rename shorthand properties correctly

sokra · sokra · commit ec966354ca0b · 2017-06-13T13:38:12.000+02:00
fixes webpack#5027
diff --git a/lib/optimize/ConcatenatedModule.js b/lib/optimize/ConcatenatedModule.js
@@ -83,6 +83,49 @@ function reduceSet(a, b) {
 	return a;
 }
 
+function getPathInAst(ast, node) {
+	if(ast === node) {
+		return [];
+	}
+	const nr = node.range;
+	var i;
+	if(Array.isArray(ast)) {
+		for(i = 0; i < ast.length; i++) {
+			const enterResult = enterNode(ast[i]);
+			if(typeof enterResult !== "undefined")
+				return enterResult;
+		}
+	} else if(ast && typeof ast === "object") {
+		const keys = Object.keys(ast);
+		for(i = 0; i < keys.length; i++) {
+			const value = ast[keys[i]];
+			if(Array.isArray(value)) {
+				const pathResult = getPathInAst(value, node);
+				if(typeof pathResult !== "undefined")
+					return pathResult;
+			} else if(value && typeof value === "object") {
+				const enterResult = enterNode(value);
+				if(typeof enterResult !== "undefined")
+					return enterResult;
+			}
+		}
+	}
+
+	function enterNode(n) {
+		const r = n.range;
+		if(r) {
+			if(r[0] <= nr[0] && r[1] >= nr[1]) {
+				const path = getPathInAst(n, node);
+				if(path) {
+					path.push(n);
+					return path;
+				}
+			}
+		}
+		return undefined;
+	}
+}
+
 class ConcatenatedModule extends Module {
 	constructor(rootModule, modules) {
 		super();
@@ -190,6 +233,7 @@ class ConcatenatedModule extends Module {
 			return {
 				module: m,
 				index: idx,
+				ast: undefined,
 				source: undefined,
 				globalScope: undefined,
 				moduleScope: undefined,
@@ -259,6 +303,7 @@ class ConcatenatedModule extends Module {
 			const globalScope = scopeManager.acquire(ast);
 			const moduleScope = globalScope.childScopes[0];
 			const resultSource = new ReplaceSource(source);
+			info.ast = ast;
 			info.source = resultSource;
 			info.globalScope = globalScope;
 			info.moduleScope = moduleScope;
@@ -298,7 +343,12 @@ class ConcatenatedModule extends Module {
 					const allIdentifiers = new Set(references.map(r => r.identifier).concat(variable.identifiers));
 					for(const identifier of allIdentifiers) {
 						const r = identifier.range;
-						source.replace(r[0], r[1] - 1, newName);
+						const path = getPathInAst(info.ast, identifier);
+						if(path && path.length > 1 && path[1].type === "Property" && path[1].shorthand) {
+							source.insert(r[1], `: ${newName}`);
+						} else {
+							source.replace(r[0], r[1] - 1, newName);
+						}
 					}
 				} else {
 					allUsedNames.add(name);
diff --git a/test/cases/scope-hoisting/renaming-shorthand-5027/file1.js b/test/cases/scope-hoisting/renaming-shorthand-5027/file1.js
@@ -0,0 +1 @@
+export var test = "test1";
diff --git a/test/cases/scope-hoisting/renaming-shorthand-5027/file2.js b/test/cases/scope-hoisting/renaming-shorthand-5027/file2.js
@@ -0,0 +1,2 @@
+var [ test ] = [ "test2" ];
+export { test }
diff --git a/test/cases/scope-hoisting/renaming-shorthand-5027/file3.js b/test/cases/scope-hoisting/renaming-shorthand-5027/file3.js
@@ -0,0 +1,2 @@
+var { test } = { test: "test3" };
+export { test }
diff --git a/test/cases/scope-hoisting/renaming-shorthand-5027/file4.js b/test/cases/scope-hoisting/renaming-shorthand-5027/file4.js
@@ -0,0 +1,2 @@
+var {o:[{ test }]} = {o:[{ test: "test4" }]};
+export { test }
diff --git a/test/cases/scope-hoisting/renaming-shorthand-5027/index.js b/test/cases/scope-hoisting/renaming-shorthand-5027/index.js
@@ -0,0 +1,25 @@
+import m from "./module";
+
+it("should apply shorthand properties correctly when renaming", function() {
+	m.should.be.eql({
+		obj: {
+			test: "test1",
+			test2: "test2",
+			test3: "test3",
+			test4: "test4"
+		},
+		nested: {
+			array: [{
+				test: "test1",
+				test2: "test2",
+				test3: "test3",
+				test4: "test4"
+			}]
+		},
+		test: "test1",
+		test2: "test2",
+		test3: "test3",
+		test4: "test4",
+		f: ["test2", "test2", "test3"]
+	})
+});
diff --git a/test/cases/scope-hoisting/renaming-shorthand-5027/module.js b/test/cases/scope-hoisting/renaming-shorthand-5027/module.js
@@ -0,0 +1,21 @@
+import { test } from './file1';
+import { test as test2 } from './file2';
+import { test as test3 } from './file3';
+import { test as test4 } from './file4';
+
+var obj = { test, test2, test3, test4 };
+var nested = { array: [ { test, test2, test3, test4 }]};
+
+function f(test = test2, { test2: t2 } = { test2 }, { t3 = test3 } = {}) {
+	return [test, t2, t3];
+}
+
+export default {
+	obj,
+	nested,
+	test,
+	test2,
+	test3,
+	test4,
+	f: f()
+};

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+var [ test ] = [ "test2" ];`
	`2`	`+export { test }`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+var { test } = { test: "test3" };`
	`2`	`+export { test }`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+var {o:[{ test }]} = {o:[{ test: "test4" }]};`
	`2`	`+export { test }`