Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArrowFunctionExpression's scope is always strict scope #49

Closed
futpib opened this issue Feb 16, 2019 · 2 comments · Fixed by #52
Closed

ArrowFunctionExpression's scope is always strict scope #49

futpib opened this issue Feb 16, 2019 · 2 comments · Fixed by #52

Comments

@futpib
Copy link
Contributor

futpib commented Feb 16, 2019

It seems to be deliberate that eslint-scope always sets isStrict to true for arrow function scopes. But this is not how it works in all the engines I could get my hands to.

I used assignment to an undeclared variable to check if the code is in strict mode:

With explicit strict mode, we get an error:

node -e '"use strict"; (() => { a = 5 })()'
# ReferenceError: a is not defined

Without "use strict" an error is not thrown, which means it's not in strict mode, but eslint-scope would treat the scope as strict anyway:

node -e '(() => { x = 1 })()'

I also checked with the spec's definition of strict mode code, and it seems that arrow functions should not get any special treatment (only classes should): https://www.ecma-international.org/ecma-262/#sec-strict-mode-code

// ArrowFunctionExpression's scope is always strict scope.
if (block.type === Syntax.ArrowFunctionExpression) {
return true;
}

expect(scope.isStrict).to.be.true;

@mysticatea
Copy link
Member

Thank you for the report!

Sounds like a bug.
Are you willing to submit a pull request?

not-an-aardvark added a commit that referenced this issue Mar 2, 2019
This reverts commit 2533966.

The change is causing ESLint to crash with a parsing error on the following code:

```js
console.log(this); z(x => console.log(x, this))
```

```
TypeError: Cannot read property 'length' of undefined
    at isStrictScope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope.js:93:40)
    at new Scope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope.js:254:25)
    at new FunctionScope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope.js:641:9)
    at ScopeManager.__nestFunctionScope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope-manager.js:209:33)
    at Referencer.visitFunction (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/referencer.js:200:27)
    at Referencer.ArrowFunctionExpression (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/referencer.js:567:14)
    at Referencer.Visitor.visit (/Users/tkatz/code/eslint/node_modules/esrecurse/esrecurse.js:104:34)
    at Referencer.Visitor.visitChildren (/Users/tkatz/code/eslint/node_modules/esrecurse/esrecurse.js:83:38)
    at Referencer.CallExpression (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/referencer.js:494:14)
    at Referencer.Visitor.visit (/Users/tkatz/code/eslint/node_modules/esrecurse/esrecurse.js:104:34)
```

Since this is currently affecting ESLint users, we're reverting the change for now until we figure out the root cause.
not-an-aardvark added a commit that referenced this issue Mar 2, 2019
This reverts commit 2533966.

The change is causing ESLint to crash with a parsing error on the following code:

```js
console.log(this); z(x => console.log(x, this))
```

```
TypeError: Cannot read property 'length' of undefined
    at isStrictScope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope.js:93:40)
    at new Scope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope.js:254:25)
    at new FunctionScope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope.js:641:9)
    at ScopeManager.__nestFunctionScope (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/scope-manager.js:209:33)
    at Referencer.visitFunction (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/referencer.js:200:27)
    at Referencer.ArrowFunctionExpression (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/referencer.js:567:14)
    at Referencer.Visitor.visit (/Users/tkatz/code/eslint/node_modules/esrecurse/esrecurse.js:104:34)
    at Referencer.Visitor.visitChildren (/Users/tkatz/code/eslint/node_modules/esrecurse/esrecurse.js:83:38)
    at Referencer.CallExpression (/Users/tkatz/code/eslint/node_modules/eslint-scope/lib/referencer.js:494:14)
    at Referencer.Visitor.visit (/Users/tkatz/code/eslint/node_modules/esrecurse/esrecurse.js:104:34)
```

Since this is currently affecting ESLint users, we're reverting the change for now until we figure out the root cause.
@not-an-aardvark
Copy link
Member

Reopening because the fix was reverted (see #51)

futpib added a commit to futpib/eslint-scope that referenced this issue Mar 2, 2019
not-an-aardvark pushed a commit that referenced this issue Mar 15, 2019
* Fix: Arrow function scope strictness (fixes #49) (#50)

* Fix: Error on functions with non-block body

* Apply suggestion from review

Co-authored-by: Toru Nagashima <star.ctor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants