disallow comparing to null and undefined unless they are valid cases in strict null mode

[zpdDG4gta8XKpMCd]

the rationale is the same as for --noUnusedLocals which in this case would be --noUselessNullChecks (i am not proposing a new flag, the existing --strictNullChecks flag should be used, this is for illustration only)

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 
}

[RyanCavanaugh]

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.

[Spongman]

i would suggest to make type safety the default, and have interop require explicit casting.

[DanielRosenwasser]

Internal implementation detail: right now we use the comparability relationship for type assertions (casting) - if we did this, we'd need to have a separate type relationship (or ad-hoc check) for allowing you to cast undefined/null to any other type.

[wizarrc]

#17896 (comment)

As a quick fix for now there is a lint rule: https://palantir.github.io/tslint/rules/strict-type-predicates/

This seems to work good enough for me right now, but in the long run I would like to see this baked into the language.

[devoto13]

I would also like to have it implemented for the same reason as #14764: vulnerable refactoring. This proposal is a bit more broad, then #14764. Maybe they should be merged?

In the recent refactoring I've changed public key: string | null; to public key: string;. null was used to mark a special case and now it was decided to use particular string constant for that special case instead. Deep in the codebase there was a bunch of checks like:

if (myObject.key == null) {
  // perform some logic for the special case
}

This silently stopped working and took quite a bit of effort to debug. It was also surprising that this didn't fail type checking. Maybe do it as strictNullComparisons flag or something, so it is true by default under strict mode and then consumers willing to do defensive programming can opt out? This way people will get consistent (and expected) behaviour out of the box and those, who need to defend their code from JavaScript can do so as well 😄

[jfirebaugh]

I just encountered a bug similar to @devoto13. I was making some code work with --strictNullChecks that was trying to initialize a property declared as prop?: number with null. I figured that given the type declaration, the correct fix was to initialize with undefined instead. This caused a bug because other code did a strict-equality check foo.prop === null which was now always false. This feature would have flagged the comparison so that I would have known to change the type declaration to prop: number | null instead.

[pladaria]

We had bugs in our codebase due to this:

For example:

This is a bug:

if (typeof foo === undefined) { // this is impossible
}

it should be:

if (typeof foo === 'undefined') {
}

I know it is a dumb bug but shit happens and this is hard to notice, even with code reviews.

The typesystem shouldn't allow this

[softwareCobbler]

i'd like to vote for a flag or something for this; just refactored a few things from function foo() : T | null to function foo() : T | undefined and plenty of post-use-site checks like result === null became silently borked. I saw no squiggles and thought "wow that refactoring went quick!" but then runtime failures got me.

[ugultopu]

You can use the no-unnecessary-condition rule from TypeScript ESLint for this.

[RA80533]

Has opinion changed on this at all? I see that in 2016 it was of the opinion that allowing this behavior would be better for defensive programming when dealing with non-TS code.

Strangely enough, this behavior exists on the typeof operator. In other words, you can do the following without error:

 typeof null === null;

The snippet above is likely something a beginner might do if after they attempt to check it against "null" because the compiler will correctly present an error for that (playground):

// @errors: 2367

typeof null === "null";

[c-vetter]

The rationale behind the current behavior is understandable. However, in my view it lends itself to sloppy typing. My strategy is to guard the boundaries where data comes in from unsafe (=non-TS) sources. And there, null and undefined need to be accounted for, but that is really no special case as far as typing is concerned. Rather, null and undefined are just necessarily possible value types. Based on that, strict typing makes a lot of sense.

Until today, I fully expected the behavior this issue calls for to be present when strict is active. Then I came across a case similar to the OP sample and wondered why there was no error message. I even checked the tsconfig to see if somebody had disabled strict or strictNullChecks.

Please add this behavior, possibly behind a new setting 🙂

@DanielRosenwasser

Internal implementation detail: right now we use the comparability relationship for type assertions (casting) - if we did this, we'd need to have a separate type relationship (or ad-hoc check) for allowing you to cast undefined/null to any other type.
Is this still a blocker? If yes:
How is this casting required? Isn't the problem an unintended non-distinction, which would be functionally equivalent to casting?

@RyanCavanaugh
This issue is marked Awaiting More Feedback. Can you say roughly what this issue would need to get some traction, how far off it is?

[typescript-bot]

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.

---

Comment by @RA80533

❌ Failed: -

• This comparison appears to be unintentional because the types '"string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"' and '"null"' have no overlap.

Historical Information

Version	Reproduction Outputs
4.2.2, 4.3.2, 4.4.2, 4.5.2, 4.6.2	❌ Failed: - This condition will always return 'false' since the types '"string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"' and '"null"' have no overlap.

[bradenhs]

Since this issue is "awaiting more feedback" I'd just like to add my feedback that allowing comparison to null for non-nullable types in strict mode is far from intuitive behavior. I've encountered bugs that would've been prevented had the compiler caught these type of impossible comparisons to null. I would love to see an additional flag for this if adding this behavior to strictNullChecks would be too disruptive to existing code.

[ersjim]

Just had an infinite loop that lead me to this issue. I was surprised to find out that I can compare a string to null.

const tokenStream = new Tokenizer(input);
while (tokenStream.peek() !== null) { // will never happen! Returns "" instead of null at EOF.
  processNextToken(tokenStream.next());
}

The problem is I was assuming that tokenStream.peek would return null at the end of the file, but it instead returns an empty string. The return type of the method is a non-nullable string. I really feel like the type system should have caught this before bricking my computer and making me hard reset.

[skondrashov]

Also adding that I ran into a case where I was allowed to compare a Date to null and was confused why typescript was allowing it.

no-unnecessary-condition from typescript eslint didn't help me either, would really like to know how I can enable type checking for this, or to have that feature added if it's not possible currently.