linter: unawaited_futures: narrow the reported range, matching discarded_futures

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

Author: srawlins

pkg/analysis_server/lib/src/services/correction/dart/add_await.dart

@@ -68,7 +68,13 @@ class AddAwait extends ResolvedCorrectionProducer {
           await _addAwait(builder, convertToAsync: _functionBodyIfNotAsync);
         }
       case _CorrectionKind.unawaited:
-        if (node.parent is! CascadeExpression) {
+        // The reported node may be the `identifier` in a PrefixedIdentifier,
+        // the `propertyName` in a PropertyAccess, or the `methodName` in a
+        // MethodInvocation. Check whether the grandparent is a
+        // CascadeExpression. If it is, we cannot simply add an await
+        // expression; we must also change the cascade(s) into a regular
+        // property access or method call.
+        if (node.parent?.parent is! CascadeExpression) {
           await _addAwait(builder);
         }
       case _CorrectionKind.forIn:

pkg/analysis_server/lib/src/services/correction/dart/wrap_in_unawaited.dart

@@ -25,7 +25,13 @@ class WrapInUnawaited extends ResolvedCorrectionProducer {
   @override
   Future<void> compute(ChangeBuilder builder) async {
     AstNode? node = this.node;
-    if (node.parent is CascadeExpression) {
+    // The reported node may be the `identifier` in a PrefixedIdentifier,
+    // the `propertyName` in a PropertyAccess, or the `methodName` in a
+    // MethodInvocation. Check whether the grandparent is a
+    // CascadeExpression. If it is, we cannot simply add an await
+    // expression; we must also change the cascade(s) into a regular
+    // property access or method call.
+    if (node.parent?.parent is CascadeExpression) {
       return;
     }
     if (node is SimpleIdentifier) {

pkg/front_end/test/incremental_compiler_leak_test.dart

@@ -46,8 +46,7 @@ class LeakFinder extends vmService.LaunchingVMServiceHelper {
     });
 
     Completer<String> cRunDone = new Completer();
-    // ignore: unawaited_futures
-    runInternal(
+    unawaited(runInternal(
         isolateRef,
         classInfo,
         instanceCounts,
@@ -57,7 +56,7 @@ class LeakFinder extends vmService.LaunchingVMServiceHelper {
             cTimeout.isCompleted ||
             cProcessExited.isCompleted).then((value) {
       cRunDone.complete("Done");
-    });
+    }));
 
     await Future.any([cRunDone.future, cTimeout.future, cProcessExited.future]);
     timer.cancel();

pkg/front_end/test/run_our_tests_with_coverage.dart

@@ -55,8 +55,7 @@ Future<void> main() async {
     processesRunning++;
     String testPath = testPaths.removeLast();
     Stopwatch stopwatch = new Stopwatch()..start();
-    // ignore: unawaited_futures
-    Process.run(Platform.resolvedExecutable, [
+    unawaited(Process.run(Platform.resolvedExecutable, [
       coverageRunner.toFilePath(),
       "--enable-asserts",
       testPath
@@ -69,7 +68,7 @@ Future<void> main() async {
       Completer<void> oldCompleter = completer;
       completer = new Completer();
       oldCompleter.complete();
-    });
+    }));
   }
   while (processesRunning > 0) {
     log("Awaiting $processesRunning processes");

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

@@ -10,6 +10,7 @@ import 'package:analyzer/error/error.dart';
 
 import '../analyzer.dart';
 import '../extensions.dart';
+import '../utils.dart';
 
 const _desc =
     'There should be no `Future`-returning calls in synchronous functions unless they '
@@ -63,7 +64,7 @@ class _Visitor extends SimpleAstVisitor<void> {
         return;
       }
 
-      _reportOnExpression(expr);
+      reportOnExpression(rule, expr);
     }
   }
 
@@ -94,17 +95,6 @@ class _Visitor extends SimpleAstVisitor<void> {
       expr.methodName.name == 'putIfAbsent' &&
       _isMapClass(expr.methodName.element?.enclosingElement);
 
-  void _reportOnExpression(Expression expr) {
-    rule.reportAtNode(switch (expr) {
-      MethodInvocation(:var methodName) => methodName,
-      InstanceCreationExpression(:var constructorName) => constructorName,
-      FunctionExpressionInvocation(:var function) => function,
-      PrefixedIdentifier(:var identifier) => identifier,
-      PropertyAccess(:var propertyName) => propertyName,
-      _ => expr,
-    });
-  }
-
   void _visit(Expression expr) {
     if (expr.isAwaitNotRequired) {
       return;
@@ -114,7 +104,7 @@ class _Visitor extends SimpleAstVisitor<void> {
         !_isEnclosedInAsyncFunctionBody(expr) &&
         expr is! AssignmentExpression) {
       // TODO(srawlins): Take `@awaitNotRequired` into account.
-      _reportOnExpression(expr);
+      reportOnExpression(rule, expr);
     }
   }
 }

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

@@ -11,6 +11,7 @@ import 'package:analyzer/error/error.dart';
 
 import '../analyzer.dart';
 import '../extensions.dart';
+import '../utils.dart';
 
 const _desc =
     r'`Future` results in `async` function bodies must be '
@@ -68,7 +69,7 @@ class _Visitor extends SimpleAstVisitor<void> {
     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.
-      rule.reportAtNode(node);
+      reportOnExpression(rule, node.expression);
     }
   }
 
@@ -110,7 +111,7 @@ class _Visitor extends SimpleAstVisitor<void> {
     }
 
     if (_isEnclosedInAsyncFunctionBody(expr) && expr is! AssignmentExpression) {
-      rule.reportAtNode(expr);
+      reportOnExpression(rule, expr);
     }
   }
 }

pkg/linter/lib/src/utils.dart

@@ -2,7 +2,9 @@
 // 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/dart/ast/ast.dart';
 import 'package:analyzer/src/lint/io.dart'; // ignore: implementation_imports
+import 'package:analyzer/src/lint/linter.dart'; // ignore: implementation_imports
 
 import 'ast.dart';
 import 'util/ascii_utils.dart';
@@ -103,6 +105,20 @@ void printToConsole(Object? object) {
   outSink.writeln(object);
 }
 
+/// Reports a diagnostic at a minimal sub-node of [expr],
+// TODO(srawlins): Move this to a shared visitor between
+//`discardeed_futures.dart` and `unawaited_futures.dart`.
+void reportOnExpression(AnalysisRule rule, Expression expr) {
+  rule.reportAtNode(switch (expr) {
+    MethodInvocation(:var methodName) => methodName,
+    InstanceCreationExpression(:var constructorName) => constructorName,
+    FunctionExpressionInvocation(:var function) => function,
+    PrefixedIdentifier(:var identifier) => identifier,
+    PropertyAccess(:var propertyName) => propertyName,
+    _ => expr,
+  });
+}
+
 class CamelCaseString {
   static final _camelCaseMatcher = RegExp(r'[A-Z][a-z]*');
 

pkg/linter/messages.yaml

@@ -11990,7 +11990,7 @@ LintCode:
 
       ```dart
       Future<void> f() async {
-        [!g();!]
+        [!g!]();
       }
 
       Future<int> g() => Future.value(0);

pkg/linter/test/rules/unawaited_futures_test.dart

@@ -30,7 +30,7 @@ class C {
   Future<int> operator +(C other) async => 7;
 }
 ''',
-      [lint(27, 6)],
+      [lint(27, 5)],
     );
   }
 
@@ -86,7 +86,7 @@ void f(Future2 p) async {
 Future2 g(Future2 p) => p;
 abstract class Future2 implements Future {}
 ''',
-      [lint(28, 5)],
+      [lint(28, 1)],
     );
   }
 
@@ -108,7 +108,7 @@ void f() async {
 }
 Future<int> g() => Future.value(0);
 ''',
-      [lint(22, 3)],
+      [lint(22, 1)],
     );
   }
 
@@ -133,7 +133,7 @@ Future2<int> g() => f2;
 abstract class Future2<T> implements Future<T> {}
 external Future2<int> f2;
 ''',
-      [lint(22, 3)],
+      [lint(22, 1)],
     );
   }
 
@@ -145,7 +145,7 @@ void f() async {
 }
 Future<int>? g() => Future.value(0);
 ''',
-      [lint(19, 4)],
+      [lint(19, 1)],
     );
   }
 
@@ -166,7 +166,7 @@ void f() async {
 }
 Future<int> g() => Future.value(0);
 ''',
-      [lint(19, 4)],
+      [lint(19, 1)],
     );
   }
 
@@ -191,7 +191,7 @@ class C {
   Future<void> doAsync() async {}
 }
 ''',
-      [lint(22, 11)],
+      [lint(24, 7)],
     );
   }
 
@@ -270,7 +270,7 @@ abstract class C {
   Future<int> get p;
 }
 ''',
-      [lint(22, 4)],
+      [lint(24, 1)],
     );
   }
 
@@ -310,7 +310,7 @@ void f(Future<int> p) async {
   p;
 }
 ''',
-      [lint(32, 2)],
+      [lint(32, 1)],
     );
   }
 
@@ -324,7 +324,7 @@ class C {
   Future<int> operator -() async => 7;
 }
 ''',
-      [lint(22, 3)],
+      [lint(22, 2)],
     );
   }