[linter, DAS] Adds new diagnostic opposite of unnecessary_await_in_return
Bug: https://github.com/dart-lang/sdk/issues/62555 Change-Id: Ica84ea93efcb2c74d2fd260cdceebbf6558e7bf3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/477660 Reviewed-by: Samuel Rawlins <srawlins@google.com> Auto-Submit: FMorschel <git@fmorschel.dev> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
committed by
Samuel Rawlins
parent
0ae45fcc7e
commit
ab017d3bda
@@ -1959,6 +1959,8 @@ annotate_overrides:
|
||||
status: hasFix
|
||||
annotate_redeclares:
|
||||
status: hasFix
|
||||
async_return_with_no_await:
|
||||
status: hasFix
|
||||
avoid_annotating_with_dynamic:
|
||||
status: hasFix
|
||||
avoid_bool_literals_in_conditional_expressions:
|
||||
|
||||
@@ -286,6 +286,7 @@ final _builtInLintGenerators = <DiagnosticCode, List<ProducerGenerator>>{
|
||||
diag.alwaysUsePackageImports: [ConvertToPackageImport.new],
|
||||
diag.annotateOverrides: [AddOverride.new],
|
||||
diag.annotateRedeclares: [AddRedeclare.new],
|
||||
diag.asyncReturnWithNoAwait: [AddAwait.return_],
|
||||
diag.avoidAnnotatingWithDynamic: [RemoveTypeAnnotation.other],
|
||||
diag.avoidBoolLiteralsInConditionalExpressions: [
|
||||
ConvertToBooleanExpression.new,
|
||||
|
||||
@@ -14,6 +14,7 @@ void main() {
|
||||
defineReflectiveSuite(() {
|
||||
defineReflectiveTests(AddAwaitTest);
|
||||
defineReflectiveTests(AddAwaitTestArgumentAndAssignment);
|
||||
defineReflectiveTests(AsyncReturnWithNoAwaitTest);
|
||||
defineReflectiveTests(UnawaitedReturnInTryBlockTest);
|
||||
});
|
||||
}
|
||||
@@ -353,6 +354,45 @@ Future<void> baz() async {
|
||||
}
|
||||
}
|
||||
|
||||
@reflectiveTest
|
||||
class AsyncReturnWithNoAwaitTest extends FixProcessorLintTest {
|
||||
@override
|
||||
FixKind get kind => DartFixKind.addAwait;
|
||||
|
||||
@override
|
||||
String get lintCode => LintNames.async_return_with_no_await;
|
||||
|
||||
Future<void> test_blockBody() async {
|
||||
await resolveTestCode('''
|
||||
class A {
|
||||
Future<int> foo() async {
|
||||
return Future.value(42);
|
||||
}
|
||||
}
|
||||
''');
|
||||
await assertHasFix('''
|
||||
class A {
|
||||
Future<int> foo() async {
|
||||
return await Future.value(42);
|
||||
}
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_functionExpression() async {
|
||||
await resolveTestCode('''
|
||||
class A {
|
||||
Future<int> foo() async => Future.value(42);
|
||||
}
|
||||
''');
|
||||
await assertHasFix('''
|
||||
class A {
|
||||
Future<int> foo() async => await Future.value(42);
|
||||
}
|
||||
''');
|
||||
}
|
||||
}
|
||||
|
||||
@reflectiveTest
|
||||
class UnawaitedReturnInTryBlockTest extends FixProcessorErrorCodeTest {
|
||||
@override
|
||||
|
||||
@@ -342,6 +342,16 @@ annotateRedeclares = LinterLintTemplate(
|
||||
expectedTypes: [ExpectedType.object],
|
||||
);
|
||||
|
||||
/// No parameters.
|
||||
const LinterLintWithoutArguments asyncReturnWithNoAwait =
|
||||
LinterLintWithoutArguments(
|
||||
name: 'async_return_with_no_await',
|
||||
problemMessage: "Returning a 'Future' without 'await'.",
|
||||
correctionMessage: "Try adding an 'await' or making the body non-async.",
|
||||
uniqueName: 'async_return_with_no_await',
|
||||
expectedTypes: [],
|
||||
);
|
||||
|
||||
/// No parameters.
|
||||
const LinterLintWithoutArguments avoidAnnotatingWithDynamic =
|
||||
LinterLintWithoutArguments(
|
||||
|
||||
@@ -57,6 +57,8 @@ abstract final class LintNames {
|
||||
|
||||
static const String annotate_redeclares = 'annotate_redeclares';
|
||||
|
||||
static const String async_return_with_no_await = 'async_return_with_no_await';
|
||||
|
||||
static const String avoid_annotating_with_dynamic =
|
||||
'avoid_annotating_with_dynamic';
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ import 'rules/analyzer_element_model_tracking.dart';
|
||||
import 'rules/analyzer_public_api.dart';
|
||||
import 'rules/annotate_overrides.dart';
|
||||
import 'rules/annotate_redeclares.dart';
|
||||
import 'rules/async_return_with_no_await.dart';
|
||||
import 'rules/avoid_annotating_with_dynamic.dart';
|
||||
import 'rules/avoid_as.dart';
|
||||
import 'rules/avoid_bool_literals_in_conditional_expressions.dart';
|
||||
@@ -279,6 +280,7 @@ void registerLintRules() {
|
||||
..registerLintRule(AnalyzerPublicApi())
|
||||
..registerLintRule(AnnotateOverrides())
|
||||
..registerLintRule(AnnotateRedeclares())
|
||||
..registerLintRule(AsyncReturnWithNoAwait())
|
||||
..registerLintRule(AvoidAnnotatingWithDynamic())
|
||||
..registerLintRule(avoidAs)
|
||||
..registerLintRule(AvoidBoolLiteralsInConditionalExpressions())
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:analyzer/analysis_rule/analysis_rule.dart';
|
||||
import 'package:analyzer/analysis_rule/rule_context.dart';
|
||||
import 'package:analyzer/analysis_rule/rule_visitor_registry.dart';
|
||||
import 'package:analyzer/error/error.dart';
|
||||
import 'package:analyzer/src/error/async_return_visitor.dart'; // ignore: implementation_imports
|
||||
|
||||
import '../analyzer.dart';
|
||||
import '../diagnostic.dart' as diag;
|
||||
|
||||
const _desc = r'Return with no await.';
|
||||
|
||||
class AsyncReturnWithNoAwait extends AnalysisRule {
|
||||
new() : super(name: LintNames.async_return_with_no_await, description: _desc);
|
||||
|
||||
@override
|
||||
DiagnosticCode get diagnosticCode => diag.asyncReturnWithNoAwait;
|
||||
|
||||
@override
|
||||
void registerNodeProcessors(
|
||||
RuleVisitorRegistry registry,
|
||||
RuleContext context,
|
||||
) {
|
||||
var visitor = AsyncReturnVisitor(
|
||||
reportAtToken: reportAtToken,
|
||||
typeProvider: context.typeProvider,
|
||||
typeSystem: context.typeSystem,
|
||||
);
|
||||
registry.addReturnStatement(this, visitor);
|
||||
registry.addExpressionFunctionBody(this, visitor);
|
||||
}
|
||||
}
|
||||
@@ -26,12 +26,16 @@ class DiscardedFutures extends AnalysisRule {
|
||||
RuleVisitorRegistry registry,
|
||||
RuleContext context,
|
||||
) {
|
||||
var typeProvider = context.typeProvider;
|
||||
var visitor = UnusedFuturesVisitor(
|
||||
rule: this,
|
||||
typeProvider: typeProvider,
|
||||
isInteresting: (node) {
|
||||
var type = node.staticType;
|
||||
// This rule does concern itself with `FutureOr`.
|
||||
if (type == null || !type.isOrImplementsFutureOrFutureOr) {
|
||||
if (type == null ||
|
||||
(type.asInstanceOf(typeProvider.futureElement) == null &&
|
||||
type.asInstanceOf(typeProvider.futureOrElement) == null)) {
|
||||
return false;
|
||||
}
|
||||
// This rule is only concerned with code in sync functions.
|
||||
|
||||
@@ -31,6 +31,7 @@ class UnawaitedFutures extends AnalysisRule {
|
||||
) {
|
||||
var visitor = UnusedFuturesVisitor(
|
||||
rule: this,
|
||||
typeProvider: context.typeProvider,
|
||||
isInteresting: (node) {
|
||||
var type = node.staticType;
|
||||
// This rule is not currently concerned with `FutureOr`.
|
||||
|
||||
@@ -5,7 +5,7 @@ import 'package:analyzer/analysis_rule/analysis_rule.dart';
|
||||
import 'package:analyzer/dart/ast/ast.dart';
|
||||
import 'package:analyzer/dart/ast/visitor.dart';
|
||||
import 'package:analyzer/dart/element/element.dart';
|
||||
import 'package:analyzer/dart/element/type.dart';
|
||||
import 'package:analyzer/dart/element/type_provider.dart';
|
||||
|
||||
import '../extensions.dart';
|
||||
|
||||
@@ -26,7 +26,13 @@ class UnusedFuturesVisitor extends SimpleAstVisitor<void> {
|
||||
/// might report on it.
|
||||
final IsInterestingFilter _isInteresting;
|
||||
|
||||
new({required this._rule, required this._isInteresting});
|
||||
final TypeProvider _typeProvider;
|
||||
|
||||
new({
|
||||
required this._rule,
|
||||
required this._isInteresting,
|
||||
required this._typeProvider,
|
||||
});
|
||||
|
||||
@override
|
||||
void visitCascadeExpression(CascadeExpression node) {
|
||||
@@ -94,22 +100,11 @@ class UnusedFuturesVisitor extends SimpleAstVisitor<void> {
|
||||
|
||||
var type = expr.staticType;
|
||||
if (type != null &&
|
||||
type.isOrImplementsFutureOrFutureOr &&
|
||||
(type.asInstanceOf(_typeProvider.futureElement) != null ||
|
||||
type.asInstanceOf(_typeProvider.futureOrElement) != null) &&
|
||||
_isInteresting(expr) &&
|
||||
expr is! AssignmentExpression) {
|
||||
_reportOnExpression(expr);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
extension DartTypeExtension on DartType {
|
||||
/// Whether this type is `Future` or `FutureOr` from dart:async, or is a
|
||||
/// subtype of `Future`.
|
||||
bool get isOrImplementsFutureOrFutureOr {
|
||||
var typeElement = element;
|
||||
if (typeElement is! InterfaceElement) return false;
|
||||
return isDartAsyncFuture ||
|
||||
isDartAsyncFutureOr ||
|
||||
typeElement.allSupertypes.any((t) => t.isDartAsyncFuture);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -813,6 +813,72 @@ LinterLintCode:
|
||||
}
|
||||
}
|
||||
```
|
||||
asyncReturnWithNoAwait:
|
||||
type: lint
|
||||
parameters: none
|
||||
problemMessage: "Returning a 'Future' without 'await'."
|
||||
correctionMessage: "Try adding an 'await' or making the body non-async."
|
||||
state:
|
||||
stable: "3.13"
|
||||
categories: [errorProne]
|
||||
hasPublishedDocs: false
|
||||
documentation: |-
|
||||
#### Description
|
||||
|
||||
The analyzer produces this diagnostic when a `Future` is returned from an
|
||||
`async` function without using `await`.
|
||||
|
||||
Returning an unawaited `Future` from an `async` function means any
|
||||
exception thrown by the `Future` might not propagate as expected. Having
|
||||
the exception thrown at the `await` site helps identify the source of the
|
||||
exception and makes it easier to debug.
|
||||
|
||||
#### Example
|
||||
|
||||
The following code produces this diagnostic because it returns the `Future`
|
||||
from `futureString` without using `await`:
|
||||
|
||||
```dart
|
||||
Future<String> futureString(Future<String> value) async {
|
||||
[!return!] value;
|
||||
}
|
||||
```
|
||||
|
||||
#### Common fixes
|
||||
|
||||
Add `await` before the returned `Future`:
|
||||
|
||||
```dart
|
||||
Future<String> futureString(Future<String> value) async {
|
||||
return await value;
|
||||
}
|
||||
```
|
||||
|
||||
If you don't need to use `await`, then remove `async` from the function body:
|
||||
|
||||
```dart
|
||||
Future<String> futureString(Future<String> value) {
|
||||
return value;
|
||||
}
|
||||
```
|
||||
deprecatedDetails: |-
|
||||
**DO** use `await` when returning a `Future` from an `async` function.
|
||||
|
||||
**BAD:**
|
||||
```dart
|
||||
Future<String> futureString(Future<String> value) async {
|
||||
return value;
|
||||
}
|
||||
Future<int> futureInt(Future<int> value) async => value;
|
||||
```
|
||||
|
||||
**GOOD:**
|
||||
```dart
|
||||
Future<String> futureString(Future<String> value) async {
|
||||
return await value;
|
||||
}
|
||||
Future<int> futureInt(Future<int> value) => value;
|
||||
```
|
||||
avoidAnnotatingWithDynamic:
|
||||
type: lint
|
||||
parameters: none
|
||||
|
||||
@@ -16,6 +16,7 @@ import 'analyzer_element_model_tracking_test.dart'
|
||||
import 'analyzer_public_api_test.dart' as analyzer_public_api;
|
||||
import 'annotate_overrides_test.dart' as annotate_overrides;
|
||||
import 'annotate_redeclares_test.dart' as annotate_redeclares;
|
||||
import 'async_return_with_no_await_test.dart' as async_return_with_no_await;
|
||||
import 'avoid_annotating_with_dynamic_test.dart'
|
||||
as avoid_annotating_with_dynamic;
|
||||
import 'avoid_bool_literals_in_conditional_expressions_test.dart'
|
||||
@@ -354,6 +355,7 @@ void main() {
|
||||
analyzer_public_api.main();
|
||||
annotate_overrides.main();
|
||||
annotate_redeclares.main();
|
||||
async_return_with_no_await.main();
|
||||
avoid_annotating_with_dynamic.main();
|
||||
avoid_bool_literals_in_conditional_expressions.main();
|
||||
avoid_catches_without_on_clauses.main();
|
||||
|
||||
@@ -0,0 +1,216 @@
|
||||
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
|
||||
import 'package:test_reflective_loader/test_reflective_loader.dart';
|
||||
|
||||
import '../rule_test_support.dart';
|
||||
|
||||
void main() {
|
||||
defineReflectiveSuite(() {
|
||||
defineReflectiveTests(AsyncReturnWithNoAwait);
|
||||
});
|
||||
}
|
||||
|
||||
@reflectiveTest
|
||||
class AsyncReturnWithNoAwait extends LintRuleTest {
|
||||
@override
|
||||
String get lintRule => LintNames.async_return_with_no_await;
|
||||
|
||||
Future<void> test_arrowFunction() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
Future<int> foo() async [!=>!] Future.value(42);
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_closure_declaring() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
var foo = () async {
|
||||
[!return!] Future.value(42);
|
||||
};
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_closure_initializer() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
var foo = () async {
|
||||
[!return!] Future.value(42);
|
||||
}();
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_dynamic() async {
|
||||
await assertNoDiagnostics('''
|
||||
Future<int> foo(dynamic v) async {
|
||||
return v;
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_dynamic_returnType() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
foo() async {
|
||||
[!return!] Future.value(42);
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_function() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
Future<int> foo() async {
|
||||
[!return!] Future.value(42);
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_futureOr() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
import 'dart:async';
|
||||
|
||||
Future<int> foo(FutureOr<int> v) async {
|
||||
[!return!] v;
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_futureOr_returnType() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
import 'dart:async';
|
||||
|
||||
FutureOr<int> foo() async {
|
||||
[!return!] Future.value(42);
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_futureSubtype() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
Future<int> foo() async {
|
||||
[!return!] MyFuture();
|
||||
}
|
||||
|
||||
class MyFuture implements Future<int> {
|
||||
@override
|
||||
noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_getter() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
Future<int> get foo async {
|
||||
[!return!] Future.value(42);
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_invalidType() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
foo() async {
|
||||
return unknown;
|
||||
}
|
||||
''',
|
||||
[error(diag.undefinedIdentifier, 23, 7)],
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_method() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
class A {
|
||||
Future<int> foo() async {
|
||||
[!return!] Future.value(42);
|
||||
}
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_notAsync() async {
|
||||
await assertNoDiagnostics('''
|
||||
Future<int> foo() {
|
||||
return Future.value(42);
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_sync() async {
|
||||
await assertNoDiagnostics('''
|
||||
Future<int> foo() async {
|
||||
return 0;
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_typeParameter() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
Future<int> foo<T extends Future<int>>(T v) async {
|
||||
[!return!] v;
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_withinTryBlock() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
Future<int> foo() async {
|
||||
try {
|
||||
return Future.value(42);
|
||||
} catch (_) {
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
''',
|
||||
[error(diag.unawaitedReturnInTryBlock, 38, 6)],
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_withinTryCatch() async {
|
||||
await assertDiagnosticsFromMarkdown('''
|
||||
Future<int> foo() async {
|
||||
try {} catch (_) {
|
||||
[!return!] Future.value(42);
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_withinTryCatch_withinTryBlock() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
Future<int> foo() async {
|
||||
try {
|
||||
try {} catch (_) {
|
||||
return Future.value(42);
|
||||
}
|
||||
} catch (_) {}
|
||||
return -1;
|
||||
}
|
||||
''',
|
||||
[error(diag.unawaitedReturnInTryBlock, 63, 6)],
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_wrongType() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
Future<int> foo() async {
|
||||
return '';
|
||||
}
|
||||
''',
|
||||
[error(diag.returnOfInvalidTypeFromFunction, 35, 2)],
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_wrongType2() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
Future<int>? foo() async {
|
||||
return Future<Null>.value(null);
|
||||
}
|
||||
''',
|
||||
[error(diag.returnOfInvalidTypeFromFunction, 36, 24)],
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user