Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: unsafe_variance #4111

Open
5 tasks done
eernstg opened this issue Mar 6, 2023 · 2 comments
Open
5 tasks done

proposal: unsafe_variance #4111

eernstg opened this issue Mar 6, 2023 · 2 comments
Labels
lint-proposal P3 A lower priority bug or feature request status-pending type-enhancement A request for a change that isn't a bug

Comments

@eernstg
Copy link
Member

eernstg commented Mar 6, 2023

unsafe_variance

Lint a getter or method return type if it is non-covariant.

Description

Lint the situation where an instance getter (including instance variables) has a type where one or more type variables declared by the enclosing class occur in a non-covariant position. Similarly, lint the situation where an instance method has a return type which is non-covariant in the same sense.

Details

Assume that g is an instance getter in a class that declares a type parameter X, and assume that X occurs in the type of g in a position which is not covariant. It could be a contravariant position or an invariant position, and X could occur multiple times, but if X occurs in one of the "forbidden" positions then it doesn't help that it also occurs in some covariant positions.

In this situation there will be a caller-side check of the type of the result returned by said getter, because the run-time value of that return type is not guaranteed to be a subtype of the statically known value of the return type.

The recommended action in this situation is to use a type that soundly characterizes the value returned by said getter, even though that type may be considerably more general. The point is that the static type cannot be trusted, and hence it is not helpful to use that type as the static type.

A similar consideration applies for a method whose return type has a non-covariant occurrence of one or more type parameters of the enclosing class.

Kind

This lint guards against run time errors.

Bad Examples

class A<X> {
  void Function(X) func; // LINT
  A(this.func);
}

class B<X> {
  X Function(X) foo() => (X x) => x; // LINT
  Y Function<Y extends X>(Y) bar() => <Y extends X>(y) => y; // LINT
}

void main() {
  A<num> a = A<int>((i) {});
  a.func; // Throws. We don't even have to call it.
  B<num> b = B<int>();
  b.foo(); // Throws, even though we ignore the returned value.
  b.bar(); // Ditto.
}

Good Examples

class A<X> {
  void Function(Never) func;
  A(this.func);
}

class B<X> {
  X Function(Never) foo() => (X x) => x;
  Function bar() => <Y extends X>(y) => y;
}

void main() {
  A<num> a = A<int>((i) { print(i); });
  var func = a.func; // OK.
  if (func is void Function(int)) func(42); // OK.

  B<num> b = B<int>();
  var func2 = b.foo(); // OK.
  var func3 = b.bar(); // OK.
  // func2('Hi'); // Compile-time error.
  if (func2 is num Function(num)) print(func2(-1.5)); // OK, but the condition is false at run time.
  if (func2 is int Function(int)) print(func2(43)); // OK.
  print(func3<int>(44)); // OK. A dynamic invocation, but the dynamic type checks succeed.
}

The above program prints '42', '43', and '44'. Note that func2 has static type num Function(Never), such that it is guaranteed that there are no statically safe invocations of func2: We must check the dynamic type in order to call it (or we can call it dynamically as in (func2 as dynamic)('Hi')), which means that the static typing describes the actual situation correctly.

Discussion

The underlying issue has been well-known for many years (see, for example, https://academic.oup.com/comjnl/article/32/4/305/377555, which is from 1989). The particular instance of the issue which has been known as "contravariant members" was described in dart-lang/language#296 in 2019. The reason why I'm proposing that we introduce a lint right now is that the issue has popped up more frequently in recent months.

It could be claimed that a dynamic caller-side type check is no worse than the dynamic check which is performed, e.g., whenever an element is added to a list. In general, this kind of check (let's call it a 'callee-side' check) occurs when a Dart instance method is called, and a formal parameter of that method has a type that contains a type variable from the enclosing class in a covariant position. In particular, List.add has a parameter of type E, which is a type parameter of List.

However, the caller-side check is arguably more disruptive than the callee-side checks. The point is that myList.add(42) will succeed if myList has an actual type argument T such that 42 is T. However, with a.func in the first bad example we don't even get to call the function, we incur a dynamic error simply by evaluating a.func. This means that there is no way we could adapt the actual arguments in an invocation like a.func(...) such that the resulting invocation would succeed (for instance, a.func(45) will throw, in spite of the fact that a.func is a function that accepts an argument of type int).

As the 'Good examples' illustrate, we can return the same functions using a less informative static type (but a sound one!), and then we can check the type of the returned function object and perform a statically safe invocation.

One colloquial way to say this is that the level of static typing is expressed honestly in the good examples, as opposed to the bad examples where the static type promises more than the run time semantics can fulfill.

Discussion checklist

@eernstg
Copy link
Member Author

eernstg commented May 17, 2024

Note that there is an alternate strategy to avoid run-time type errors for a class that has a "contravariant member": The member can be used in a type safe manner from inside the body of that class:

class A<X> {
  void Function(X) fun;
  X arg;
  A(this.fun, this.arg);
  void invokeFun() => fun(arg); // Safe.
}

void main() {
  A<num> a = A<int>((i) => print(i.isEven), 42);
  a.invokeFun(); // Runs and does not throw.
  a.fun(a.arg); // Throws.
  print('Not reached');
}

The primary advice given in the message from unsafe_variance would still be to avoid having a contravariant member in the first place, but it could be useful to mention this strategy as well: "Use the contravariant member only from inside the class body".

@eernstg
Copy link
Member Author

eernstg commented Sep 11, 2024

https://dart-review.googlesource.com/c/sdk/+/384700 provides an implementation of this lint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lint-proposal P3 A lower priority bug or feature request status-pending type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants