fix corner cases in optional_chains (#5110)

This commit is contained in:
Alex Lam S.L
2021-08-20 03:10:10 +01:00
committed by GitHub
parent befb99bd71
commit 9634a9d1fd
8 changed files with 228 additions and 93 deletions

View File

@@ -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;

View File

@@ -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) {

View File

@@ -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) {

View File

@@ -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) {

View File

@@ -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;

View File

@@ -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,

View File

@@ -0,0 +1 @@
console?.log``;

View File

@@ -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,