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

no-unnecessary-type-assertion complains over valid code #3505

Closed
jeffijoe opened this issue Nov 21, 2017 · 12 comments
Closed

no-unnecessary-type-assertion complains over valid code #3505

jeffijoe opened this issue Nov 21, 2017 · 12 comments

Comments

@jeffijoe
Copy link

Bug Report

  • TSLint version: 5.8.0
  • TypeScript version: 2.6.1
  • Running TSLint via: CLI

TypeScript code being linted

image

const { response } = await throws<AxiosError>(
  api.user1.createRole({ name: 'Nope' })
)
expect(response!.status).toBe(400)
expect(response!.data.message).toMatch(/admin/)

with tslint.json configuration:

{
  "extends": ["tslint-config-standard", "tslint-config-prettier"],
  "rules": {
    "no-var-requires": false,
    "ordered-imports": [false],
    "no-console": [false],
    "member-access": [false],
    "object-literal-sort-keys": false,
    "await-promise": false,
    "no-use-before-declare": false
  }
}

Actual behavior

no-unnecessary-type-assertion complains over response, even though response is AxiosResponse<any> | undefined

ERROR: myfile.ts[13, 14]: This assertion is unnecessary since it does not change the type of the expression.
ERROR: myfile.ts[14, 14]: This assertion is unnecessary since it does not change the type of the expression.

Expected behavior

It should not error.

@ajafff
Copy link
Contributor

ajafff commented Nov 22, 2017

@jeffijoe can you please try to make a create a minimal repro so I can debug this locally?

@jeffijoe
Copy link
Author

@ajafff of course! I've attached a ZIP.

tslint-repro.zip

npm install, then open src/test.ts to see the correct code.

Now run npm run lint and see the correct code being broken by tslint.

@aluanhaddad
Copy link

I took a look. This is a downstream issue that is caused by the package tslint-config-standard. If I place the no-unnecessary-type-assertion rule directly in your tslint and remove the aforementioned package from tslint.json everything works as expected.

Moral of the story: Don't trust linter configs written by people who rely on ASI 😛.

@jeffijoe
Copy link
Author

@aluanhaddad what is the issue with the tslint-config-standard package? What's making it "not work"? And what does ASI have to do with anything? 😛

@ajafff
Copy link
Contributor

ajafff commented Nov 27, 2017

@aluanhaddad thanks for digging into this.

@jeffijoe This is actually caused by no-unused-variable. See #3455 for further details.

@ajafff ajafff closed this as completed Nov 27, 2017
@aluanhaddad
Copy link

@jeffijoe I was just trying to get your goat 😃

@jeffijoe
Copy link
Author

@ajafff I just read through that issue, not sure what the solution is? 😄

@ajafff
Copy link
Contributor

ajafff commented Nov 27, 2017

@jeffijoe sorry, I picked the wrong of the dozens of similar issues. #3478 (comment) provides more information

@jeffijoe
Copy link
Author

@ajafff thanks! 👍 Too bad workarounds are needed though. 😞

@buu700
Copy link
Contributor

buu700 commented Dec 2, 2017

I'm running into a similar issue with this code: const elements = <HTMLElement[]> Array.from(document.querySelectorAll('balls'));, where the type of elements without the cast would have actually been Element[], breaking code further down that relies on properties of HTMLElement.

Does this sound like the same root cause, @ajafff, or should I submit a new issue?

@ajafff
Copy link
Contributor

ajafff commented Dec 2, 2017

@buu700 if you disable no-unused-variable and it works as expected, it's the same issue. Otherwise you should submit a new issue.

@buu700
Copy link
Contributor

buu700 commented Dec 2, 2017

Got it, thanks! Just tested with no-unnecessary-type-assertion as the only rule enabled and reproduced the problem, so I'll submit a new issue.

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

No branches or pull requests

4 participants