Issue 42337. Report BODY_MIGHT_COMPLETE_NORMALLY for factory constructors.

Bug: #42337
Change-Id: I9209d7bf519ec61026ffe22bb05145e776e8517d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151344
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

Author: scheglov

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

@@ -273,7 +273,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
 
   @override
   void visitConstructorDeclaration(ConstructorDeclaration node) {
-    if (node.declaredElement.isFactory) {
+    if (!_isNonNullableByDefault && node.declaredElement.isFactory) {
       if (node.body is BlockFunctionBody) {
         // Check the block for a return statement, if not, create the hint.
         if (!ExitDetector.exits(node.body)) {

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

@@ -414,10 +414,17 @@ class ResolverVisitor extends ScopedVisitor {
       }
 
       if (typeSystem.isPotentiallyNonNullable(returnType)) {
-        errorReporter.reportErrorForNode(
-          CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY,
-          errorNode,
-        );
+        if (errorNode is ConstructorDeclaration) {
+          errorReporter.reportErrorForName(
+            CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY,
+            errorNode,
+          );
+        } else {
+          errorReporter.reportErrorForNode(
+            CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY,
+            errorNode,
+          );
+        }
       }
     }
   }
@@ -807,24 +814,41 @@ class ResolverVisitor extends ScopedVisitor {
   @override
   void visitConstructorDeclaration(ConstructorDeclaration node) {
     ExecutableElement outerFunction = _enclosingFunction;
-    try {
-      _flowAnalysis?.topLevelDeclaration_enter(
-          node, node.parameters, node.body);
-      _flowAnalysis?.executableDeclaration_enter(node, node.parameters, false);
+    _enclosingFunction = node.declaredElement;
+
+    if (_flowAnalysis != null) {
+      _flowAnalysis.topLevelDeclaration_enter(node, node.parameters, node.body);
+      _flowAnalysis.executableDeclaration_enter(node, node.parameters, false);
+    } else {
       _promoteManager.enterFunctionBody(node.body);
-      _enclosingFunction = node.declaredElement;
-      FunctionType type = _enclosingFunction.type;
-      InferenceContext.setType(node.body, type.returnType);
-      super.visitConstructorDeclaration(node);
-    } finally {
-      _flowAnalysis?.executableDeclaration_exit(node.body, false);
-      _flowAnalysis?.topLevelDeclaration_exit();
+    }
+
+    var returnType = _enclosingFunction.type.returnType;
+    InferenceContext.setType(node.body, returnType);
+
+    super.visitConstructorDeclaration(node);
+
+    if (_flowAnalysis != null) {
+      var bodyContext = BodyInferenceContext.of(node.body);
+      if (node.factoryKeyword != null) {
+        checkForBodyMayCompleteNormally(
+          returnType: bodyContext?.contextType,
+          body: node.body,
+          errorNode: node,
+        );
+      }
+      _flowAnalysis.executableDeclaration_exit(node.body, false);
+      _flowAnalysis.topLevelDeclaration_exit();
+      nullSafetyDeadCodeVerifier?.flowEnd(node);
+    } else {
       _promoteManager.exitFunctionBody();
-      _enclosingFunction = outerFunction;
     }
+
     ConstructorElementImpl constructor = node.declaredElement;
     constructor.constantInitializers =
         _createCloner().cloneNodeList(node.initializers);
+
+    _enclosingFunction = outerFunction;
   }
 
   @override

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

@@ -22,6 +22,26 @@ class BodyMayCompleteNormallyTest extends DriverResolutionTest {
     ..contextFeatures = FeatureSet.forTesting(
         sdkVersion: '2.7.0', additionalFeatures: [Feature.non_nullable]);
 
+  test_factoryConstructor_named_blockBody() async {
+    await assertErrorsInCode(r'''
+class A {
+  factory A.named() {}
+}
+''', [
+      error(CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY, 20, 7),
+    ]);
+  }
+
+  test_factoryConstructor_unnamed_blockBody() async {
+    await assertErrorsInCode(r'''
+class A {
+  factory A() {}
+}
+''', [
+      error(CompileTimeErrorCode.BODY_MIGHT_COMPLETE_NORMALLY, 20, 1),
+    ]);
+  }
+
   test_function_future_int_blockBody_async() async {
     await assertErrorsInCode(r'''
 Future<int> foo() async {}
@@ -219,6 +239,22 @@ main() {
 ''');
   }
 
+  test_generativeConstructor_blockBody() async {
+    await assertNoErrorsInCode(r'''
+class A {
+  A() {}
+}
+''');
+  }
+
+  test_generativeConstructor_emptyBody() async {
+    await assertNoErrorsInCode(r'''
+class A {
+  A();
+}
+''');
+  }
+
   test_method_future_int_blockBody_async() async {
     await assertErrorsInCode(r'''
 class A {