From 22258923488242498ea504482dcb8fe56d88371b Mon Sep 17 00:00:00 2001 From: Peeyush Kushwaha Date: Tue, 25 Jul 2017 00:13:17 +0530 Subject: [PATCH] Use first binding for multiple var declarations (#5745) * Use first binding for multiple var declarations Since var declarations after initial binding have no effect, use the first declaration. Fixes #2378 * Include hoisted function bindings * Missing newline in expected.js * Simplify constantViolations in new Binding on existing * clarify comment language --- packages/babel-traverse/src/scope/binding.js | 16 ++++++++-------- .../rename/parameter-redeclaration/actual.js | 11 +++++++++++ .../rename/parameter-redeclaration/expected.js | 12 ++++++++++++ .../rename/parameter-redeclaration/options.json | 3 +++ .../rename/parameter-redeclaration/plugin.js | 9 +++++++++ 5 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/actual.js create mode 100644 packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/expected.js create mode 100644 packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/options.json create mode 100644 packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/plugin.js diff --git a/packages/babel-traverse/src/scope/binding.js b/packages/babel-traverse/src/scope/binding.js index 694becff86..3c5c645dfb 100644 --- a/packages/babel-traverse/src/scope/binding.js +++ b/packages/babel-traverse/src/scope/binding.js @@ -13,6 +13,14 @@ import type NodePath from "../path"; export default class Binding { constructor({ existing, identifier, scope, path, kind }) { + // this if condition is true only if the re-binding does not result in an error + // e.g. if rebinding kind is 'var' or 'hoisted', and previous was 'param' + if (existing) { + // we maintain the original binding but update constantViolations + existing.constantViolations = existing.constantViolations.concat(path); + return existing; + } + this.identifier = identifier; this.scope = scope; this.path = path; @@ -26,14 +34,6 @@ export default class Binding { this.references = 0; this.clearValue(); - - if (existing) { - this.constantViolations = [].concat( - existing.path, - existing.constantViolations, - this.constantViolations, - ); - } } constantViolations: Array; diff --git a/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/actual.js b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/actual.js new file mode 100644 index 0000000000..9f0435f951 --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/actual.js @@ -0,0 +1,11 @@ +function f(a, b) { + var a = "redeclared"; + return b; +} + +function g(a) { + function a() { + return "function, redeclared"; + } + return a(); +} diff --git a/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/expected.js b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/expected.js new file mode 100644 index 0000000000..64957cbe2c --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/expected.js @@ -0,0 +1,12 @@ +function f(z, b) { + var z = "redeclared"; + return b; +} + +function g(z) { + function z() { + return "function, redeclared"; + } + + return z(); +} diff --git a/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/options.json b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/options.json new file mode 100644 index 0000000000..14af0e5fea --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["./plugin"] +} diff --git a/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/plugin.js b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/plugin.js new file mode 100644 index 0000000000..4cc132b829 --- /dev/null +++ b/packages/babel-traverse/test/fixtures/rename/parameter-redeclaration/plugin.js @@ -0,0 +1,9 @@ +module.exports = function() { + return { + visitor: { + Function(path) { + path.scope.rename("a", "z"); + } + } + }; +}