fix corner cases in `optional_chains` (#5110)

8 files changed, 228 insertions, 93 deletions

diff --git a/lib/ast.js b/lib/ast.js
index 76db9f95..c4866f85 100644
--- a/lib/ast.js
+++ b/lib/ast.js
@@ -1289,13 +1289,14 @@ function must_be_expressions(node, prop, allow_spread, allow_hole) {
     });
 }
 
-var AST_Call = DEFNODE("Call", "args expression optional pure", {
+var AST_Call = DEFNODE("Call", "args expression optional pure terminal", {
     $documentation: "A function call expression",
     $propdoc: {
         args: "[AST_Node*] array of arguments",
         expression: "[AST_Node] expression to invoke as function",
         optional: "[boolean] whether the expression is optional chaining",
         pure: "[string/S] marker for side-effect-free call expression",
+        terminal: "[boolean] whether the chain has ended",
     },
     walk: function(visitor) {
         var node = this;
@@ -1316,6 +1317,7 @@ var AST_New = DEFNODE("New", null, {
     $documentation: "An object instantiation.  Derives from a function call since it has exactly the same properties",
     _validate: function() {
         if (this.optional) throw new Error("optional must be false");
+        if (this.terminal) throw new Error("terminal must be false");
     },
 }, AST_Call);
 
@@ -1338,12 +1340,18 @@ var AST_Sequence = DEFNODE("Sequence", "expressions", {
     },
 });
 
-var AST_PropAccess = DEFNODE("PropAccess", "expression optional property", {
+function root_expr(prop) {
+    while (prop instanceof AST_PropAccess) prop = prop.expression;
+    return prop;
+}
+
+var AST_PropAccess = DEFNODE("PropAccess", "expression optional property terminal", {
     $documentation: "Base class for property access expressions, i.e. `a.foo` or `a[\"foo\"]`",
     $propdoc: {
         expression: "[AST_Node] the “container” expression",
         optional: "[boolean] whether the expression is optional chaining",
         property: "[AST_Node|string] the property to access.  For AST_Dot this is always a plain string, while for AST_Sub it's an arbitrary AST_Node",
+        terminal: "[boolean] whether the chain has ended",
     },
     get_property: function() {
         var p = this.property;
diff --git a/lib/compress.js b/lib/compress.js
index 3ffb4a1c..274a8399 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -1721,11 +1721,6 @@ merge(Compressor.prototype, {
         return x;
     }
 
-    function root_expr(prop) {
-        while (prop instanceof AST_PropAccess) prop = prop.expression;
-        return prop;
-    }
-
     function is_iife_call(node) {
         if (node.TYPE != "Call") return false;
         do {
@@ -9070,16 +9065,20 @@ merge(Compressor.prototype, {
     OPT(AST_Const, varify);
     OPT(AST_Let, varify);
 
-    function trim_optional_chain(self, compressor) {
+    function trim_optional_chain(node, compressor) {
         if (!compressor.option("optional_chains")) return;
-        if (!self.optional) return;
-        var expr = self.expression;
-        var ev = fuzzy_eval(compressor, expr, true);
-        if (ev == null) return make_node(AST_UnaryPrefix, self, {
-            operator: "void",
-            expression: expr,
-        }).optimize(compressor);
-        if (!(ev instanceof AST_Node)) self.optional = false;
+        if (node.terminal) do {
+            var expr = node.expression;
+            if (node.optional) {
+                var ev = fuzzy_eval(compressor, expr, true);
+                if (ev == null) return make_node(AST_UnaryPrefix, node, {
+                    operator: "void",
+                    expression: expr,
+                }).optimize(compressor);
+                if (!(ev instanceof AST_Node)) node.optional = false;
+            }
+            node = expr;
+        } while ((node.TYPE == "Call" || node instanceof AST_PropAccess) && !node.terminal);
     }
 
     function lift_sequence_in_expression(node, compressor) {
diff --git a/lib/mozilla-ast.js b/lib/mozilla-ast.js
index 15f53849..8bf6a97b 100644
--- a/lib/mozilla-ast.js
+++ b/lib/mozilla-ast.js
@@ -556,7 +556,9 @@
             return node;
         },
         ChainExpression: function(M) {
-            return from_moz(M.expression);
+            var node = from_moz(M.expression);
+            node.terminal = true;
+            return node;
         },
     };
 
@@ -868,7 +870,7 @@
 
     def_to_moz(AST_PropAccess, function To_Moz_MemberExpression(M) {
         var computed = M instanceof AST_Sub;
-        return {
+        var expr = {
             type: "MemberExpression",
             object: to_moz(M.expression),
             computed: computed,
@@ -878,6 +880,10 @@
                 name: M.property,
             },
         };
+        return M.terminal ? {
+            type: "ChainExpression",
+            expression: expr,
+        } : expr;
     });
 
     def_to_moz(AST_Unary, function To_Moz_Unary(M) {
diff --git a/lib/output.js b/lib/output.js
index 0cf379dd..c35c8ffc 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -790,35 +790,26 @@ function OutputStream(options) {
         if (p instanceof AST_Unary) return true;
     });
 
-    function lhs_has_optional(node, output) {
-        var p = output.parent();
-        if (p instanceof AST_PropAccess && p.expression === node && is_lhs(p, output.parent(1))) {
-            // ++(foo?.bar).baz
-            // (foo?.()).bar = baz
-            do {
-                if (node.optional) return true;
-                node = node.expression;
-            } while (node.TYPE == "Call" || node instanceof AST_PropAccess);
-        }
+    function need_chain_parens(node, parent) {
+        if (!node.terminal) return false;
+        if (!(parent instanceof AST_Call || parent instanceof AST_PropAccess)) return false;
+        return parent.expression === node;
     }
 
     PARENS(AST_PropAccess, function(output) {
         var node = this;
         var p = output.parent();
-        if (p instanceof AST_New) {
-            if (p.expression !== node) return false;
-            // i.e. new (foo().bar)
-            //
-            // if there's one call into this subtree, then we need
-            // parens around it too, otherwise the call will be
-            // interpreted as passing the arguments to the upper New
-            // expression.
-            do {
-                node = node.expression;
-            } while (node instanceof AST_PropAccess);
-            return node.TYPE == "Call";
-        }
-        return lhs_has_optional(node, output);
+        // i.e. new (foo().bar)
+        //
+        // if there's one call into this subtree, then we need
+        // parens around it too, otherwise the call will be
+        // interpreted as passing the arguments to the upper New
+        // expression.
+        if (p instanceof AST_New && p.expression === node && root_expr(node).TYPE == "Call") return true;
+        // (foo?.bar)()
+        // (foo?.bar).baz
+        // new (foo?.bar)()
+        return need_chain_parens(node, p);
     });
 
     PARENS(AST_Call, function(output) {
@@ -833,7 +824,10 @@ function OutputStream(options) {
             var g = output.parent(1);
             if (g instanceof AST_Assign && g.left === p) return true;
         }
-        return lhs_has_optional(node, output);
+        // (foo?.())()
+        // (foo?.()).bar
+        // new (foo?.())()
+        return need_chain_parens(node, p);
     });
 
     PARENS(AST_New, function(output) {
diff --git a/lib/parse.js b/lib/parse.js
index eb2dc01a..a80fcaf9 100644
--- a/lib/parse.js
+++ b/lib/parse.js
@@ -1818,10 +1818,7 @@ function parse($TEXT, options) {
         if (is("punc")) {
             switch (start.value) {
               case "`":
-                var tmpl = template(null);
-                tmpl.start = start;
-                tmpl.end = prev();
-                return subscripts(tmpl, allow_calls);
+                return subscripts(template(null), allow_calls);
               case "(":
                 next();
                 if (is("punc", ")")) {
@@ -2230,6 +2227,7 @@ function parse($TEXT, options) {
     }
 
     function template(tag) {
+        var start = tag ? tag.start : S.token;
         var read = S.input.context().read_template;
         var strings = [];
         var expressions = [];
@@ -2240,58 +2238,60 @@ function parse($TEXT, options) {
         }
         next();
         return new AST_Template({
+            start: start,
             expressions: expressions,
             strings: strings,
             tag: tag,
+            end: prev(),
         });
     }
 
-    var subscripts = function(expr, allow_calls, optional) {
+    function subscripts(expr, allow_calls) {
         var start = expr.start;
-        if (is("punc", "[")) {
-            next();
-            var prop = expression();
-            expect("]");
-            return subscripts(new AST_Sub({
-                start: start,
-                optional: optional,
-                expression: expr,
-                property: prop,
-                end: prev(),
-            }), allow_calls);
-        }
-        if (allow_calls && is("punc", "(")) {
-            next();
-            var call = new AST_Call({
-                start: start,
-                optional: optional,
-                expression: expr,
-                args: expr_list(")", !options.strict),
-                end: prev(),
-            });
-            return subscripts(call, true);
-        }
-        if (optional || is("punc", ".")) {
-            if (!optional) next();
-            return subscripts(new AST_Dot({
-                start: start,
-                optional: optional,
-                expression: expr,
-                property: as_name(),
-                end: prev(),
-            }), allow_calls);
-        }
-        if (is("punc", "`")) {
-            var tmpl = template(expr);
-            tmpl.start = expr.start;
-            tmpl.end = prev();
-            return subscripts(tmpl, allow_calls);
-        }
-        if (is("operator", "?") && is_token(peek(), "punc", ".")) {
-            next();
-            next();
-            return subscripts(expr, allow_calls, true);
+        var optional = null;
+        while (true) {
+            if (is("operator", "?") && is_token(peek(), "punc", ".")) {
+                next();
+                next();
+                optional = expr;
+            }
+            if (is("punc", "[")) {
+                next();
+                var prop = expression();
+                expect("]");
+                expr = new AST_Sub({
+                    start: start,
+                    optional: optional === expr,
+                    expression: expr,
+                    property: prop,
+                    end: prev(),
+                });
+            } else if (allow_calls && is("punc", "(")) {
+                next();
+                expr = new AST_Call({
+                    start: start,
+                    optional: optional === expr,
+                    expression: expr,
+                    args: expr_list(")", !options.strict),
+                    end: prev(),
+                });
+            } else if (optional === expr || is("punc", ".")) {
+                if (optional !== expr) next();
+                expr = new AST_Dot({
+                    start: start,
+                    optional: optional === expr,
+                    expression: expr,
+                    property: as_name(),
+                    end: prev(),
+                });
+            } else if (is("punc", "`")) {
+                if (optional) croak("Invalid template on optional chain");
+                expr = template(expr);
+            } else {
+                break;
+            }
         }
+        if (optional) expr.terminal = true;
         if (expr instanceof AST_Call && !expr.pure) {
             var start = expr.start;
             var comments = start.comments_before;
@@ -2305,7 +2305,7 @@ function parse($TEXT, options) {
             }
         }
         return expr;
-    };
+    }
 
     function maybe_unary(no_in) {
         var start = S.token;
diff --git a/test/compress/optional-chains.js b/test/compress/optional-chains.js
index 6e06f49c..8d16fdac 100644
--- a/test/compress/optional-chains.js
+++ b/test/compress/optional-chains.js
@@ -58,7 +58,7 @@ assign_parentheses_dot: {
     input: {
         (console?.log).name.p = console.log("PASS");
     }
-    expect_exact: '(console?.log.name).p=console.log("PASS");'
+    expect_exact: '(console?.log).name.p=console.log("PASS");'
     expect_stdout: "PASS"
     node_version: ">=14"
 }
@@ -72,6 +72,26 @@ assign_no_parentheses: {
     node_version: ">=14"
 }
 
+call_parentheses: {
+    input: {
+        (function(o) {
+            console.log(o.f("FAIL"), (o.f)("FAIL"), (0, o.f)(42));
+            console.log(o?.f("FAIL"), (o?.f)("FAIL"), (0, o?.f)(42));
+        })({
+            a: "PASS",
+            f(b) {
+                return this.a || b;
+            },
+        });
+    }
+    expect_exact: '(function(o){console.log(o.f("FAIL"),o.f("FAIL"),(0,o.f)(42));console.log(o?.f("FAIL"),(o?.f)("FAIL"),(0,o?.f)(42))})({a:"PASS",f(b){return this.a||b}});'
+    expect_stdout: [
+        "PASS PASS 42",
+        "PASS PASS 42",
+    ]
+    node_version: ">=14"
+}
+
 unary_parentheses: {
     input: {
         var o = { p: 41 };
@@ -237,6 +257,99 @@ trim_2: {
     node_version: ">=14"
 }
 
+trim_dot_call_1: {
+    options = {
+        evaluate: true,
+        optional_chains: true,
+    }
+    input: {
+        console.log(null?.f());
+    }
+    expect: {
+        console.log(void 0);
+    }
+    expect_stdout: "undefined"
+    node_version: ">=14"
+}
+
+trim_dot_call_2: {
+    options = {
+        evaluate: true,
+        optional_chains: true,
+        unsafe: true,
+    }
+    input: {
+        try {
+            (null?.p)();
+        } catch (e) {
+            console.log("PASS");
+        }
+    }
+    expect: {
+        try {
+            (void 0)();
+        } catch (e) {
+            console.log("PASS");
+        }
+    }
+    expect_stdout: "PASS"
+    node_version: ">=14"
+}
+
+trim_dot_call_3: {
+    options = {
+        evaluate: true,
+        optional_chains: true,
+        unsafe: true,
+    }
+    input: {
+        try {
+            ({ p: null })?.p();
+        } catch (e) {
+            console.log("PASS");
+        }
+    }
+    expect: {
+        try {
+            null();
+        } catch (e) {
+            console.log("PASS");
+        }
+    }
+    expect_stdout: "PASS"
+    node_version: ">=14"
+}
+
+trim_dot_sub: {
+    options = {
+        evaluate: true,
+        optional_chains: true,
+    }
+    input: {
+        console.log(null?.p[42]);
+    }
+    expect: {
+        console.log(void 0);
+    }
+    expect_stdout: "undefined"
+    node_version: ">=14"
+}
+
+trim_sub_call_call: {
+    options = {
+        evaluate: true,
+        optional_chains: true,
+    }
+    input: {
+        console.log(null?.[42]()());
+    }
+    expect: {
+        console.log(void 0);
+    }
+    expect_stdout: "undefined"
+    node_version: ">=14"
+}
+
 issue_4906: {
     options = {
         toplevel: true,
diff --git a/test/input/invalid/optional-template.js b/test/input/invalid/optional-template.js
new file mode 100644
index 00000000..608f4831
--- /dev/null
+++ b/test/input/invalid/optional-template.js
@@ -0,0 +1 @@
+console?.log``;
diff --git a/test/mocha/cli.js b/test/mocha/cli.js
index 150af5a3..9fb10a16 100644
--- a/test/mocha/cli.js
+++ b/test/mocha/cli.js
@@ -721,6 +721,20 @@ describe("bin/uglifyjs", function() {
             done();
         });
     });
+    it("Should throw syntax error (console?.log``)", function(done) {
+        var command = uglifyjscmd + " test/input/invalid/optional-template.js";
+        exec(command, function(err, stdout, stderr) {
+            assert.ok(err);
+            assert.strictEqual(stdout, "");
+            assert.strictEqual(stderr.split(/\n/).slice(0, 4).join("\n"), [
+                "Parse error at test/input/invalid/optional-template.js:1,12",
+                "console?.log``;",
+                "            ^",
+                "ERROR: Invalid template on optional chain",
+            ].join("\n"));
+            done();
+        });
+    });
     it("Should handle literal string as source map input", function(done) {
         var command = [
             uglifyjscmd,