[no-shadow] new false positive in 4.0.0 for function argument types #2447

mceachen · 2020-08-31T20:25:30Z

I have tried restarting my IDE and the issue persists.
I have updated to the latest version of the packages.
I have read the FAQ and my problem is not listed.

Repro

export function map<T, U>(
  obj: T | undefined, 
  f: (obj: T) => U): U | undefined {
  return obj == null ? undefined : f(obj)
}

Note that obj is used both as the name of the first argument, and for typing the second argument. The second argument's obj is unused, so no-shadow shouldn't have a problem with this, and indeed, this code when using the no-shadow rule with

    "@typescript-eslint/eslint-plugin": "^3.10.1",
    "@typescript-eslint/parser": "^3.10.1",

passes linting. But after upgrading to

    "@typescript-eslint/eslint-plugin": "^4.0.1",
    "@typescript-eslint/parser": "^4.0.1",

I see 21:7 error 'obj' is already declared in the upper scope no-shadow

If I switch my .eslintrc.js to

    "no-shadow": "off",
    "@typescript-eslint/no-shadow": ["error", { ignoreTypeValueShadow: true }],

I still see error 'obj' is already declared in the upper scope @typescript-eslint/no-shadow.

Additional Info

yarn eslint src/fe/Maybe.ts --debug
yarn run v1.22.5
$ /tmp/example/node_modules/.bin/eslint src/fe/Maybe.ts --debug
  eslint:cli CLI args: [ 'src/fe/Maybe.ts', '--debug' ] +0ms
  eslint:cli Running on files +2ms
  eslint:config-array-factory Loading JSON config file: /tmp/example/package.json +0ms
  eslint:ignore-pattern Create with: [ IgnorePattern { patterns: [ '/**/node_modules/*' ], basePath: '/tmp/example', loose: false } ] +0ms
  eslint:ignore-pattern   processed: { basePath: '/tmp/example', patterns: [ '/**/node_modules/*' ] } +1ms
  eslint:ignore-pattern Create with: [ IgnorePattern { patterns: [ '/**/node_modules/*' ], basePath: '/tmp/example', loose: false } ] +1ms
  eslint:ignore-pattern   processed: { basePath: '/tmp/example', patterns: [ '/**/node_modules/*' ] } +0ms
  eslint:file-enumerator Start to iterate files: [ 'src/fe/Maybe.ts' ] +0ms
  eslint:file-enumerator File: /tmp/example/src/fe/Maybe.ts +0ms
  eslint:cascading-config-array-factory Load config files for /tmp/example/src/fe. +0ms
  eslint:cascading-config-array-factory No cache found: /tmp/example/src/fe. +0ms
  eslint:config-array-factory Loading package.json config file: /tmp/example/src/fe/package.json +3ms
  eslint:config-array-factory Loading JSON config file: /tmp/example/src/fe/package.json +0ms
  eslint:config-array-factory Error reading package.json file: /tmp/example/src/fe/package.json +0ms
  eslint:config-array-factory Config file not found on /tmp/example/src/fe +0ms
  eslint:cascading-config-array-factory No cache found: /tmp/example/src. +1ms
  eslint:config-array-factory Loading JS config file: /tmp/example/src/.eslintrc.js +0ms
  eslint:config-array-factory Config file found: /tmp/example/src/.eslintrc.js +1ms
  eslint:config-array-factory Loading {extends:"plugin:@typescript-eslint/eslint-recommended"} relative to /tmp/example/src/.eslintrc.js +0ms
  eslint:config-array-factory Loading plugin "@typescript-eslint" from /tmp/example/src/.eslintrc.js +0ms
  eslint:config-array-factory Loaded: @typescript-eslint/eslint-plugin@4.0.1 (/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js) +1ms
  eslint:config-array-factory Plugin /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js loaded in: 182ms +182ms
  eslint:config-array-factory Loading {extends:"plugin:@typescript-eslint/recommended"} relative to /tmp/example/src/.eslintrc.js +2ms
  eslint:config-array-factory Loading plugin "@typescript-eslint" from /tmp/example/src/.eslintrc.js +0ms
  eslint:config-array-factory Loaded: @typescript-eslint/eslint-plugin@4.0.1 (/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js) +0ms
  eslint:config-array-factory Plugin /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js loaded in: 0ms +0ms
  eslint:config-array-factory Loading {extends:"./configs/base"} relative to /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js +0ms
  eslint:config-array-factory package.json was not found: Cannot find module './configs/base/package.json'
