From 2b42773e01726fa080057bed9382f71ea64e8482 Mon Sep 17 00:00:00 2001 From: Sebastian McKenzie Date: Mon, 11 May 2015 22:08:38 +0100 Subject: [PATCH] explode duplicate identifiers in export/import specifiers and property shorthand - fixes #1458 --- src/babel/generation/generators/modules.js | 4 +- src/babel/generation/generators/types.js | 10 ++++- .../transformers/es6/properties.shorthand.js | 3 -- .../transformation/transformers/index.js | 1 + .../transformers/internal/explode.js | 18 ++++++++ .../transformers/internal/modules.js | 3 +- .../transformers/spec/function-name.js | 11 +++-- src/babel/traversal/scope.js | 41 +------------------ src/babel/types/validators.js | 2 +- .../shorthand-property/actual.js | 11 +++++ .../shorthand-property/expected.js | 13 ++++++ 11 files changed, 66 insertions(+), 51 deletions(-) create mode 100644 src/babel/transformation/transformers/internal/explode.js create mode 100644 test/core/fixtures/transformation/spec.function-name/shorthand-property/actual.js create mode 100644 test/core/fixtures/transformation/spec.function-name/shorthand-property/expected.js diff --git a/src/babel/generation/generators/modules.js b/src/babel/generation/generators/modules.js index 8c606d35b7..833b4ca2e8 100644 --- a/src/babel/generation/generators/modules.js +++ b/src/babel/generation/generators/modules.js @@ -3,7 +3,7 @@ import * as t from "../../types"; export function ImportSpecifier(node, print) { print(node.imported); - if (node.local && node.local !== node.imported) { + if (node.local && node.local.name !== node.imported.name) { this.push(" as "); print(node.local); } @@ -19,7 +19,7 @@ export function ExportDefaultSpecifier(node, print) { export function ExportSpecifier(node, print) { print(node.local); - if (node.exported && node.local !== node.exported) { + if (node.exported && node.local.name !== node.exported.name) { this.push(" as "); print(node.exported); } diff --git a/src/babel/generation/generators/types.js b/src/babel/generation/generators/types.js index c686dc7acf..391a7b273f 100644 --- a/src/babel/generation/generators/types.js +++ b/src/babel/generation/generators/types.js @@ -1,4 +1,5 @@ import each from "lodash/collection/each"; +import * as t from "../../types"; export function Identifier(node) { this.push(node.name); @@ -39,7 +40,14 @@ export function Property(node, print) { this.push("]"); } else { print(node.key); - if (node.shorthand) return; + + // shorthand! + if (node.shorthand && + (t.isIdentifier(node.key) && + t.isIdentifier(node.value) && + node.key.name === node.value.name)) { + return; + } } this.push(":"); diff --git a/src/babel/transformation/transformers/es6/properties.shorthand.js b/src/babel/transformation/transformers/es6/properties.shorthand.js index b5140ac364..e25f43c8ab 100644 --- a/src/babel/transformation/transformers/es6/properties.shorthand.js +++ b/src/babel/transformation/transformers/es6/properties.shorthand.js @@ -1,5 +1,3 @@ -import * as t from "../../../types"; - export function Property(node) { if (node.method) { node.method = false; @@ -7,6 +5,5 @@ export function Property(node) { if (node.shorthand) { node.shorthand = false; - node.key = t.removeComments(t.clone(node.key)); } } diff --git a/src/babel/transformation/transformers/index.js b/src/babel/transformation/transformers/index.js index c67b4aeb70..e6231da863 100644 --- a/src/babel/transformation/transformers/index.js +++ b/src/babel/transformation/transformers/index.js @@ -1,5 +1,6 @@ export default { //- builtin-setup + _explode: require("./internal/explode"), _validation: require("./internal/validation"), _hoistDirectives: require("./internal/hoist-directives"), "minification.removeDebugger": require("./minification/remove-debugger"), diff --git a/src/babel/transformation/transformers/internal/explode.js b/src/babel/transformation/transformers/internal/explode.js new file mode 100644 index 0000000000..e26bc44634 --- /dev/null +++ b/src/babel/transformation/transformers/internal/explode.js @@ -0,0 +1,18 @@ +import clone from "lodash/lang/clone"; +import * as t from "../../../types"; + +export var metadata = { + group: "builtin-setup" +}; + +function buildClone(bindingKey, refKey) { + return function (node) { + if (node[bindingKey] === node[refKey]) { + node[refKey] = t.removeComments(clone(node[refKey])); + } + }; +} + +export var Property = buildClone("value", "key"); +export var ExportSpecifier = buildClone("local", "exported"); +export var ImportSpecifier = buildClone("local", "imported"); diff --git a/src/babel/transformation/transformers/internal/modules.js b/src/babel/transformation/transformers/internal/modules.js index ac64b0e455..75bc4f6edb 100644 --- a/src/babel/transformation/transformers/internal/modules.js +++ b/src/babel/transformation/transformers/internal/modules.js @@ -4,6 +4,7 @@ // a generator function as a default then regenerator will destroy the export // declaration and leave a variable declaration in it's place... yeah, handy. +import clone from "lodash/lang/clone"; import * as t from "../../../types"; export var metadata = { @@ -75,7 +76,7 @@ export function ExportNamedDeclaration(node, parent, scope) { var bindings = this.get("declaration").getBindingIdentifiers(); for (var key in bindings) { var id = bindings[key]; - specifiers.push(t.exportSpecifier(id, id)); + specifiers.push(t.exportSpecifier(clone(id), clone(id))); } return [declar, t.exportNamedDeclaration(null, specifiers)]; } diff --git a/src/babel/transformation/transformers/spec/function-name.js b/src/babel/transformation/transformers/spec/function-name.js index 2b384fcddb..dc5e21085b 100644 --- a/src/babel/transformation/transformers/spec/function-name.js +++ b/src/babel/transformation/transformers/spec/function-name.js @@ -1,8 +1,11 @@ +import { bare } from "../../helpers/name-method"; + export var metadata = { group: "builtin-setup" }; -export { - bare as FunctionExpression, - bare as ArrowFunctionExpression -} from "../../helpers/name-method"; +export var FunctionExpression = { + exit: bare +}; + +export { FunctionExpression as ArrowFunctionExpression }; diff --git a/src/babel/traversal/scope.js b/src/babel/traversal/scope.js index db5681f2c6..0facdf60ea 100644 --- a/src/babel/traversal/scope.js +++ b/src/babel/traversal/scope.js @@ -90,50 +90,13 @@ var blockVariableVisitor = explode({ var renameVisitor = explode({ ReferencedIdentifier(node, parent, scope, state) { - if (node.name !== state.oldName) return; - - if (this.parentPath.isProperty() && this.key === "key" && parent.shorthand) { - var value = t.identifier(state.newName);; - - if (parent.value === state.binding) { - state.info.identifier = state.binding = value; - } - - parent.shorthand = false; - parent.value = value; - parent.key = t.identifier(state.oldName); - } else { + if (node.name === state.oldName) { node.name = state.newName; } }, Declaration(node, parent, scope, state) { - var ids = {}; - - var matchesLocal = (node, key) => { - return node.local === node[key] && (node.local.name === state.oldName || node.local.name === state.newName); - }; - - if (this.isExportDeclaration() && this.has("specifiers")) { - var specifiers = this.get("specifiers"); - for (var specifier of (specifiers: Array)) { - if (specifier.isExportSpecifier() && matchesLocal(specifier.node, "exported")) { - specifier.get("exported").replaceWith(t.identifier(state.oldName)); - } - } - } else if (this.isImportDeclaration() && this.has("specifiers")) { - var specifiers = this.get("specifiers"); - for (var specifier of (specifiers: Array)) { - if (specifier.isImportSpecifier() && matchesLocal(specifier.node, "imported")) { - state.binding = state.info.identifier = t.identifier(state.newName); - specifier.get("local").replaceWith(state.binding); - } else { - extend(ids, specifier.getBindingIdentifiers()); - } - } - } else { - ids = this.getBindingIdentifiers(); - } + var ids = this.getBindingIdentifiers();; for (var name in ids) { if (name === state.oldName) ids[name].name = state.newName; diff --git a/src/babel/types/validators.js b/src/babel/types/validators.js index 997c7bfb6e..660d597103 100644 --- a/src/babel/types/validators.js +++ b/src/babel/types/validators.js @@ -29,7 +29,7 @@ export function isReferenced(node: Object, parent: Object): boolean { // no: { NODE: "" } case "Property": if (parent.key === node) { - return parent.computed || parent.shorthand; + return parent.computed; } // no: var NODE = init; diff --git a/test/core/fixtures/transformation/spec.function-name/shorthand-property/actual.js b/test/core/fixtures/transformation/spec.function-name/shorthand-property/actual.js new file mode 100644 index 0000000000..2c06014fa3 --- /dev/null +++ b/test/core/fixtures/transformation/spec.function-name/shorthand-property/actual.js @@ -0,0 +1,11 @@ +var Utils = { + get: function() {} +}; + +var { get } = Utils; + +var bar = { + get: function(arg) { + get(arg, "baz"); + } +}; diff --git a/test/core/fixtures/transformation/spec.function-name/shorthand-property/expected.js b/test/core/fixtures/transformation/spec.function-name/shorthand-property/expected.js new file mode 100644 index 0000000000..374fc7c053 --- /dev/null +++ b/test/core/fixtures/transformation/spec.function-name/shorthand-property/expected.js @@ -0,0 +1,13 @@ +"use strict"; + +var Utils = { + get: function get() {} +}; + +var _get = Utils.get; + +var bar = { + get: function get(arg) { + _get(arg, "baz"); + } +};