Add a warning for methods named factory

The primary constructors language feature changes the interpretation of
members of the form `factory() {}`. Before the feature that code defines
a method named `factory`, but with the feature enabled that now defines
a factory constructor.

This CL adds a warning that applies when the feature is not enabled that
tells users that methods named `factory`, when there is nothing before
the name of the method such as a return type, will no longer be
supported after the feature is enabled.

In addition, it associates a fix with the diagnostic in order to make it
easier for users to update their code in a way that will preserve the
semantics of the code after the feature is enabled.

I added the check in the `BestPracticesVerifier`, but that might not be
the best place for it. Please let me know if it should be moved.

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

Author: bwilkerson

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

@@ -3898,3 +3898,5 @@ uri_does_not_exist_in_doc_import:
   status: needsFix
   notes: |-
     The same fix as CompileTimeErrorCode.URI_DOES_NOT_EXIST
+deprecated_factory_method:
+  status: hasFix

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

@@ -525,6 +525,7 @@ final _builtInNonLintGenerators = <DiagnosticCode, List<ProducerGenerator>>{
     RemoveDefaultValue.new,
     RemoveRequired.new,
   ],
+  diag.deprecatedFactoryMethod: [AddReturnType.new],
   diag.dotShorthandUndefinedGetter: [
     AddEnumConstant.new,
     ChangeTo.getterOrSetter,

pkg/analysis_server/test/src/services/correction/fix/add_return_type_test.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:_fe_analyzer_shared/src/base/errors.dart';
 import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:linter/src/lint_names.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -12,6 +14,7 @@ import 'fix_processor.dart';
 void main() {
   defineReflectiveSuite(() {
     defineReflectiveTests(AddReturnType_AlwaysDeclareReturnTypesTest);
+    defineReflectiveTests(AddReturnType_DeprecatedFactoryMethodTest);
     defineReflectiveTests(AddReturnType_StrictTopLevelInferenceTest);
     defineReflectiveTests(AddReturnTypeBulkTest);
   });
@@ -300,6 +303,31 @@ Iterable<num> f() sync* {
   }
 }
 
+@reflectiveTest
+class AddReturnType_DeprecatedFactoryMethodTest
+    extends FixProcessorErrorCodeTest {
+  @override
+  DiagnosticCode get diagnosticCode => diag.deprecatedFactoryMethod;
+
+  @override
+  FixKind get kind => DartFixKind.addReturnType;
+
+  Future<void> test_notAnOverride() async {
+    await resolveTestCode('''
+// @dart=2.12
+class C {
+  factory() => 3;
+}
+''');
+    await assertHasFix('''
+// @dart=2.12
+class C {
+  int factory() => 3;
+}
+''');
+  }
+}
+
 @reflectiveTest
 class AddReturnType_StrictTopLevelInferenceTest extends FixProcessorLintTest {
   @override

pkg/analyzer/lib/src/diagnostic/diagnostic.g.dart

@@ -3223,6 +3223,21 @@ const DiagnosticWithoutArguments deprecatedExtendsFunction =
       expectedTypes: [],
     );
 
+/// No parameters.
+const DiagnosticWithoutArguments deprecatedFactoryMethod =
+    DiagnosticWithoutArgumentsImpl(
+      name: 'deprecated_factory_method',
+      problemMessage:
+          "Methods named 'factory' will become constructors when the "
+          "primary_constructors feature is enabled.",
+      correctionMessage:
+          "Try adding a return type or modifier before the method's name, or "
+          "change the name of the method.",
+      type: DiagnosticType.STATIC_WARNING,
+      uniqueName: 'deprecated_factory_method',
+      expectedTypes: [],
+    );
+
 /// Parameters:
 /// String p0: the name of the field
 const DiagnosticWithArguments<

pkg/analyzer/lib/src/diagnostic/diagnostic_code_values.g.dart

@@ -214,6 +214,7 @@ const List<DiagnosticCode> diagnosticCodeValues = [
   diag.deprecatedExportUse,
   diag.deprecatedExtend,
   diag.deprecatedExtendsFunction,
+  diag.deprecatedFactoryMethod,
   diag.deprecatedField,
   diag.deprecatedImplement,
   diag.deprecatedImplementsFunction,

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

@@ -676,6 +676,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
   @override
   void visitMethodDeclaration(MethodDeclaration node) {
     bool wasInDoNotStoreMember = _inDoNotStoreMember;
+    var nameToken = node.name;
     var element = node.declaredFragment!.element;
     var enclosingElement = element.enclosingElement;
 
@@ -706,7 +707,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
         _checkStrictInferenceReturnType(
           node.returnType,
           node,
-          node.name.lexeme,
+          nameToken.lexeme,
         );
       }
       if (!elementIsOverride) {
@@ -723,15 +724,31 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
         // always named, so we can safely assume
         // `overriddenElement.enclosingElement3.name` is non-`null`.
         _diagnosticReporter.atToken(
-          node.name,
+          nameToken,
           diag.invalidOverrideOfNonVirtualMember,
           arguments: [
-            node.name.lexeme,
+            nameToken.lexeme,
             overriddenElement.enclosingElement!.displayName,
           ],
         );
       }
 
+      bool isAmbiguousFactoryMethod() {
+        if (nameToken.lexeme != Keyword.FACTORY.lexeme) return false;
+        var firstToken = node.firstTokenAfterCommentAndMetadata;
+        if (firstToken == nameToken) return true;
+        if (firstToken.lexeme == Keyword.EXTERNAL.lexeme ||
+            firstToken.lexeme == Keyword.AUGMENT.lexeme) {
+          return firstToken.next == nameToken;
+        }
+        return false;
+      }
+
+      if (!_currentLibrary.featureSet.isEnabled(Feature.primary_constructors) &&
+          isAmbiguousFactoryMethod()) {
+        _diagnosticReporter.atToken(nameToken, diag.deprecatedFactoryMethod);
+      }
+
       super.visitMethodDeclaration(node);
     } finally {
       for (var v in _elementUsageFrontierDetectors) {

pkg/analyzer/messages.yaml

@@ -25329,6 +25329,12 @@ WarningCode:
       ```dart
       class F {}
       ```
+  DEPRECATED_FACTORY_METHOD:
+    type: staticWarning
+    parameters: none
+    problemMessage: Methods named 'factory' will become constructors when the primary_constructors feature is enabled.
+    correctionMessage: Try adding a return type or modifier before the method's name, or change the name of the method.
+    hasPublishedDocs: false
   DEPRECATED_IMPLEMENT:
     type: staticWarning
     parameters:

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

@@ -0,0 +1,138 @@
+// Copyright (c) 2025, 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/diagnostic/diagnostic.dart' as diag;
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(DeprecatedFactoryMethodTest);
+  });
+}
+
+@reflectiveTest
+class DeprecatedFactoryMethodTest extends PubPackageResolutionTest {
+  test_noTypeOrModifier_after() async {
+    await assertNoErrorsInCode('''
+class C {
+  factory() => throw 0;
+}
+''');
+  }
+
+  test_noTypeOrModifier_before() async {
+    await assertErrorsInCode(
+      '''
+// @dart=2.12
+class C {
+  factory() => throw 0;
+}
+''',
+      [error(diag.deprecatedFactoryMethod, 26, 7)],
+    );
+  }
+
+  test_withAnnotation_after() async {
+    await assertNoErrorsInCode('''
+class C {
+  @deprecated
+  factory() => throw 0;
+}
+''');
+  }
+
+  test_withAnnotation_before() async {
+    await assertErrorsInCode(
+      '''
+// @dart=2.12
+class C {
+  @deprecated
+  factory() => throw 0;
+}
+''',
+      [error(diag.deprecatedFactoryMethod, 40, 7)],
+    );
+  }
+
+  test_withModifier_augment_after() async {
+    await assertNoErrorsInCode('''
+class C {
+  augment factory() => throw 0;
+}
+''');
+  }
+
+  test_withModifier_augment_before() async {
+    await assertErrorsInCode(
+      '''
+// @dart=2.12
+class C {
+  augment factory() => throw 0;
+}
+''',
+      // TODO(brianwilkerson): The `undefinedClass` diagnostic should not be
+      //  produced here.
+      [
+        error(diag.undefinedClass, 26, 7),
+        error(diag.deprecatedFactoryMethod, 34, 7),
+      ],
+    );
+  }
+
+  test_withModifier_external_after() async {
+    await assertNoErrorsInCode('''
+class C {
+  external factory();
+}
+''');
+  }
+
+  test_withModifier_external_before() async {
+    await assertErrorsInCode(
+      '''
+// @dart=2.12
+class C {
+  external factory();
+}
+''',
+      [error(diag.deprecatedFactoryMethod, 35, 7)],
+    );
+  }
+
+  test_withModifier_static_after() async {
+    await assertNoErrorsInCode('''
+class C {
+  static factory() {}
+}
+''');
+  }
+
+  test_withModifier_static_before() async {
+    await assertNoErrorsInCode('''
+// @dart=2.12
+class C {
+  static factory() {}
+}
+''');
+  }
+
+  test_withType_after() async {
+    await assertNoErrorsInCode('''
+class C {
+  int factory() => 0;
+}
+''');
+  }
+
+  test_withType_before() async {
+    await assertNoErrorsInCode('''
+// @dart=2.12
+class C {
+  int factory() => 0;
+}
+''');
+  }
+}

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

@@ -183,6 +183,7 @@ import 'deprecated_colon_for_default_value_test.dart'
 import 'deprecated_export_use_test.dart' as deprecated_export_use;
 import 'deprecated_extend_test.dart' as deprecated_extend;
 import 'deprecated_extends_function_test.dart' as deprecated_extends_function;
+import 'deprecated_factory_method_test.dart' as deprecated_factory_method;
 import 'deprecated_implement_test.dart' as deprecated_implement;
 import 'deprecated_implements_function_test.dart'
     as deprecated_implements_function;
@@ -1079,6 +1080,7 @@ main() {
     deprecated_export_use.main();
     deprecated_extend.main();
     deprecated_extends_function.main();
+    deprecated_factory_method.main();
     deprecated_implement.main();
     deprecated_implements_function.main();
     deprecated_instantiate.main();