use-flow-type marks a reference as used with the wrong reference when names are the same

[willmruzek]

Code

import {
	contextType,
} from 'someModule';

export function getLabelSets(
	contextType: $Keys<typeof contextType>,
) { ... }

Versions

"babel-eslint": "^8.2.3",
"eslint": "^4.19.1",
"eslint-plugin-flowtype": "^2.48.0",

Expected

No errors.

Actual

error  'contextType' is defined but never used  no-unused-vars

[gajus]

@mruzekw Can you give a try fixing this?

[willmruzek]

Sure. I don't know what the full scope of the bug is. Is it all typeof clauses in type annotations? Just in generic types? So I'll have to do some digging.

[gajus]

Couldn't tell without digging into it myself.

[willmruzek]

So it looks like this only happens when a module def is the same name as a function param.

Here are a couple of examples where this takes affect:

// @flow

import { contextType } from './someModule';

// Should error but doesn't
export function getLabelSets(
	contextType: $Keys<typeof contextType>,
) {
  return; // Notice how contextType function param is not used
}

// Doesn't count the module level definition because the identifier was found in the function scope
export function getLabelSets(
	contextType: $Keys<typeof contextType>,
) {
  return contextType;
}

[willmruzek]

Alright, in an attempt to fix this I've found that context.markVariableAsUsed(name) does not provide a way to to set a starting scope. The scope returned for a GenericTypeAnnotation for a function parameter is the function scope. Since the param name is the same as the module-level variable, it still counts it. What we need though is a way to say "skip the function scope because the type reference will be in a parent scope".

Not sure how to proceed here other than raise an issue in the ESLint repo.

[pnevyk]

I may be missing something, but would it help if this scope lookup started one scope up?

[willmruzek]

@pnevyk That's how I attempted to fix it. But notice the markVariableAsUsed implementation here:

https://github.com/eslint/eslint/blob/master/lib/linter.js#L660-L678

[pnevyk]

@mruzekw Thanks for clarification.