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-unnecessary-type-assertion] false positive in strict mode #2248

Closed
aiwenar opened this issue Jun 24, 2020 · 4 comments
Closed

[no-unnecessary-type-assertion] false positive in strict mode #2248

aiwenar opened this issue Jun 24, 2020 · 4 comments
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@aiwenar
Copy link

aiwenar commented Jun 24, 2020

Repro

parser: '@typescript-eslint/parser'

parserOptions:
  project: ./tsconfig.json

plugins:
  - '@typescript-eslint'

rules:
  '@typescript-eslint/no-unnecessary-type-assertion': error
interface State {}
interface Foo {}

// Those four ‘functions’ were longer fragments of code in the original non-reduced version.
declare function initialState(): State | null
declare function nextState(state: State): [Foo, State] | undefined
declare function condition(state: Foo): boolean
declare function useState(foo: Foo): void

export function bar(): void {
    let state = initialState()

    if (state == null) return

    for (;;) {
        const [foo, next]: [Foo, State] | [] = nextState(state) ?? []

        if (foo == null) return

        if (condition(foo)) {
            state = next!
            continue
        }

        useState(next!)

        break
    }
}

Expected Result

No error.

Actual Result

With code posted above ESLint reports

  20:21  error  This assertion is unnecessary since it does not change the type of the expression  @typescript-eslint/no-unnecessary-type-assertion
  24:18  error  This assertion is unnecessary since it does not change the type of the expression  @typescript-eslint/no-unnecessary-type-assertion

however, if any of those assertions is removed TypeScript instead reports

test.ts:15:58 - error TS2345: Argument of type 'State | null' is not assignable to parameter of type 'State'.
  Type 'null' is not assignable to type 'State'.

15         const [foo, next]: [Foo, State] | [] = nextState(state) ?? []
                                                            ~~~~~

test.ts:20:13 - error TS2322: Type 'State | undefined' is not assignable to type 'State | null'.
  Type 'undefined' is not assignable to type 'State | null'.

20             state = next
               ~~~~~

test.ts:24:18 - error TS2345: Argument of type 'State | undefined' is not assignable to parameter of type 'Foo'.
  Type 'undefined' is not assignable to type 'Foo'.

24         useState(next)
                    ~~~~

Additional Info

TypeScript error is only present when compiling in strict mode. The fact that initialState returns State | null also seems important; if it is changed to simply return State and the null check in line 12 is removed then, while TypeScript still requires type assertions, ESLint stops reporting any problems.

Versions

package version
@typescript-eslint/eslint-plugin 3.4.0
@typescript-eslint/parser 3.4.0
TypeScript 3.9.5
ESLint 7.3.1
node 12.16.3
npm 6.14.4
yarn 1.12.4
@aiwenar aiwenar added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Jun 24, 2020
@bradzacher
Copy link
Member

bradzacher commented Jun 24, 2020

This looks like a bug in typescript.
The API is reporting to us that the type of next is any.

image
repl

image
repl

This is obviously incorrect, as internally it's able to resolve the type of next to State | undefined.

Please file an issue with typescript - microsoft/TypeScript#36687

@bradzacher bradzacher added bug Something isn't working external This issue is with another package, not typescript-eslint itself and removed triage Waiting for maintainers to take a look labels Jun 24, 2020
@JoshuaKGoldberg
Copy link
Member

I reduced this down a bit to still get next: any:

interface State {}
interface Foo {}

declare function initialState(): State | null
declare function nextState(state: State): [Foo, State] | undefined

export function bar(): void {
    let state = initialState()

    if (state == null) return

    for (;;) {
        const [_, next]: [Foo, State] | [] = nextState(state) ?? []

        state = next!
    }
}

If you remove foo/Foo altogether TypeScript displays a more explicit 'next' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.:

interface State {}

declare function initialState(): State | null
declare function nextState(state: State | null): [State] | undefined

export function bar(): void {
    let state = initialState()

    if (state == null) return

    for (;;) {
        const [next]: [State] | [] = nextState(state) ?? []
             // ~~~~ 'next' implicitly has type 'any' because it does not have a
             // type annotation and is referenced directly or indirectly in its own initializer.

        state = next!
    }
}

That erroneous error goes away and next is correctly inferred as type State if you remove the state = next! line.

@bradzacher should this issue be closed, since the bug seems pretty clearly in TypeScript land?

@bradzacher
Copy link
Member

I think we should leave this open to help stop people from raising a duplicate.

@Josh-Cena
Copy link
Member

I can no longer reproduce this on the latest versions (playground). What about others?

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working external This issue is with another package, not typescript-eslint itself package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

No branches or pull requests

4 participants