Require stack:
- /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js +0ms
  eslint:config-array-factory Loaded: ./configs/base (/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/base.js) +0ms
  eslint:config-array-factory Loading JS config file: /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/base.js +0ms
  eslint:config-array-factory Loading parser "@typescript-eslint/parser" from /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/base.js +1ms
  eslint:config-array-factory Loaded: @typescript-eslint/parser@4.0.1 (/tmp/example/node_modules/@typescript-eslint/parser/dist/index.js) +0ms
  eslint:config-array-factory Loading plugin "@typescript-eslint" from /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/base.js +58ms
  eslint:config-array-factory Loaded: @typescript-eslint/eslint-plugin@4.0.1 (/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js) +0ms
  eslint:config-array-factory Plugin /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js loaded in: 0ms +1ms
  eslint:config-array-factory Loading {extends:"./configs/eslint-recommended"} relative to /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js +0ms
  eslint:config-array-factory package.json was not found: Cannot find module './configs/eslint-recommended/package.json'
Require stack:
- /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js +0ms
  eslint:config-array-factory Loaded: ./configs/eslint-recommended (/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/eslint-recommended.js) +0ms
  eslint:config-array-factory Loading JS config file: /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/eslint-recommended.js +0ms
  eslint:config-array-factory Loading parser "@typescript-eslint/parser" from /tmp/example/src/.eslintrc.js +0ms
  eslint:config-array-factory Loaded: @typescript-eslint/parser@4.0.1 (/tmp/example/node_modules/@typescript-eslint/parser/dist/index.js) +0ms
  eslint:config-array-factory Loading plugin "@typescript-eslint" from /tmp/example/src/.eslintrc.js +0ms
  eslint:config-array-factory Loaded: @typescript-eslint/eslint-plugin@4.0.1 (/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js) +0ms
  eslint:config-array-factory Plugin /tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js loaded in: 0ms +0ms
  eslint:cascading-config-array-factory No cache found: /tmp/example. +246ms
  eslint:config-array-factory Loading package.json config file: /tmp/example/package.json +0ms
  eslint:config-array-factory Loading JSON config file: /tmp/example/package.json +0ms
  eslint:config-array-factory Error reading package.json file: /tmp/example/package.json +1ms
  eslint:config-array-factory Config file not found on /tmp/example +0ms
  eslint:cascading-config-array-factory No cache found: /home/mrm/src. +1ms
  eslint:config-array-factory Config file not found on /home/mrm/src +0ms
  eslint:cascading-config-array-factory No cache found: /home/mrm. +0ms
  eslint:cascading-config-array-factory Stop traversing because of considered root. +0ms
  eslint:rules Loading rule 'constructor-super' (remaining=281) +0ms
  eslint:rules Loading rule 'getter-return' (remaining=280) +0ms
  eslint:rules Loading rule 'no-const-assign' (remaining=279) +0ms
  eslint:rules Loading rule 'no-dupe-args' (remaining=278) +0ms
  eslint:rules Loading rule 'no-dupe-class-members' (remaining=277) +1ms
  eslint:rules Loading rule 'no-dupe-keys' (remaining=276) +0ms
  eslint:rules Loading rule 'no-func-assign' (remaining=275) +0ms
  eslint:rules Loading rule 'no-import-assign' (remaining=274) +0ms
  eslint:rules Loading rule 'no-new-symbol' (remaining=273) +0ms
  eslint:rules Loading rule 'no-obj-calls' (remaining=272) +1ms
  eslint:rules Loading rule 'no-redeclare' (remaining=271) +0ms
  eslint:rules Loading rule 'no-setter-return' (remaining=270) +0ms
  eslint:rules Loading rule 'no-this-before-super' (remaining=269) +0ms
  eslint:rules Loading rule 'no-undef' (remaining=268) +0ms
  eslint:rules Loading rule 'no-unreachable' (remaining=267) +1ms
  eslint:rules Loading rule 'no-unsafe-negation' (remaining=266) +0ms
  eslint:rules Loading rule 'no-var' (remaining=265) +0ms
  eslint:rules Loading rule 'prefer-const' (remaining=264) +1ms
  eslint:rules Loading rule 'prefer-rest-params' (remaining=263) +0ms
  eslint:rules Loading rule 'prefer-spread' (remaining=262) +1ms
  eslint:rules Loading rule 'valid-typeof' (remaining=261) +0ms
  eslint:rules Loading rule 'no-array-constructor' (remaining=260) +3ms
  eslint:rules Loading rule 'no-empty-function' (remaining=259) +0ms
  eslint:rules Loading rule 'no-extra-semi' (remaining=258) +1ms
  eslint:rules Loading rule 'no-unused-vars' (remaining=257) +1ms
  eslint:rules Loading rule 'no-shadow' (remaining=256) +2ms
  eslint:rules Loading rule 'no-unused-expressions' (remaining=255) +0ms
  eslint:rules Loading rule 'eqeqeq' (remaining=254) +3ms
  eslint:rules Loading rule 'no-undef-init' (remaining=253) +0ms
  eslint:rules Loading rule 'no-empty' (remaining=252) +2ms
  eslint:cascading-config-array-factory Configuration was determined: ConfigArray(8) [ { type: 'config', name: 'DefaultIgnorePattern', filePath: '', criteria: null, env: undefined, globals: undefined, ignorePattern: IgnorePattern { patterns: [Array], basePath: '/tmp/example', loose: false }, noInlineConfig: undefined, parser: undefined, parserOptions: undefined, plugins: undefined, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: undefined, settings: undefined }, { type: 'config', name: 'src/.eslintrc.js » plugin:@typescript-eslint/eslint-recommended', filePath: '/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js', criteria: null, env: undefined, globals: undefined, ignorePattern: undefined, noInlineConfig: undefined, parser: undefined, parserOptions: undefined, plugins: undefined, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: undefined, settings: undefined }, { type: 'config', name: 'src/.eslintrc.js » plugin:@typescript-eslint/eslint-recommended#overrides[0]', filePath: '/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js', criteria: { includes: [Array], excludes: null, basePath: '/tmp/example/src' }, env: undefined, globals: undefined, ignorePattern: undefined, noInlineConfig: undefined, parser: undefined, parserOptions: undefined, plugins: undefined, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: { 'constructor-super': 'off', 'getter-return': 'off', 'no-const-assign': 'off', 'no-dupe-args': 'off', 'no-dupe-class-members': 'off', 'no-dupe-keys': 'off', 'no-func-assign': 'off', 'no-import-assign': 'off', 'no-new-symbol': 'off', 'no-obj-calls': 'off', 'no-redeclare': 'off', 'no-setter-return': 'off', 'no-this-before-super': 'off', 'no-undef': 'off', 'no-unreachable': 'off', 'no-unsafe-negation': 'off', 'no-var': 'error', 'prefer-const': 'error', 'prefer-rest-params': 'error', 'prefer-spread': 'error', 'valid-typeof': 'off' }, settings: undefined }, { type: 'config', name: 'src/.eslintrc.js » plugin:@typescript-eslint/recommended » ./configs/base', filePath: '/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/base.js', criteria: null, env: undefined, globals: undefined, ignorePattern: undefined, noInlineConfig: undefined, parser: { error: null, filePath: '/tmp/example/node_modules/@typescript-eslint/parser/dist/index.js', id: '@typescript-eslint/parser', importerName: 'src/.eslintrc.js » plugin:@typescript-eslint/recommended » ./configs/base', importerPath: '/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/base.js' }, parserOptions: { sourceType: 'module' }, plugins: { '@typescript-eslint': [Object] }, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: undefined, settings: undefined }, { type: 'config', name: 'src/.eslintrc.js » plugin:@typescript-eslint/recommended » ./configs/eslint-recommended', filePath: '/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/eslint-recommended.js', criteria: null, env: undefined, globals: undefined, ignorePattern: undefined, noInlineConfig: undefined, parser: undefined, parserOptions: undefined, plugins: undefined, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: undefined, settings: undefined }, { type: 'config', name: 'src/.eslintrc.js » plugin:@typescript-eslint/recommended » ./configs/eslint-recommended#overrides[0]', filePath: '/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/configs/eslint-recommended.js', criteria: { includes: [Array], excludes: null, basePath: '/tmp/example/src' }, env: undefined, globals: undefined, ignorePattern: undefined, noInlineConfig: undefined, parser: undefined, parserOptions: undefined, plugins: undefined, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: { 'constructor-super': 'off', 'getter-return': 'off', 'no-const-assign': 'off', 'no-dupe-args': 'off', 'no-dupe-class-members': 'off', 'no-dupe-keys': 'off', 'no-func-assign': 'off', 'no-import-assign': 'off', 'no-new-symbol': 'off', 'no-obj-calls': 'off', 'no-redeclare': 'off', 'no-setter-return': 'off', 'no-this-before-super': 'off', 'no-undef': 'off', 'no-unreachable': 'off', 'no-unsafe-negation': 'off', 'no-var': 'error', 'prefer-const': 'error', 'prefer-rest-params': 'error', 'prefer-spread': 'error', 'valid-typeof': 'off' }, settings: undefined }, { type: 'config', name: 'src/.eslintrc.js » plugin:@typescript-eslint/recommended', filePath: '/tmp/example/node_modules/@typescript-eslint/eslint-plugin/dist/index.js', criteria: null, env: undefined, globals: undefined, ignorePattern: undefined, noInlineConfig: undefined, parser: undefined, parserOptions: undefined, plugins: undefined, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: { '@typescript-eslint/adjacent-overload-signatures': 'error', '@typescript-eslint/ban-ts-comment': 'error', '@typescript-eslint/ban-types': 'error', '@typescript-eslint/explicit-module-boundary-types': 'warn', 'no-array-constructor': 'off', '@typescript-eslint/no-array-constructor': 'error', 'no-empty-function': 'off', '@typescript-eslint/no-empty-function': 'error', '@typescript-eslint/no-empty-interface': 'error', '@typescript-eslint/no-explicit-any': 'warn', '@typescript-eslint/no-extra-non-null-assertion': 'error', 'no-extra-semi': 'off', '@typescript-eslint/no-extra-semi': 'error', '@typescript-eslint/no-inferrable-types': 'error', '@typescript-eslint/no-misused-new': 'error', '@typescript-eslint/no-namespace': 'error', '@typescript-eslint/no-non-null-asserted-optional-chain': 'error', '@typescript-eslint/no-non-null-assertion': 'warn', '@typescript-eslint/no-this-alias': 'error', 'no-unused-vars': 'off', '@typescript-eslint/no-unused-vars': 'warn', '@typescript-eslint/no-var-requires': 'error', '@typescript-eslint/prefer-as-const': 'error', '@typescript-eslint/prefer-namespace-keyword': 'error', '@typescript-eslint/triple-slash-reference': 'error' }, settings: undefined }, { type: 'config', name: 'src/.eslintrc.js', filePath: '/tmp/example/src/.eslintrc.js', criteria: null, env: { node: true }, globals: undefined, ignorePattern: undefined, noInlineConfig: undefined, parser: { error: null, filePath: '/tmp/example/node_modules/@typescript-eslint/parser/dist/index.js', id: '@typescript-eslint/parser', importerName: 'src/.eslintrc.js', importerPath: '/tmp/example/src/.eslintrc.js' }, parserOptions: { project: 'tsconfig.json', sourceType: 'module' }, plugins: { '@typescript-eslint': [Object] }, processor: undefined, reportUnusedDisableDirectives: undefined, root: undefined, rules: { 'no-shadow': 'off', '@typescript-eslint/no-shadow': [Array], 'no-unused-expressions': 'off', 'no-unused-vars': 'off', '@typescript-eslint/no-unused-vars': 'off', '@typescript-eslint/camelcase': 'off', '@typescript-eslint/explicit-function-return-type': 'off', '@typescript-eslint/interface-name-prefix': 'off', '@typescript-eslint/no-explicit-any': 'off', '@typescript-eslint/no-extra-semi': 'off', '@typescript-eslint/no-inferrable-types': 'off', '@typescript-eslint/no-namespace': 'off', '@typescript-eslint/no-var-requires': 'off', 'no-extra-semi': 'off', 'node/no-missing-import': 'off', 'node/no-unsupported-features/es-syntax': 'off', '@typescript-eslint/explicit-module-boundary-types': 'off', '@typescript-eslint/unbound-method': 'error', '@typescript-eslint/member-delimiter-style': [Array], eqeqeq: [Array], 'no-undef-init': 'warn', '@typescript-eslint/no-floating-promises': 'warn', '@typescript-eslint/no-non-null-assertion': 'off', '@typescript-eslint/no-use-before-define': 'off', '@typescript-eslint/strict-boolean-expressions': 'warn', 'no-empty': 'warn', '@typescript-eslint/ban-types': 'warn' }, settings: undefined } ] on /tmp/example/src/fe +18ms
  eslint:ignore-pattern Create with: [ IgnorePattern { patterns: [ '/**/node_modules/*' ], basePath: '/tmp/example', loose: false } ] +269ms
  eslint:ignore-pattern   processed: { basePath: '/tmp/example', patterns: [ '/**/node_modules/*' ] } +0ms
  eslint:ignore-pattern Check {
  filePath: '/tmp/example/src/fe/Maybe.ts',
  dot: false,
  relativePath: 'src/fe/Maybe.ts',
  result: false
} +1ms
  eslint:cli-engine Lint /tmp/example/src/fe/Maybe.ts +0ms
  eslint:linter Linting code for /tmp/example/src/fe/Maybe.ts (pass 1) +0ms
  eslint:linter Verify +0ms
  eslint:linter With ConfigArray: /tmp/example/src/fe/Maybe.ts +0ms
  eslint:linter Generating fixed text for /tmp/example/src/fe/Maybe.ts (pass 1) +3s
  eslint:source-code-fixer Applying fixes +0ms
  eslint:source-code-fixer shouldFix parameter was false, not attempting fixes +0ms
  eslint:file-enumerator Complete iterating files: ["src/fe/Maybe.ts"] +3s
  eslint:cli-engine Linting complete in: 3241ms +3s

