linter: Consider @awaitNotRequired in the discared_futures rule

Work towards #46218

Change-Id: I9e356ab28996351d2ebfb15f655816f35a825339
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424480
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/linter/lib/src/extensions.dart

@@ -328,6 +328,11 @@ extension ElementExtension on Element2? {
     _ => this,
   };
 
+  /// Whether this is an [Annotatable] which is annotated with `@awaitNotRequired`.
+  bool get hasAwaitNotRequired =>
+      this is Annotatable &&
+      (this! as Annotatable).metadata2.hasAwaitNotRequired;
+
   bool get isDartCorePrint {
     var self = this;
     return self is TopLevelFunctionElement &&
@@ -336,7 +341,23 @@ extension ElementExtension on Element2? {
   }
 }
 
-extension ExpressionExtension on Expression? {
+extension ExpressionExtension on Expression {
+  /// Returns whether `await` is not required for this expression.
+  // TODO(srawlins): Handle inheritence in each of these cases; the
+  // `@awaitNotRequired` annotation should be inherited.
+  bool get isAwaitNotRequired => switch (this) {
+    BinaryExpression(:var element) => element.hasAwaitNotRequired,
+    MethodInvocation(:var methodName) => methodName.element.hasAwaitNotRequired,
+    PrefixedIdentifier(:var identifier) =>
+      identifier.element.hasAwaitNotRequired,
+    PrefixExpression(:var element) => element.hasAwaitNotRequired,
+    PropertyAccess(:var propertyName) =>
+      propertyName.element.hasAwaitNotRequired,
+    _ => false,
+  };
+}
+
+extension ExpressionNullableExtension on Expression? {
   /// A very, very, very rough approximation of the context type of this node.
   ///
   /// This approximation will never be accurate for some expressions.

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

@@ -7,6 +7,7 @@ import 'package:analyzer/dart/element/element2.dart';
 import 'package:analyzer/dart/element/type.dart';
 
 import '../analyzer.dart';
+import '../extensions.dart';
 
 const _desc =
     'There should be no `Future`-returning calls in synchronous functions unless they '
@@ -57,6 +58,10 @@ class _Visitor extends SimpleAstVisitor<void> {
       expr = expression;
     }
 
+    if (expr.isAwaitNotRequired) {
+      return;
+    }
+
     var type = expr.staticType;
     if (type == null) {
       return;
@@ -70,8 +75,6 @@ class _Visitor extends SimpleAstVisitor<void> {
 
       _reportOnExpression(expr);
     }
-
-    // TODO(srawlins): Take `@awaitNotRequired` into account.
   }
 
   @override
@@ -113,6 +116,10 @@ class _Visitor extends SimpleAstVisitor<void> {
   }
 
   void _visit(Expression expr) {
+    if (expr.isAwaitNotRequired) {
+      return;
+    }
+
     if ((expr.staticType.isFutureOrFutureOr) &&
         !_isEnclosedInAsyncFunctionBody(expr) &&
         expr is! AssignmentExpression) {

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

@@ -64,7 +64,7 @@ class _Visitor extends SimpleAstVisitor<void> {
       return;
     }
 
-    if (_isAwaitNotRequired(expr)) {
+    if (expr.isAwaitNotRequired) {
       return;
     }
 
@@ -80,39 +80,6 @@ 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;
@@ -136,7 +103,7 @@ class _Visitor extends SimpleAstVisitor<void> {
       _isMapClass(expr.methodName.element?.enclosingElement2);
 
   void _visit(Expression expr) {
-    if (_isAwaitNotRequired(expr)) {
+    if (expr.isAwaitNotRequired) {
       return;
     }
 
@@ -148,9 +115,3 @@ class _Visitor extends SimpleAstVisitor<void> {
     }
   }
 }
-
-extension on Element2? {
-  bool get hasAwaitNotRequired =>
-      this is Annotatable &&
-      (this! as Annotatable).metadata2.hasAwaitNotRequired;
-}

pkg/linter/test/rules/discarded_futures_test.dart

@@ -14,6 +14,9 @@ void main() {
 
 @reflectiveTest
 class DiscardedFuturesTest extends LintRuleTest {
+  @override
+  bool get addMetaPackageDep => true;
+
   @override
   String get lintRule => LintNames.discarded_futures;
 
@@ -101,7 +104,7 @@ void foo() {
 }
 
 Future<int> g() async => 0;
-  ''');
+''');
   }
 
   Future<void> test_assignment_ok_implicit_setOfFuture() async {
@@ -111,7 +114,7 @@ void foo() {
 }
 
 Future<int> g() async => 0;
-  ''');
+''');
   }
 
   Future<void> test_cascade_ok_future() async {
@@ -146,7 +149,7 @@ void foo() {
 extension on int {
   Future<int> g() async => this;
 }
-  ''',
+''',
       [lint(26, 1)],
     );
   }
@@ -306,18 +309,28 @@ Future<int> g() async => 0;
   Future<void> test_function() async {
     await assertDiagnostics(
       r'''
-void recreateDir(String path) {
-  deleteDir(path);
-  createDir(path);
+void f() {
+  g();
 }
 
-Future<void> deleteDir(String path) async {}
-Future<void> createDir(String path) async {}
+Future<int> g() async => 7;
 ''',
-      [lint(34, 9), lint(53, 9)],
+      [lint(13, 1)],
     );
   }
 
+  Future<void> test_function_awaitNotRequired() async {
+    await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+void f() {
+  g();
+}
+
+@awaitNotRequired
+Future<int> g() async => 7;
+''');
+  }
+
   Future<void> test_function_closure() async {
     await assertDiagnostics(
       r'''
@@ -361,6 +374,21 @@ Future<int> g() async => 0;
 ''');
   }
 
+  Future<void> test_function_futureOr() async {
+    await assertDiagnostics(
+      '''
+import 'dart:async';
+
+void f() {
+  g();
+}
+
+FutureOr<int> g() async => 0;
+''',
+      [lint(35, 1)],
+    );
+  }
+
   Future<void> test_function_ok_async() async {
     await assertNoDiagnostics(r'''
 Future<void> recreateDir(String path) async {
@@ -431,17 +459,15 @@ Future<int> g() async => 0;
   Future<void> test_method() async {
     await assertDiagnostics(
       r'''
-class Dir{
-  void recreateDir(String path) {
-    deleteDir(path);
-    createDir(path);
+class C {
+  void m() {
+    g();
   }
 
-  Future<void> deleteDir(String path) async {}
-  Future<void> createDir(String path) async {}
+  Future<void> g() async {}
 }
 ''',
-      [lint(49, 9), lint(70, 9)],
+      [lint(27, 1)],
     );
   }
 
@@ -497,6 +523,20 @@ Future<int> g() async => 0;
 ''');
   }
 
+  Future<void> test_method_awaitNotRequired() async {
+    await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+class C {
+  void m() {
+    g();
+  }
+
+  @awaitNotRequired
+  Future<void> g() async {}
+}
+''');
+  }
+
   Future<void> test_method_record_namedAssignment_ok_future() async {
     await assertNoDiagnostics(r'''
 void foo() {
@@ -702,21 +742,6 @@ Future<int> g() async => 0;
 ''');
   }
 
-  Future<void> test_trigger_futureOr() async {
-    await assertDiagnostics(
-      '''
-import 'dart:async';
-
-void foo() {
-  g();
-}
-
-FutureOr<int> g() async => 0;
-''',
-      [lint(37, 1)],
-    );
-  }
-
   Future<void> test_variable_assignment() async {
     await assertDiagnostics(
       r'''