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

Syntactic checks against comparing to null/undefined in strictNullChecks mode #60425

Open
6 tasks done
lauraharker opened this issue Nov 5, 2024 · 7 comments
Open
6 tasks done
Labels
Experimentation Needed Someone needs to try this out to see what happens Suggestion An idea for TypeScript

Comments

@lauraharker
Copy link

πŸ” Search Terms

nullish, strictNullChecks, equals, undefined, null

βœ… Viability Checklist

⭐ Suggestion

When strictNullChecks are enabled, disallow == and === comparisons to null/undefined for expressions that are syntactically known to not be undefined.

I'd imagine the definition of syntactically not null/undefined expressions would be similar to that from #59217.

This is similar to #11920, but I think would be easier to land because it won't error on defensive programming null checks.

πŸ“ƒ Motivating Example

TypeScript's typeof operator always returns a string, but it can be hard to spot the difference between checking "undefined" and just undefined in code like this:

if (typeof foo === undefined) {

In strictNullChecks mode, TypeScript will catch unnecessary comparisons like this, for expressions that at runtime it knows can never be null or undefined.

πŸ’» Use Cases

  1. What do you want to use this for? catch user errors
  2. What shortcomings exist with current approaches? there's the https://typescript-eslint.io/rules/no-unnecessary-condition/, but that's type-based so does have the problem of flagging defensive programming. it's also nice to have things be in the compiler instead of a separate tool.
  3. What workarounds are you using in the meantime? nothing at the moment, if this is infeasible will probably create a custom lint check.
@MartinJohns
Copy link
Contributor

Duplicate of #29200.

@lauraharker
Copy link
Author

I'd argue this is broader in scope than #29200. Typeof was the motivating example, but TS could also check other code patterns as it does for nullish coalesing after #59217.

@MartinJohns
Copy link
Contributor

I mean.. the other cases are covered by #11920.

@lauraharker
Copy link
Author

My understanding is that a major concern about #11920 was breaking code intentionally checking for null/undefined against an API excluding null/undefined.

We intentionally allow this for the sake of defensive programming (i.e. defending against missing inputs from non-TS code). If there's enough demand we could add a flag or something.

& the discussion on the PR to fix that issue #59559 (comment) seems to be concluding that it's too breaking to land without adding an extra flag.

I'd expect this change to be significantly less breaking specifically because it would be syntactic only.

If the TS team is interested in rescoping #11920 to be about cases only syntactically known to be not null/undefined, or doesn't want to add a syntactic-only check, I understand, but I would like to get feedback specifically on the idea of a syntactic-only check so that it's not lost in the noise in #11920.

@lauraharker
Copy link
Author

To clarify, I'm not proposing to flag this code from #11920 (comment):

declare const value: number;
if (value !== null) { // <-- somehow valid, expected to be invalid, since number doesn't have null as a possible value
}
if (value !== '') { // <-- invalid as expected 
}

because value is not syntactically known to always be null, that's just a type system construct.

@lauraharker
Copy link
Author

Other cases that could be covered, motivated by the nullish coalesce checks from #59217:

if (((x) => x) !== undefined) {}
if ("" !== undefined) {}
if ({foo: 3} !== undefined) {}

@RyanCavanaugh
Copy link
Member

I think this is a decent idea now that we have a notion of "syntactically provable to not be undefined". I put up a very drafty draft at #60433 to see if this is likely to be useful.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experimentation Needed Someone needs to try this out to see what happens labels Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experimentation Needed Someone needs to try this out to see what happens Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants