Skip to content

Commit

Permalink
feat(es): Add an option to preserve all comments (#3815)
Browse files Browse the repository at this point in the history
  • Loading branch information
dpchamps authored Mar 11, 2022
1 parent 6257f0f commit c5a0c9a
Show file tree
Hide file tree
Showing 18 changed files with 408 additions and 2 deletions.
19 changes: 17 additions & 2 deletions crates/swc/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ use swc_ecma_visit::{Fold, VisitMutWith};
use self::util::BoolOrObject;
use crate::{
builder::PassBuilder,
dropped_comments_preserver::dropped_comments_preserver,
plugin::{PluginConfig, PluginContext},
SwcImportResolver,
};
Expand Down Expand Up @@ -296,6 +297,7 @@ impl Options {
minify: mut js_minify,
experimental,
lints,
preserve_all_comments,
..
} = config.jsc;

Expand Down Expand Up @@ -368,7 +370,11 @@ impl Options {

let regenerator = transform.regenerator.clone();

let preserve_comments = js_minify.as_ref().map(|v| v.format.comments.clone());
let preserve_comments = if preserve_all_comments {
Some(BoolOrObject::from(true))
} else {
js_minify.as_ref().map(|v| v.format.comments.clone())
};

if syntax.typescript() {
transform.legacy_decorator = true;
Expand Down Expand Up @@ -498,7 +504,11 @@ impl Options {
syntax.jsx()
),
pass,
Optional::new(jest::jest(), transform.hidden.jest)
Optional::new(jest::jest(), transform.hidden.jest),
Optional::new(
dropped_comments_preserver(comments.cloned()),
preserve_all_comments
),
);

Ok(BuiltInput {
Expand Down Expand Up @@ -1031,6 +1041,9 @@ pub struct JscConfig {

#[serde(default)]
pub lints: LintConfig,

#[serde(default)]
pub preserve_all_comments: bool,
}

/// `jsc.experimental` in `.swcrc`
Expand Down Expand Up @@ -1567,6 +1580,8 @@ impl Merge for JscConfig {
self.paths.merge(&from.paths);
self.minify.merge(&from.minify);
self.experimental.merge(&from.experimental);
self.preserve_all_comments
.merge(&from.preserve_all_comments)
}
}

Expand Down
138 changes: 138 additions & 0 deletions crates/swc/src/dropped_comments_preserver.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
use swc_common::{
comments::{Comment, Comments, SingleThreadedComments},
BytePos, Span, DUMMY_SP,
};
use swc_ecma_ast::{Module, Script};
use swc_ecma_visit::{as_folder, noop_visit_mut_type, Fold, VisitMut, VisitMutWith};

/// Preserves comments that would otherwise be dropped.
///
/// If during compilation an ast node associated with
/// a comment is dropped, the comment will not appear in the final emitted
/// output. This can create problems in the JavaScript ecosystem, particularly
/// around instanbul coverage and other tooling that relies on comment
/// directives.
///
/// This transformer shifts orphaned comments to the next closest known span
/// while making a best-effort to preserve the "general orientation" of
/// comments.
pub fn dropped_comments_preserver(
comments: Option<SingleThreadedComments>,
) -> impl Fold + VisitMut {
as_folder(DroppedCommentsPreserver {
comments,
is_first_span: true,
known_spans: Vec::new(),
})
}

struct DroppedCommentsPreserver {
comments: Option<SingleThreadedComments>,
is_first_span: bool,
known_spans: Vec<Span>,
}

type CommentEntries = Vec<(BytePos, Vec<Comment>)>;

impl VisitMut for DroppedCommentsPreserver {
noop_visit_mut_type!();

fn visit_mut_module(&mut self, module: &mut Module) {
module.visit_mut_children_with(self);
self.shift_comments_to_known_spans();
}

fn visit_mut_script(&mut self, script: &mut Script) {
script.visit_mut_children_with(self);
self.shift_comments_to_known_spans();
}

fn visit_mut_span(&mut self, span: &mut Span) {
if span.is_dummy() || self.is_first_span {
self.is_first_span = false;
return;
}

self.known_spans.push(*span);
span.visit_mut_children_with(self)
}
}

impl DroppedCommentsPreserver {
fn shift_comments_to_known_spans(&self) {
if let Some(comments) = &self.comments {
let trailing_comments = self.shift_leading_comments(comments);

self.shift_trailing_comments(trailing_comments);
}
}

/// We'll be shifting all comments to known span positions, so drain the
/// current comments first to limit the amount of look ups needed into
/// the hashmaps.
///
/// This way, we only need to take the comments once, and then add them back
/// once.
fn collect_existing_comments(&self, comments: &SingleThreadedComments) -> CommentEntries {
let (mut leading_comments, mut trailing_comments) = comments.borrow_all_mut();
let mut existing_comments: CommentEntries = leading_comments
.drain()
.chain(trailing_comments.drain())
.collect();

existing_comments.sort_by(|(bp_a, _), (bp_b, _)| bp_a.cmp(bp_b));

existing_comments
}

/// Shift all comments to known leading positions.
/// This prevents trailing comments from ending up associated with
/// nodes that will not emit trailing comments, while
/// preserving any comments that might show up after all code positions.
///
/// This maintains the highest fidelity between existing comment positions
/// of pre and post compiled code.
fn shift_leading_comments(&self, comments: &SingleThreadedComments) -> CommentEntries {
let mut existing_comments = self.collect_existing_comments(comments);

for span in self.known_spans.iter() {
let (comments_to_move, next_byte_positions): (CommentEntries, CommentEntries) =
existing_comments
.drain(..)
.partition(|(bp, _)| *bp <= span.lo);

existing_comments.extend(next_byte_positions);

let collected_comments = comments_to_move.into_iter().flat_map(|(_, c)| c).collect();

self.comments
.add_leading_comments(span.lo, collected_comments)
}

existing_comments
}

/// These comments trail all known span lo byte positions.
/// Therefore, by shifting them to trail the highest known hi position, we
/// ensure that any remaining trailing comments are emitted in a
/// similar location
fn shift_trailing_comments(&self, remaining_comment_entries: CommentEntries) {
let last_trailing = self
.known_spans
.iter()
.copied()
.fold(
DUMMY_SP,
|acc, span| if span.hi > acc.hi { span } else { acc },
);

self.comments.add_trailing_comments(
last_trailing.hi,
remaining_comment_entries
.into_iter()
.flat_map(|(_, c)| c)
.collect(),
);
}
}
1 change: 1 addition & 0 deletions crates/swc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ use crate::config::{

mod builder;
pub mod config;
mod dropped_comments_preserver;
mod plugin;
pub mod resolver {
use std::path::PathBuf;
Expand Down
16 changes: 16 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-1/input/.swcrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"jsc": {
"parser": {
"syntax": "typescript",
"tsx": false,
"decorators": false,
"dynamicImport": false
},
"transform": null,
"target": "es5",
"loose": false,
"externalHelpers": false,
"keepClassNames": false,
"preserveAllComments": true
}
}
11 changes: 11 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-1/input/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* istanbul ignore next */
type Z = number;
// preserved comment
const x = 1;

// Stacked Comment
// Another comment
type Y = string;
const a = "";

// trailing comment
5 changes: 5 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-1/output/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/* istanbul ignore next */ // preserved comment
var x = 1;
// Stacked Comment
// Another comment
var a = ""; // trailing comment
33 changes: 33 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-2/input/.swcrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"sourceMaps": false,
"module": {
"type": "commonjs"
},
"jsc": {
"parser": {
"syntax": "typescript",
"tsx": true,
"dynamicImport": true,
"decorators": false
},
"transform": {
"react": {
"pragma": "React.createElement",
"pragmaFrag": "React.Fragment",
"throwIfNamespace": true,
"development": false,
"useBuiltins": false
}
},
"target": "es2015",
"loose": false,
"externalHelpers": false,
"keepClassNames": false,
"minify": {
"compress": false,
"mangle": false
},
"preserveAllComments": true
},
"minify": false
}
12 changes: 12 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-2/input/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//top comment
export const noop = () => {};
/* istanbul ignore next */
export const badIstanbul = (test: Record<string, unknown>) => {
const { value, ...pixelParams } = test;
console.log('fail');
};

/* istanbul ignore next: UI-5137 */
export const downloadDocument = (): void => {
console.log('fail');
};
20 changes: 20 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-2/output/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
"use strict";
Object.defineProperty(exports, "__esModule", {
value: true
});
exports.downloadDocument = exports.badIstanbul = exports.noop = void 0;
var swcHelpers = require("@swc/helpers");
//top comment
const noop = ()=>{};
exports.noop = noop;
var /* istanbul ignore next */ badIstanbul = (test)=>{
const { value } = test, pixelParams = swcHelpers.objectWithoutProperties(test, [
"value"
]);
console.log('fail');
};
exports.badIstanbul = badIstanbul;
/* istanbul ignore next: UI-5137 */ const downloadDocument = ()=>{
console.log('fail');
};
exports.downloadDocument = downloadDocument;
32 changes: 32 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-3/input/.swcrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"sourceMaps": false,
"module": {
"type": "commonjs"
},
"jsc": {
"parser": {
"syntax": "typescript",
"tsx": false,
"dynamicImport": true,
"decorators": false
},
"transform": {
"react": {
"pragma": "React.createElement",
"pragmaFrag": "React.Fragment",
"throwIfNamespace": true,
"development": false,
"useBuiltins": false
},
"hidden": {
"jest": true
}
},
"target": "es2015",
"loose": false,
"externalHelpers": false,
"keepClassNames": false,
"preserveAllComments": true
},
"minify": false
}
7 changes: 7 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-3/input/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// single line comment
const x = ({y, ...rest}: /*todo: refactor any type*/ any) => {
return {
y, // another comment
z: rest.z // final comment
}
}
14 changes: 14 additions & 0 deletions crates/swc/tests/fixture/issue-2964.case-3/output/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
"use strict";
var swcHelpers = require("@swc/helpers");
// single line comment
const x = (_param)=>/*todo: refactor any type*/ {
var { y } = _param, rest = swcHelpers.objectWithoutProperties(_param, [
"y"
]);
return {
y,
// another comment
z: rest.z
};
} // final comment
;
1 change: 1 addition & 0 deletions crates/swc/tests/rust_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ fn shopify_2_same_opt() {
experimental: Default::default(),
lints: Default::default(),
assumptions: Default::default(),
preserve_all_comments: false,
},
module: None,
minify: false,
Expand Down
Loading

1 comment on commit c5a0c9a

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: c5a0c9a Previous: fcbd3c5 Ratio
full_es2015 145501297 ns/iter (± 5764341) 149036204 ns/iter (± 8071373) 0.98
full_es2016 145990028 ns/iter (± 6482731) 148655719 ns/iter (± 6711514) 0.98
full_es2017 145416292 ns/iter (± 5022142) 147344131 ns/iter (± 7438269) 0.99
full_es2018 145159325 ns/iter (± 8832138) 147924028 ns/iter (± 6288444) 0.98
full_es2019 144238028 ns/iter (± 8771262) 145375089 ns/iter (± 8285404) 0.99
full_es2020 132803290 ns/iter (± 6187321) 142354371 ns/iter (± 6629692) 0.93
full_es3 198157615 ns/iter (± 7801543) 206327265 ns/iter (± 10659522) 0.96
full_es5 186048251 ns/iter (± 7338644) 195970389 ns/iter (± 13946861) 0.95
parser 532353 ns/iter (± 11744) 546573 ns/iter (± 12972) 0.97

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.