Proposal: Error when using non-null assertion on already non-null expression

[Sheeo]

Original idea from this stack overflow post.

Code

let x: string = "foo";
let y: string = x!;

Wanted behavior:

A compiler error or warning about superflous use of the non-null assertion operator !.

Actual behavior:

No compiler warnings or errors.

Implementation details

Naive implementation using the current control-flow assigned type can be found here.

It poses a problem on code like the following:

let x: string | undefined | null;
x = "";
x!.indexOf("foo"); // Error provided here, because type has been narrowed to just `string`

Whether control-flow type narrowing should affect the error or the analysis should prefer declared types is a point of discussion.

As a corollary, I also made using the operator an error when not compiling with strictNullChecks, but this produces errors when compiling the ts codebase itself.

Language feature checklist

• No syntactical implications of this feature
• Semantic changes described above
• No emission changes
• Breaking changes only on codebases superflously using the operator

[rotemdan]

I can see this as one of those checks that would sometimes be useful but at other times create unnecessary issues, say, when a variable's type is changed from a nullable to a non-nullable and back again to a nullable (when the type is inferred, this can happen in a very different part of the code). As you mentioned, if applied to types inferred through flow analysis it could make this a "programming annoyance" in some cases (like the way Go effectively tortures the programmer to remove package imports whenever they become unused and add them back when they are needed, over and over again).

I guess it can technically be covered by a type-checked tslint rule, but I'll take it a step further and suggest that if TS provided a type-checker plugin API that would integrate natively into the language service (e.g. would even be run in incremental compilation etc.) I would consider implementing it myself, along with many other similarly simple rules/checks (some heuristic, some precise).

[RyanCavanaugh]

See #8453

[Sheeo]

@RyanCavanaugh from what I see in the linked discussion, it was only about superflous non-null warnings. This issue also discusses introducing a warning when strictNullChecks aren't enabled.

Would a PR implementing the superflous warning, but preferring declared types so the described annoying case doesn't happen be considered for merge?

[RyanCavanaugh]

but preferring declared types so the described annoying case doesn't happen be considered for merge?

I'm not sure which behavior you're referring to here. Can you show an example?

[Sheeo]

I described it in the original post, perhaps not clearly enough. The naïve implementation I did provides an error for the following code:

let x: string | undefined | null;
x = "";
x!.indexOf("foo"); // Error provided here, because type has been narrowed to just `string`

If instead we made it only consider--if present--the declared type of variables, the above code wouldn't error.

There's technically nothing wrong with providing the error using the control flow narrowed type, but judging from the original discussion in the linked issue, it seems like this is a concern.