Shortened the documentation of unsafe_variance

It is now 69 lines, and not the longest.

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

Author: eernstg

pkg/analyzer/tool/diagnostics/diagnostics.md

@@ -29153,111 +29153,83 @@ _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.
+The analyzer produces this diagnostic when an instance member has a result
+type which is [contravariant or invariant](https://dart.dev/resources/glossary#variance)
+in a type parameter of the enclosing declaration. The result type of a
+variable is its type, and the result type of a getter or method is its
+return type. This lint warns against such members because they are likely
+to cause a failing type check at run time, with no static warning or error
+at the call site.
 
 #### Example
 
-**BAD:**
+The following code produces this diagnostic because `X` occurs
+as a parameter type in the type of `f`, which is a
+contravariant occurrence of this type parameter:
+
 ```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.
+  bool Function([!X!]) f;
+  C(this.f);
 }
 ```
 
-The problem is that `X` occurs as a parameter type in the type
-of `fun`.
+This is unsafe: If `c` has static type `C<num>` and run-time type `C<int>`
+then `c.f` will throw. Hence, every invocation `c.f(a)` will also throw,
+even in the case where `a` has a correct type as an argument to `c.f`.
 
 #### 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:
+If the linted member is or can be private then you may be able
+to enforce that it is never accessed on any other receiver than `this`.
+This is sufficient to ensure that that the run-time type error does not
+occur. For example:
 
-**BETTER:**
 ```dart
 class C<X> {
+  // NB: Ensure manually that `_f` is only accessed on `this`.
   // ignore: unsafe_variance
-  final bool Function(X) _fun;
-  bool fun(X x) => _fun(x);
-  C(this._fun);
-}
+  bool Function(X) _f;
 
-void main() {
-  C<num> c = C<int>((i) => i.isEven);
-  c.fun(10); // Succeeds.
+  C(this._f);
+
+  // We can write a forwarding method to allow clients to call `_f`.
+  bool f(X x) => _f(x);
 }
 ```
 
-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:
+You can eliminate the unsafe variance by using a more general type for
+the linted member. In this case you may need to check the run-time type
+and perform a downcast at call sites.
 
-**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.
+class C<X> {
+  bool Function(Never) f;
+  C(this.f);
 }
 ```
 
-With this approach, `C<int>` is not a subtype of `C<num>`, so
-`c` must have a different declared type.
+If `c` has static type `C<num>` then you may test the type. For example,
+`c.f is bool Function(num)`. You may safely call it with an argument of
+type `num` if it has that 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:
+You can also eliminate the unsafe variance by using a much more general
+type like `Function`, which is essentially the type `dynamic` for
+functions.
 
-**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.
+  Function f;
+  C(this.f);
 }
 ```
 
+This will make `c.f(a)` dynamically safe: It will throw if and only if the
+argument `a` does not have the type required by the function. This is
+better than the original version because it will not throw because of a
+mismatched static type. It only throws when it _must_ throw for soundness
+reasons.
+
 ### use_build_context_synchronously
 
 _Don't use 'BuildContext's across async gaps, guarded by an unrelated 'mounted'

pkg/linter/messages.yaml

@@ -13195,8 +13195,83 @@ LintCode:
     documentation: |-
       #### Description
 
-      This lint warns against declaring non-covariant members.
+      The analyzer produces this diagnostic when an instance member has a result
+      type which is [contravariant or invariant](https://dart.dev/resources/glossary#variance)
+      in a type parameter of the enclosing declaration. The result type of a
+      variable is its type, and the result type of a getter or method is its
+      return type. This lint warns against such members because they are likely
+      to cause a failing type check at run time, with no static warning or error
+      at the call site.
 
+      #### Example
+
+      The following code produces this diagnostic because `X` occurs
+      as a parameter type in the type of `f`, which is a
+      contravariant occurrence of this type parameter:
+
+      ```dart
+      class C<X> {
+        bool Function([!X!]) f;
+        C(this.f);
+      }
+      ```
+
+      This is unsafe: If `c` has static type `C<num>` and run-time type `C<int>`
+      then `c.f` will throw. Hence, every invocation `c.f(a)` will also throw,
+      even in the case where `a` has a correct type as an argument to `c.f`.
+
+      #### Common fixes
+
+      If the linted member is or can be private then you may be able
+      to enforce that it is never accessed on any other receiver than `this`.
+      This is sufficient to ensure that that the run-time type error does not
+      occur. For example:
+
+      ```dart
+      class C<X> {
+        // NB: Ensure manually that `_f` is only accessed on `this`.
+        // ignore: unsafe_variance
+        bool Function(X) _f;
+
+        C(this._f);
+
+        // We can write a forwarding method to allow clients to call `_f`.
+        bool f(X x) => _f(x);
+      }
+      ```
+
+      You can eliminate the unsafe variance by using a more general type for
+      the linted member. In this case you may need to check the run-time type
+      and perform a downcast at call sites.
+
+      ```dart
+      class C<X> {
+        bool Function(Never) f;
+        C(this.f);
+      }
+      ```
+
+      If `c` has static type `C<num>` then you may test the type. For example,
+      `c.f is bool Function(num)`. You may safely call it with an argument of
+      type `num` if it has that type.
+
+      You can also eliminate the unsafe variance by using a much more general
+      type like `Function`, which is essentially the type `dynamic` for
+      functions.
+
+      ```dart
+      class C<X> {
+        Function f;
+        C(this.f);
+      }
+      ```
+
+      This will make `c.f(a)` dynamically safe: It will throw if and only if the
+      argument `a` does not have the type required by the function. This is
+      better than the original version because it will not throw because of a
+      mismatched static type. It only throws when it _must_ throw for soundness
+      reasons.
+    deprecatedDetails: |-
       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
@@ -13210,8 +13285,6 @@ LintCode:
 
       This lint flags this kind of member declaration.
 
-      #### Example
-
       **BAD:**
       ```dart
       class C<X> {
@@ -13228,8 +13301,6 @@ LintCode:
       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
@@ -13299,79 +13370,6 @@ LintCode:
         if (cfun is bool Function(bool)) cfun(true); // Not called.
       }
       ```
-    deprecatedDetails: |-
-      Don't declare 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.
-
-      **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`. A better approach 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`:
-
-      **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`).
-
-      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>((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_async_use:
     sharedName: use_build_context_synchronously
     problemMessage: "Don't use 'BuildContext's across async gaps."

pkg/linter/tool/machine/rules.json

@@ -2938,7 +2938,7 @@
     "incompatible": [],
     "sets": [],
     "fixStatus": "noFix",
-    "details": "Don't declare non-covariant members.\n\nAn instance variable whose type contains a type parameter of the\nenclosing class, mixin, or enum in a non-covariant position is\nlikely to cause run-time failures due to failing type\nchecks. For example, in `class C<X> {...}`, an instance variable\nof the form `void Function(X) myVariable;` may cause this kind\nof run-time failure.\n\nThe same is true for a getter or method whose return type has a\nnon-covariant occurrence of a type parameter of the enclosing\ndeclaration.\n\nThis lint flags this kind of member declaration.\n\n**BAD:**\n```dart\nclass C<X> {\n  final bool Function(X) fun; // LINT\n  C(this.fun);\n}\n\nvoid main() {\n  C<num> c = C<int>((i) => i.isEven);\n  c.fun(10); // Throws.\n}\n```\n\nThe problem is that `X` occurs as a parameter type in the type\nof `fun`. A better approach is to ensure that the non-covariant\nmember `fun` is _only_ used on `this`. We cannot strictly\nenforce this, but we can make it private and add a forwarding\nmethod `fun`:\n\n**BETTER:**\n```dart\nclass C<X> {\n  // ignore: unsafe_variance\n  final bool Function(X) _fun;\n  bool fun(X x) => _fun(x);\n  C(this.fun);\n}\n\nvoid main() {\n  C<num> c = C<int>((i) => i.isEven);\n  c.fun(10); // Succeeds.\n}\n```\n\nA fully safe approach requires a feature that Dart does not yet\nhave, namely statically checked variance. With that, we could\nspecify that the type parameter `X` is invariant (`inout X`).\n\nAnother possibility is to declare the variable to have a safe\nbut more general type. It is then safe to use the variable\nitself, but every invocation will have to be checked at run\ntime:\n\n**HONEST:**\n```dart\nclass C<X> {\n  final bool Function(Never) fun;\n  C(this.fun);\n}\n\nvoid main() {\n  C<num> c = C<int>((i) => i.isEven);\n  var cfun = c.fun; // Local variable, enables promotion.\n  if (cfun is bool Function(int)) cfun(10); // Succeeds.\n  if (cfun is bool Function(bool)) cfun(true); // Not called.\n}\n```",
+    "details": "An instance variable whose type contains a type parameter of the\nenclosing class, mixin, or enum in a non-covariant position is\nlikely to cause run-time failures due to failing type\nchecks. For example, in `class C<X> {...}`, an instance variable\nof the form `void Function(X) myVariable;` may cause this kind\nof run-time failure.\n\nThe same is true for a getter or method whose return type has a\nnon-covariant occurrence of a type parameter of the enclosing\ndeclaration.\n\nThis lint flags this kind of member declaration.\n\n**BAD:**\n```dart\nclass C<X> {\n  final bool Function([!X!]) fun; // LINT\n  C(this.fun);\n}\n\nvoid main() {\n  C<num> c = C<int>((i) => i.isEven);\n  c.fun(10); // Throws.\n}\n```\n\nThe problem is that `X` occurs as a parameter type in the type\nof `fun`.\n\nOne way to reduce the potential for run-time type errors is to\nensure that the non-covariant member `fun` is _only_ used on\n`this`. We cannot strictly enforce this, but we can make it\nprivate and add a forwarding method `fun` such that we can check\nlocally in the same library that this constraint is satisfied:\n\n**BETTER:**\n```dart\nclass C<X> {\n  // ignore: unsafe_variance\n  final bool Function(X) _fun;\n  bool fun(X x) => _fun(x);\n  C(this._fun);\n}\n\nvoid main() {\n  C<num> c = C<int>((i) => i.isEven);\n  c.fun(10); // Succeeds.\n}\n```\n\nA fully safe approach requires a feature that Dart does not yet\nhave, namely statically checked variance. With that, we could\nspecify that the type parameter `X` is invariant (`inout X`).\n\nIt is possible to emulate invariance without support for statically\nchecked variance. This puts some restrictions on the creation of\nsubtypes, but faithfully provides the typing that `inout` would\ngive:\n\n**GOOD:**\n```dart\ntypedef Inv<X> = X Function(X);\ntypedef C<X> = _C<X, Inv<X>>;\n\nclass _C<X, Invariance extends Inv<X>> {\n  // ignore: unsafe_variance\n  final bool Function(X) fun; // Safe!\n  _C(this.fun);\n}\n\nvoid main() {\n  C<int> c = C<int>((i) => i.isEven);\n  c.fun(10); // Succeeds.\n}\n```\n\nWith this approach, `C<int>` is not a subtype of `C<num>`, so\n`c` must have a different declared type.\n\nAnother possibility is to declare the variable to have a safe\nbut more general type. It is then safe to use the variable\nitself, but every invocation will have to be checked at run\ntime:\n\n**HONEST:**\n```dart\nclass C<X> {\n  final bool Function(Never) fun;\n  C(this.fun);\n}\n\nvoid main() {\n  C<num> c = C<int>((int i) => i.isEven);\n  var cfun = c.fun; // Local variable, enables promotion.\n  if (cfun is bool Function(int)) cfun(10); // Succeeds.\n  if (cfun is bool Function(bool)) cfun(true); // Not called.\n}\n```",
     "sinceDartSdk": "3.7"
   },
   {