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

[no-unused-vars] TypeError: Cannot read property 'type' of undefined on first import when having private methods #13575

Closed
piranna opened this issue Aug 16, 2020 · 18 comments
Labels
3rd party plugin This is an issue related to a 3rd party plugin, config, or parser archived due to age This issue has been archived; please open a new issue for any further discussion experimental syntax This change is related to experimental syntax (stage 3 or below)

Comments

@piranna
Copy link
Contributor

piranna commented Aug 16, 2020

Tell us about your environment

Environment Info:

Node version: v14.4.0
npm version: v6.14.5
Local ESLint version: v7.7.0 (Currently used)
Global ESLint version: Not found

What parser (default, Babel-ESLint, etc.) are you using?

@babel/eslint-parser

Please show your full configuration:

Configuration
{
  "env": {
    "browser": true,
    "es2020": true,
    "jest": true,
    "node": true
  },
  "extends": "eslint:recommended",
  "parser": "@babel/eslint-parser"
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

I have updated babel-eslint to new @babel/eslint-parser so I can make use of private methods (there was an error on previous ones that only allowed private properties). After doing so, I'm getting the error attached at the end.

The code at WebRtcPeerCore.js:30 is import EventEmitter from 'events', and the failing repository is https://github.com/piranna/kurento-utils-js/blob/master/src/WebRtcPeerCore.js (there's another one, but it's company private). It only happen when changing any of the arrow functions set to a private property to be a private method, like this:

// TODO eslint doesn't fully support private methods, replace arrow function
  #setVideoStream = stream =>
  {
    this.#videoStream = stream

    this.emit('setLocalVideo')
  }
  #setVideoStream(stream)
  {
    this.#videoStream = stream

    this.emit('setLocalVideo')
  }
eslint src/* testutils/* __tests__/*

What did you expect to happen?

Eslint parsing the file as normally does.

What actually happened? Please include the actual, raw output from ESLint.

Oops! Something went wrong! :(

ESLint: 7.7.0

TypeError: Cannot read property 'type' of undefined
Occurred while linting /home/piranna/Trabajo/Veedeo.me/kurento-utils-js/src/WebRtcPeerCore.js:30
    at collectUnusedVariables (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/rules/no-unused-vars.js:569:50)
    at collectUnusedVariables (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/rules/no-unused-vars.js:603:17)
    at collectUnusedVariables (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/rules/no-unused-vars.js:603:17)
    at collectUnusedVariables (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/rules/no-unused-vars.js:603:17)
    at Program:exit (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/rules/no-unused-vars.js:615:36)
    at /home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (/home/piranna/Trabajo/Veedeo.me/kurento-utils-js/node_modules/eslint/lib/linter/node-event-generator.js:283:22)

Are you willing to submit a pull request to fix this bug?

Yes, I can write one with some guidance about what's the source of this error.

@piranna piranna added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Aug 16, 2020
@piranna
Copy link
Contributor Author

piranna commented Aug 16, 2020

It seems could be somewhat related to #12584, but I'm not using Typescript, just only vanilla Javascript, I'm using Babel to transpile to older browser versions, no more.

@anikethsaha
Copy link
Member

Thanks for the report!

private fields (now class fields) are still in stage 3 and eslint only supports stage 4 features. So that's why the core eslint rules like no-unused-vars don't support these. Even though the parse (@babel/eslint-parser) does supports it, but eslint's rule doesn't.

The solution can be, Either the core rule has to be extended to a new rule from babel eslint plugin or needs to wait for the proposal to reach stage 4 and then implementing the support in eslint rules and the underlining parser/analyzers.

@anikethsaha anikethsaha added experimental syntax This change is related to experimental syntax (stage 3 or below) and removed bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Aug 16, 2020
@piranna
Copy link
Contributor Author

piranna commented Aug 16, 2020

I think it makes more sense to wait for stage 4... The idea of adding an overwrite rule in @babel/eslint-plugin would be nice, and later move the code to core eslint, but are we sure does it concern to the no-unused-vars rule? I find it really strange that the error happens on the import only when there's a private method, like it could be a rules conflict or corner case... what do you think?

@anikethsaha
Copy link
Member

Actually eslint runs the rules that are mentioned in the config i.e for your config it is recommended so maybe no-unused-vars was the one that got this error first. Most of the rules (including no-unused-vars) need to be updated for this proposal.

@piranna
Copy link
Contributor Author

piranna commented Aug 16, 2020

Make sense... then probably the best would be to wait until it gets to stage 4. At least I've been able to replace the deprecated packages for the new ones :-)

@mdjermanovic
Copy link
Member

I find it really strange that the error happens on the import only when there's a private method, like it could be a rules conflict or corner case... what do you think?

I don't think this is happening on the import. ESLint prints the line where the currently traversed node starts, which is usually very helpful, but in the case of a crash in the no-unused-vars rule it will always print the line where the whole program starts because the rule listens "Program" node only.

@mdjermanovic mdjermanovic added the 3rd party plugin This is an issue related to a 3rd party plugin, config, or parser label Aug 16, 2020
@mdjermanovic
Copy link
Member

It's crashing on private method params. This is reproducible with the following code:

/*eslint no-unused-vars: ["error"]*/

class A {
  #foo(p) {}
}

I believe this is an issue with @babel/eslint-parser. The core no-unused-vars rule itself could probably work fine with this code.

The main problem is: visitor keys for ClassPrivateMethod type do not match the structure of ClassPrivateMethod nodes.

Because of that, ESLint doesn't set parent property on ClassPrivateMethod#value (a FunctionExpression node). The rule crashes here because def.node (which is ClassPrivateMethod#value) doesn't have parent.

@piranna can you please open an issue in https://github.com/babel/babel repository?

@mdjermanovic
Copy link
Member

Just to note that per our policy about experimental features we do make changes in core rules to avoid crashes on stage 3 syntax:

We will make changes to core rules in order to avoid crashes on stage 3 ECMAScript syntax proposals (as long as they are implemented using the correct experimental ESTree syntax).

However, this particular problem seems to be something that should be fixed in @babel/eslint-parser, rather than in the rules. Also, generated AST doesn't follow the experimental ESTree class-features specification, though it's probably intentional in order to avoid breaking a number of rules.

Anyway, I think this should be reported in the babel/babel repo.

@piranna
Copy link
Contributor Author

piranna commented Aug 17, 2020

I don't think this is happening on the import. ESLint prints the line where the currently traversed node starts, which is usually very helpful, but in the case of a crash in the no-unused-vars rule it will always print the line where the whole program starts because the rule listens "Program" node only.

Make sense, probably that's what happening here.

Anyway, I think this should be reported in the babel/babel repo.

Done at babel/babel#11972.

@piranna
Copy link
Contributor Author

piranna commented Aug 18, 2020

this particular problem seems to be something that should be fixed in @babel/eslint-parser

Fix at babel/babel#11973 scheduled for Babel 7.11.4.

@piranna
Copy link
Contributor Author

piranna commented Aug 18, 2020

Fix in babel merged in main branch.

@mdjermanovic
Copy link
Member

Thanks for the info!

We can keep this issue open to see if the new @babel/eslint-parser version fixes the problem.

@piranna
Copy link
Contributor Author

piranna commented Aug 23, 2020

Checked on two different projects, I confirm @babel/eslint-parser version 7.11.4 at last fix this :-D

@piranna piranna closed this as completed Aug 23, 2020
@mdjermanovic
Copy link
Member

Glad the issue is solved, thanks for the update!

@piranna
Copy link
Contributor Author

piranna commented Aug 24, 2020

Glad the issue is solved, thanks for the update!

You are welcome :-)

