Merge pull request #4881 from webpack/bugfix/destructing

support destructing on assignment

lib/Parser.js

@@ -636,6 +636,9 @@ class Parser extends Tapable {
 	}
 
 	walkFunctionDeclaration(statement) {
+		statement.params.forEach(param => {
+			this.walkPattern(param);
+		});
 		this.inScope(statement.params, function() {
 			if(statement.body.type === "BlockStatement") {
 				this.prewalkStatement(statement.body);
@@ -797,24 +800,15 @@ class Parser extends Tapable {
 			switch(declarator.type) {
 				case "VariableDeclarator":
 					{
-						const renameIdentifier = declarator.init && this.getRenameIdentifier(declarator.init);
-						if(renameIdentifier && declarator.id.type === "Identifier" && this.applyPluginsBailResult1("can-rename " + renameIdentifier, declarator.init)) {
-							// renaming with "var a = b;"
-							if(!this.applyPluginsBailResult1("rename " + renameIdentifier, declarator.init)) {
-								this.scope.renames["$" + declarator.id.name] = this.scope.renames["$" + renameIdentifier] || renameIdentifier;
-								const idx = this.scope.definitions.indexOf(declarator.id.name);
-								if(idx >= 0) this.scope.definitions.splice(idx, 1);
-							}
-						} else {
-							this.enterPattern(declarator.id, (name, decl) => {
-								if(!this.applyPluginsBailResult1("var-" + declarator.kind + " " + name, decl)) {
-									if(!this.applyPluginsBailResult1("var " + name, decl)) {
-										this.scope.renames["$" + name] = undefined;
+						this.enterPattern(declarator.id, (name, decl) => {
+							if(!this.applyPluginsBailResult1("var-" + declarator.kind + " " + name, decl)) {
+								if(!this.applyPluginsBailResult1("var " + name, decl)) {
+									this.scope.renames["$" + name] = undefined;
+									if(this.scope.definitions.indexOf(name) < 0)
 										this.scope.definitions.push(name);
-									}
 								}
-							});
-						}
+							}
+						});
 						break;
 					}
 			}
@@ -827,7 +821,14 @@ class Parser extends Tapable {
 				case "VariableDeclarator":
 					{
 						const renameIdentifier = declarator.init && this.getRenameIdentifier(declarator.init);
-						if(!renameIdentifier || declarator.id.type !== "Identifier" || !this.applyPluginsBailResult1("can-rename " + renameIdentifier, declarator.init)) {
+						if(renameIdentifier && declarator.id.type === "Identifier" && this.applyPluginsBailResult1("can-rename " + renameIdentifier, declarator.init)) {
+							// renaming with "var a = b;"
+							if(!this.applyPluginsBailResult1("rename " + renameIdentifier, declarator.init)) {
+								this.scope.renames["$" + declarator.id.name] = this.scope.renames["$" + renameIdentifier] || renameIdentifier;
+								const idx = this.scope.definitions.indexOf(declarator.id.name);
+								if(idx >= 0) this.scope.definitions.splice(idx, 1);
+							}
+						} else {
 							this.walkPattern(declarator.id);
 							if(declarator.init)
 								this.walkExpression(declarator.init);
@@ -845,6 +846,11 @@ class Parser extends Tapable {
 			this["walk" + pattern.type](pattern);
 	}
 
+	walkAssignmentPattern(pattern) {
+		this.walkExpression(pattern.right);
+		this.walkPattern(pattern.left);
+	}
+
 	walkObjectPattern(pattern) {
 		for(let i = 0, len = pattern.properties.length; i < len; i++) {
 			const prop = pattern.properties[i];
@@ -912,6 +918,9 @@ class Parser extends Tapable {
 	}
 
 	walkFunctionExpression(expression) {
+		expression.params.forEach(param => {
+			this.walkPattern(param);
+		});
 		this.inScope(expression.params, function() {
 			if(expression.body.type === "BlockStatement") {
 				this.prewalkStatement(expression.body);
@@ -923,6 +932,9 @@ class Parser extends Tapable {
 	}
 
 	walkArrowFunctionExpression(expression) {
+		expression.params.forEach(param => {
+			this.walkPattern(param);
+		});
 		this.inScope(expression.params, function() {
 			if(expression.body.type === "BlockStatement") {
 				this.prewalkStatement(expression.body);
@@ -993,8 +1005,10 @@ class Parser extends Tapable {
 			}
 		} else {
 			this.walkExpression(expression.right);
-			this.scope.renames["$" + expression.left.name] = undefined;
-			this.walkExpression(expression.left);
+			this.walkPattern(expression.left);
+			this.enterPattern(expression.left, (name, decl) => {
+				this.scope.renames["$" + name] = undefined;
+			});
 		}
 	}
 
@@ -1191,7 +1205,6 @@ class Parser extends Tapable {
 
 	enterAssignmentPattern(pattern, onIdent) {
 		this.enterPattern(pattern.left, onIdent);
-		this.walkExpression(pattern.right);
 	}
 
 	evaluateExpression(expression) {

test/cases/parsing/issue-3273/index.js

@@ -26,8 +26,13 @@ it("should hide import by pattern in function", function() {
 	}({ test: "ok" }));
 });
 
-it("should allow import in default", function() {
+it("should allow import in default (incorrect)", function() {
 	var { other = test, test } = { test: "ok" };
 	test.should.be.eql("ok");
+	(typeof other).should.be.eql("undefined");
+});
+
+it("should allow import in default", function() {
+	var { other = test } = { test: "ok" };
 	other.should.be.eql("test");
 });

test/cases/parsing/issue-4870/file.js

@@ -0,0 +1 @@
+export var test = "test";

test/cases/parsing/issue-4870/index.js

@@ -0,0 +1,13 @@
+import { test } from "./file";
+
+it("should allow import in array destructing", function() {
+	var other;
+	[other = test] = [];
+	other.should.be.eql("test");
+});
+
+it("should allow import in object destructing", function() {
+	var other;
+	({other = test} = {});
+	other.should.be.eql("test");
+});

test/cases/parsing/issue-4870/test.filter.js

@@ -0,0 +1,6 @@
+var supportsIteratorDestructuring = require("../../../helpers/supportsIteratorDestructuring");
+var supportsObjectDestructuring = require("../../../helpers/supportsObjectDestructuring");
+
+module.exports = function(config) {
+	return !config.minimize && supportsObjectDestructuring() && supportsIteratorDestructuring();
+};