analyzer: Report missing return for nullable return types

This is implemented as a new HintCode, but it could be a new Warning, if
we'd like to stop adding new Hints.

There is also a dartfix available in this change.

Fixes https://github.com/dart-lang/sdk/issues/46656

Change-Id: I8e93e576d2bd09a8ff02d52c12bbb9ec6adff9c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220803
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>

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

@@ -0,0 +1,76 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// 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:analysis_server/src/services/correction/dart/abstract_producer.dart';
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+
+class AddReturnNull extends CorrectionProducer {
+  @override
+  bool get canBeAppliedInBulk => true;
+
+  @override
+  bool get canBeAppliedToFile => true;
+
+  @override
+  FixKind get fixKind => DartFixKind.ADD_RETURN_NULL;
+
+  @override
+  FixKind? get multiFixKind => DartFixKind.ADD_RETURN_NULL_MULTI;
+
+  @override
+  Future<void> compute(ChangeBuilder builder) async {
+    Block block;
+
+    var coveringNode = coveredNode;
+    if (coveringNode is Block) {
+      block = coveringNode;
+    } else if (coveringNode is SimpleIdentifier) {
+      var declaration = coveringNode.parent;
+      if (declaration is FunctionDeclaration) {
+        var body = declaration.functionExpression.body;
+        if (body is BlockFunctionBody) {
+          block = body.block;
+        } else {
+          return;
+        }
+      } else if (declaration is MethodDeclaration) {
+        var body = declaration.body;
+        if (body is BlockFunctionBody) {
+          block = body.block;
+        } else {
+          return;
+        }
+      } else {
+        return;
+      }
+    } else {
+      return;
+    }
+    int position;
+    String returnStatement;
+    if (block.statements.isEmpty) {
+      position = block.offset + 1;
+      var prefix = utils.getLinePrefix(block.offset);
+      returnStatement =
+          '$eol$prefix${utils.getIndent(1)}return null;$eol$prefix';
+    } else {
+      var lastStatement = block.statements.last;
+      position = lastStatement.offset + lastStatement.length;
+      var prefix = utils.getNodePrefix(lastStatement);
+      returnStatement = '$eol${prefix}return null;';
+    }
+
+    await builder.addDartFileEdit(file, (builder) {
+      builder.addInsertion(position, (builder) {
+        builder.write(returnStatement);
+      });
+    });
+  }
+
+  /// Return an instance of this class. Used as a tear-off in `FixProcessor`.
+  static AddReturnNull newInstance() => AddReturnNull();
+}

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

@@ -213,6 +213,16 @@ class DartFixKind {
     DartFixKindPriority.IN_FILE,
     "Add 'required' keywords everywhere in file",
   );
+  static const ADD_RETURN_NULL = FixKind(
+    'dart.fix.add.returnNull',
+    DartFixKindPriority.DEFAULT,
+    "Add 'return null'",
+  );
+  static const ADD_RETURN_NULL_MULTI = FixKind(
+    'dart.fix.add.returnNull.multi',
+    DartFixKindPriority.IN_FILE,
+    "Add 'return null' everywhere in file",
+  );
   static const ADD_RETURN_TYPE = FixKind(
     'dart.fix.add.returnType',
     DartFixKindPriority.DEFAULT,

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

@@ -25,6 +25,7 @@ import 'package:analysis_server/src/services/correction/dart/add_null_check.dart
 import 'package:analysis_server/src/services/correction/dart/add_override.dart';
 import 'package:analysis_server/src/services/correction/dart/add_required.dart';
 import 'package:analysis_server/src/services/correction/dart/add_required_keyword.dart';
+import 'package:analysis_server/src/services/correction/dart/add_return_null.dart';
 import 'package:analysis_server/src/services/correction/dart/add_return_type.dart';
 import 'package:analysis_server/src/services/correction/dart/add_static.dart';
 import 'package:analysis_server/src/services/correction/dart/add_super_constructor_invocation.dart';
@@ -1077,6 +1078,9 @@ class FixProcessor extends BaseProcessor {
       MakeReturnTypeNullable.newInstance,
     ],
 
+    HintCode.BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE: [
+      AddReturnNull.newInstance,
+    ],
     HintCode.CAN_BE_NULL_AFTER_NULL_AWARE: [
       ReplaceWithNullAware.inChain,
     ],

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

@@ -0,0 +1,125 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// 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:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'fix_processor.dart';
+
+void main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(AddReturnNullBulkTest);
+    defineReflectiveTests(AddReturnNullTest);
+  });
+}
+
+@reflectiveTest
+class AddReturnNullBulkTest extends BulkFixProcessorTest {
+  Future<void> test_singleFile() async {
+    await resolveTestCode('''
+int? f() {
+  // ignore: unused_element
+  int? g() {
+    if (1 == 2) return 5;
+  }
+  if (1 == 2) return 7;
+}
+''');
+    await assertHasFix('''
+int? f() {
+  // ignore: unused_element
+  int? g() {
+    if (1 == 2) return 5;
+    return null;
+  }
+  if (1 == 2) return 7;
+  return null;
+}
+''');
+  }
+}
+
+@reflectiveTest
+class AddReturnNullTest extends FixProcessorTest {
+  @override
+  FixKind get kind => DartFixKind.ADD_RETURN_NULL;
+
+  Future<void> test_functionExpression() async {
+    await resolveTestCode('''
+int? Function() f = () {
+  if (1 == 2) return 7;
+};
+''');
+    await assertHasFix('''
+int? Function() f = () {
+  if (1 == 2) return 7;
+  return null;
+};
+''');
+  }
+
+  Future<void> test_functionExpression_empty() async {
+    await resolveTestCode('''
+int? Function() f = () {};
+''');
+    await assertHasFix('''
+int? Function() f = () {
+  return null;
+};
+''');
+  }
+
+  Future<void> test_localFunction() async {
+    await resolveTestCode('''
+void m() {
+  // ignore: unused_element
+  int? f() {
+    if (1 == 2) return 7;
+  }
+}
+''');
+    await assertHasFix('''
+void m() {
+  // ignore: unused_element
+  int? f() {
+    if (1 == 2) return 7;
+    return null;
+  }
+}
+''');
+  }
+
+  Future<void> test_method() async {
+    await resolveTestCode('''
+class A {
+  int? m() {
+    if (1 == 2) return 7;
+  }
+}
+''');
+    await assertHasFix('''
+class A {
+  int? m() {
+    if (1 == 2) return 7;
+    return null;
+  }
+}
+''');
+  }
+
+  Future<void> test_topLevelFunction_block() async {
+    await resolveTestCode('''
+int? f() {
+  if (1 == 2) return 7;
+}
+''');
+    await assertHasFix('''
+int? f() {
+  if (1 == 2) return 7;
+  return null;
+}
+''');
+  }
+}

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

@@ -30,6 +30,7 @@ import 'add_ne_null_test.dart' as add_ne_null;
 import 'add_null_check_test.dart' as add_null_check;
 import 'add_override_test.dart' as add_override;
 import 'add_required_test.dart' as add_required;
+import 'add_return_null_test.dart' as add_return_null;
 import 'add_return_type_test.dart' as add_return_type;
 import 'add_static_test.dart' as add_static;
 import 'add_super_constructor_invocation_test.dart'
