dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
630 stars 170 forks source link

proposal: `nullable_extension_type_with_nullable_representation` #4842

Open eernstg opened 9 months ago

eernstg commented 9 months ago

nullable_extension_type_with_nullable_representation

Description

Flag every occurrence of a type of the form E? where E is an extension type whose instantiated representation type is nullable.

Details

If E is an extension type whose instantiated representation type is nullable then E? is "doubly nullable" because it is possible for an expression of that type to be null because of the ?, and it is also possible for the expression to have the value null because its representation object is the null object.

The purpose of this lint is to avoid unnecessary complexity by nudging developers in the direction of the most useful perspective, which is the latter (the representation object is the null object).

Kind

This lint guards against useless complexity, hence helping developers to avoid bugs and waste of time.

Bad Examples

extension type E(int? it) {...} // No issues here, but this declaration is used below.

void f(E? e) {} // Lint.
E? variable = ...; // Lint.

void main() {
  List<E?> xs = ...; // Lint.
  ...
  xs.add(null); // Allowed because the element type is of the form `T?`.
}

Good Examples

extension type E(int? it) {...}

void f(E e) {}
E variable = ...;

void main() {
  List<E> xs = ...;
  ...
  xs.add(E(null));
}

Discussion

To recap, if E is an extension type whose instantiated representation type is nullable then E? is "doubly nullable" because it is possible for an expression of that type to be null because of the ?, and it is also possible for the expression to have the value null because its representation object is the null object.

(Note that we're not talking about extension types whose representation type is non-nullable or even potentially non-nullable. If E2 is such an extension type, e.g., extension type E2(int it) {} then E2? is perfectly fine.)

This situation is overly complex, and the purpose of this lint is to ensure that such extension types are generally used without the ?, such that the value null is interpreted in one, well-defined manner: The representation object is the null object.

Why wouldn't we use the opposite approach, and consider null to indicate that the given expression of type E? has a value which "isn't there"? It seems reasonable to assume that an extension type E whose representation object can be null was created in order to support certain extension type members on the null object (as well as some other objects, of course). If a receiver has type E?, and the null object is interpreted to mean "this receiver isn't there" then we can't invoke those extension members. In other words, it seems to be the most useful default if we use the perspective that the representation object is the null object, not the perspective that the expression has a value which "isn't there". Hence we should prefer E rather than E?, so we flag the occurrences of E?.

One property of this approach may be debated: Do we never want to use E? as the type of a variable v because it is more convenient to say v = null than v = E(null)? (see the last part of the 'Good Examples'). This is a non-trivial trade-off, but the assumption which is used here is that it is more important to avoid the useless complexity than it is to make assignment of null textually concise. After all, v as E will still succeed, and we may then invoke members of E on null, so we might as well use v = E(null); as a reminder that the null object may indeed potentially be the receiver of invocations of members of E.

Discussion checklist

lrhn commented 9 months ago

A type like E? can be introduced through generics.

Map<String, E> map = ...;
var v = map["ok"]; // Inferred type `E?`.

The return type of map[] is E?. The v variable will get that type. You can't just type v as E v = map["ok"];, because that's not assignable.

Would the lint have a way to avoid warning about this, or do you have to do var v = map["ok"] as E;?

eernstg commented 9 months ago

Great question!

Assume that E is an extension type whose representation type is nullable.

Map<String, E> map = ...;
var v = map["ok"]; // Lint: `v` has inferred type `E?`.

The following considerations might be specific for the given example, but the example does illustrate some relevant facts:

I think it is useful for a developer who writes the above code to be aware of the fact that v could be null because map does not contain "ok" as a key, but v could also be null because "ok" is a key whose value is null (which is viewed as E(null)).

The developer could then respond to the lint by invoking containsKey, if the distinction matters, or making it map["ok"] as E if the key is definitely expected to exist and it is safe to consider E(null) as the value. A particularly tricky situation arises if v is null, and this is considered to show that the key wasn't there, but the key was actually present, and the developer just forgot that null can be a perfectly meaningful run-time value for an expression of type E.

Similar considerations could of course be relevant for a Map<String, T> where T is any potentially nullable type, but the case where the value type of a map is an extension type with a nullable representation is particularly non-obvious.