When are non-null assertions acceptable?

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"};

const canvas = document.createElement("canvas");
const ctx = canvas.getContext("2d")!; // returns `CanvasRenderingContext2D | null`

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.

One side note: I haven’t been using Sonar for very long. It’s already caught a few outright mistakes in my code, and I’m grateful for that. But I view each flagged rule violation as a failure on my part – either I went against a best practice, or I configured my Sonar project incorrectly such that it’s enforcing something that it shouldn’t. I’ve come here a few times since our latest server upgrade to argue about rules that I think could be better. In each case, it’s because I recognize the value in what the rule is trying to accomplish, but the implementation flags too many lines that simply should not be changed.

If a rule flags 100 lines, and I have to mark 95 of them as “false positive”, I want to see that rule improved – even if that means changing the way I annotate my code. The most important thing to me in this case is avoiding those 95 interactions with the Sonar web interface.

1 Like