analyzer/linter: Support an @awaitNotRequired annotation

Work towards #46218

This implements the bulk of the `@awaitNotRequired` support as specified here: #46218 (comment)

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 <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: srawlins

pkg/analyzer/api.txt

@@ -2983,6 +2983,7 @@ package:analyzer/dart/element/element.dart:
     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 @@ package:analyzer/dart/element/element2.dart:
     new (constructor: Metadata Function())
     annotations (getter: List<ElementAnnotation>)
     hasAlwaysThrows (getter: bool)
+    hasAwaitNotRequired (getter: bool)
     hasDeprecated (getter: bool)
     hasDoNotStore (getter: bool)
     hasDoNotSubmit (getter: bool)

pkg/analyzer/lib/dart/element/element.dart

@@ -115,6 +115,10 @@ abstract class ElementAnnotation implements ConstantEvaluationTarget {
   /// 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;
 

pkg/analyzer/lib/dart/element/element2.dart

@@ -2139,6 +2139,9 @@ abstract class Metadata {
   /// 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;

pkg/analyzer/lib/src/dart/element/element.dart

@@ -1992,6 +1992,10 @@ class ElementAnnotationImpl implements ElementAnnotation {
   /// 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';
 
@@ -2184,6 +2188,10 @@ class ElementAnnotationImpl implements ElementAnnotation {
   @override
   bool get isAlwaysThrows => _isPackageMetaGetter(_alwaysThrowsVariableName);
 
+  @override
+  bool get isAwaitNotRequired =>
+      _isPackageMetaGetter(_awaitNotRequiredVariableName);
+
   @override
   bool get isConstantEvaluated => evaluationResult != null;
 
@@ -7137,6 +7145,18 @@ final class MetadataImpl implements Metadata {
     return false;
   }
 
+  @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) {

pkg/analyzer/lib/src/error/annotation_verifier.dart

@@ -43,7 +43,10 @@ class AnnotationVerifier {
       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);

pkg/analyzer_utilities/lib/test/mock_packages/package_content/meta/lib/meta.dart

@@ -58,6 +58,38 @@ import 'meta_meta.dart';
 @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 @@ class _AlwaysThrows {
   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();
 }

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

@@ -70,6 +70,8 @@ class _Visitor extends SimpleAstVisitor<void> {
 
       _reportOnExpression(expr);
     }
+
+    // TODO(srawlins): Take `@awaitNotRequired` into account.
   }
 
   @override
@@ -114,6 +116,7 @@ class _Visitor extends SimpleAstVisitor<void> {
     if ((expr.staticType.isFutureOrFutureOr) &&
         !_isEnclosedInAsyncFunctionBody(expr) &&
         expr is! AssignmentExpression) {
+      // TODO(srawlins): Take `@awaitNotRequired` into account.
       _reportOnExpression(expr);
     }
   }

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

@@ -64,6 +64,10 @@ class _Visitor extends SimpleAstVisitor<void> {
       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 @@ class _Visitor extends SimpleAstVisitor<void> {
     _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 @@ class _Visitor extends SimpleAstVisitor<void> {
       _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 @@ class _Visitor extends SimpleAstVisitor<void> {
     }
   }
 }
+
+extension on Element2? {
+  bool get hasAwaitNotRequired =>
+      this is Annotatable &&
+      (this! as Annotatable).metadata2.hasAwaitNotRequired;
+}

pkg/linter/test/rules/unawaited_futures_test.dart

@@ -14,6 +14,9 @@ void main() {
 
 @reflectiveTest
 class UnawaitedFuturesTest extends LintRuleTest {
+  @override
+  bool get addMetaPackageDep => true;
+
   @override
   String get lintRule => LintNames.unawaited_futures;
 
@@ -31,6 +34,19 @@ class C {
     );
   }
 
+  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 @@ Future<int> g() => Future.value(0);
     );
   }
 
+  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,19 +156,18 @@ Future<int> g() => Future.value(0);
     );
   }
 
-  test_functionCallInCascade_assignment() async {
+  test_functionCall_unawaited_awaitNotRequired() async {
     await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
 void f() async {
-  C()..futureField = g();
+  g();
 }
+@awaitNotRequired
 Future<int> g() => Future.value(0);
-class C {
-  Future<int>? futureField;
-}
 ''');
   }
 
-  test_functionCallInCascade_inAsync() async {
+  test_functionCallInCascade() async {
     await assertDiagnostics(
       r'''
 void f() async {
@@ -155,6 +181,33 @@ class C {
     );
   }
 
+  // TODO(srawlins): Test that `@awaitNotRequired` is inherited.
+
+  test_functionCallInCascade_assignment() async {
+    await assertNoDiagnostics(r'''
+void f() async {
+  C()..futureField = g();
+}
+Future<int> g() => Future.value(0);
+class C {
+  Future<int>? futureField;
+}
+''');
+  }
+
+  test_functionCallInCascade_awaitNotRequired() async {
+    await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+void f() async {
+  C()..doAsync();
+}
+class C {
+  @awaitNotRequired
+  Future<void> doAsync() async {}
+}
+''');
+  }
+
   test_functionCallInCascade_indexAssignment() async {
     await assertNoDiagnostics(r'''
 void f() async {
@@ -180,6 +233,19 @@ class C {
   }
 
   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 @@ void f(Future<int> p) async {
     );
   }
 
-  test_unaryExpression_unawaited() async {
+  test_prefixExpression_unawaited() async {
     await assertDiagnostics(
       r'''
 void f(C a) async {
@@ -218,6 +284,19 @@ class C {
     );
   }
 
+  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'''