Skip to content

feat(eslint-plugin): add no-unnecessary-type-assertion rule #157

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

Merged
merged 8 commits into from
Jan 29, 2019

Conversation

uniqueiniquity
Copy link
Contributor

Adding equivalent of TSLint's no-unnecessary-type-assertion.

@codecov
Copy link

codecov bot commented Jan 28, 2019

Codecov Report

Merging #157 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
+ Coverage   95.69%   95.76%   +0.06%     
==========================================
  Files          49       50       +1     
  Lines        2487     2526      +39     
  Branches      373      373              
==========================================
+ Hits         2380     2419      +39     
  Misses         63       63              
  Partials       44       44
Impacted Files Coverage Δ
...-plugin/lib/rules/no-unnecessary-type-assertion.js 100% <100%> (ø)

Copy link
Collaborator

@armano2 armano2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm unsure if adding fixer for this rule is good idea, if someone did cast, than most likely it was needed for something.

@adidahiya
Copy link

if someone did cast, than most likely it was needed for something

Sometimes extraneous casts are left around after refactoring, or an upgrade of the compiler where it got smarter about inferring types. Either way, I think that fixing is the right move for no-op casts because it is always safe... do you have any counter examples where it's not safe?

@uniqueiniquity uniqueiniquity merged commit 38abc28 into typescript-eslint:master Jan 29, 2019
@uniqueiniquity uniqueiniquity deleted the noUnnecAssert branch January 29, 2019 20:58
*/

'use strict';
const tsutils = require('tsutils');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uniqueiniquity @JamesHenry Upgraded to latest canary (1.1.2-alpha.4) to work around #161.

Which then triggered

$ yarn run lint:ts
$ eslint . --ext .ts,.tsx --max-warnings=0
Error: Cannot find module 'tsutils'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:581:15)
    at Function.Module._load (internal/modules/cjs/loader.js:507:25)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:22:18)
    at Object.<anonymous> (C:\REDACTED\node_modules\@typescript-eslint\eslint-plugin\lib\rules\no-unnecessary-type-assertion.js:7:17)

Had to add tsutils to package.json. Is that expected ?

Maybe a slight addition to the README would be helpful regarding this. Or even better, add tsutils as a dependency of that plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nulltoken My mistake. I'm not sure how I missed adding it to the package.json and it still managed to pass the tests...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I get it; tslint is being installed as a dev dependency, and that happens to pull in tsutils so it works in a dev scenario. I'll get a fix up right now.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

uniqueiniquity added a commit that referenced this pull request Jan 30, 2019
Managed to forget to add `tsutils` as a true dependency to `eslint-plugin` in #157.
@j-f1
Copy link
Contributor

j-f1 commented Jan 31, 2019

LGTM

@j-f1 j-f1 removed their request for review January 31, 2019 16:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants