Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

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

Closed
0xdevalias opened this issue Nov 16, 2023 · 5 comments · Fixed by #111 or #107 · May be fixed by Stanislav1975/eslint#11
Closed

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

0xdevalias opened this issue Nov 16, 2023 · 5 comments · Fixed by #111 or #107 · May be fixed by Stanislav1975/eslint#11

Comments

@0xdevalias
Copy link

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
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Nov 16, 2023
@0xdevalias 0xdevalias changed the title Bug: eslint-scope dails to create new Scope for block element Bug: eslint-scope fails to create new Scope for block element Nov 16, 2023
@bradzacher
Copy link

bradzacher commented Nov 16, 2023

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

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

@mdjermanovic
Copy link
Member

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
Copy link
Author

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:

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:

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 nzakas moved this from Needs Triage to Ready to Implement in Triage Nov 17, 2023
@nzakas
Copy link
Member

nzakas commented Nov 17, 2023

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. 👍

nzakas added a commit that referenced this issue Nov 17, 2023
@nzakas nzakas moved this from Ready to Implement to Implementing in Triage Nov 17, 2023
@0xdevalias
Copy link
Author

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 :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
4 participants