Bug: eslint-scope fails to create new Scope for block element

[0xdevalias]

Background Context

While exploring a variable renaming issue (pionxzh/wakaru#32) there was a reference to some example code that failed in another parser when block statements were involved (facebook/jscodeshift#263)

In trying this out with eslint-scope (pionxzh/wakaru#32 (comment)), it seems that the bug also may exist here.

Description

I've encountered an issue with eslint-scope where it does not seem to create a new scope for a block element in JavaScript code. According to ECMAScript specifications, let and const declarations within a block { ... } should have block-level scope. However, eslint-scope seems to be treating the block as if it doesn't create a new scope.

Code to Reproduce

Here is a simple code snippet that illustrates the issue:

const x = 42;
{
  const x = 47;
  console.log(x); // Should log 47, as this `x` is in the block scope
}
console.log(x); // Should log 42, as this `x` is in the global scope

In this code, there are two separate scopes: the global scope and the block scope. The variable x is declared twice, once in each scope. According to JavaScript scoping rules, these should be treated as two different variables.

Expected Behavior

eslint-scope should recognize and create a new scope for the block element, treating the inner const x = 47; as a separate variable scoped within the block.

Actual Behavior

Currently, eslint-scope does not seem to differentiate between the two scopes in the provided code snippet. It treats the entire code as if it's in a single scope, which is not consistent with standard JavaScript scoping rules.

Additional Information

• eslint-scope version: 7.2.2
• Node.js version: 18.12.1

[bradzacher]

https://github.com/eslint/eslint-scope/blob/main/tests/es6-block-scope.js

According to these tests eslint-scope does the correct thing?

[mdjermanovic]

I've encountered an issue with eslint-scope where it does not seem to create a new scope for a block element in JavaScript code.

Looks like you are getting the default ES5 scoping. Did you pass the ecmaVersion option with a value >=6?

[0xdevalias]

Oo.. let me try that.. this could definitely be a PEBKAC issue.

This is what I was using in my test REPL (Ref):

  const ast = espree.parse(jsCode, {
    ecmaVersion: 2020,
    sourceType: 'module',
    range: true
  });

  // Analyze the scopes in the AST
  const scopeManager = eslintScope.analyze(ast);

Which was based off the usage example in the README:

• https://github.com/eslint/eslint-scope#-usage

I didn't notice any links to docs pages/similar suggesting that the .analyze method took options (and didn't think to check the source).

Beyond what's written here, are there any more details about the options? Eg. what does optimistic do, as the comment only says that it's a flag.

/**
..snip..
 * @param {Object} providedOptions Options that tailor the scope analysis
 * @param {boolean} [providedOptions.optimistic=false] the optimistic flag
 * @param {boolean} [providedOptions.directive=false] the directive flag
 * @param {boolean} [providedOptions.ignoreEval=false] whether to check 'eval()' calls
 * @param {boolean} [providedOptions.nodejsScope=false] whether the whole
 * script is executed under node.js environment. When enabled, escope adds
 * a function scope immediately following the global scope.
 * @param {boolean} [providedOptions.impliedStrict=false] implied strict mode
 * (if ecmaVersion >= 5).
 * @param {string} [providedOptions.sourceType='script'] the source type of the script. one of 'script', 'module', and 'commonjs'
 * @param {number} [providedOptions.ecmaVersion=5] which ECMAScript version is considered
 * @param {Object} [providedOptions.childVisitorKeys=null] Additional known visitor keys. See [esrecurse](https://github.com/estools/esrecurse)'s the `childVisitorKeys` option.
 * @param {string} [providedOptions.fallback='iteration'] A kind of the fallback in order to encounter with unknown node. See [esrecurse](https://github.com/estools/esrecurse)'s the `fallback` option.
..snip

I stumbled onto this ScopeManager docs page through google, but didn't see it linked either:

• https://eslint.org/docs/latest/extend/scope-manager-interface

It might be useful to update the README with a little more info/links about the .analyze options/ScopeManager interface/etc?

---

With my old REPL test code, the output was:

$ node espree-eslint-scope_2.js 

-= SCOPE INFO =-
Scope: type=global (block.type=Program) block.id?.name=undefined implicit=x,x,console,x,console,x
Variable: x [
  { type: 'Variable', kind: 'const' },
  { type: 'Variable', kind: 'const' }
] References: 4

After correctly setting it to parse with ecmaVersion: 6, the output was:

$ node espree-eslint-scope_2.js 

-= SCOPE INFO =-
Scope: type=global (block.type=Program) block.id?.name=undefined implicit=x,console,console,x
Variable: x [ { type: 'Variable', kind: 'const' } ] References: 2

-= SCOPE INFO =-
Scope: type=block (block.type=BlockStatement) block.id?.name=undefined implicit=undefined
Variable: x [ { type: 'Variable', kind: 'const' } ] References: 2

After setting it to parse with ecmaVersion: 6, sourceType: 'module', the output was:

$ node espree-eslint-scope_2.js 

-= SCOPE INFO =-
Scope: type=global (block.type=Program) block.id?.name=undefined implicit=console,console

-= SCOPE INFO =-
Scope: type=module (block.type=Program) block.id?.name=undefined implicit=undefined
Variable: x [ { type: 'Variable', kind: 'const' } ] References: 2

-= SCOPE INFO =-
Scope: type=block (block.type=BlockStatement) block.id?.name=undefined implicit=undefined
Variable: x [ { type: 'Variable', kind: 'const' } ] References: 2

---

So it seems it's not a bug after all, and just user error on my part. Better docs in the README would definitely help avoid cases like this in future though too.

[nzakas]

So it seems it's not a bug after all, and just user error on my part. Better docs in the README would definitely help avoid cases like this in future though too.

Yeah, we just kind of lazily forked escope and never really did any docs updates, but we should. 👍

[0xdevalias]

Yeah, we just kind of lazily forked escope and never really did any docs updates, but we should.

@nzakas That makes sense. The docs update in #111 looks good :)