Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 18b93ca

Browse filesBrowse files
bwilkersonCommit Queue
bwilkerson
authored and
Commit Queue
committed
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 <paulberry@google.com> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Samuel Rawlins <srawlins@google.com>
1 parent 0e9487d commit 18b93ca
Copy full SHA for 18b93ca

File tree

Expand file treeCollapse file tree

9 files changed

+213
-3
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+213
-3
lines changed
Open diff view settings
Collapse file

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

Copy file name to clipboardExpand all lines: pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3898,3 +3898,5 @@ uri_does_not_exist_in_doc_import:
38983898
status: needsFix
38993899
notes: |-
39003900
The same fix as CompileTimeErrorCode.URI_DOES_NOT_EXIST
3901+
deprecated_factory_method:
3902+
status: hasFix
Collapse file

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

Copy file name to clipboardExpand all lines: pkg/analysis_server/lib/src/services/correction/fix_internal.dart
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,7 @@ final _builtInNonLintGenerators = <DiagnosticCode, List<ProducerGenerator>>{
525525
RemoveDefaultValue.new,
526526
RemoveRequired.new,
527527
],
528+
diag.deprecatedFactoryMethod: [AddReturnType.new],
528529
diag.dotShorthandUndefinedGetter: [
529530
AddEnumConstant.new,
530531
ChangeTo.getterOrSetter,
Collapse file

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

Copy file name to clipboardExpand all lines: pkg/analysis_server/test/src/services/correction/fix/add_return_type_test.dart
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:_fe_analyzer_shared/src/base/errors.dart';
56
import 'package:analysis_server/src/services/correction/fix.dart';
7+
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
68
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
79
import 'package:linter/src/lint_names.dart';
810
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -12,6 +14,7 @@ import 'fix_processor.dart';
1214
void main() {
1315
defineReflectiveSuite(() {
1416
defineReflectiveTests(AddReturnType_AlwaysDeclareReturnTypesTest);
17+
defineReflectiveTests(AddReturnType_DeprecatedFactoryMethodTest);
1518
defineReflectiveTests(AddReturnType_StrictTopLevelInferenceTest);
1619
defineReflectiveTests(AddReturnTypeBulkTest);
1720
});
@@ -300,6 +303,31 @@ Iterable<num> f() sync* {
300303
}
301304
}
302305

306+
@reflectiveTest
307+
class AddReturnType_DeprecatedFactoryMethodTest
308+
extends FixProcessorErrorCodeTest {
309+
@override
310+
DiagnosticCode get diagnosticCode => diag.deprecatedFactoryMethod;
311+
312+
@override
313+
FixKind get kind => DartFixKind.addReturnType;
314+
315+
Future<void> test_notAnOverride() async {
316+
await resolveTestCode('''
317+
// @dart=2.12
318+
class C {
319+
factory() => 3;
320+
}
321+
''');
322+
await assertHasFix('''
323+
// @dart=2.12
324+
class C {
325+
int factory() => 3;
326+
}
327+
''');
328+
}
329+
}
330+
303331
@reflectiveTest
304332
class AddReturnType_StrictTopLevelInferenceTest extends FixProcessorLintTest {
305333
@override
Collapse file

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

Copy file name to clipboardExpand all lines: pkg/analyzer/lib/src/diagnostic/diagnostic.g.dart
+15Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3223,6 +3223,21 @@ const DiagnosticWithoutArguments deprecatedExtendsFunction =
32233223
expectedTypes: [],
32243224
);
32253225

3226+
/// No parameters.
3227+
const DiagnosticWithoutArguments deprecatedFactoryMethod =
3228+
DiagnosticWithoutArgumentsImpl(
3229+
name: 'deprecated_factory_method',
3230+
problemMessage:
3231+
"Methods named 'factory' will become constructors when the "
3232+
"primary_constructors feature is enabled.",
3233+
correctionMessage:
3234+
"Try adding a return type or modifier before the method's name, or "
3235+
"change the name of the method.",
3236+
type: DiagnosticType.STATIC_WARNING,
3237+
uniqueName: 'deprecated_factory_method',
3238+
expectedTypes: [],
3239+
);
3240+
32263241
/// Parameters:
32273242
/// String p0: the name of the field
32283243
const DiagnosticWithArguments<
Collapse file

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

Copy file name to clipboardExpand all lines: pkg/analyzer/lib/src/diagnostic/diagnostic_code_values.g.dart
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ const List<DiagnosticCode> diagnosticCodeValues = [
214214
diag.deprecatedExportUse,
215215
diag.deprecatedExtend,
216216
diag.deprecatedExtendsFunction,
217+
diag.deprecatedFactoryMethod,
217218
diag.deprecatedField,
218219
diag.deprecatedImplement,
219220
diag.deprecatedImplementsFunction,
Collapse file

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

Copy file name to clipboardExpand all lines: pkg/analyzer/lib/src/error/best_practices_verifier.dart
+20-3Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
676676
@override
677677
void visitMethodDeclaration(MethodDeclaration node) {
678678
bool wasInDoNotStoreMember = _inDoNotStoreMember;
679+
var nameToken = node.name;
679680
var element = node.declaredFragment!.element;
680681
var enclosingElement = element.enclosingElement;
681682

@@ -706,7 +707,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
706707
_checkStrictInferenceReturnType(
707708
node.returnType,
708709
node,
709-
node.name.lexeme,
710+
nameToken.lexeme,
710711
);
711712
}
712713
if (!elementIsOverride) {
@@ -723,15 +724,31 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
723724
// always named, so we can safely assume
724725
// `overriddenElement.enclosingElement3.name` is non-`null`.
725726
_diagnosticReporter.atToken(
726-
node.name,
727+
nameToken,
727728
diag.invalidOverrideOfNonVirtualMember,
728729
arguments: [
729-
node.name.lexeme,
730+
nameToken.lexeme,
730731
overriddenElement.enclosingElement!.displayName,
731732
],
732733
);
733734
}
734735

736+
bool isAmbiguousFactoryMethod() {
737+
if (nameToken.lexeme != Keyword.FACTORY.lexeme) return false;
738+
var firstToken = node.firstTokenAfterCommentAndMetadata;
739+
if (firstToken == nameToken) return true;
740+
if (firstToken.lexeme == Keyword.EXTERNAL.lexeme ||
741+
firstToken.lexeme == Keyword.AUGMENT.lexeme) {
742+
return firstToken.next == nameToken;
743+
}
744+
return false;
745+
}
746+
747+
if (!_currentLibrary.featureSet.isEnabled(Feature.primary_constructors) &&
748+
isAmbiguousFactoryMethod()) {
749+
_diagnosticReporter.atToken(nameToken, diag.deprecatedFactoryMethod);
750+
}
751+
735752
super.visitMethodDeclaration(node);
736753
} finally {
737754
for (var v in _elementUsageFrontierDetectors) {
Collapse file

‎pkg/analyzer/messages.yaml‎

Copy file name to clipboardExpand all lines: pkg/analyzer/messages.yaml
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25329,6 +25329,12 @@ WarningCode:
2532925329
```dart
2533025330
class F {}
2533125331
```
25332+
DEPRECATED_FACTORY_METHOD:
25333+
type: staticWarning
25334+
parameters: none
25335+
problemMessage: Methods named 'factory' will become constructors when the primary_constructors feature is enabled.
25336+
correctionMessage: Try adding a return type or modifier before the method's name, or change the name of the method.
25337+
hasPublishedDocs: false
2533225338
DEPRECATED_IMPLEMENT:
2533325339
type: staticWarning
2533425340
parameters:
Collapse file

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

Copy file name to clipboard
+138Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag;
6+
import 'package:test_reflective_loader/test_reflective_loader.dart';
7+
8+
import '../dart/resolution/context_collection_resolution.dart';
9+
10+
main() {
11+
defineReflectiveSuite(() {
12+
defineReflectiveTests(DeprecatedFactoryMethodTest);
13+
});
14+
}
15+
16+
@reflectiveTest
17+
class DeprecatedFactoryMethodTest extends PubPackageResolutionTest {
18+
test_noTypeOrModifier_after() async {
19+
await assertNoErrorsInCode('''
20+
class C {
21+
factory() => throw 0;
22+
}
23+
''');
24+
}
25+
26+
test_noTypeOrModifier_before() async {
27+
await assertErrorsInCode(
28+
'''
29+
// @dart=2.12
30+
class C {
31+
factory() => throw 0;
32+
}
33+
''',
34+
[error(diag.deprecatedFactoryMethod, 26, 7)],
35+
);
36+
}
37+
38+
test_withAnnotation_after() async {
39+
await assertNoErrorsInCode('''
40+
class C {
41+
@deprecated
42+
factory() => throw 0;
43+
}
44+
''');
45+
}
46+
47+
test_withAnnotation_before() async {
48+
await assertErrorsInCode(
49+
'''
50+
// @dart=2.12
51+
class C {
52+
@deprecated
53+
factory() => throw 0;
54+
}
55+
''',
56+
[error(diag.deprecatedFactoryMethod, 40, 7)],
57+
);
58+
}
59+
60+
test_withModifier_augment_after() async {
61+
await assertNoErrorsInCode('''
62+
class C {
63+
augment factory() => throw 0;
64+
}
65+
''');
66+
}
67+
68+
test_withModifier_augment_before() async {
69+
await assertErrorsInCode(
70+
'''
71+
// @dart=2.12
72+
class C {
73+
augment factory() => throw 0;
74+
}
75+
''',
76+
// TODO(brianwilkerson): The `undefinedClass` diagnostic should not be
77+
// produced here.
78+
[
79+
error(diag.undefinedClass, 26, 7),
80+
error(diag.deprecatedFactoryMethod, 34, 7),
81+
],
82+
);
83+
}
84+
85+
test_withModifier_external_after() async {
86+
await assertNoErrorsInCode('''
87+
class C {
88+
external factory();
89+
}
90+
''');
91+
}
92+
93+
test_withModifier_external_before() async {
94+
await assertErrorsInCode(
95+
'''
96+
// @dart=2.12
97+
class C {
98+
external factory();
99+
}
100+
''',
101+
[error(diag.deprecatedFactoryMethod, 35, 7)],
102+
);
103+
}
104+
105+
test_withModifier_static_after() async {
106+
await assertNoErrorsInCode('''
107+
class C {
108+
static factory() {}
109+
}
110+
''');
111+
}
112+
113+
test_withModifier_static_before() async {
114+
await assertNoErrorsInCode('''
115+
// @dart=2.12
116+
class C {
117+
static factory() {}
118+
}
119+
''');
120+
}
121+
122+
test_withType_after() async {
123+
await assertNoErrorsInCode('''
124+
class C {
125+
int factory() => 0;
126+
}
127+
''');
128+
}
129+
130+
test_withType_before() async {
131+
await assertNoErrorsInCode('''
132+
// @dart=2.12
133+
class C {
134+
int factory() => 0;
135+
}
136+
''');
137+
}
138+
}
Collapse file

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

Copy file name to clipboardExpand all lines: pkg/analyzer/test/src/diagnostics/test_all.dart
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ import 'deprecated_colon_for_default_value_test.dart'
183183
import 'deprecated_export_use_test.dart' as deprecated_export_use;
184184
import 'deprecated_extend_test.dart' as deprecated_extend;
185185
import 'deprecated_extends_function_test.dart' as deprecated_extends_function;
186+
import 'deprecated_factory_method_test.dart' as deprecated_factory_method;
186187
import 'deprecated_implement_test.dart' as deprecated_implement;
187188
import 'deprecated_implements_function_test.dart'
188189
as deprecated_implements_function;
@@ -1079,6 +1080,7 @@ main() {
10791080
deprecated_export_use.main();
10801081
deprecated_extend.main();
10811082
deprecated_extends_function.main();
1083+
deprecated_factory_method.main();
10821084
deprecated_implement.main();
10831085
deprecated_implements_function.main();
10841086
deprecated_instantiate.main();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.