Merge pull request #528 from rollup/gh-527

Make sure shorthand destructuring assignments don't break

src/Statement.js

@@ -44,7 +44,7 @@ export default class Statement {
 		let readDepth = 0;
 
 		walk( this.node, {
-			enter ( node, parent ) {
+			enter ( node, parent, prop ) {
 				// warn about eval
 				if ( node.type === 'CallExpression' && node.callee.name === 'eval' && !scope.contains( 'eval' ) ) {
 					module.bundle.onwarn( `Use of \`eval\` (in ${module.id}) is discouraged, as it may cause issues with minification. See https://github.com/rollup/rollup/wiki/Troubleshooting#avoiding-eval for more details` );
@@ -61,15 +61,6 @@ export default class Statement {
 				if ( node._scope ) scope = node._scope;
 				if ( /Function/.test( node.type ) ) readDepth += 1;
 
-				// special case – shorthand properties. because node.key === node.value,
-				// we can't differentiate once we've descended into the node
-				if ( node.type === 'Property' && node.shorthand && parent.type !== 'ObjectPattern' ) {
-					const reference = new Reference( node.key, scope );
-					reference.isShorthandProperty = true; // TODO feels a bit kludgy
-					references.push( reference );
-					return this.skip();
-				}
-
 				let isReassignment;
 
 				if ( parent && isModifierNode( parent ) ) {
@@ -106,9 +97,19 @@ export default class Statement {
 						scope.parent :
 						scope;
 
+					const isShorthandProperty = parent.type === 'Property' && parent.shorthand;
+
+					// Since `node.key` can equal `node.value` for shorthand properties
+					// we must use the `prop` argument provided by `estree-walker` to determine
+					// if we're looking at the key or the value.
+					// If they are equal, we'll return to not create duplicate references.
+					if ( isShorthandProperty && parent.value === parent.key && prop === 'value' ) {
+						return;
+					}
+
 					const reference = new Reference( node, referenceScope, statement );
 					reference.isReassignment = isReassignment;
-
+					reference.isShorthandProperty = isShorthandProperty;
 					references.push( reference );
 
 					this.skip(); // don't descend from `foo.bar.baz` into `foo.bar`

test/function/object-destructuring-renaming/_config.js

@@ -0,0 +1,12 @@
+var assert = require( 'assert' );
+
+module.exports = {
+	description: 'renaming destructured object properties should request the correct property (#527)',
+
+	// we must transpile the object destructuring to test it
+	babel: true,
+
+	exports: function ( exports ) {
+		assert.equal( exports.env, process.env );
+	}
+};

test/function/object-destructuring-renaming/main.js

@@ -0,0 +1,3 @@
+import { getEnv } from './module.js';
+
+export var env = getEnv();

test/function/object-destructuring-renaming/module.js

@@ -0,0 +1,5 @@
+const { env } = process;
+
+export function getEnv() {
+	return env;
+}