/tmp/example/src/fe/Maybe.ts
  21:7  error  'obj' is already declared in the upper scope  @typescript-eslint/no-shadow

✖ 1 problem (1 error, 0 warnings)

error Command failed with exit code 1.

Versions

package	version
`@typescript-eslint/eslint-plugin`	`4.0.1`
`@typescript-eslint/parser`	`4.0.1`
`TypeScript`	`4.0.2`
`ESLint`	`7.7.0`
`node`	`14.5.0`

The text was updated successfully, but these errors were encountered:

bradzacher · 2020-08-31T20:32:05Z

Believe it or not, this isn't actually a false-positive.

Each parameter in a function type declares a value variable so that you can reference it later within a typeof type.

So there is shadowing that occurs here.
Here is a simplified example which illustrates why:

const obj = 1;
type Fn = (obj: string) => typeof obj;

declare const fn: Fn;

const result = fn(''); // what's the type of result?

repl

You might think that typeof result === 1, because typeof obj === 1, however because the parameter declared in the function type shadows the variable name, typeof result === string.

This is probably a rare case, so I could see value behind the addition of an option to ignore this? I guess it depends on how rare it actually is.

mceachen · 2020-08-31T21:20:27Z

Thanks for the great example, that made it clear (I'd link to that from the the no-shadow docs!).

If I had to guess, though, I'd agree with you that defining a return type using typeof is going to be rare. It'd be great to have an option to ignore this case.

…terNameValueShadow` Fixes #2447 Technically this is backwards incompatible, because I'm adding the option true by default, but I believe that the case it protects against is so rare that it's a substantially better experience this way, and it's what people expect

mceachen added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Aug 31, 2020

bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Aug 31, 2020

bradzacher added enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed awaiting response Issues waiting for a reply from the OP or another party labels Aug 31, 2020

bradzacher added this to the 4.0 Scope Analyser Release Bugs milestone Aug 31, 2020

glen-84 mentioned this issue Sep 1, 2020

[no-shadow] v4 false positive: argument name and property in type have same name #2454

Closed

3 tasks

This comment has been minimized.

Sign in to view

This was referenced Sep 1, 2020

[no-shadow] False positive when declaring any TypeScript enum #2466

Closed

[@typescript-eslint/no-shadow] Variables are incorrectly marked as shadowed #2469

Closed

domoritz mentioned this issue Sep 2, 2020

Variables are incorrectly marked as shadowed eslint/eslint#13649

Closed

bradzacher mentioned this issue Sep 2, 2020

feat(eslint-plugin): [no-shadow] add option ignoreFunctionTypeParameterNameValueShadow #2470

Merged

bradzacher closed this as completed in #2470 Sep 2, 2020

FrankEssenberger mentioned this issue Oct 2, 2020

[Chore] Increase versions manual SAP/cloud-sdk-js#547

Merged

github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[no-shadow] new false positive in 4.0.0 for function argument types #2447

[no-shadow] new false positive in 4.0.0 for function argument types #2447

mceachen commented Aug 31, 2020

bradzacher commented Aug 31, 2020

Uh oh!

mceachen commented Aug 31, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

[no-shadow] new false positive in 4.0.0 for function argument types #2447

[no-shadow] new false positive in 4.0.0 for function argument types #2447

Comments

mceachen commented Aug 31, 2020

Repro

Additional Info

Versions

bradzacher commented Aug 31, 2020

Uh oh!

mceachen commented Aug 31, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.