Introduce the lint unsafe_variance

This CL implements a new lint, `unsafe_variance`. This lint emits a
warning whenever an instance member declaration has a signature where
a type variable declared by the enclosing class/mixin/enum occurs in
a non-covariant position in the return type (including the type of
an instance variable).

Issues: https://github.com/dart-lang/linter/issues/4111, with goals related to dart-lang/language#296 and dart-lang/language#524.

Change-Id: I1352d71d61fece03a432ccf0d98825a69e3a457f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384700
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Erik Ernst <[email protected]>

Author: eernstg

Diff for: pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -2441,6 +2441,13 @@ LintCode.unrelated_type_equality_checks_in_expression:
   status: needsEvaluation
 LintCode.unrelated_type_equality_checks_in_pattern:
   status: needsEvaluation
+LintCode.unsafe_variance:
+  status: noFix
+  notes: |-
+    A type like `X Function([X])` could be replaced by `X Function()`,
+    or by `X Function([Never])`, or by `Function`, depending on the
+    situation and the preferences of the developer. There is no
+    universally applicable fix.
 LintCode.use_build_context_synchronously_async_use:
   status: noFix
 LintCode.use_build_context_synchronously_wrong_mounted:

Diff for: pkg/analyzer/tool/diagnostics/diagnostics.md

@@ -29147,6 +29147,117 @@ bool f(String s) {
 }
 ```
 
+### unsafe_variance
+
+_This type is unsafe: a type parameter occurs in a non-covariant position._
+
+#### Description
+
+This lint warns against declaring non-covariant members.
+
+An instance variable whose type contains a type parameter of the
+enclosing class, mixin, or enum in a non-covariant position is
+likely to cause run-time failures due to failing type
+checks. For example, in `class C<X> {...}`, an instance variable
+of the form `void Function(X) myVariable;` may cause this kind
+of run-time failure.
+
+The same is true for a getter or method whose return type has a
+non-covariant occurrence of a type parameter of the enclosing
+declaration.
+
+This lint flags this kind of member declaration.
+
+#### Example
+
+**BAD:**
+```dart
+class C<X> {
+  final bool Function([!X!]) fun; // LINT
+  C(this.fun);
+}
+
+void main() {
+  C<num> c = C<int>((i) => i.isEven);
+  c.fun(10); // Throws.
+}
+```
+
+The problem is that `X` occurs as a parameter type in the type
+of `fun`.
+
+#### Common fixes
+
+One way to reduce the potential for run-time type errors is to
+ensure that the non-covariant member `fun` is _only_ used on
+`this`. We cannot strictly enforce this, but we can make it
+private and add a forwarding method `fun` such that we can check
+locally in the same library that this constraint is satisfied:
+
+**BETTER:**
+```dart
+class C<X> {
+  // ignore: unsafe_variance
+  final bool Function(X) _fun;
+  bool fun(X x) => _fun(x);
+  C(this._fun);
+}
+
+void main() {
+  C<num> c = C<int>((i) => i.isEven);
+  c.fun(10); // Succeeds.
+}
+```
+
+A fully safe approach requires a feature that Dart does not yet
+have, namely statically checked variance. With that, we could
+specify that the type parameter `X` is invariant (`inout X`).
+
+It is possible to emulate invariance without support for statically
+checked variance. This puts some restrictions on the creation of
+subtypes, but faithfully provides the typing that `inout` would
+give:
+
+**GOOD:**
+```dart
+typedef Inv<X> = X Function(X);
+typedef C<X> = _C<X, Inv<X>>;
+
+class _C<X, Invariance extends Inv<X>> {
+  // ignore: unsafe_variance
+  final bool Function(X) fun; // Safe!
+  _C(this.fun);
+}
+
+void main() {
+  C<int> c = C<int>((i) => i.isEven);
+  c.fun(10); // Succeeds.
+}
+```
+
+With this approach, `C<int>` is not a subtype of `C<num>`, so
+`c` must have a different declared type.
+
+Another possibility is to declare the variable to have a safe
+but more general type. It is then safe to use the variable
+itself, but every invocation will have to be checked at run
+time:
+
+**HONEST:**
+```dart
+class C<X> {
+  final bool Function(Never) fun;
+  C(this.fun);
+}
+
+void main() {
+  C<num> c = C<int>((int i) => i.isEven);
+  var cfun = c.fun; // Local variable, enables promotion.
+  if (cfun is bool Function(int)) cfun(10); // Succeeds.
+  if (cfun is bool Function(bool)) cfun(true); // Not called.
+}
+```
+
 ### use_build_context_synchronously
 
 _Don't use 'BuildContext's across async gaps, guarded by an unrelated 'mounted'

Diff for: pkg/linter/CHANGELOG.md

@@ -6,6 +6,7 @@
 - new _(experimental)_ lint: `omit_obvious_property_types`
 - new _(experimental)_ lint: `specify_nonobvious_property_types`
 - removed lint: `unsafe_html`
+- new _(experimental)_ lint: `unsafe_variance`
 
 # 3.6.0
 

Diff for: pkg/linter/example/all.yaml

@@ -206,6 +206,7 @@ linter:
     - unnecessary_to_list_in_spreads
     - unreachable_from_main
     - unrelated_type_equality_checks
+    - unsafe_variance
     - use_build_context_synchronously
     - use_colored_box
     - use_decorated_box

Diff for: pkg/linter/lib/src/lint_codes.g.dart

@@ -1762,6 +1762,14 @@ class LinterLintCode extends LintCode {
         uniqueName: 'unrelated_type_equality_checks_in_pattern',
       );
 
+  static const LintCode unsafe_variance = LinterLintCode(
+    LintNames.unsafe_variance,
+    "This type is unsafe: a type parameter occurs in a non-covariant position.",
+    correctionMessage:
+        "Try using a more general type that doesn't contain any type "
+        "parameters in such a position.",
+  );
+
   static const LintCode
   use_build_context_synchronously_async_use = LinterLintCode(
     LintNames.use_build_context_synchronously,

Diff for: pkg/linter/lib/src/lint_names.g.dart

@@ -546,6 +546,8 @@ abstract final class LintNames {
 
   static const String unsafe_html = 'unsafe_html';
 
+  static const String unsafe_variance = 'unsafe_variance';
+
   static const String use_build_context_synchronously =
       'use_build_context_synchronously';
 

Diff for: pkg/linter/lib/src/rules.dart

@@ -225,6 +225,7 @@ import 'rules/unnecessary_to_list_in_spreads.dart';
 import 'rules/unreachable_from_main.dart';
 import 'rules/unrelated_type_equality_checks.dart';
 import 'rules/unsafe_html.dart';
+import 'rules/unsafe_variance.dart';
 import 'rules/use_build_context_synchronously.dart';
 import 'rules/use_colored_box.dart';
 import 'rules/use_decorated_box.dart';
@@ -470,6 +471,7 @@ void registerLintRules() {
     ..registerLintRule(UnreachableFromMain())
     ..registerLintRule(UnrelatedTypeEqualityChecks())
     ..registerLintRule(UnsafeHtml())
+    ..registerLintRule(UnsafeVariance())
     ..registerLintRule(UseBuildContextSynchronously())
     ..registerLintRule(UseColoredBox())
     ..registerLintRule(UseDecoratedBox())