Eliminate prefer_void_to_null?

[eernstg]

Describe the issue

The lint prefer_void_to_null is probably obsolete. It is certainly possible that it has been helpful in the effort to eliminate some suboptimal usages of the type Null, but it is most likely not a good fit for the language today. I'd recommend that this lint is removed entirely.

To Reproduce

Several examples have been given where this lint emits a message, but the advice is not applicable or useful. Here is one example:

void main() {
  List<(int?, int?)> xs;
  ...
  xs = <(Null, int)>[(null, 10)]; // LINT
}

In this case it is not possible to replace the type Null by the type void, as recommended by the lint (it's a compile-time error to do that, as it should be). Also, the chosen types may be slightly odd, but they do make sense in this context.

Further discussions and examples can be found in several issues:

• False positive with prefer_void_to_null #4444
• prefer_void_to_null false positive in case of nullable generics #3894
• Adjust documentation of prefer_void_to_null #3794
• void should not have the same behavior as dynamic sdk#53759

Finally, consider the examples given in the documentation for the lint itself (renaming the declarations because they were all named f):

// BAD:

Null f1() {}
Future<Null> f2() {}
Stream<Null> f3() {}
f4(Null x) {}

// GOOD:

void f1() {}
Future<void> f2() {}
Stream<void> f3() {}
f4(void x) {}

It is not obvious why the type Null has been used in the signature of these functions (and some of them might actually need to be updated to use Never today in order to make sense), but we should note that all of those changes would be breaking.

If f1 is called and the result is assigned to a variable of type T? for any T then it would now be a compile-time error. Similarly for f2 and f3 if the returned value is assigned to Future<T?> or Stream<T?> for any T. Finally, f4 could be an instance method which is overridden by f4(int? iq), and the change to f4(void x) {} would now break that override.

The motivation for the lint was probably that this is good breakage, because those functions should not have been declared using the type Null in the first place. However, it is not obvious to me that this kind of motivation is justified today: Is it really true that the change from Null to void is useful, and worth the breakage?

I tend to think that the spurious occurrence of null at run time used to be an issue worth fighting against, but with null safety in the type system we already know how to avoid null when it really shouldn't occur, and if nulls are allowed in any given context then every non-Object? usage of the null will be guarded by a null check.

Expected behavior

The lint should only give advice which is applicable and useful, but it seems to produce a large amount of false positives.

Additional context

Null safety has completely changed the role played by the type Null. In this new context, prefer_void_to_null does not play a constructive role in most of the situations where it used to be helpful. I don't think there is a natural and useful modification which can be used to bring this lint up to date, hence I'm suggesting that get gets removed.

Does anyone have some really compelling reasons why this lint is still useful?

[srawlins]

I agree I think it has outlived its usefulness; I was getting analysis_server compliant and it was pretty awkward; there were plenty of valid uses of Null.

[pq]

I'm inclined to agree. Thanks for opening this @eernstg!

[lrhn]

I'm not sure the lint is that bad, as long as it's restrained to where it fits best.
But it's also possible that it's not necessary any more, since people are doing the right thing anyway.

Where the lint makes sense is function parameter and return types.
A function returning Null, an async function returning Future<Null>, or a parameter type being Null all mean that the value is not important. It can only be one thing, so there is no need to ever look at the value. And a value you never look at is better represented as void.

The lint shouldn't apply anywhere else. That might be the issue here, it's being over-applied to places where you do care.

So the real problem is false positives.

If a return type is constrained so that it cannot be made void, then the lint shouldn't apply.

It may be possible to have other false positives, where a return type really should be null, even if it's not constrained so that it has to be so. I can't come up with a good example, but it's possible.

[eernstg]

The main reason why I think this lint is less helpful than it used to be is that the main motivation has been transformed drastically. In pre-null-safety code we could have this, with no compile-time errors:

// @dart = 2.9

Future<Null> f() async {
  return;
}

void main() async {
  int i = await f(); // Look out!
}

At the comment, we'd initialize i using an expression of type Null, and that was OK according to the type checker because Null was a subtype of int. However, today we'd have this:

Future<Null> f() async {
  return;
}

void main() async {
  int i = await f(); // Compile-time error, not assignable.
  Null n = await f(); // OK. Silly, but honest.
  int? j = await f(); // OK, but doesn't forget about null.

  j.isEven; // Compile-time error, we still haven't forgotten about null.
}

Hence, because we're tracking null precisely today, the null object doesn't get a chance to escape scrutiny the way it did in the olden days. I think this means that the lint doesn't help anyone so much today.

@lrhn wrote:

a value you never look at is better represented as void.

This is a point well taken. An expression with static type void does get an even more harsh treatment than an expression of type Null.

However, the null object is actually a quite powerful representation of "nothing to see here!", because it has a safe dynamic test: If an expression has static type int? then it could represent an int result as an int value, and it could represent "no result available" as null, and we would be able to use if (it is int) ... to make the distinction, precisely.

An expression of type void doesn't have a similar ability, because there is no way we can express a type that says "we might have an int or we might have nothing" using void, and it wouldn't be safe anyway because any object is a type correct value of an expression of type void.

In other words, we should probably get used to the idea that void can be used to indicate "do not use this object", and does that quite well, but the null object can also represent a similar concept ("there's nothing here"), and none of those is better than the other in all respects.

[srawlins]

If a return type is constrained so that it cannot be made void, then the lint shouldn't apply.

This would be great. Can it be known? This is the example from the analysis_server package:

String? foo() {
  if (1 == 2) return reportBad();
  // ...
}

Null reportBad() {
  print('bad');
  return null;
}

Changing reportBad to return void, and then all of the call sites to reportBad(); return null; got very noisy. I didn't write this API, but after trying to change the return type of all of the reportX methods to void, and inserting dozens of return null; statements, I really prefer the Null API.

[eernstg]

Can it be known?

I'd say no: The analyzer needs to be able to perform the static analysis on a library knowing only the transitive closure of its imports, and we have no reason to assume that there aren't thousands of clients (in libraries that aren't in that transitive closure) who are depending on the given function/method declaration. Each of them may break if the return type is modified to a (much) more general type.

.. after trying to change the return type of all of the reportX methods to void, and inserting dozens of return null; statements, I really prefer the Null API.

I think this is a good example, showing that we should just stop shouting at Null APIs in general, because they are not all bad. (And I suspect that the ones which are actually bad are basically already receiving as many diagnostic messages as they'd ever need, because of the strict null checking that we have today. Oh, btw, isn't that a great fact! ;-)

[lrhn]

It's hard to know why someone chose Null. It's only really for overriding instance methods that you can clearly see the constraints.

So while there are cases where it's correct to recommend void over Null, they're not always easy to distinguish from cases where using Null is correct (so, false positives), and because Null is no longer assignable to any type, using Null is more likely to be a deliberate choice.

Null is no longer special, it's just a singleton type, which means that having a value of that type is irrelevant, because you know ahead of time which value it will be. That's why using void better shows that the value doesn't matter. That goes for () and a single-element enum too, or any other type that we can statically identify as only ever having precisely one value. Null no longer deserves a lint of its own.

[eernstg]

while there are cases where it's correct to recommend void over Null, they're not always easy to distinguish from cases where using Null is correct

I think this supports the view that prefer_void_to_null is likely to be obsolete.