Skip to content

Commit 5c1b908

Browse files
srawlinsCommit Queue
authored andcommitted
linter: Document and test more specifically what the rule does
In preparation for the `@awaitNotRequired` annotation, I looked into the implementation of this rule, and changed the documentation to be more specific about the very specific situations in which lints are reported (there are only three). This should alleviate issues like #60006. I also added some test cases that will be useful in exploring what the new annotation will be able to suppress. Change-Id: Iea3a90b6c79df4b8eee7c33063780b3cf0298109 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415780 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Phil Quitslund <[email protected]>
1 parent 47f7972 commit 5c1b908

File tree

3 files changed

+102
-16
lines changed

3 files changed

+102
-16
lines changed

pkg/linter/lib/src/rules/unawaited_futures.dart

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,20 @@ class _Visitor extends SimpleAstVisitor<void> {
5454
if (type == null) {
5555
return;
5656
}
57-
if (type.implementsInterface('Future', 'dart.async')) {
58-
// Ignore a couple of special known cases.
59-
if (_isFutureDelayedInstanceCreationWithComputation(expr) ||
60-
_isMapPutIfAbsentInvocation(expr)) {
61-
return;
62-
}
63-
64-
if (_isEnclosedInAsyncFunctionBody(node)) {
65-
// Future expression statement that isn't awaited in an async function:
66-
// while this is legal, it's a very frequent sign of an error.
67-
rule.reportLint(node);
68-
}
57+
if (!type.implementsInterface('Future', 'dart.async')) {
58+
return;
59+
}
60+
61+
// Ignore a couple of special known cases.
62+
if (_isFutureDelayedInstanceCreationWithComputation(expr) ||
63+
_isMapPutIfAbsentInvocation(expr)) {
64+
return;
65+
}
66+
67+
if (_isEnclosedInAsyncFunctionBody(node)) {
68+
// Future expression statement that isn't awaited in an async function:
69+
// while this is legal, it's a very frequent sign of an error.
70+
rule.reportLint(node);
6971
}
7072
}
7173

@@ -97,6 +99,7 @@ class _Visitor extends SimpleAstVisitor<void> {
9799
_isMapClass(expr.methodName.element?.enclosingElement2);
98100

99101
void _visit(Expression expr) {
102+
// TODO(srawlins): Check whether `expr`'s static type _implements_ `Future`.
100103
if ((expr.staticType?.isDartAsyncFuture ?? false) &&
101104
_isEnclosedInAsyncFunctionBody(expr) &&
102105
expr is! AssignmentExpression) {

pkg/linter/messages.yaml

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11438,10 +11438,18 @@ LintCode:
1143811438
documentation: |-
1143911439
#### Description
1144011440
11441-
The analyzer produces this diagnostic when an instance of `Future` is
11442-
returned from an invocation within an `async` (or `async*`) method or
11443-
function and the future is neither awaited nor passed to the `unawaited`
11444-
function.
11441+
The analyzer produces this diagnostic on an expression with a `Future`
11442+
type, only in a few specific cases:
11443+
11444+
* when the expression is itself a statement (like `f();`),
11445+
* when the expression is part of a cascade (like `C()..f()`),
11446+
* when the expression is a String interpolation (like `'${f()}'`).
11447+
11448+
The analyzer only produces this diagnostic on expressions inside an
11449+
`async` or `async*` function.
11450+
11451+
The two common corrections are to 'await' the expression, or to wrap the
11452+
expression in a call to `unawaited()`.
1144511453
1144611454
#### Example
1144711455

pkg/linter/test/rules/unawaited_futures_test.dart

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,30 @@ class UnawaitedFuturesTest extends LintRuleTest {
1717
@override
1818
String get lintRule => LintNames.unawaited_futures;
1919

20+
test_binaryExpression_unawaited() async {
21+
await assertDiagnostics(
22+
r'''
23+
void f(C a, C b) async {
24+
a + b;
25+
}
26+
class C {
27+
Future<int> operator +(C other) async => 7;
28+
}
29+
''',
30+
[lint(27, 6)],
31+
);
32+
}
33+
34+
test_boundToFuture_unawaited() async {
35+
// This behavior was not necessarily designed, but this test documents the
36+
// current behavior.
37+
await assertNoDiagnostics(r'''
38+
void f<T extends Future<void>>(T p) async {
39+
p;
40+
}
41+
''');
42+
}
43+
2044
test_classImplementsFuture() async {
2145
// https://github.com/dart-lang/linter/issues/2211
2246
await assertDiagnostics(
@@ -72,6 +96,18 @@ Future<int> g() => Future.value(0);
7296
);
7397
}
7498

99+
test_functionCall_nullableFuture_unawaited() async {
100+
await assertDiagnostics(
101+
r'''
102+
void f() async {
103+
g();
104+
}
105+
Future<int>? g() => Future.value(0);
106+
''',
107+
[lint(19, 4)],
108+
);
109+
}
110+
75111
test_functionCall_returnedWithFutureType() async {
76112
await assertNoDiagnostics(r'''
77113
void f() async {
@@ -143,6 +179,45 @@ class C {
143179
''');
144180
}
145181

182+
test_instanceProperty_unawaited() async {
183+
await assertDiagnostics(
184+
r'''
185+
void f(C c) async {
186+
c.p;
187+
}
188+
abstract class C {
189+
Future<int> get p;
190+
}
191+
''',
192+
[lint(22, 4)],
193+
);
194+
}
195+
196+
test_parameter_unawaited() async {
197+
await assertDiagnostics(
198+
r'''
199+
void f(Future<int> p) async {
200+
p;
201+
}
202+
''',
203+
[lint(32, 2)],
204+
);
205+
}
206+
207+
test_unaryExpression_unawaited() async {
208+
await assertDiagnostics(
209+
r'''
210+
void f(C a) async {
211+
-a;
212+
}
213+
class C {
214+
Future<int> operator -() async => 7;
215+
}
216+
''',
217+
[lint(22, 3)],
218+
);
219+
}
220+
146221
test_undefinedIdentifier() async {
147222
await assertDiagnostics(
148223
r'''

0 commit comments

Comments
 (0)