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

no-unnecessary-type-assertion: Don't check ! if --strictNullChecks is not enabled#3724

Merged
JoshuaKGoldberg merged 4 commits intomasterfrom
unknown repository
Jun 15, 2019
Merged

no-unnecessary-type-assertion: Don't check ! if --strictNullChecks is not enabled#3724
JoshuaKGoldberg merged 4 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Feb 16, 2018

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

Doesn't check whether x! is an unnecessary non-null assertion if --strictNullChecks is not enabled.

CHANGELOG.md entry:

[bugfix] no-unnecessary-type-assertion: Don't check x! non-null assertions if --strictNullChecks is not enabled

@ajafff
Copy link
Contributor

ajafff commented Feb 22, 2018

@Andy-MS what's the motivation for this change? Adding assertions during migration to --strictNullChecks?

@ghost
Copy link
Author

ghost commented Feb 22, 2018

With --strictNullChecks on this change is unnecessary. But without --strictNullChecks, the rule currently produces a lot of false positives at x! because it gets a non-null type for x.

@ajafff
Copy link
Contributor

ajafff commented Feb 22, 2018

Yeah, I get that. But what's the use case?
There's no reason to use non null assertions without strictNullChecks. The assertion is really unnecessary.
It only makes sense if you are migrating to strictNullChecks.

@ghost
Copy link
Author

ghost commented Feb 22, 2018

At least for TypeScript, we annotated our code as best as we could even without having --strictNullChecks enabled. I wouldn't want to have to remove all of those assertions to enable this lint rule, then have to add them all back when enabling --strictNullChecks.

@giladgray
Copy link

@Andy-MS this change looks good! would you mind merging latest master and resolving merge conflicts?

@NaridaL
Copy link
Contributor

NaridaL commented Jul 5, 2018

What's the status on this?

@JoshuaKGoldberg
Copy link
Contributor

👋 @Andy-MS and @NaridaL! I don't know if this is still relevant or useful to you, but heads up that this is now merged in and will be available in the next TSLint release. Sorry this took so long!

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.

4 participants