Skip to content

Commit 609f2f9

Browse files
scheglovCommit Queue
authored and
Commit Queue
committed
Linter. Issue 59814. Report unnecessary_async.
Bug: #59814 Change-Id: Iebf90315e7c443e00ea84c380f85ac6c84bc86c6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403320 Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Phil Quitslund <[email protected]>
1 parent 810eeb4 commit 609f2f9

File tree

12 files changed

+722
-3
lines changed

12 files changed

+722
-3
lines changed

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -2401,6 +2401,10 @@ LintCode.unintended_html_in_doc_comment:
24012401
status: needsFix
24022402
notes: |-
24032403
The fix is to encode the angle brackets.
2404+
LintCode.unnecessary_async:
2405+
status: needsFix
2406+
notes: |-
2407+
And probably also quick assist that works even if returns `Future`.
24042408
LintCode.unnecessary_await_in_return:
24052409
status: hasFix
24062410
LintCode.unnecessary_brace_in_string_interps:

pkg/analyzer/lib/src/dart/resolver/body_inference_context.dart

+8
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ class BodyInferenceContext {
2626
/// Types of all `return` or `yield` statements in the body.
2727
final List<DartType> _returnTypes = [];
2828

29+
/// Whether the execution flow can reach the end of the body.
30+
///
31+
/// For example here, because there is no `return` at the end.
32+
/// ```
33+
/// void f() {}
34+
/// ```
35+
bool mayCompleteNormally = true;
36+
2937
factory BodyInferenceContext({
3038
required TypeSystemImpl typeSystem,
3139
required FunctionBodyImpl node,

pkg/analyzer/lib/src/generated/resolver.dart

+5-3
Original file line numberDiff line numberDiff line change
@@ -543,14 +543,16 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
543543
required FunctionBodyImpl body,
544544
required SyntacticEntity errorNode,
545545
}) {
546-
if (!flowAnalysis.flow!.isReachable) {
546+
var bodyContext = body.bodyContext;
547+
if (bodyContext == null) {
547548
return;
548549
}
549550

550-
var bodyContext = body.bodyContext;
551-
if (bodyContext == null) {
551+
if (!flowAnalysis.flow!.isReachable) {
552+
bodyContext.mayCompleteNormally = false;
552553
return;
553554
}
555+
554556
var returnType = bodyContext.contextType;
555557
if (returnType == null) {
556558
if (errorNode is BlockFunctionBody) {

pkg/linter/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
- new lint: `strict_top_level_inference`
55
- new _(experimental)_ lint: `omit_obvious_property_types`
66
- new _(experimental)_ lint: `specify_nonobvious_property_types`
7+
- new _(experimental)_ lint: `unnecessary_async`
78
- new _(experimental)_ lint: `unsafe_variance`
89
- removed lint: `package_api_docs`
910
- removed lint: `unsafe_html`

pkg/linter/example/all.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ linter:
180180
- type_literal_in_constant_pattern
181181
- unawaited_futures
182182
- unintended_html_in_doc_comment
183+
- unnecessary_async
183184
- unnecessary_await_in_return
184185
- unnecessary_brace_in_string_interps
185186
- unnecessary_breaks

pkg/linter/lib/src/lint_codes.g.dart

+6
Original file line numberDiff line numberDiff line change
@@ -1572,6 +1572,12 @@ class LinterLintCode extends LintCode {
15721572
hasPublishedDocs: true,
15731573
);
15741574

1575+
static const LintCode unnecessary_async = LinterLintCode(
1576+
LintNames.unnecessary_async,
1577+
"Don't make a function 'async' if it doesn't use 'await'.",
1578+
correctionMessage: "Try removing the 'async' modifier.",
1579+
);
1580+
15751581
static const LintCode unnecessary_await_in_return = LinterLintCode(
15761582
LintNames.unnecessary_await_in_return,
15771583
"Unnecessary 'await'.",

pkg/linter/lib/src/lint_names.g.dart

+2
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,8 @@ abstract final class LintNames {
480480
static const String unintended_html_in_doc_comment =
481481
'unintended_html_in_doc_comment';
482482

483+
static const String unnecessary_async = 'unnecessary_async';
484+
483485
static const String unnecessary_await_in_return =
484486
'unnecessary_await_in_return';
485487

pkg/linter/lib/src/rules.dart

+2
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ import 'rules/type_init_formals.dart';
198198
import 'rules/type_literal_in_constant_pattern.dart';
199199
import 'rules/unawaited_futures.dart';
200200
import 'rules/unintended_html_in_doc_comment.dart';
201+
import 'rules/unnecessary_async.dart';
201202
import 'rules/unnecessary_await_in_return.dart';
202203
import 'rules/unnecessary_brace_in_string_interps.dart';
203204
import 'rules/unnecessary_breaks.dart';
@@ -446,6 +447,7 @@ void registerLintRules() {
446447
..registerLintRule(TypeLiteralInConstantPattern())
447448
..registerLintRule(UnawaitedFutures())
448449
..registerLintRule(UnintendedHtmlInDocComment())
450+
..registerLintRule(UnnecessaryAsync())
449451
..registerLintRule(UnnecessaryAwaitInReturn())
450452
..registerLintRule(UnnecessaryBraceInStringInterps())
451453
..registerLintRule(UnnecessaryBreaks())
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/dart/ast/token.dart';
6+
import 'package:analyzer/dart/ast/visitor.dart';
7+
import 'package:analyzer/dart/element/nullability_suffix.dart';
8+
import 'package:analyzer/dart/element/type.dart';
9+
// ignore: implementation_imports
10+
import 'package:analyzer/src/dart/ast/ast.dart';
11+
12+
import '../analyzer.dart';
13+
14+
const _desc = r'No await no async.';
15+
16+
class UnnecessaryAsync extends LintRule {
17+
UnnecessaryAsync()
18+
: super(
19+
name: LintNames.unnecessary_async,
20+
description: _desc,
21+
state: const State.experimental(),
22+
);
23+
24+
@override
25+
LintCode get lintCode => LinterLintCode.unnecessary_async;
26+
27+
@override
28+
void registerNodeProcessors(
29+
NodeLintRegistry registry,
30+
LinterContext context,
31+
) {
32+
var visitor = _Visitor(this);
33+
registry.addFunctionDeclaration(this, visitor);
34+
registry.addFunctionExpression(this, visitor);
35+
registry.addMethodDeclaration(this, visitor);
36+
}
37+
}
38+
39+
class _HasAwaitVisitor extends RecursiveAstVisitor<void> {
40+
bool hasAwait = false;
41+
bool everyReturnHasValue = true;
42+
bool returnsOnlyFuture = true;
43+
44+
@override
45+
void visitAwaitExpression(AwaitExpression node) {
46+
hasAwait = true;
47+
super.visitAwaitExpression(node);
48+
}
49+
50+
@override
51+
void visitExpressionFunctionBody(ExpressionFunctionBody node) {
52+
_updateWithExpression(node.expression);
53+
super.visitExpressionFunctionBody(node);
54+
}
55+
56+
@override
57+
void visitForElement(ForElement node) {
58+
hasAwait |= node.awaitKeyword != null;
59+
super.visitForElement(node);
60+
}
61+
62+
@override
63+
void visitForStatement(ForStatement node) {
64+
hasAwait |= node.awaitKeyword != null;
65+
super.visitForStatement(node);
66+
}
67+
68+
@override
69+
void visitFunctionExpression(FunctionExpression node) {
70+
// Stop the recursion.
71+
}
72+
73+
@override
74+
void visitReturnStatement(ReturnStatement node) {
75+
var expression = node.expression;
76+
if (expression != null) {
77+
_updateWithExpression(expression);
78+
} else {
79+
everyReturnHasValue = false;
80+
}
81+
82+
super.visitReturnStatement(node);
83+
}
84+
85+
void _updateWithExpression(Expression expression) {
86+
var type = expression.staticType;
87+
if (!(type is InterfaceType &&
88+
type.isDartAsyncFutureOrSubtype &&
89+
type.nullabilitySuffix == NullabilitySuffix.none)) {
90+
returnsOnlyFuture = false;
91+
}
92+
}
93+
}
94+
95+
class _Visitor extends SimpleAstVisitor<void> {
96+
final LintRule rule;
97+
98+
_Visitor(this.rule);
99+
100+
@override
101+
void visitFunctionDeclaration(covariant FunctionDeclarationImpl node) {
102+
var element = node.declaredFragment!.element;
103+
104+
_checkBody(
105+
body: node.functionExpression.body,
106+
returnType: element.returnType,
107+
);
108+
}
109+
110+
@override
111+
void visitFunctionExpression(covariant FunctionExpressionImpl node) {
112+
// Here we handle only closures.
113+
if (node.parent is FunctionDeclaration) {
114+
return;
115+
}
116+
117+
var bodyContext = node.body.bodyContext;
118+
119+
_checkBody(
120+
body: node.body,
121+
returnType: bodyContext?.imposedType,
122+
);
123+
}
124+
125+
@override
126+
void visitMethodDeclaration(covariant MethodDeclarationImpl node) {
127+
var element = node.declaredFragment!.element;
128+
129+
_checkBody(
130+
body: node.body,
131+
returnType: element.returnType,
132+
);
133+
}
134+
135+
void _checkBody({
136+
required FunctionBodyImpl body,
137+
required DartType? returnType,
138+
}) {
139+
var asyncKeyword = body.keyword;
140+
if (asyncKeyword == null || asyncKeyword.keyword != Keyword.ASYNC) {
141+
return;
142+
}
143+
144+
if (body.star != null) {
145+
return;
146+
}
147+
148+
var bodyContext = body.bodyContext;
149+
if (bodyContext == null) {
150+
return;
151+
}
152+
153+
var visitor = _HasAwaitVisitor();
154+
body.accept(visitor);
155+
156+
if (visitor.hasAwait) {
157+
return;
158+
}
159+
160+
// If no imposed return type, then any type is OK.
161+
if (returnType == null) {
162+
rule.reportLintForToken(asyncKeyword);
163+
return;
164+
}
165+
166+
// We don't have to return anything.
167+
// So, the generated `Future` is not necessary.
168+
if (returnType is VoidType) {
169+
rule.reportLintForToken(asyncKeyword);
170+
return;
171+
}
172+
173+
// It is OK to return values into `FutureOr`.
174+
// So, wrapping values into `Future` is not necessary.
175+
if (returnType.isDartAsyncFutureOr) {
176+
rule.reportLintForToken(asyncKeyword);
177+
return;
178+
}
179+
180+
// We handle only `Future<T>` below.
181+
if (!returnType.isDartAsyncFuture) {
182+
return;
183+
}
184+
185+
// If the body may complete normally, we cannot remove `async`.
186+
// This would make the body return `null`.
187+
// And `null` is not the same as `Future.value(null)`.
188+
if (bodyContext.mayCompleteNormally) {
189+
return;
190+
}
191+
192+
// If every `return` returns `Future`, we don't need wrapping.
193+
if (visitor.everyReturnHasValue && visitor.returnsOnlyFuture) {
194+
rule.reportLintForToken(asyncKeyword);
195+
return;
196+
}
197+
}
198+
}
199+
200+
extension on InterfaceType {
201+
bool get isDartAsyncFutureOrSubtype {
202+
var typeProvider = element3.library2.typeProvider;
203+
return asInstanceOf2(typeProvider.futureElement2) != null;
204+
}
205+
}

pkg/linter/messages.yaml

+29
Original file line numberDiff line numberDiff line change
@@ -11632,6 +11632,35 @@ LintCode:
1163211632
/// \<assignment\> -> \<variable\> = \<expression\>`
1163311633
/// <http://foo.bar.baz>
1163411634
```
11635+
unnecessary_async:
11636+
problemMessage: "Don't make a function 'async' if it doesn't use 'await'."
11637+
correctionMessage: "Try removing the 'async' modifier."
11638+
state:
11639+
experimental: "3.7"
11640+
categories: [style]
11641+
hasPublishedDocs: false
11642+
deprecatedDetails: |-
11643+
Functions that don't do `await` don't have to be `async`.
11644+
11645+
Usually such functions also don't have to return a `Future`, which allows
11646+
an invoker to avoid `await` in its code, etc. Synchronous code in
11647+
general runs faster, and is easier to reason about.
11648+
11649+
**BAD:**
11650+
```dart
11651+
void f() async {
11652+
// await Future.delayed(const Duration(seconds: 2));
11653+
print(0);
11654+
}
11655+
```
11656+
11657+
**GOOD:**
11658+
```dart
11659+
void f() {
11660+
// await Future.delayed(const Duration(seconds: 2));
11661+
print(0);
11662+
}
11663+
```
1163511664
unnecessary_await_in_return:
1163611665
problemMessage: "Unnecessary 'await'."
1163711666
correctionMessage: "Try removing the 'await'."

pkg/linter/test/rules/all.dart

+2
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ import 'type_literal_in_constant_pattern_test.dart'
253253
import 'unawaited_futures_test.dart' as unawaited_futures;
254254
import 'unintended_html_in_doc_comment_test.dart'
255255
as unintended_html_in_doc_comment;
256+
import 'unnecessary_async_test.dart' as unnecessary_async;
256257
import 'unnecessary_await_in_return_test.dart' as unnecessary_await_in_return;
257258
import 'unnecessary_brace_in_string_interps_test.dart'
258259
as unnecessary_brace_in_string_interps;
@@ -506,6 +507,7 @@ void main() {
506507
type_literal_in_constant_pattern.main();
507508
unawaited_futures.main();
508509
unintended_html_in_doc_comment.main();
510+
unnecessary_async.main();
509511
unnecessary_await_in_return.main();
510512
unnecessary_brace_in_string_interps.main();
511513
unnecessary_breaks.main();

0 commit comments

Comments
 (0)