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

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 
}

About this issue

  • Original URL
  • State: open
  • Created 8 years ago
  • Reactions: 41
  • Comments: 15 (3 by maintainers)

Most upvoted comments

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?

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

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.

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

I would also like to have it implemented for the same reason as https://github.com/microsoft/TypeScript/issues/14764: vulnerable refactoring. This proposal is a bit more broad, then https://github.com/microsoft/TypeScript/issues/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 šŸ˜„

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.