fix(es/fixer): Preserve parenthesis for optional chaining (#8399)

**Related issue:** - Closes #8398
swc-project · Dec 11, 2023 · a69f172 · a69f172
1 parent 3b845c3
commit a69f172
Show file tree

Hide file tree

Showing 21 changed files with 171 additions and 146 deletions.
diff --git a/crates/swc/tests/fixture/issues-8xxx/8398/input/.swcrc b/crates/swc/tests/fixture/issues-8xxx/8398/input/.swcrc
@@ -0,0 +1,15 @@
+{
+    "jsc": {
+        "parser": {
+            "syntax": "typescript",
+            "tsx": true
+        },
+        "target": "es2022",
+        "minify": {
+            "compress": true
+        },
+        "loose": false
+    },
+    "isModule": true,
+    "minify": true
+}
diff --git a/crates/swc/tests/fixture/issues-8xxx/8398/input/index.js b/crates/swc/tests/fixture/issues-8xxx/8398/input/index.js
@@ -0,0 +1,12 @@
+(obj?.a).b;
+(obj?.a)();
+(obj?.a)[0];
+(obj?.a)`hello`;
+
+(obj.a?.()).a;
+(obj.a?.())();
+(obj.a?.())[0];
+(obj.a?.())`hello`;
+
+(((a?.b)));
+(((a?.())));
diff --git a/crates/swc/tests/fixture/issues-8xxx/8398/output/index.js b/crates/swc/tests/fixture/issues-8xxx/8398/output/index.js
@@ -0,0 +1 @@
+(obj?.a).b,(obj?.a)(),(obj?.a)[0],(obj?.a)`hello`,(obj.a?.()).a,(obj.a?.())(),(obj.a?.())[0],(obj.a?.())`hello`,a?.b,a?.();
diff --git a/crates/swc/tests/tsc-references/checkObjectDefineProperty.1.normal.js b/crates/swc/tests/tsc-references/checkObjectDefineProperty.1.normal.js
@@ -46,7 +46,7 @@ var needsExemplar = function() {
     var _ = arguments.length > 0 && arguments[0] !== void 0 ? arguments[0] : x;
     return void 0;
 };
-var expected = /** @type {{name: string, readonly middleInit: string, readonly lastName: string, zip: number, readonly houseNumber: number, zipStr: string}} */ /** @type {*} */ null;
+var expected = /** @type {*} */ null;
 /**
  * 
  * @param {typeof returnExemplar} a 

diff --git a/crates/swc/tests/tsc-references/deleteChain.1.normal.js b/crates/swc/tests/tsc-references/deleteChain.1.normal.js
@@ -1,15 +1,15 @@
 //// [deleteChain.ts]
-var _o3_b, _o3_b1, _o4_b_c_d, _o4_b, _this, _o4_b1, _o4_b_c_d1, _o4_b2, _o5_b_c_d, _o5_b, _o5_b_c_d1, _o5_b1, _o6_b_c_d, _o6_b, _o6_b_c_d1, _o6_b1;
+var _o3_b, _o3_b1, _o4_b_c_d, _o4_b, _o4_b_c_d1, _o4_b1, _o4_b_c_d2, _o4_b2, _o5_b_c_d, _o5_b, _o5_b_c_d1, _o5_b1, _o6_b_c_d, _o6_b, _o6_b_c_d1, _o6_b1;
 o1 === null || o1 === void 0 ? true : delete o1.b;
-delete (o1 === null || o1 === void 0 ? void 0 : o1.b);
+o1 === null || o1 === void 0 ? true : delete o1.b;
+o2 === null || o2 === void 0 ? true : delete o2.b.c;
 o2 === null || o2 === void 0 ? true : delete o2.b.c;
-delete (o2 === null || o2 === void 0 ? void 0 : o2.b.c);
 (_o3_b = o3.b) === null || _o3_b === void 0 ? true : delete _o3_b.c;
-delete ((_o3_b1 = o3.b) === null || _o3_b1 === void 0 ? void 0 : _o3_b1.c);
+(_o3_b1 = o3.b) === null || _o3_b1 === void 0 ? true : delete _o3_b1.c;
 (_o4_b = o4.b) === null || _o4_b === void 0 ? true : (_o4_b_c_d = _o4_b.c.d) === null || _o4_b_c_d === void 0 ? true : delete _o4_b_c_d.e;
-(_this = (_o4_b1 = o4.b) === null || _o4_b1 === void 0 ? void 0 : _o4_b1.c.d) === null || _this === void 0 ? true : delete _this.e;
-delete ((_o4_b2 = o4.b) === null || _o4_b2 === void 0 ? void 0 : (_o4_b_c_d1 = _o4_b2.c.d) === null || _o4_b_c_d1 === void 0 ? void 0 : _o4_b_c_d1.e);
+(_o4_b1 = o4.b) === null || _o4_b1 === void 0 ? true : (_o4_b_c_d1 = _o4_b1.c.d) === null || _o4_b_c_d1 === void 0 ? true : delete _o4_b_c_d1.e;
+(_o4_b2 = o4.b) === null || _o4_b2 === void 0 ? true : (_o4_b_c_d2 = _o4_b2.c.d) === null || _o4_b_c_d2 === void 0 ? true : delete _o4_b_c_d2.e;
 (_o5_b = o5.b) === null || _o5_b === void 0 ? true : (_o5_b_c_d = _o5_b.call(o5).c.d) === null || _o5_b_c_d === void 0 ? true : delete _o5_b_c_d.e;
-delete ((_o5_b1 = o5.b) === null || _o5_b1 === void 0 ? void 0 : (_o5_b_c_d1 = _o5_b1.call(o5).c.d) === null || _o5_b_c_d1 === void 0 ? void 0 : _o5_b_c_d1.e);
+(_o5_b1 = o5.b) === null || _o5_b1 === void 0 ? true : (_o5_b_c_d1 = _o5_b1.call(o5).c.d) === null || _o5_b_c_d1 === void 0 ? true : delete _o5_b_c_d1.e;
 (_o6_b = o6.b) === null || _o6_b === void 0 ? true : (_o6_b_c_d = _o6_b["c"].d) === null || _o6_b_c_d === void 0 ? true : delete _o6_b_c_d["e"];
-delete ((_o6_b1 = o6.b) === null || _o6_b1 === void 0 ? void 0 : (_o6_b_c_d1 = _o6_b1["c"].d) === null || _o6_b_c_d1 === void 0 ? void 0 : _o6_b_c_d1["e"]);
+(_o6_b1 = o6.b) === null || _o6_b1 === void 0 ? true : (_o6_b_c_d1 = _o6_b1["c"].d) === null || _o6_b_c_d1 === void 0 ? true : delete _o6_b_c_d1["e"];
diff --git a/crates/swc/tests/tsc-references/deleteChain.2.minified.js b/crates/swc/tests/tsc-references/deleteChain.2.minified.js
diff --git a/crates/swc/tests/tsc-references/importTypeInJSDoc.1.normal.js b/crates/swc/tests/tsc-references/importTypeInJSDoc.1.normal.js
@@ -11,7 +11,7 @@
 //// [index.js]
 /**
  * @typedef {import("./externs")} Foo
- */ let a = /** @type {Foo} */ /** @type {*} */ undefined;
+ */ let a = /** @type {*} */ undefined;
 a = new Foo({
     doer: Foo.Bar
 });

diff --git a/crates/swc/tests/tsc-references/optionalChainingInTypeAssertions(target=es2015).1.normal.js b/crates/swc/tests/tsc-references/optionalChainingInTypeAssertions(target=es2015).1.normal.js
@@ -8,8 +8,8 @@ const foo = new Foo();
 (_foo_m1 = foo.m) === null || _foo_m1 === void 0 ? void 0 : _foo_m1.call(foo);
 (_foo_m2 = foo.m) === null || _foo_m2 === void 0 ? void 0 : _foo_m2.call(foo);
 (_foo_m3 = foo.m) === null || _foo_m3 === void 0 ? void 0 : _foo_m3.call(foo);
-// https://github.com/microsoft/TypeScript/issues/50148
-(foo === null || foo === void 0 ? void 0 : foo.m).length;
+(// https://github.com/microsoft/TypeScript/issues/50148
+foo === null || foo === void 0 ? void 0 : foo.m).length;
 (foo === null || foo === void 0 ? void 0 : foo.m).length;
 (foo === null || foo === void 0 ? void 0 : foo["m"]).length;
 (foo === null || foo === void 0 ? void 0 : foo["m"]).length;
diff --git a/...es/swc/tests/tsc-references/optionalChainingInTypeAssertions(target=es2022).2.minified.js b/...es/swc/tests/tsc-references/optionalChainingInTypeAssertions(target=es2022).2.minified.js
@@ -2,4 +2,4 @@
 const foo = new class {
     m() {}
 }();
-foo.m?.(), foo.m?.(), foo.m?.(), foo.m?.(), foo?.m.length, foo?.m.length, foo?.m.length, foo?.m.length;
+foo.m?.(), foo.m?.(), foo.m?.(), foo.m?.(), (foo?.m).length, (foo?.m).length, (foo?.m).length, (foo?.m).length;
diff --git a/crates/swc_ecma_minifier/src/compress/pure/mod.rs b/crates/swc_ecma_minifier/src/compress/pure/mod.rs
@@ -334,10 +334,6 @@ impl VisitMut for Pure<'_> {
     }
 
     fn visit_mut_expr(&mut self, e: &mut Expr) {
-        if let Expr::Paren(p) = e {
-            *e = *p.expr.take();
-        }
-
         {
             let ctx = Ctx {
                 in_first_expr: false,

diff --git a/crates/swc_ecma_minifier/src/metadata/mod.rs b/crates/swc_ecma_minifier/src/metadata/mod.rs
@@ -97,7 +97,13 @@ impl VisitMut for InfoMarker<'_> {
     fn visit_mut_call_expr(&mut self, n: &mut CallExpr) {
         n.visit_mut_children_with(self);
 
-        if has_noinline(self.comments, n.span) {
+        // TODO: remove after we figure out how to move comments properly
+        if has_noinline(self.comments, n.span)
+            || match &n.callee {
+                Callee::Expr(e) => has_noinline(self.comments, e.span()),
+                _ => false,
+            }
+        {
             n.span = n.span.apply_mark(self.marks.noinline);
         }
 

diff --git a/crates/swc_ecma_minifier/src/pass/precompress.rs b/crates/swc_ecma_minifier/src/pass/precompress.rs
@@ -8,6 +8,7 @@ use crate::HEAVY_TASK_PARALLELS;
 /// Optimizer invoked before invoking compressor.
 ///
 /// - Remove parens.
+/// TODO: remove completely after #8333
 pub(crate) fn precompress_optimizer<'a>() -> impl 'a + VisitMut {
     PrecompressOptimizer {}
 }
@@ -26,14 +27,6 @@ impl Parallel for PrecompressOptimizer {
 impl VisitMut for PrecompressOptimizer {
     noop_visit_mut_type!();
 
-    fn visit_mut_expr(&mut self, e: &mut Expr) {
-        e.visit_mut_children_with(self);
-
-        if let Expr::Paren(p) = e {
-            *e = *p.expr.take();
-        }
-    }
-
     fn visit_mut_pat_or_expr(&mut self, n: &mut PatOrExpr) {
         n.visit_mut_children_with(self);
 

diff --git a/crates/swc_ecma_minifier/tests/compress.rs b/crates/swc_ecma_minifier/tests/compress.rs
@@ -39,7 +39,11 @@ use swc_ecma_parser::{
     EsConfig, Parser, Syntax,
 };
 use swc_ecma_testing::{exec_node_js, JsExecOptions};
-use swc_ecma_transforms_base::{fixer::fixer, hygiene::hygiene, resolver};
+use swc_ecma_transforms_base::{
+    fixer::{fixer, paren_remover},
+    hygiene::hygiene,
+    resolver,
+};
 use swc_ecma_utils::drop_span;
 use swc_ecma_visit::{FoldWith, Visit, VisitMut, VisitMutWith, VisitWith};
 use testing::{assert_eq, unignore_fixture, DebugUsingDisplay, NormalizedOutput};
@@ -194,7 +198,12 @@ fn run(
             .map_err(|err| {
                 err.into_diagnostic(handler).emit();
             })
-            .map(|module| module.fold_with(&mut resolver(unresolved_mark, top_level_mark, false)));
+            .map(|mut module| {
+                module.visit_mut_with(&mut paren_remover(Some(&comments)));
+                module.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));
+
+                module
+            });
 
         // Ignore parser errors.
         //

diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8398/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8398/input.js
@@ -0,0 +1 @@
+(obj?.a).b;
diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8398/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8398/output.js
@@ -0,0 +1 @@
+(obj?.a).b;
diff --git a/crates/swc_ecma_minifier/tests/terser_exec.rs b/crates/swc_ecma_minifier/tests/terser_exec.rs
@@ -32,7 +32,11 @@ use swc_ecma_parser::{
     lexer::{input::SourceFileInput, Lexer},
     EsConfig, Parser, Syntax,
 };
-use swc_ecma_transforms_base::{fixer::fixer, hygiene::hygiene, resolver};
+use swc_ecma_transforms_base::{
+    fixer::{fixer, paren_remover},
+    hygiene::hygiene,
+    resolver,
+};
 use swc_ecma_visit::{FoldWith, VisitMutWith};
 use testing::assert_eq;
 
@@ -197,7 +201,12 @@ fn run(cm: Lrc<SourceMap>, handler: &Handler, input: &Path, config: &str) -> Opt
             .map_err(|err| {
                 err.into_diagnostic(handler).emit();
             })
-            .map(|module| module.fold_with(&mut resolver(unresolved_mark, top_level_mark, false)));
+            .map(|mut module| {
+                module.visit_mut_with(&mut paren_remover(Some(&comments)));
+                module.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));
+
+                module
+            });
 
         // Ignore parser errors.
         //
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		(obj?.a).b,(obj?.a)(),(obj?.a)[0],(obj?.a)`hello`,(obj.a?.()).a,(obj.a?.())(),(obj.a?.())[0],(obj.a?.())`hello`,a?.b,a?.();