@@ -234,6 +235,7 @@ void main() {
     add_null_check.main();
     add_override.main();
     add_required.main();
+    add_return_null.main();
     add_return_type.main();
     add_static.main();
     add_super_constructor_invocation.main();

pkg/analyzer/lib/error/error.dart

@@ -523,6 +523,7 @@ const List<ErrorCode> errorCodeValues = [
   FfiCode.SUBTYPE_OF_STRUCT_CLASS_IN_WITH,
   HintCode.ARGUMENT_TYPE_NOT_ASSIGNABLE_TO_ERROR_HANDLER,
   HintCode.ASSIGNMENT_OF_DO_NOT_STORE,
+  HintCode.BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE,
   HintCode.CAN_BE_NULL_AFTER_NULL_AWARE,
   HintCode.DEAD_CODE,
   HintCode.DEAD_CODE_CATCH_FOLLOWING_CATCH,

pkg/analyzer/lib/src/dart/error/hint_codes.g.dart

@@ -90,6 +90,19 @@ class HintCode extends AnalyzerErrorCode {
     correctionMessage: "Try removing the assignment.",
   );
 
+  /**
+   * Parameters:
+   * 0: the name of the declared return type
+   */
+  static const HintCode BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE = HintCode(
+    'BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE',
+    "This function has a nullable return type of '{0}', but ends without "
+        "returning a value.",
+    correctionMessage:
+        "Try adding a return statement, or if no value is ever returned, try "
+        "changing the return type to 'void'.",
+  );
+
   /**
    * When the target expression uses '?.' operator, it can be `null`, so all the
    * subsequent invocations should also use '?.' operator.

pkg/analyzer/lib/src/dart/resolver/function_expression_resolver.dart

@@ -9,7 +9,6 @@ import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
-import 'package:analyzer/src/dart/resolver/body_inference_context.dart';
 import 'package:analyzer/src/dart/resolver/invocation_inference_helper.dart';
 import 'package:analyzer/src/generated/migration.dart';
 import 'package:analyzer/src/generated/resolver.dart';
@@ -64,9 +63,7 @@ class FunctionExpressionResolver {
     _resolve2(node);
 
     if (_resolver.flowAnalysis.flow != null && !isFunctionDeclaration) {
-      var bodyContext = BodyInferenceContext.of(node.body);
       _resolver.checkForBodyMayCompleteNormally(
-        returnType: bodyContext?.contextType,
         body: body,
         errorNode: body,
       );

pkg/analyzer/lib/src/error/codes.g.dart

@@ -1115,13 +1115,15 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
   // }
   // ```
   //
-  // If the method intentionally returns `null` at the end, then change the
+  // If the method intentionally returns `null` at the end, then add an
+  // explicit return of `null` at the end of the method and change the
   // return type so that it's valid to return `null`:
   //
   // ```dart
   // class C<T> {
   //   T? m(T t) {
   //     print(t);
+  //     return null;
   //   }
   // }
   // ```
@@ -1129,7 +1131,7 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
       CompileTimeErrorCode(
     'BODY_MIGHT_COMPLETE_NORMALLY',
     "The body might complete normally, causing 'null' to be returned, but the "
-        "return type is a potentially non-nullable type.",
+        "return type, '{0}', is a potentially non-nullable type.",
     correctionMessage:
         "Try adding either a return or a throw statement at the end.",
     hasPublishedDocs: true,

pkg/analyzer/lib/src/generated/resolver.dart

@@ -472,7 +472,6 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
   }
 
   void checkForBodyMayCompleteNormally({
-    required DartType? returnType,
     required FunctionBody body,
     required AstNode errorNode,
   }) {
@@ -481,6 +480,12 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
       return;
     }
 
+    // TODO(scheglov) encapsulate
+    var bodyContext = BodyInferenceContext.of(body);
+    if (bodyContext == null) {
+      return null;
+    }
+    var returnType = bodyContext.contextType;
     if (returnType == null) {
       return;
     }
@@ -490,24 +495,59 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
         return;
       }
 
-      if (typeSystem.isPotentiallyNonNullable(returnType)) {
-        if (errorNode is ConstructorDeclaration) {
-          errorReporter.reportErrorForName(
-            CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY,
-            errorNode,
-          );
-        } else if (errorNode is BlockFunctionBody) {
-          errorReporter.reportErrorForToken(
-            CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY,
-            errorNode.block.leftBracket,
-          );
-        } else {
-          errorReporter.reportErrorForNode(
-            CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY,
-            errorNode,
-          );
+      if (body.isAsynchronous) {
+        // Check whether the return type is legal. If not, return rather than
+        // reporting a second error.
+
+        // This is the same check as [ReturnTypeVerifier._isLegalReturnType].
+        // TODO(srawlins): When this check is moved into the resolution stage,
+        // use the result of that check to determine whether this check should
+        // be done.
+        var lowerBound = typeProvider.futureElement.instantiate(
+          typeArguments: [NeverTypeImpl.instance],
+          nullabilitySuffix: NullabilitySuffix.star,
+        );
+        var imposedType = bodyContext.imposedType;
+        if (imposedType != null &&
+            !typeSystem.isSubtypeOf(lowerBound, imposedType)) {
+          // [imposedType] is an illegal return type for an asynchronous
+          // non-generator function; do not report an additional error here.
+          return;
         }
       }
+
+      ErrorCode errorCode;
+      if (typeSystem.isPotentiallyNonNullable(returnType)) {
+        errorCode = CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY;
+      } else {
+        var returnTypeBase = typeSystem.futureOrBase(returnType);
+        if (returnTypeBase.isVoid ||
+            returnTypeBase.isDynamic ||
+            returnTypeBase.isDartCoreNull) {
+          return;
+        } else {
+          errorCode = HintCode.BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE;
+        }
+      }
+      if (errorNode is ConstructorDeclaration) {
+        errorReporter.reportErrorForName(
+          errorCode,
+          errorNode,
+          arguments: [returnType],
+        );
+      } else if (errorNode is BlockFunctionBody) {
+        errorReporter.reportErrorForToken(
+          errorCode,
+          errorNode.block.leftBracket,
+          [returnType],
+        );
+      } else {
+        errorReporter.reportErrorForNode(
+          errorCode,
+          errorNode,
+          [returnType],
+        );
+      }
     }
   }
 
@@ -1342,9 +1382,7 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
     }
 
     if (node.factoryKeyword != null) {
-      var bodyContext = BodyInferenceContext.of(node.body);
       checkForBodyMayCompleteNormally(
-        returnType: bodyContext?.contextType,
         body: node.body,
         errorNode: node,
       );
@@ -1573,12 +1611,7 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
     }
 
     if (!node.isSetter) {
-      // TODO(scheglov) encapsulate
-      var bodyContext = BodyInferenceContext.of(
-        node.functionExpression.body,
-      );
       checkForBodyMayCompleteNormally(
-        returnType: bodyContext?.contextType,
         body: node.functionExpression.body,
         errorNode: node.name,
       );
@@ -1804,10 +1837,7 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
     }
 
     if (!node.isSetter) {
-      // TODO(scheglov) encapsulate
-      var bodyContext = BodyInferenceContext.of(node.body);
       checkForBodyMayCompleteNormally(
-        returnType: bodyContext?.contextType,
         body: node.body,
         errorNode: node.name,
       );

pkg/analyzer/messages.yaml

@@ -1082,7 +1082,7 @@ CompileTimeErrorCode:
       immediately enclosing _a_ is not declared asynchronous. (Where _a_ is the
       await expression.)
   BODY_MIGHT_COMPLETE_NORMALLY:
-    problemMessage: "The body might complete normally, causing 'null' to be returned, but the return type is a potentially non-nullable type."
+    problemMessage: "The body might complete normally, causing 'null' to be returned, but the return type, '{0}', is a potentially non-nullable type."
     correctionMessage: Try adding either a return or a throw statement at the end.
     hasPublishedDocs: true
     comment: No parameters.
@@ -1146,13 +1146,15 @@ CompileTimeErrorCode:
       }
       ```
 
-      If the method intentionally returns `null` at the end, then change the
+      If the method intentionally returns `null` at the end, then add an
+      explicit return of `null` at the end of the method and change the
       return type so that it's valid to return `null`:
 
       ```dart
       class C<T> {
         T? m(T t) {
           print(t);
+          return null;
         }
       }
       ```
@@ -14069,6 +14071,13 @@ HintCode:
     problemMessage: "'{0}' is marked 'doNotStore' and shouldn't be assigned to a field or top-level variable."
     correctionMessage: Try removing the assignment.
     comment: Users should not assign values marked `@doNotStore`.
+  BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE:
+    problemMessage: "This function has a nullable return type of '{0}', but ends without returning a value."
+    correctionMessage: "Try adding a return statement, or if no value is ever returned, try changing the return type to 'void'."
+    hasPublishedDocs: false
+    comment: |-
+      Parameters:
+      0: the name of the declared return type
   CAN_BE_NULL_AFTER_NULL_AWARE:
     problemMessage: "The receiver uses '?.', so its value can be null."
     correctionMessage: "Replace the '.' with a '?.' in the invocation."

pkg/analyzer/test/src/diagnostics/body_might_complete_normally_nullable_test.dart

@@ -0,0 +1,74 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// 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/src/error/codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(BodyMightCompleteNormallyNullableTest);
+  });
+}
+
+@reflectiveTest
+class BodyMightCompleteNormallyNullableTest extends PubPackageResolutionTest {
+  test_function_async_block_futureOrIntQuestion() async {
+    await assertErrorsInCode('''
+import 'dart:async';
+FutureOr<int?> f(Future f) async {}
+''', [
+      error(HintCode.BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE, 36, 1),
+    ]);
+  }
+
+  test_function_async_block_futureOrVoid() async {
+    await assertNoErrorsInCode('''
+import 'dart:async';
+FutureOr<void> f(Future f) async {}
+''');
+  }
+
+  test_function_async_block_void() async {
+    await assertNoErrorsInCode('''
+import 'dart:async';
+void f(Future f) async {}
+''');
+  }
+
+  test_function_sync_block_dynamic() async {
+    await assertNoErrorsInCode('''
+dynamic f() {}
+''');
+  }
+
+  test_function_sync_block_intQuestion() async {
+    await assertErrorsInCode('''
+int? f() {}
+''', [
+      error(HintCode.BODY_MIGHT_COMPLETE_NORMALLY_NULLABLE, 5, 1),
+    ]);
+  }
+
+  test_function_sync_block_intQuestion_definiteReturn() async {
+    await assertNoErrorsInCode('''
+int? f() {
+  return null;
+}
+''');
+  }
+
+  test_function_sync_block_Null() async {
+    await assertNoErrorsInCode('''
+Null f() {}
+''');
+  }
+
+  test_function_sync_block_void() async {
+    await assertNoErrorsInCode('''
+void f() {}
+''');
+  }
+}

pkg/analyzer/test/src/diagnostics/body_might_complete_normally_test.dart

@@ -222,16 +222,6 @@ main() {
 ''');
   }
 
-  test_functionExpression_nullable_blockBody() async {
-    await assertNoErrorsInCode(r'''
-main() {
-  int? Function() foo = () {
-  };
-  foo;
-}
-''');
-  }
-
   test_generativeConstructor_blockBody() async {
     await assertNoErrorsInCode(r'''
 class A {
@@ -350,14 +340,6 @@ class A {
 ''');
   }
 
-  test_method_nullable_blockBody() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  int? foo() {}
-}
-''');
-  }
-
   test_method_nullable_blockBody_return() async {
     await assertNoErrorsInCode(r'''
 class A {

pkg/analyzer/test/src/diagnostics/illegal_async_return_type_test.dart

@@ -17,7 +17,9 @@ main() {
 class IllegalAsyncReturnTypeTest extends PubPackageResolutionTest {
   test_function_nonFuture() async {
     await assertErrorsInCode('''
-int f() async {}
+int f() async {
+  return 1;
+}
 ''', [
       error(CompileTimeErrorCode.ILLEGAL_ASYNC_RETURN_TYPE, 0, 3),
     ]);
@@ -53,7 +55,9 @@ SubFuture<int> f() async {
   test_method_nonFuture() async {
     await assertErrorsInCode('''
 class C {
-  int m() async {}
+  int m() async {
+    return 1;
+  }
 }
 ''', [
       error(CompileTimeErrorCode.ILLEGAL_ASYNC_RETURN_TYPE, 12, 3),

pkg/analyzer/test/src/diagnostics/missing_return_test.dart

@@ -140,7 +140,7 @@ int Function() f = () {};
   }
 
   test_functionExpression_sync_dynamic() async {
-    await assertNoErrorsInCode(r'''
+    await assertNoErrorsInCode('''
 Function() f = () {};
 ''');
   }
@@ -153,7 +153,7 @@ int Function() f = () => null;
 
   test_localFunction_sync_dynamic() async {
     await assertNoErrorsInCode(r'''
-main() {
+void foo() {
   f() {}
   f;
 }
@@ -217,7 +217,26 @@ class B extends A {
 
 @reflectiveTest
 class MissingReturnWithNullSafetyTest extends PubPackageResolutionTest {
-  test_returnNever() async {
+  test_function_async_block_futureOrVoid() async {
+    await assertNoErrorsInCode('''
+import 'dart:async';
+FutureOr<void> f() async {}
+''');
+  }
+
+  test_function_async_block_void() async {
+    await assertNoErrorsInCode('''
+void f() async {}
+''');
+  }
+
+  test_function_sync_block_dynamic() async {
+    await assertNoErrorsInCode('''
+dynamic f() {}
+''');
+  }
+
+  test_function_sync_block_Never() async {
     newFile('$testPackageLibPath/a.dart', content: r'''
 Never foo() {
   throw 0;
@@ -230,6 +249,18 @@ import 'a.dart';
 int f() {
   foo();
 }
+''');
+  }
+
+  test_function_sync_block_Null() async {
+    await assertNoErrorsInCode('''
+Null f() {}
+''');
+  }
+
+  test_function_sync_block_void() async {
+    await assertNoErrorsInCode('''
+void f() {}
 ''');
   }
 }

pkg/analyzer/test/src/diagnostics/test_all.dart

@@ -41,6 +41,8 @@ import 'await_in_late_local_variable_initializer_test.dart'
     as await_in_late_local_variable_initializer;
 import 'await_in_wrong_context_test.dart' as await_in_wrong_context;
 import 'binary_operator_written_out_test.dart' as binary_operator_written_out;
+import 'body_might_complete_normally_nullable_test.dart'
+    as body_might_complete_normally_nullable;
 import 'body_might_complete_normally_test.dart' as body_might_complete_normally;
 import 'built_in_identifier_as_extension_name_test.dart'
     as built_in_as_extension_name;
@@ -751,6 +753,7 @@ main() {
     await_in_late_local_variable_initializer.main();
     await_in_wrong_context.main();
     binary_operator_written_out.main();
+    body_might_complete_normally_nullable.main();
     body_might_complete_normally.main();
     built_in_as_extension_name.main();
     built_in_as_prefix_name.main();

pkg/analyzer/tool/diagnostics/diagnostics.md

@@ -1323,7 +1323,7 @@ Future<int> f() async {
 ### body_might_complete_normally
 
 _The body might complete normally, causing 'null' to be returned, but the return
-type is a potentially non-nullable type._
+type, '{0}', is a potentially non-nullable type._
 
 #### Description
 
@@ -1384,13 +1384,15 @@ class C<T> {
 }
 {% endprettify %}
 
-If the method intentionally returns `null` at the end, then change the
+If the method intentionally returns `null` at the end, then add an
+explicit return of `null` at the end of the method and change the
 return type so that it's valid to return `null`:
 
 {% prettify dart tag=pre+code %}
 class C<T> {
   T? m(T t) {
     print(t);
+    return null;
   }
 }
 {% endprettify %}