Merge pull request #1167 from rollup/gh-1078

deshadow destructured parameters with assignments

.travis.yml

@@ -1,7 +1,6 @@
 sudo: false
 language: node_js
 node_js:
-  - "0.12"
   - "4"
   - "6"
 env:

appveyor.yml

@@ -10,7 +10,6 @@ init:
 environment:
   matrix:
     # node.js
-    - nodejs_version: 0.12
     - nodejs_version: 4
     - nodejs_version: 6
 

package.json

@@ -48,13 +48,14 @@
   "homepage": "https://github.com/rollup/rollup",
   "devDependencies": {
     "acorn": "^4.0.1",
-    "buble": "^0.12.5",
+    "buble": "^0.15.1",
     "chalk": "^1.1.3",
     "codecov.io": "^0.1.6",
     "console-group": "^0.3.1",
-    "eslint": "^2.13.0",
+    "eslint": "^2.13.1",
-    "eslint-plugin-import": "^1.14.0",
+    "eslint-plugin-import": "^2.2.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';