Open scheglov opened 1 year ago
It would be nice to be a little more explicit by describing when they can be avoided, or at least the conditions under which this lint will fire. Is the example you gave the only code pattern that should be caught, or are there others?
Maybe this is less about using the non-null-assert operator and more about the redundant test for null
(using both != null
and !
?
Either way, do we want it to catch more complex cases, such as
void f(int? x, int? y) {
if (x != null && y != null) {
x! + y!;
}
}
Is this something that would benefit from flow analysis? (@stereotype441)
Yes, I thought that there might be more cases when this lint could be reported. But this specific example that I provided is often used in our code base, as the failures here show.
The example that you provided, with formal parameters, actually is not what we want to cover by this lint. We specifically want it for places where we don't get type inference, because the checked value is returned by a getter, and we implicitly expect that the send read will return non-nullable value. So, this would be:
class A {
int? foo;
int? bar;
void f() {
if (foo != null && bar != null) {
foo! + bar!;
}
}
}
avoidable_null_assertion
Replace above with your proposed rule name. (See notes on naming in Writing Lints.)
Description
AVOID null assertions when they can be avoided.
You can instead use null check patters, or extract a local variable.
Details
Give a detailed description that should provide context and motivation. Ideally this could be used directly in the rule's documentation.
Kind
This guards against unsafe null assertions. Potentially faster performance, because null assertion costs something.
Bad Examples
Good Examples
Discussion
We have quite a few instances of such code in the analyzer, that would be reported by this lint, and hopefully automatically fixed.
Discussion checklist
Not yet ready CL.