@adelespinasse
Copy link

I seem to be having this issue, or something similar. But I'm not using @babel/eslint-parser. I assume there's probably some out-of-date package somewhere, but I'm not sure what to look for.

I'm using create-react-app so it's a bit hard to tell what I really need in terms of dependencies, but I've got ESLint 7.17.0 and most of the @babel/ packages are versions like 7.12.x, a few 7.11.x and a few 7.10.x.

I have a class that uses private properties (not private methods, I already was avoiding those because of lack of support in various tools). ESLint gives me this error:

Oops! Something went wrong! :(

ESLint: 7.17.0

TypeError: Cannot read property 'type' of undefined
Occurred while linting /home/aldel/emailabz/src/IdeaGraph/controller.js:6
    at isForInRef (/home/aldel/emailabz/node_modules/eslint/lib/rules/no-unused-vars.js:449:24)
    at variable.references.some.ref (/home/aldel/emailabz/node_modules/eslint/lib/rules/no-unused-vars.js:486:21)
    at Array.some (<anonymous>)
    at isUsedVariable (/home/aldel/emailabz/node_modules/eslint/lib/rules/no-unused-vars.js:485:40)
    at collectUnusedVariables (/home/aldel/emailabz/node_modules/eslint/lib/rules/no-unused-vars.js:596:26)
    at collectUnusedVariables (/home/aldel/emailabz/node_modules/eslint/lib/rules/no-unused-vars.js:603:17)
    at Program:exit (/home/aldel/emailabz/node_modules/eslint/lib/rules/no-unused-vars.js:615:36)
    at listeners.(anonymous function).forEach.listener (/home/aldel/emailabz/node_modules/eslint/lib/linter/safe-emitter.js:45:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/home/aldel/emailabz/node_modules/eslint/lib/linter/safe-emitter.js:45:38)

Oddly, I never got this error in other files where I used private properties. But the error goes away if I rename all the private properties to make them not private, so I assume it's related.

What could be causing this?

My .eslintrc (minus the rules) is:

{
  "extends": ["airbnb", "react-app"],
  "plugins": [
    "flowtype",
    "react-hooks"
  ],
  "env": {
    "browser": true,
    "node": true,
    "es6": true,
    "mocha": true,
    "jest": true
  },
  "settings": {
    "react": {
      "pragma": "React",
      "version": "16.7"
    }
  },
  "globals": {
    "d3": true
  },
  "rules": {
...

@mdjermanovic
Copy link
Member

@adelespinasse that could be a different problem, can you please open a new bug report issue with a code example and other details?

@adelespinasse
Copy link

@adelespinasse that could be a different problem, can you please open a new bug report issue with a code example and other details?

Ok, to my surprise, I've managed to make a fairly minimal-ish reproduction of the issue, reported separately here: #13994

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Mar 18, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3rd party plugin This is an issue related to a 3rd party plugin, config, or parser archived due to age This issue has been archived; please open a new issue for any further discussion experimental syntax This change is related to experimental syntax (stage 3 or below)
Projects
None yet
Development

No branches or pull requests

4 participants