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

4.18.2 - "Cannot read property 'type' of undefined" in no-unused-vars when checking flow type definitions #10090

Closed
tomasz-sodzawiczny opened this issue Mar 15, 2018 · 8 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@tomasz-sodzawiczny
Copy link

Tell us about your environment

  • ESLint Version: 4.18.2 (it works in 4.18.1)!
  • Node Version: 9.8.0
  • yarn Version: 1.5.1

What parser (default, Babel-ESLint, etc.) are you using?
babel-eslint@8.2.2

Please show your full configuration:

Configuration
module.exports = {
  parser: 'babel-eslint',
  rules: {
    'no-unused-vars': 'error'
  }
};

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

The issue only occures for files where a result type is defined for an arrow function:

// @flow

type Type = null;
// Defining result type causes the crash
export const f = (): Type => null;
// This would work
export const f2 = () => null;
eslint source.js

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

Cannot read property 'type' of undefined
TypeError: Cannot read property 'type' of undefined
    at isForInRef (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/rules/no-unused-vars.js:406:24)
    at variable.references.some.ref (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/rules/no-unused-vars.js:443:21)
    at Array.some (<anonymous>)
    at isUsedVariable (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/rules/no-unused-vars.js:442:40)
    at collectUnusedVariables (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/rules/no-unused-vars.js:565:26)
    at collectUnusedVariables (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/rules/no-unused-vars.js:572:17)
    at Program:exit (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/rules/no-unused-vars.js:617:36)
    at listeners.(anonymous function).forEach.listener (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/util/safe-emitter.js:47:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/tgs/projects/brainly/brainly-frontend/node_modules/eslint/lib/util/safe-emitter.js:47:38)

It works correctly in 4.18.1!

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Mar 15, 2018
@tomasz-sodzawiczny tomasz-sodzawiczny changed the title 4.18.2: no-unused-vars crashes on flow type definitions 4.18.2 - "Cannot read property 'type' of undefined" in no-unused-vars when checking flow type definitions Mar 15, 2018
@not-an-aardvark not-an-aardvark added bug ESLint is working incorrectly core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Mar 16, 2018
@ilyavolodin
Copy link
Member

When you tested it with 4.18.1 did you use the same version of babel-eslint? Because the last change to no-unused-vars rule (the one that's failing), was done in 4.18.0, so if that change is the cause of the issue, 4.18.1 should also be failing as well.

@not-an-aardvark
Copy link
Member

not-an-aardvark commented Mar 19, 2018

@ilyavolodin If I had to guess, this was caused by the refactoring in core that went into 4.18.2. It might have changed something about how scope analysis is resolved.

@platinumazure
Copy link
Member

Incidentally, this seems to be the same error that redux is showing in eslint-canary.

I'm going to do a quick bisect to try to learn more.

@platinumazure
Copy link
Member

@tomasz-sodzawiczny I can't reproduce on my machine. Am I doing something wrong?

I created a subdirectory in the eslint repo and created these files:

  • .eslintrc.json (N.B. root: true is to avoid pulling in ancestor eslint config files and should not affect test):
    {
        "parser": "babel-eslint",
        "rules": {
            "no-unused-vars": "error"
        },
        "root": true
    }
  • file.js:
    // @flow
    
    type Type = null;
    
    export const f = (): Type => null;
  • test.sh:
    #!/bin/bash
    
    echo "Running npm install..."
    npm install --quiet
    
    echo "Installing babel-eslint..."
    npm install babel-eslint@8.2.2 --quiet
    
    echo "Linting the test file..."
    
    if node ../bin/eslint.js ./file.js; then
        echo "Linting successful!"
    else
        echo "Linting failed!"
    fi

The last is intended to be a script for bisecting, run after checking out a commit. I've run it on both latest master and on the v4.18.2 tag, with the expectation that linting should fail/crash. Just in case my bash skills are rusty, I've also run ESLint directly, outside the script (using the same node ../bin/eslint.js ./file.js input) and also saw no linting errors in both latest master and v4.18.2 tag.

Have I missed something?

@not-an-aardvark
Copy link
Member

Incidentally, this seems to be the same error that redux is showing in eslint-canary.

This is also the error that was occurring in #9762 (where scope analysis with babel-eslint was incorrect when we added the ability to use custom scope analysis). However, aside from this issue I'm not aware of a case where this occurs when using the latest version of babel-eslint.

@tomasz-sodzawiczny
Copy link
Author

@platinumazure @not-an-aardvark Ugh, OK, I tried to reproduce that in an isolated setup and I found the issue - it's not with eslint, it's problem with yarn workspaces, false alarm.

In case some has the same thing: it seems when using yarn workspaces eslint binary might run against a different babel-eslint than the one defined in your package. Yuck.

Sorry I wasted your time. :(

@platinumazure
Copy link
Member

I've created #10100 to add some extra debugging output that might come in handy in similar situations in the future.

@not-an-aardvark
Copy link
Member

Closing this issue since it turned out to not be a bug. Thanks for the report regardless -- I think we have room for improvement in helping the user debug this type of issue.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 16, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

No branches or pull requests

4 participants