After a recent upgrade, my Sonar server started applying the rule typescript:S2966, “Non-null assertions should not be used”. I agree that use of the assertion operator is a red flag because it can sometimes be used to work around the type checker and just “make it work”, at the cost of runtime safety.
However: there are also cases where you have outside knowledge that a variable must be defined at a given point in program execution, but circumstances prevent you from codifying that in the type information. Consider:
interface Foo {
x?: string;
}
const f: Foo = {x: "hey"};
f.x!.charCodeAt(0);
const canvas = document.createElement("canvas");
const ctx = canvas.getContext("2d")!; // returns `CanvasRenderingContext2D | null`
ctx.drawImage(...);
I contend that the assertion operators here represent the only correct way to write this code, and further, that encouraging people to avoid using them results in objectively worse, less-readable code.
The first example is a pretty simple case. I could use as const typing instead of explicitly specifying the type on the variable declaration, but this doesn’t scale well to large or complex interfaces. Alternately, I could break out the value of x as its own variable (const x = "hey", const f: Foo = {x};) but it introduces needless complexity and again doesn’t scale well. The assertion is much easier to read.
The second case is a stronger argument. The getContext method is typed the way it is because the function returns null if the canvas has been set to another context-mode previously, or if the string argument is invalid. In the above code, we know that the canvas has not been previously initialized, and that the passed string is valid, so the method can never return null. The only way to avoid using an assertion operator is to perform a runtime check and error out. This would introduce an error pathway that can never be tested, which is inarguably worse.
So, I’m trying to work out how to handle this in my code. I believe it would be appropriate to annotate each use of the non-null assertion operator with a comment explaining why it’s needed – in fact, I try to do that already. I would definitely support a rule that enforces this. (Is there a way to tell Sonar to ignore a specific error on a given line? I see //NOSONAR in the docs but I’d prefer to ignore only this one error, if possible.) But I strongly disagree with branding the non-null assertion operator as “considered harmful” generally, because there are a number of occasions where all the alternatives are worse.