deshadow destructured parameters with assignments (#1078)

package.json

@@ -55,6 +55,7 @@
     "eslint": "^2.13.0",
     "eslint-plugin-import": "^1.14.0",
     "estree-walker": "^0.2.1",
+    "is-reference": "^1.0.0",
     "istanbul": "^0.4.3",
     "magic-string": "^0.15.2",
     "minimist": "^1.2.0",

src/ast/nodes/Identifier.js

@@ -1,9 +1,24 @@
 import Node from '../Node.js';
-import isReference from '../utils/isReference.js';
+import isReference from 'is-reference';
+
+function isAssignmentPatternLhs ( node, parent ) {
+	// special case: `({ foo = 42 }) => {...}`
+	// `foo` actually has two different parents, the Property of the
+	// ObjectPattern, and the AssignmentPattern. In one case it's a
+	// reference, in one case it's not, because it's shorthand for
+	// `({ foo: foo = 42 }) => {...}`. But unlike a regular shorthand
+	// property, the `foo` node appears at different levels of the tree
+	return (
+		parent.type === 'Property' &&
+		parent.shorthand &&
+		parent.value.type === 'AssignmentPattern' &&
+		parent.value.left === node
+	);
+}
 
 export default class Identifier extends Node {
 	bind ( scope ) {
-		if ( isReference( this, this.parent ) ) {
+		if ( isReference( this, this.parent ) || isAssignmentPatternLhs( this, this.parent ) ) {
 			this.declaration = scope.findDeclaration( this.name );
 			this.declaration.addReference( this ); // TODO necessary?
 		}

src/ast/nodes/MemberExpression.js

@@ -1,6 +1,6 @@
+import isReference from 'is-reference';
 import getLocation from '../../utils/getLocation.js';
 import relativeId from '../../utils/relativeId.js';
-import isReference from '../utils/isReference.js';
 import Node from '../Node.js';
 import { UNKNOWN } from '../values.js';
 

src/ast/nodes/shared/callHasEffects.js

@@ -1,5 +1,5 @@
+import isReference from 'is-reference';
 import flatten from '../../utils/flatten.js';
-import isReference from '../../utils/isReference.js';
 import pureFunctions from './pureFunctions.js';
 import { UNKNOWN } from '../../values.js';
 

src/ast/utils/isReference.js

@@ -1,28 +0,0 @@
-export default function isReference ( node, parent ) {
-	if ( node.type === 'MemberExpression' ) {
-		return !node.computed && isReference( node.object, node );
-	}
-
-	if ( node.type === 'Identifier' ) {
-		// the only time we could have an identifier node without a parent is
-		// if it's the entire body of a function without a block statement –
-		// i.e. an arrow function expression like `a => a`
-		if ( !parent ) return true;
-
-		// TODO is this right?
-		if ( parent.type === 'MemberExpression' || parent.type === 'MethodDefinition' ) {
-			return parent.computed || node === parent.object;
-		}
-
-		// disregard the `bar` in `{ bar: foo }`, but keep it in `{ [bar]: foo }`
-		if ( parent.type === 'Property' ) return parent.computed || node === parent.value;
-
-		// disregard the `bar` in `class Foo { bar () {...} }`
-		if ( parent.type === 'MethodDefinition' ) return false;
-
-		// disregard the `bar` in `export { foo as bar }`
-		if ( parent.type === 'ExportSpecifier' && node !== parent.local ) return;
-
-		return true;
-	}
-}

test/function/deshadowed-destructured-parameter/_config.js

@@ -0,0 +1,4 @@
+module.exports = {
+	description: 'correctly deshadows destructured function parameters (#1008)',
+	buble: true
+};

test/function/deshadowed-destructured-parameter/main.js

@@ -0,0 +1,10 @@
+import { x } from './x.js';
+
+let y;
+
+function foo ({ x = 42 }) {
+	y = x;
+}
+
+foo({});
+assert.equal( y, 42 );

test/function/deshadowed-destructured-parameter/x.js

@@ -0,0 +1 @@
+export var x = 'whatever';