analyzer/linter: Support an @awaitNotRequired annotation
Work towards https://github.com/dart-lang/sdk/issues/46218
This implements the bulk of the `@awaitNotRequired` support as specified here: https://github.com/dart-lang/sdk/issues/46218#issuecomment-2725895048
Remaining work:
* implement inheritence
* report a badly-placed annotation
* add the annotation to the real meta package
Change-Id: Ida16018746704d0baa165ab9be95187b52a79061
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423923
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analyzer/api.txt b/pkg/analyzer/api.txt
index 0d7b7db..50236f4 100644
--- a/pkg/analyzer/api.txt
+++ b/pkg/analyzer/api.txt
@@ -2983,6 +2983,7 @@
constantEvaluationErrors (getter: List<AnalysisError>?)
element2 (getter: Element2?)
isAlwaysThrows (getter: bool)
+ isAwaitNotRequired (getter: bool)
isDeprecated (getter: bool)
isDoNotStore (getter: bool)
isDoNotSubmit (getter: bool)
@@ -3563,6 +3564,7 @@
new (constructor: Metadata Function())
annotations (getter: List<ElementAnnotation>)
hasAlwaysThrows (getter: bool)
+ hasAwaitNotRequired (getter: bool)
hasDeprecated (getter: bool)
hasDoNotStore (getter: bool)
hasDoNotSubmit (getter: bool)
diff --git a/pkg/analyzer/lib/dart/element/element.dart b/pkg/analyzer/lib/dart/element/element.dart
index 930443c..fa64210 100644
--- a/pkg/analyzer/lib/dart/element/element.dart
+++ b/pkg/analyzer/lib/dart/element/element.dart
@@ -115,6 +115,10 @@
/// Whether the annotation marks the associated function as always throwing.
bool get isAlwaysThrows;
+ /// Whether the annotation marks the associated element as not needing to be
+ /// awaited.
+ bool get isAwaitNotRequired;
+
/// Whether the annotation marks the associated element as being deprecated.
bool get isDeprecated;
diff --git a/pkg/analyzer/lib/dart/element/element2.dart b/pkg/analyzer/lib/dart/element/element2.dart
index a2e53f1..4343e0d 100644
--- a/pkg/analyzer/lib/dart/element/element2.dart
+++ b/pkg/analyzer/lib/dart/element/element2.dart
@@ -2139,6 +2139,9 @@
/// Whether the receiver has an annotation of the form `@alwaysThrows`.
bool get hasAlwaysThrows;
+ /// Whether the receiver has an annotation of the form `@awaitNotRequired`.
+ bool get hasAwaitNotRequired;
+
/// Whether the receiver has an annotation of the form `@deprecated`
/// or `@Deprecated('..')`.
bool get hasDeprecated;
diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart
index 55bd0b1..bf35e97 100644
--- a/pkg/analyzer/lib/src/dart/element/element.dart
+++ b/pkg/analyzer/lib/src/dart/element/element.dart
@@ -1992,6 +1992,10 @@
/// throws, for dead code purposes.
static const String _alwaysThrowsVariableName = 'alwaysThrows';
+ /// The name of the top-level variable used to mark an element as not needing
+ /// to be awaited.
+ static const String _awaitNotRequiredVariableName = 'awaitNotRequired';
+
/// The name of the class used to mark an element as being deprecated.
static const String _deprecatedClassName = 'Deprecated';
@@ -2185,6 +2189,10 @@
bool get isAlwaysThrows => _isPackageMetaGetter(_alwaysThrowsVariableName);
@override
+ bool get isAwaitNotRequired =>
+ _isPackageMetaGetter(_awaitNotRequiredVariableName);
+
+ @override
bool get isConstantEvaluated => evaluationResult != null;
bool get isDartInternalSince {
@@ -7138,6 +7146,18 @@
}
@override
+ bool get hasAwaitNotRequired {
+ var annotations = this.annotations;
+ for (var i = 0; i < annotations.length; i++) {
+ var annotation = annotations[i];
+ if (annotation.isAwaitNotRequired) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @override
bool get hasDeprecated {
if (_metadataFlags < 0) {
// TODO(augmentations): Consider optimizing this similar to `ElementImpl`.
diff --git a/pkg/analyzer/lib/src/error/annotation_verifier.dart b/pkg/analyzer/lib/src/error/annotation_verifier.dart
index dde9b49..c1ff62e 100644
--- a/pkg/analyzer/lib/src/error/annotation_verifier.dart
+++ b/pkg/analyzer/lib/src/error/annotation_verifier.dart
@@ -43,7 +43,10 @@
return;
}
var parent = node.parent;
- if (element.isFactory) {
+ if (element.isAwaitNotRequired) {
+ // TODO(srawlins): Check that functions return Future and fields and
+ // getters have a static type of Future.
+ } else if (element.isFactory) {
_checkFactory(node);
} else if (element.isInternal) {
_checkInternal(node);
diff --git a/pkg/analyzer_utilities/lib/test/mock_packages/package_content/meta/lib/meta.dart b/pkg/analyzer_utilities/lib/test/mock_packages/package_content/meta/lib/meta.dart
index ebc7ed6..46c7639 100644
--- a/pkg/analyzer_utilities/lib/test/mock_packages/package_content/meta/lib/meta.dart
+++ b/pkg/analyzer_utilities/lib/test/mock_packages/package_content/meta/lib/meta.dart
@@ -58,6 +58,38 @@
@Deprecated("Use a return type of 'Never' instead")
const _AlwaysThrows alwaysThrows = _AlwaysThrows();
+/// Used to annotate a [Future]-returning function (including constructors,
+/// getters, methods, and operators), or a [Future]-typed field (including
+/// top-level, instance, and static) `f`. Indicates that the [Future] value that
+/// `f` returns does not need to be awaited. Any methods that override `f` in
+/// class inheritance, are also expected to conform to this contract.
+///
+/// Tools, such as the analyzer, can use this to understand whether to report
+/// that a [Future]-typed value does not need to be awaited:
+///
+/// ```dart
+/// @awaitNotRequired Future<LogMessage> log(String message) { ... }
+///
+/// int fn() {
+/// log('Message'); // Not important to wait for logging to complete.
+/// }
+/// ```
+///
+/// Without the annotation on `log`, the analyzer may report a lint diagnostic
+/// at the call to `log`, such as `discarded_futures` or `unawaited_futures`,
+/// regarding the danger of not awaiting the function call, depending on what
+/// lint rules are enabled.
+///
+/// Tools, such as the analyzer, can also provide feedback if
+///
+/// * the annotation is associated with anything other than a constructor,
+/// function, method, operator, field, or top-level variable, or
+/// * the annotation is associated with a constructor, function, method, or
+/// operator that does not return a [Future], or
+/// * the annotation is associated with a field or top-level variable that is
+/// not typed as a [Future].
+const _AwaitNotRequired awaitNotRequired = _AwaitNotRequired();
+
/// Used to annotate a parameter of an instance method that overrides another
/// method.
///
@@ -621,6 +653,19 @@
const _AlwaysThrows();
}
+/// See [awaitNotRequired] for more details.
+@Target({
+ TargetKind.constructor,
+ TargetKind.field,
+ TargetKind.function,
+ TargetKind.getter,
+ TargetKind.method,
+ TargetKind.topLevelVariable,
+})
+class _AwaitNotRequired {
+ const _AwaitNotRequired();
+}
+
class _Checked {
const _Checked();
}
diff --git a/pkg/linter/lib/src/rules/discarded_futures.dart b/pkg/linter/lib/src/rules/discarded_futures.dart
index 1046428..cafa370 100644
--- a/pkg/linter/lib/src/rules/discarded_futures.dart
+++ b/pkg/linter/lib/src/rules/discarded_futures.dart
@@ -70,6 +70,8 @@
_reportOnExpression(expr);
}
+
+ // TODO(srawlins): Take `@awaitNotRequired` into account.
}
@override
@@ -114,6 +116,7 @@
if ((expr.staticType.isFutureOrFutureOr) &&
!_isEnclosedInAsyncFunctionBody(expr) &&
expr is! AssignmentExpression) {
+ // TODO(srawlins): Take `@awaitNotRequired` into account.
_reportOnExpression(expr);
}
}
diff --git a/pkg/linter/lib/src/rules/unawaited_futures.dart b/pkg/linter/lib/src/rules/unawaited_futures.dart
index f00646e..2061707 100644
--- a/pkg/linter/lib/src/rules/unawaited_futures.dart
+++ b/pkg/linter/lib/src/rules/unawaited_futures.dart
@@ -64,6 +64,10 @@
return;
}
+ if (_isAwaitNotRequired(expr)) {
+ return;
+ }
+
if (_isEnclosedInAsyncFunctionBody(node)) {
// Future expression statement that isn't awaited in an async function:
// while this is legal, it's a very frequent sign of an error.
@@ -76,6 +80,39 @@
_visit(node.expression);
}
+ bool _isAwaitNotRequired(Expression node) {
+ // TODO(srawlins): Handle inheritence in each of these cases; the
+ // `@awaitNotRequired` annotation should be inherited.
+ switch (node) {
+ case BinaryExpression():
+ if (node.element.hasAwaitNotRequired) {
+ return true;
+ }
+
+ case MethodInvocation():
+ if (node.methodName.element.hasAwaitNotRequired) {
+ return true;
+ }
+
+ case PrefixedIdentifier():
+ if (node.identifier.element.hasAwaitNotRequired) {
+ return true;
+ }
+
+ case PrefixExpression():
+ if (node.element.hasAwaitNotRequired) {
+ return true;
+ }
+
+ case PropertyAccess():
+ if (node.propertyName.element.hasAwaitNotRequired) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
bool _isEnclosedInAsyncFunctionBody(AstNode node) {
var enclosingFunctionBody = node.thisOrAncestorOfType<FunctionBody>();
return enclosingFunctionBody?.isAsynchronous ?? false;
@@ -99,6 +136,10 @@
_isMapClass(expr.methodName.element?.enclosingElement2);
void _visit(Expression expr) {
+ if (_isAwaitNotRequired(expr)) {
+ return;
+ }
+
// TODO(srawlins): Check whether `expr`'s static type _implements_ `Future`.
if ((expr.staticType?.isDartAsyncFuture ?? false) &&
_isEnclosedInAsyncFunctionBody(expr) &&
@@ -107,3 +148,9 @@
}
}
}
+
+extension on Element2? {
+ bool get hasAwaitNotRequired =>
+ this is Annotatable &&
+ (this! as Annotatable).metadata2.hasAwaitNotRequired;
+}
diff --git a/pkg/linter/test/rules/unawaited_futures_test.dart b/pkg/linter/test/rules/unawaited_futures_test.dart
index 32d7554..2aa0854 100644
--- a/pkg/linter/test/rules/unawaited_futures_test.dart
+++ b/pkg/linter/test/rules/unawaited_futures_test.dart
@@ -15,6 +15,9 @@
@reflectiveTest
class UnawaitedFuturesTest extends LintRuleTest {
@override
+ bool get addMetaPackageDep => true;
+
+ @override
String get lintRule => LintNames.unawaited_futures;
test_binaryExpression_unawaited() async {
@@ -31,6 +34,19 @@
);
}
+ test_binaryExpression_unawaited_awaitNotRequired() async {
+ await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+void f(C a, C b) async {
+ a + b;
+}
+class C {
+ @awaitNotRequired
+ Future<int> operator +(C other) async => 7;
+}
+''');
+ }
+
test_boundToFuture_unawaited() async {
// This behavior was not necessarily designed, but this test documents the
// current behavior.
@@ -96,6 +112,17 @@
);
}
+ test_functionCall_interpolated_unawaited_awaitNotRequired() async {
+ await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+void f() async {
+ '${g()}';
+}
+@awaitNotRequired
+Future<int> g() => Future.value(0);
+''');
+ }
+
test_functionCall_nullableFuture_unawaited() async {
await assertDiagnostics(
r'''
@@ -129,6 +156,33 @@
);
}
+ test_functionCall_unawaited_awaitNotRequired() async {
+ await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+void f() async {
+ g();
+}
+@awaitNotRequired
+Future<int> g() => Future.value(0);
+''');
+ }
+
+ test_functionCallInCascade() async {
+ await assertDiagnostics(
+ r'''
+void f() async {
+ C()..doAsync();
+}
+class C {
+ Future<void> doAsync() async {}
+}
+''',
+ [lint(22, 11)],
+ );
+ }
+
+ // TODO(srawlins): Test that `@awaitNotRequired` is inherited.
+
test_functionCallInCascade_assignment() async {
await assertNoDiagnostics(r'''
void f() async {
@@ -141,18 +195,17 @@
''');
}
- test_functionCallInCascade_inAsync() async {
- await assertDiagnostics(
- r'''
+ test_functionCallInCascade_awaitNotRequired() async {
+ await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
void f() async {
C()..doAsync();
}
class C {
+ @awaitNotRequired
Future<void> doAsync() async {}
}
-''',
- [lint(22, 11)],
- );
+''');
}
test_functionCallInCascade_indexAssignment() async {
@@ -180,6 +233,19 @@
}
test_instanceProperty_unawaited() async {
+ await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+void f(C c) async {
+ c.p;
+}
+abstract class C {
+ @awaitNotRequired
+ Future<int> get p;
+}
+''');
+ }
+
+ test_instanceProperty_unawaited_awaitNotRequired() async {
await assertDiagnostics(
r'''
void f(C c) async {
@@ -204,7 +270,7 @@
);
}
- test_unaryExpression_unawaited() async {
+ test_prefixExpression_unawaited() async {
await assertDiagnostics(
r'''
void f(C a) async {
@@ -218,6 +284,19 @@
);
}
+ test_prefixExpression_unawaited_awaitNotRequired() async {
+ await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+void f(C a) async {
+ -a;
+}
+class C {
+ @awaitNotRequired
+ Future<int> operator -() async => 7;
+}
+''');
+ }
+
test_undefinedIdentifier() async {
await assertDiagnostics(
r'''
