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

Type annotation identifier does not always have range information #473

Closed
platinumazure opened this issue May 20, 2017 · 7 comments
Closed

Comments

@platinumazure
Copy link

Upstream issue for eslint/eslint#8630.

Given this source code:

// @flow
type FieldOptions<T> = {| defaultValue: T |}
type DateFieldOptions = {| ...FieldOptions<Date> |}

This generates the AST shown at this gist.

It looks like both identifiers in FieldOptions<Date> do not get ranges:

I'm hazarding a guess that this might have to do with being preceded by the spread operator, but I can't say for sure.

Unfortunately, I don't know enough about babel-eslint or Flow to have any idea where to start with this. But if someone can give me some guidance on where to start, I could theoretically at least attempt a solution.

@rosskevin
Copy link

rosskevin commented Jun 1, 2017

I'm seeing this same exact thing with flow type object spread, such as type X = {...Y}.

Cannot read property '1' of undefined
TypeError: Cannot read property '1' of undefined
    at scope.references.forEach.reference (/Users/kross/projects/ui/node_modules/eslint/lib/rules/no-use-before-define.js:206:83)
    at Array.forEach (native)
    at findVariablesInScope (/Users/kross/projects/ui/node_modules/eslint/lib/rules/no-use-before-define.js:194:30)
    at EventEmitter.Program:exit (/Users/kross/projects/ui/node_modules/eslint/lib/rules/no-use-before-define.js:242:21)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:191:7)
    at NodeEventGenerator.applySelector (/Users/kross/projects/ui/node_modules/eslint/lib/util/node-event-generator.js:265:26)
    at NodeEventGenerator.applySelectors (/Users/kross/projects/ui/node_modules/eslint/lib/util/node-event-generator.js:294:22)
    at NodeEventGenerator.leaveNode (/Users/kross/projects/ui/node_modules/eslint/lib/util/node-event-generator.js:317:14)
    at CodePathAnalyzer.leaveNode (/Users/kross/projects/ui/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:622:23)

I've tried to use the absolute latest releases though I'm unsure how there is an old babylon 6.17.1:

❯❯❯ yarn list babylon babel-eslint eslint                                                                                                                                                                                     develop ⬆ ✭ ✱
yarn list v0.21.3
├─ @alienfast/build@2.0.62
│  ├─ babel-traverse@7.0.0-alpha.12
│  │  ├─ babel-template@7.0.0-alpha.7
│  │  │  └─ babylon@7.0.0-beta.8
│  │  └─ babel-traverse@7.0.0-alpha.7
│  │  │  └─ babylon@7.0.0-beta.8
│  └─ babylon@7.0.0-beta.12
├─ babel-core@7.0.0-alpha.12
│  ├─ babel-helper-function-name@7.0.0-alpha.7
│  │  └─ babylon@7.0.0-beta.8
│  └─ babylon@7.0.0-beta.12
├─ babel-eslint@7.2.3
├─ babel-helper-remap-async-to-generator@7.0.0-alpha.12
│  ├─ babel-traverse@7.0.0-alpha.12
│  │  ├─ babel-template@7.0.0-alpha.7
│  │  │  └─ babylon@7.0.0-beta.8
│  │  └─ babel-traverse@7.0.0-alpha.7
│  │  │  └─ babylon@7.0.0-beta.8
│  └─ babylon@7.0.0-beta.12
├─ babel-helpers@7.0.0-alpha.12
│  ├─ babel-helper-function-name@7.0.0-alpha.7
│  │  └─ babylon@7.0.0-beta.8
│  └─ babylon@7.0.0-beta.12
├─ babel-plugin-transform-decorators@7.0.0-alpha.12
│  ├─ babel-helper-function-name@7.0.0-alpha.7
│  │  └─ babylon@7.0.0-beta.8
│  └─ babylon@7.0.0-beta.12
├─ babel-preset-es2016@7.0.0-alpha.12
│  ├─ babel-helper-function-name@7.0.0-alpha.7
│  │  └─ babylon@7.0.0-beta.8
│  ├─ babel-template@7.0.0-alpha.7
│  │  └─ babylon@7.0.0-beta.8
│  └─ babylon@7.0.0-beta.12
├─ babel-preset-stage-2@7.0.0-alpha.12
│  ├─ babel-traverse@7.0.0-alpha.12
│  │  ├─ babel-template@7.0.0-alpha.7
│  │  │  └─ babylon@7.0.0-beta.8
│  │  └─ babel-traverse@7.0.0-alpha.7
│  │  │  └─ babylon@7.0.0-beta.8
│  └─ babylon@7.0.0-beta.12
├─ babylon@6.17.1
└─ eslint@3.19.0
❯❯❯ yarn why babylon                                                                                                                                                                                                          develop ⬆ ✭ ✱
yarn why v0.21.3
[1/4] 🤔  Why do we have the module "babylon"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
info Has been hoisted to "babylon"
info Reasons this module exists
   - "babel-eslint" depends on it
   - "babel-eslint#babel-traverse" depends on it
   - "babel-plugin-flow-react-proptypes#babel-core" depends on it

@kentcdodds
Copy link
Member

Thanks for the report! Do either of you think you could create a small repository to reproduce the issue and maybe even make a pull request with a fix?

@rosskevin
Copy link

rosskevin commented Jun 1, 2017

Repo with reproduction:
https://github.com/rosskevin/babel-eslint-473-flow-spread

I did this quickly and did not try to pare down the other eslint plugins, but perhaps it is simple enough as is.

/related #443 #465

@soda0289
Copy link
Contributor

soda0289 commented Jun 2, 2017

@rosskevin I looked into this.

The problem is that there is no range information. This is because babel-eslint is not traversing into the ObjectTypeSpreadProperty node arguments. This node type was added to babel-types by this PR babel/babel#5653 but there has been no release and this means that the package babel-traverse is not including the newest type definitions.

To fix this I think there just has to be a new release of babel v6.

@rosskevin
Copy link

@soda0289 my repo is using the latest 7.x alpha, which I have seen has the original PR merged so I would expect it to work. I don't ​know the version of babel-traverse as I am mobile now, but I'll check in the morning.

@rosskevin
Copy link

rosskevin commented Jun 2, 2017

I was able to do a quick check, it appears my test repo has a mixture of babel-traverse:

~/p/babel-eslint-473-flow-spread ❯❯❯ yarn list babel-traverse                                                                                                                                                                         master ✚ ✱ ◼
yarn list v0.21.3
├─ babel-eslint@7.2.3
│  └─ babel-traverse@6.24.1
├─ babel-plugin-flow-react-proptypes@3.2.0
│  └─ babel-traverse@6.24.1
├─ babel-plugin-transform-dev-warning@0.1.0
│  └─ babel-traverse@6.24.1
├─ babel-plugin-transform-react-remove-prop-types@0.4.5
│  └─ babel-traverse@6.24.1
└─ babel-traverse@7.0.0-alpha.12
│  └─ babel-traverse@7.0.0-alpha.7
✨  Done in 0.53s.

I tried to be explicit with "babel-traverse": "^7.0.0-alpha.12" in the package.json but it didn't affect the plugins' versions of it.

@kaicataldo
Copy link
Member

Thank you for the issue. Now that @babel/eslint-parser has been released, we are making this repository read-only. If this is a change you would still like to advocate for, please reopen this in the babel/babel monorepo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants