DAS: Add a fix for strict_top_level_inference

Change-Id: I990542bd6455113a70ead1d680f50b1163e6e263
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401800
Reviewed-by: Phil Quitslund <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

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

@@ -33,30 +33,29 @@ class AddReturnType extends ResolvedCorrectionProducer {
 
   @override
   Future<void> compute(ChangeBuilder builder) async {
-    Token? insertBeforeEntity;
-    FunctionBody? body;
-    var executable = node;
-    if (executable is MethodDeclaration && executable.name == token) {
-      if (executable.returnType != null) {
+    if (node
+        case MethodDeclaration(:var name, :var returnType, :var isSetter) ||
+            FunctionDeclaration(:var name, :var returnType, :var isSetter)) {
+      if (name != token) {
         return;
       }
-      if (executable.isSetter) {
+      if (returnType != null) {
         return;
       }
-      insertBeforeEntity =
-          executable.operatorKeyword ??
-          executable.propertyKeyword ??
-          executable.name;
-      body = executable.body;
-    } else if (executable is FunctionDeclaration && executable.name == token) {
-      if (executable.returnType != null) {
-        return;
-      }
-      if (executable.isSetter) {
+      if (isSetter) {
         return;
       }
-      insertBeforeEntity = executable.propertyKeyword ?? executable.name;
-      body = executable.functionExpression.body;
+    }
+
+    Token insertBeforeEntity;
+    FunctionBody body;
+    if (node case MethodDeclaration method) {
+      insertBeforeEntity =
+          method.operatorKeyword ?? method.propertyKeyword ?? method.name;
+      body = method.body;
+    } else if (node case FunctionDeclaration method) {
+      insertBeforeEntity = method.propertyKeyword ?? method.name;
+      body = method.functionExpression.body;
     } else {
       return;
     }
@@ -66,10 +65,9 @@ class AddReturnType extends ResolvedCorrectionProducer {
       return;
     }
 
-    var insertBeforeEntity_final = insertBeforeEntity;
     await builder.addDartFileEdit(file, (builder) {
       if (returnType is DynamicType || builder.canWriteType(returnType)) {
-        builder.addInsertion(insertBeforeEntity_final.offset, (builder) {
+        builder.addInsertion(insertBeforeEntity.offset, (builder) {
           if (returnType is DynamicType) {
             builder.write('dynamic');
           } else {
@@ -81,7 +79,7 @@ class AddReturnType extends ResolvedCorrectionProducer {
     });
   }
 
-  /// Return the type of value returned by the function [body], or `null` if a
+  /// Returns the type of value returned by the function [body], or `null` if a
   /// type can't be inferred.
   DartType? _inferReturnType(FunctionBody body) {
     DartType? baseType;
@@ -100,10 +98,8 @@ class AddReturnType extends ResolvedCorrectionProducer {
       return null;
     }
 
-    var isAsynchronous = body.isAsynchronous;
-    var isGenerator = body.isGenerator;
-    if (isAsynchronous) {
-      if (isGenerator) {
+    if (body.isAsynchronous) {
+      if (body.isGenerator) {
         return typeProvider.streamElement2.instantiate(
           typeArguments: [baseType],
           nullabilitySuffix: baseType.nullabilitySuffix,
@@ -114,7 +110,7 @@ class AddReturnType extends ResolvedCorrectionProducer {
           nullabilitySuffix: baseType.nullabilitySuffix,
         );
       }
-    } else if (isGenerator) {
+    } else if (body.isGenerator) {
       return typeProvider.iterableElement2.instantiate(
         typeArguments: [baseType],
         nullabilitySuffix: baseType.nullabilitySuffix,
@@ -144,13 +140,8 @@ class _ReturnTypeComputer extends RecursiveAstVisitor<void> {
   void visitReturnStatement(ReturnStatement node) {
     hasReturn = true;
     // prepare expression
-    var expression = node.expression;
-    if (expression == null) {
-      return;
-    }
-    // prepare type
-    var type = expression.typeOrThrow;
-    if (type.isBottom) {
+    var type = node.expression?.typeOrThrow;
+    if (type == null || type.isBottom) {
       return;
     }
     // combine types

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -44,9 +44,9 @@
 #   issue created for it.
 #
 # Stats:
-# -  54 "needsEvaluation"
+# -  58 "needsEvaluation"
 # - 308 "needsFix"
-# - 465 "hasFix"
+# - 470 "hasFix"
 # - 514 "noFix"
 
 AnalysisOptionsErrorCode.INCLUDED_FILE_PARSE_ERROR:
@@ -2368,17 +2368,19 @@ LintCode.specify_nonobvious_local_variable_types:
 LintCode.specify_nonobvious_property_types:
   status: hasFix
 LintCode.strict_top_level_inference_add_type:
-  status: noFix
-  notes: |-
-    There is no obvious way to pick a good candidate type.
+  status: hasFix
 LintCode.strict_top_level_inference_replace_keyword:
   status: noFix
   notes: |-
-    There is no obvious way to pick a good candidate type.
+    Conceivably for variable declarations we can search some space (like the
+    library or the compilation unit) for what values are assigned, and suggest
+    the LUB of those value(s)' type(s).
 LintCode.strict_top_level_inference_split_to_types:
   status: noFix
   notes: |-
-    There is no obvious way to pick a good candidate type.
+    Conceivably for variable declarations we can search some space (like the
+    library or the compilation unit) for what values are assigned, and suggest
+    the LUB of those value(s)' type(s).
 LintCode.test_types_in_equals:
   status: noFix
 LintCode.throw_in_finally:

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

@@ -467,6 +467,7 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
   LinterLintCode.specify_nonobvious_property_types: [
     AddTypeAnnotation.bulkFixable,
   ],
+  LinterLintCode.strict_top_level_inference_add_type: [AddReturnType.new],
   LinterLintCode.type_annotate_public_apis: [AddTypeAnnotation.bulkFixable],
   LinterLintCode.type_init_formals: [RemoveTypeAnnotation.other],
   LinterLintCode.type_literal_in_constant_pattern: [

pkg/analysis_server/test/src/services/correction/fix/add_return_type_test.dart

@@ -11,38 +11,14 @@ import 'fix_processor.dart';
 
 void main() {
   defineReflectiveSuite(() {
+    defineReflectiveTests(AddReturnType_AlwaysDeclareReturnTypesTest);
+    defineReflectiveTests(AddReturnType_StrictTopLevelInferenceTest);
     defineReflectiveTests(AddReturnTypeBulkTest);
-    defineReflectiveTests(AddReturnTypeLintTest);
   });
 }
 
 @reflectiveTest
-class AddReturnTypeBulkTest extends BulkFixProcessorTest {
-  @override
-  String get lintCode => LintNames.always_declare_return_types;
-
-  Future<void> test_singleFile() async {
-    await resolveTestCode('''
-class A {
-  get foo => 0;
-  m(p) {
-    return p;
-  }
-}
-''');
-    await assertHasFix('''
-class A {
-  int get foo => 0;
-  dynamic m(p) {
-    return p;
-  }
-}
-''');
-  }
-}
-
-@reflectiveTest
-class AddReturnTypeLintTest extends FixProcessorLintTest {
+class AddReturnType_AlwaysDeclareReturnTypesTest extends FixProcessorLintTest {
   @override
   FixKind get kind => DartFixKind.ADD_RETURN_TYPE;
 
@@ -238,3 +214,80 @@ int get foo => 0;
 ''');
   }
 }
+
+@reflectiveTest
+class AddReturnType_StrictTopLevelInferenceTest extends FixProcessorLintTest {
+  @override
+  FixKind get kind => DartFixKind.ADD_RETURN_TYPE;
+
+  @override
+  String get lintCode => LintNames.strict_top_level_inference;
+
+  Future<void> test_instanceMethod_typeVariable() async {
+    await resolveTestCode('''
+class C<T> {
+  f(T p) {
+    return p;
+  }
+}
+''');
+    await assertHasFix('''
+class C<T> {
+  T f(T p) {
+    return p;
+  }
+}
+''');
+  }
+
+  Future<void> test_topLevelFunction() async {
+    await resolveTestCode('''
+f() {
+  return '';
+}
+''');
+    await assertHasFix('''
+String f() {
+  return '';
+}
+''');
+  }
+
+  Future<void> test_topLevelFunction_typeVariable() async {
+    await resolveTestCode('''
+f<T>(T p) {
+  return [p];
+}
+''');
+    await assertHasFix('''
+List<T> f<T>(T p) {
+  return [p];
+}
+''');
+  }
+}
+
+@reflectiveTest
+class AddReturnTypeBulkTest extends BulkFixProcessorTest {
+  @override
+  String get lintCode => LintNames.always_declare_return_types;
+
+  Future<void> test_singleFile() async {
+    await resolveTestCode('''
+class A {
+  get foo => 0;
+  m(p) {
+    return p;
+  }
+}
+''');
+    await assertHasFix('''
+class A {
+  int get foo => 0;
+  dynamic m(p) {
+    return p;
+  }
+}
+''');
+  }
+}