Skip to content

Commit

Permalink
Shortened the documentation of unsafe_variance
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@eernstg
eernstg authored and Commit Queue committed Nov 29, 2024
1 parent b196577 commit e88f081
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 153 deletions.
120 changes: 46 additions & 74 deletions pkg/analyzer/tool/diagnostics/diagnostics.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
154 changes: 76 additions & 78 deletions pkg/linter/messages.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -13210,8 +13285,6 @@ LintCode:
This lint flags this kind of member declaration.
#### Example
**BAD:**
```dart
class C<X> {
Expand All @@ -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
Expand Down Expand Up @@ -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."
Expand Down
2 changes: 1 addition & 1 deletion pkg/linter/tool/machine/rules.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
{
Expand Down

0 comments on commit e88f081

Please sign in to comment.