Destructuring consistency fixes (#1417)

- Use AST_Destructuring for lhf assignment patterns
- Use AST_DefaultAssign for default assignments
- Add more checks for lhs expressions
- Add lots of testing
- Cleanup ast (e.g. remove default property)
- Fix #1402 based on a patch from @kzc
- Refine spread allowance in array destructring pattern
- Add destructuring AST tree checker
This commit is contained in:
Anthony Van de Gejuchte
2017-02-24 01:49:19 +01:00
committed by Alex Lam S.L
parent 85c1cba760
commit 07734b000a
16 changed files with 1253 additions and 141 deletions

View File

@@ -392,46 +392,56 @@ var AST_ArrowParametersOrSeq = DEFNODE("ArrowParametersOrSeq", "expressions", {
// We don't want anything which doesn't belong in a destructuring
var root = this;
return this.expressions.map(function to_fun_args(ex, _, __, default_seen_above) {
var insert_default = function(ex, default_value) {
if (default_value) {
return new AST_DefaultAssign({
start: ex.start,
left: ex,
operator: "=",
right: default_value,
end: default_value.end
});
}
return ex;
}
if (ex instanceof AST_Object) {
return new AST_Destructuring({
return insert_default(new AST_Destructuring({
start: ex.start,
end: ex.end,
is_array: false,
default: default_seen_above,
names: ex.properties.map(to_fun_args)
});
}), default_seen_above);
} else if (ex instanceof AST_ObjectKeyVal) {
if (ex.key instanceof AST_SymbolRef) {
ex.key = to_fun_args(ex.key, 0, [ex.key], ex.default);
ex.key = to_fun_args(ex.key, 0, [ex.key]);
}
ex.value = to_fun_args(ex.value, 0, [ex.key], ex.default);
return ex;
ex.value = to_fun_args(ex.value, 0, [ex.key]);
return insert_default(ex, default_seen_above);
} else if (ex instanceof AST_Hole) {
return ex;
} else if (ex instanceof AST_Destructuring) {
if (ex.names.length == 0)
croak("Invalid destructuring function parameter", ex.start.line, ex.start.col);
ex.names = ex.names.map(to_fun_args);
return ex;
return insert_default(ex, default_seen_above);
} else if (ex instanceof AST_SymbolRef) {
return new AST_SymbolFunarg({
return insert_default(new AST_SymbolFunarg({
name: ex.name,
default: default_seen_above,
start: ex.start,
end: ex.end
});
}), default_seen_above);
} else if (ex instanceof AST_Expansion) {
return ex;
ex.expression = to_fun_args(ex.expression);
return insert_default(ex, default_seen_above);
} else if (ex instanceof AST_Array) {
return new AST_Destructuring({
return insert_default(new AST_Destructuring({
start: ex.start,
end: ex.end,
is_array: true,
default: default_seen_above,
names: ex.elements.map(to_fun_args)
});
}), default_seen_above);
} else if (ex instanceof AST_Assign) {
return to_fun_args(ex.left, undefined, undefined, ex.right);
return insert_default(to_fun_args(ex.left, undefined, undefined, ex.right), default_seen_above);
} else {
croak("Invalid function parameter", ex.start.line, ex.start.col);
}
@@ -447,7 +457,7 @@ var AST_Lambda = DEFNODE("Lambda", "name argnames uses_arguments is_generator",
$propdoc: {
is_generator: "[boolean] is generatorFn or not",
name: "[AST_SymbolDeclaration?] the name of this function",
argnames: "[AST_SymbolFunarg|AST_Destructuring|AST_Expansion*] array of function arguments, destructurings, or expanding arguments",
argnames: "[AST_SymbolFunarg|AST_Destructuring|AST_Expansion|AST_DefaultAssign*] array of function arguments, destructurings, or expanding arguments",
uses_arguments: "[boolean/S] tells whether this function accesses the arguments array"
},
args_as_names: function () {
@@ -489,12 +499,11 @@ var AST_Defun = DEFNODE("Defun", null, {
}, AST_Lambda);
/* -----[ DESTRUCTURING ]----- */
var AST_Destructuring = DEFNODE("Destructuring", "names is_array default", {
var AST_Destructuring = DEFNODE("Destructuring", "names is_array", {
$documentation: "A destructuring of several names. Used in destructuring assignment and with destructuring function argument names",
$propdoc: {
"names": "[AST_Destructuring|AST_Expansion|AST_Hole|AST_ObjectKeyVal|AST_Symbol] Array of properties or elements",
"is_array": "[Boolean] Whether the destructuring represents an object or array",
"default": "[AST_Node?] Default assign value"
"names": "[AST_Node*] Array of properties or elements",
"is_array": "[Boolean] Whether the destructuring represents an object or array"
},
_walk: function(visitor) {
return visitor._visit(this, function(){
@@ -672,7 +681,7 @@ var AST_Try = DEFNODE("Try", "bcatch bfinally", {
var AST_Catch = DEFNODE("Catch", "argname", {
$documentation: "A `catch` node; only makes sense as part of a `try` statement",
$propdoc: {
argname: "[AST_SymbolCatch] symbol for the exception"
argname: "[AST_SymbolCatch|AST_Destructuring|AST_Expansion|AST_DefaultAssign] symbol for the exception"
},
_walk: function(visitor) {
return visitor._visit(this, function(){
@@ -957,6 +966,10 @@ var AST_Assign = DEFNODE("Assign", null, {
$documentation: "An assignment expression — `a = b + 5`",
}, AST_Binary);
var AST_DefaultAssign = DEFNODE("DefaultAssign", null, {
$documentation: "A default assignment expression like in `(a = 3) => a`"
}, AST_Binary);
/* -----[ LITERALS ]----- */
var AST_Array = DEFNODE("Array", "elements", {
@@ -991,8 +1004,7 @@ var AST_ObjectProperty = DEFNODE("ObjectProperty", "key value", {
$documentation: "Base class for literal object properties",
$propdoc: {
key: "[string|AST_Node] the property name converted to a string for ObjectKeyVal. For setters, getters and computed property this is an arbitrary AST_Node",
value: "[AST_Node] property value. For setters and getters this is an AST_Function.",
default: "[AST_Expression] The default for this parameter, only used when nested inside a binding pattern"
value: "[AST_Node] property value. For setters and getters this is an AST_Function."
},
_walk: function(visitor) {
return visitor._visit(this, function(){
@@ -1003,11 +1015,10 @@ var AST_ObjectProperty = DEFNODE("ObjectProperty", "key value", {
}
});
var AST_ObjectKeyVal = DEFNODE("ObjectKeyVal", "quote default", {
var AST_ObjectKeyVal = DEFNODE("ObjectKeyVal", "quote", {
$documentation: "A key: value object property",
$propdoc: {
quote: "[string] the original quote character",
default: "[AST_Expression] The default parameter value, only used when nested inside a binding pattern"
quote: "[string] the original quote character"
}
}, AST_ObjectProperty);
@@ -1066,19 +1077,13 @@ var AST_ClassExpression = DEFNODE("ClassExpression", null, {
$documentation: "A class expression."
}, AST_Class);
var AST_Symbol = DEFNODE("Symbol", "scope name thedef default", {
var AST_Symbol = DEFNODE("Symbol", "scope name thedef", {
$propdoc: {
name: "[string] name of this symbol",
scope: "[AST_Scope/S] the current scope (not necessarily the definition scope)",
thedef: "[SymbolDef/S] the definition of this symbol",
default: "[AST_Expression] The default parameter value, only used when nested inside a binding pattern"
thedef: "[SymbolDef/S] the definition of this symbol"
},
$documentation: "Base class for all symbols",
_walk: function (visitor) {
return visitor._visit(this, function() {
if (this.default) this.default._walk(visitor);
});
}
$documentation: "Base class for all symbols"
});
var AST_NewTarget = DEFNODE("NewTarget", null, {

View File

@@ -74,6 +74,7 @@ function Compressor(options, false_by_default) {
pure_funcs : null,
negate_iife : !false_by_default,
screw_ie8 : true,
ecma : 5,
drop_console : false,
angular : false,
warnings : true,
@@ -2317,7 +2318,7 @@ merge(Compressor.prototype, {
if (!fun) return self;
var args = fun.argnames.map(function(arg, i){
return make_node(AST_String, self.args[i], {
value: arg.print_to_string()
value: arg.print_to_string({ecma: compressor.option("ecma")})
});
});
var code = OutputStream();
@@ -2886,6 +2887,22 @@ merge(Compressor.prototype, {
return self;
});
OPT(AST_DefaultAssign, function(self, compressor){
if (!compressor.option("evaluate")) {
return self;
}
var evaluateRight = self.right.evaluate(compressor);
// `[x = undefined] = foo` ---> `[x] = foo`
if (evaluateRight.length > 1 && evaluateRight[1] === undefined) {
self = self.left;
} else {
self.right = evaluateRight[0];
}
return self;
});
OPT(AST_Conditional, function(self, compressor){
if (!compressor.option("conditionals")) return self;
if (self.condition instanceof AST_Seq) {

View File

@@ -641,13 +641,10 @@ function OutputStream(options) {
|| p instanceof AST_PropAccess // (1, {foo:2}).foo or (1, {foo:2})["foo"] ==> 2
|| p instanceof AST_Array // [ 1, (2, 3), 4 ] ==> [ 1, 3, 4 ]
|| p instanceof AST_ObjectProperty // { foo: (1, 2) }.foo ==> 2
|| (p instanceof AST_SymbolConst && p.default === this) // const { xCover = (0, function() {}) }
|| (p instanceof AST_SymbolLet && p.default === this) // let { xCover = (0, function() {}) }
|| (p instanceof AST_SymbolVar && p.default === this) // var { xCover = (0, function() {}) }
|| (p instanceof AST_SymbolCatch && p.default === this) // } catch (xCover = (0, function() {}) ) {
|| p instanceof AST_Conditional /* (false, true) ? (a = 10, b = 20) : (c = 30)
* ==> 20 (side effect, set a := 10 and b := 20) */
|| p instanceof AST_Arrow // x => (x, x)
|| p instanceof AST_DefaultAssign // x => (x = (0, function(){}))
;
});
@@ -761,6 +758,9 @@ function OutputStream(options) {
// (a = foo)["prop"] —or— (a = foo).prop
if (p instanceof AST_PropAccess && p.expression === this)
return true;
// ({a, b} = {a: 1, b: 2}), a destructuring assignment
if (this instanceof AST_Assign && this.left instanceof AST_Destructuring && this.left.is_array === false)
return true;
});
/* -----[ PRINTERS ]----- */
@@ -789,12 +789,6 @@ function OutputStream(options) {
output.comma();
})
output.print(self.is_array ? "]" : "}");
if (self.default) {
output.space();
output.print('=');
output.space();
self.default.print(output)
}
});
DEFPRINT(AST_Debugger, function(self, output){
@@ -1003,7 +997,7 @@ function OutputStream(options) {
parent instanceof AST_Unary ||
(parent instanceof AST_Call && self === parent.expression);
if (needs_parens) { output.print("(") }
if (self.argnames.length === 1 && self.argnames[0] instanceof AST_Symbol && !self.argnames[0].default) {
if (self.argnames.length === 1 && self.argnames[0] instanceof AST_Symbol) {
self.argnames[0].print(output);
} else {
output.with_parens(function(){
@@ -1497,15 +1491,27 @@ function OutputStream(options) {
});
DEFPRINT(AST_ObjectKeyVal, function(self, output){
function get_name(self) {
var def = self.value.definition();
return def ? def.mangled_name || def.name : self.value.name;
var def = self.definition();
return def ? def.mangled_name || def.name : self.name;
}
if (output.option("shorthand") &&
var allowShortHand = output.option("shorthand");
if (allowShortHand &&
self.value instanceof AST_Symbol &&
is_identifier_string(self.key) &&
get_name(self) === self.key
get_name(self.value) === self.key
) {
self.print_property_name(self.key, self.quote, output);
} else if (allowShortHand &&
self.value instanceof AST_DefaultAssign &&
self.value.left instanceof AST_Symbol &&
is_identifier_string(self.key) &&
get_name(self.value.left) === self.key
) {
self.print_property_name(self.key, self.quote, output);
output.print("=");
self.value.right.print(output);
} else {
if (!(self.key instanceof AST_Node)) {
self.print_property_name(self.key, self.quote, output);
@@ -1517,12 +1523,6 @@ function OutputStream(options) {
output.colon();
self.value.print(output);
}
if (self.default) {
output.space();
output.print('=');
output.space();
self.default.print(output);
}
});
AST_ObjectProperty.DEFMETHOD("_print_getter_setter", function(type, self, output) {
if (self.static) {
@@ -1573,12 +1573,6 @@ function OutputStream(options) {
});
DEFPRINT(AST_SymbolDeclaration, function(self, output){
self._do_print(output);
if (self.default) {
output.space();
output.print('=');
output.space();
self.default.print(output)
}
});
DEFPRINT(AST_Undefined, function(self, output){
output.print("void 0");

View File

@@ -1106,11 +1106,9 @@ function parse($TEXT, options) {
case "export":
return export_();
default:
unexpected();
}
}
unexpected();
});
function labeled_statement() {
@@ -1384,7 +1382,13 @@ function parse($TEXT, options) {
if (is("operator", "=") && expand === false) {
used_parameters.mark_default_assignment(S.token);
next();
param.default = expression(false);
param = new AST_DefaultAssign({
start: param.start,
left: param,
operator: "=",
right: expression(false),
end: S.token
});
}
if (expand !== false) {
@@ -1458,7 +1462,13 @@ function parse($TEXT, options) {
if (is("operator", "=") && is_expand === false) {
used_parameters.mark_default_assignment(S.token);
next();
elements[elements.length - 1].default = expression(false);
elements[elements.length - 1] = new AST_DefaultAssign({
start: elements[elements.length - 1].start,
left: elements[elements.length - 1],
operator: "=",
right: expression(false),
end: S.token
});
}
if (is_expand) {
if (!is("punc", "]")) {
@@ -1489,10 +1499,15 @@ function parse($TEXT, options) {
}
if (is("name") && (is_token(peek(), "punc") || is_token(peek(), "operator")) && [",", "}", "="].indexOf(peek().value) !== -1) {
used_parameters.add_parameter(S.token);
elements.push(new symbol_type({
start: S.token,
name: S.token.value,
end: S.token
elements.push(new AST_ObjectKeyVal({
start: prev(),
key: S.token.value,
value: new symbol_type({
start: S.token,
name: S.token.value,
end: S.token
}),
end: prev()
}));
next();
} else if (is("punc", "}")) {
@@ -1503,9 +1518,14 @@ function parse($TEXT, options) {
if (property === null) {
unexpected(prev());
} else if (prev().type === "name" && !is("punc", ":")) {
elements.push(new AST_SymbolFunarg({
elements.push(new AST_ObjectKeyVal({
start: prev(),
name: property_token,
key: property,
value: new symbol_type({
start: prev(),
name: property,
end: prev()
}),
end: prev()
}));
} else {
@@ -1513,16 +1533,22 @@ function parse($TEXT, options) {
elements.push(new AST_ObjectKeyVal({
start: property_token,
quote: property_token.quote,
end: prev(),
key: property,
value: binding_element(used_parameters, symbol_type)
value: binding_element(used_parameters, symbol_type),
end: prev()
}));
}
}
if (is("operator", "=")) {
used_parameters.mark_default_assignment(S.token);
next();
elements[elements.length - 1].default = expression(false);
elements[elements.length - 1].value = new AST_DefaultAssign({
start: elements[elements.length - 1].value.start,
left: elements[elements.length - 1].value,
operator: "=",
right: expression(false),
end: S.token
});
}
}
expect("}");
@@ -1558,7 +1584,7 @@ function parse($TEXT, options) {
next();
a.push(new AST_Expansion({
start: prev(),
expression: binding_element(undefined, AST_SymbolFunarg),
expression: expression(false),
end: S.token,
}));
if (!is("punc", ")")) {
@@ -1943,7 +1969,7 @@ function parse($TEXT, options) {
a.push(new AST_Hole({ start: S.token, end: S.token }));
} else if (is("expand", "...")) {
next();
a.push(new AST_Expansion({start: S.token, expression: expression(),end: S.token}));
a.push(new AST_Expansion({start: prev(), expression: expression(),end: S.token}));
} else {
a.push(expression(false));
}
@@ -1970,63 +1996,51 @@ function parse($TEXT, options) {
start = S.token;
var type = start.type;
var name = as_property_name();
var value;
// Check property and fetch value
if (!is("punc", ":")) {
var concise = concise_method_or_getset(name, start);
if (concise) {
a.push(concise);
continue;
}
if (!(start.type !== name)) {
unexpected(S.token);
}
value = new AST_SymbolRef({
start: prev(),
name: name,
end: prev()
});
} else if (name === null) {
unexpected(prev());
} else {
next(); // `:` - see first condition
value = expression(false);
}
if (type == "punc" && start.value == "[") {
expect(":");
a.push(new AST_ObjectKeyVal({
start: start,
key: name,
value: expression(false),
end: prev()
}));
continue;
}
// Check for default value and alter value accordingly if necessary
if (is("operator", "=")) {
next();
a.push(new AST_Assign({
value = new AST_Assign({
start: start,
// Symbol class doesn't matter. This is only meant to carry the symbol name into .as_params() since this is not normally valid.
left: new AST_SymbolRef({
start: start,
end: start,
name: name
}),
left: value,
operator: "=",
right: expression(false),
end: prev()
}));
} else if (!is("punc", ":")) {
// It's one of those object destructurings, the value is its own name
a.push(new AST_ObjectKeyVal({
start: start,
key: name,
value: new AST_SymbolRef({
start: start,
end: prev(),
name: name
}),
end: start,
}));
} else {
expect(":");
a.push(new AST_ObjectKeyVal({
start : start,
quote : start.quote,
key : name,
value : expression(false),
end : prev()
}));
});
}
// Create property
a.push(new AST_ObjectKeyVal({
start: start,
quote: start.quote,
key: name,
value: value,
end: prev()
}));
}
next();
return new AST_Object({ properties: a })
@@ -2493,6 +2507,55 @@ function parse($TEXT, options) {
if (is("operator") && ASSIGNMENT(val)) {
if (is_assignable(left)) {
var walk = function(node) {
var newNode;
if (node instanceof AST_Object) {
newNode = new AST_Destructuring({
start: node.start,
names: node.properties.map(walk),
is_array: false,
end: node.end
});
node = newNode;
} else if (node instanceof AST_Array) {
var names = [];
for (var i = 0; i < node.elements.length; i++) {
// Only allow expansion as last element
if (node.elements[i] instanceof AST_Expansion) {
if (i + 1 !== node.elements.length) {
token_error(node.elements[i].start, "SyntaxError: Spread must the be last element in destructuring array");
}
node.elements[i].expression = walk(node.elements[i].expression);
}
names.push(walk(node.elements[i]));
}
newNode = new AST_Destructuring({
start: node.start,
names: names,
is_array: true,
end: node.end
});
node = newNode;
} else if (node instanceof AST_ObjectProperty) {
node.value = walk(node.value);
} else if (node instanceof AST_Assign) {
node = new AST_DefaultAssign({
start: node.start,
left: node.left,
operator: "=",
right: node.right,
end: node.end
});
}
return node;
}
left = walk(left);
next();
return new AST_Assign({
start : start,