Variables used in computed method key are not scoped correctly

[overlookmotel]

Bug Report

• I would like to work on a fix!

Current behavior

Where an object/class method has a computed key containing a variable, and the method includes a variable with same name in its body or parameters, the scope of the variable in method key is incorrectly identified.

Babel determines the scope of the var in the method key to be inside the method's body, not outside as it actually is.

Input Code

https://codesandbox.io/s/babel-issue-12790-b9y6b

Input code:

const x = 'q';
const obj = {
  [x](x) {
    return x;
  }
};

Plugin:

() => ({
  visitor: {
    Program(path) {
      path.scope.rename('x');
    }
  }
})

Expected behavior

Expected output to be:

const _x = 'q';
const obj = {
  [_x](x) {
    return x;
  }
};

but instead get:

const _x = 'q';
const obj = {
  [x](x) { // <-- `[x]` refers to non-existent variable
    return x;
  }
};

Babel Configuration (babel.config.js, .babelrc, package.json#babel, cli command, .eslintrc)

None.

Environment

System:
  OS: macOS Mojave 10.14.6
Binaries:
  Node: 14.15.5 - ~/.nvm/versions/node/v14.15.5/bin/node
  npm: 6.14.11 - ~/.nvm/versions/node/v14.15.5/bin/npm
npmPackages:
  @babel/core: ^7.12.13 => 7.12.13

Additional context

I am very happy to work on a fix. But I'm unfamiliar with how/where Babel calculates scope of identifiers, so if someone could point me in the right direction, that would be a big help.

[babel-bot]

Hey @overlookmotel! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

[char0n]

Yes I can confirm, I'm seeing this in multiple projects I work on. Babel no longer works on code like described in description of this issue.

Ref char0n/ramda-adjunct#1750

[nicolo-ribaudo]

@char0n So a previous version of Babel used to work correctly?

[char0n]

@nicolo-ribaudo, yep @babel/preset-env@7.12.11 works like a charm. Anything with higher semver breaks.

[nicolo-ribaudo]

@char0n I think your bug is a different (but closely related) one, because the code in the OP also shows the bug with Babel 7.10.0.

EDIT: Probably some unrelated change made this exact bug appear when compiling your code.

[nicolo-ribaudo]

@overlookmotel It seems that there are two different bugs in Babel:

1. When path refers to the x inside the computed key, path.scope is the method scope rather than the program scope. This also causes scoping problems such as in this example
2. When renaming a variable, we stop traversing the AST when we enter a node which introduces a new scope (such as ObjectMethod).

We can probably fix (1) by adding a list of "nodes whose keys should refer to the grandparent scope", and start from path.parentPath in

babel/packages/babel-traverse/src/path/context.ts

Line 121 in 30dc25d

let path = this.parentPath;

in those cases.

We can fix (2) by modifying

babel/packages/babel-traverse/src/scope/lib/renamer.ts

Line 6 in 30dc25d

const renameVisitor: Visitor<Renamer> = {

to do something similar to

babel/packages/babel-helper-replace-supers/src/index.js

Line 47 in 30dc25d

export const environmentVisitor = {

: when there is a computed method, we can use skipAllButComputedKey to continue visiting the method key.

[overlookmotel]

@nicolo-ribaudo Thanks for the diagnosis and hints. I'll make a stab at a fix in next couple of weeks.

[overlookmotel]

I've submitted PR #12812 to fix bug (1). It was more complicated than I expected, so I'm not sure now when I'll get time to attack bug (2).

[overlookmotel]

Fixed by #12812, now merged.

[nicolo-ribaudo]

Thanks @overlookmotel, we completely forgot to close this 😅