Skip to content

Commit 85a7836

Browse files
munificentCommit Queue
authored andcommitted
[private-named-parameters] Report error if the public name collides with another parameter.
Bug: #61643 Change-Id: I219a5310a67459735c61ee9187abd24f4c8cc209 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464605 Commit-Queue: Bob Nystrom <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Auto-Submit: Bob Nystrom <[email protected]>
1 parent 1123046 commit 85a7836

File tree

7 files changed

+280
-5
lines changed

7 files changed

+280
-5
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,6 +1392,8 @@ PREFIX_SHADOWED_BY_LOCAL_DECLARATION:
13921392
status: noFix
13931393
PRIVATE_COLLISION_IN_MIXIN_APPLICATION:
13941394
status: noFix
1395+
PRIVATE_NAMED_PARAMETER_DUPLICATE_PUBLIC_NAME:
1396+
status: noFix
13951397
PRIVATE_NAMED_PARAMETER_WITHOUT_PUBLIC_NAME:
13961398
status: noFix
13971399
PRIVATE_SETTER:

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13521,6 +13521,25 @@ privateNamedNonFieldParameter = DiagnosticWithoutArgumentsImpl(
1352113521
expectedTypes: [],
1352213522
);
1352313523

13524+
/// Parameters:
13525+
/// String publicName: the corresponding public name of private named
13526+
/// parameter
13527+
const DiagnosticWithArguments<
13528+
LocatableDiagnostic Function({required String publicName})
13529+
>
13530+
privateNamedParameterDuplicatePublicName = DiagnosticWithArguments(
13531+
name: 'PRIVATE_NAMED_PARAMETER_DUPLICATE_PUBLIC_NAME',
13532+
problemMessage:
13533+
"The corresponding public name '{0}' is already the name of another "
13534+
"parameter.",
13535+
correctionMessage: "Try renaming one of the parameters.",
13536+
type: DiagnosticType.COMPILE_TIME_ERROR,
13537+
uniqueName:
13538+
'CompileTimeErrorCode.PRIVATE_NAMED_PARAMETER_DUPLICATE_PUBLIC_NAME',
13539+
withArguments: _withArgumentsPrivateNamedParameterDuplicatePublicName,
13540+
expectedTypes: [ExpectedType.string],
13541+
);
13542+
1352413543
/// No parameters.
1352513544
const DiagnosticWithoutArguments
1352613545
privateNamedParameterWithoutPublicName = DiagnosticWithoutArgumentsImpl(
@@ -20278,6 +20297,15 @@ LocatableDiagnostic _withArgumentsPrivateCollisionInMixinApplication({
2027820297
]);
2027920298
}
2028020299

20300+
LocatableDiagnostic _withArgumentsPrivateNamedParameterDuplicatePublicName({
20301+
required String publicName,
20302+
}) {
20303+
return LocatableDiagnosticImpl(
20304+
diag.privateNamedParameterDuplicatePublicName,
20305+
[publicName],
20306+
);
20307+
}
20308+
2028120309
LocatableDiagnostic _withArgumentsPrivateSetter({required String p0}) {
2028220310
return LocatableDiagnosticImpl(diag.privateSetter, [p0]);
2028320311
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,7 @@ const List<DiagnosticCode> diagnosticCodeValues = [
874874
diag.prefixShadowedByLocalDeclaration,
875875
diag.privateCollisionInMixinApplication,
876876
diag.privateNamedNonFieldParameter,
877+
diag.privateNamedParameterDuplicatePublicName,
877878
diag.privateNamedParameterWithoutPublicName,
878879
diag.privateOptionalParameter,
879880
diag.privateSetter,

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,20 @@ class DuplicateDefinitionVerifier {
7676

7777
scope.add(identifier, node: parameter);
7878
}
79+
80+
// For private named parameters, also look for collisions with their public
81+
// name and other parameters.
82+
for (var parameter in node.parameters) {
83+
if (parameter.declaredFragment
84+
case FieldFormalParameterFragment fragment) {
85+
if (fragment.privateName != null) {
86+
scope.checkPublicName(
87+
privateName: parameter.name!,
88+
publicName: fragment.name!,
89+
);
90+
}
91+
}
92+
}
7993
}
8094

8195
/// Check that all of the variables have unique names.
@@ -958,13 +972,10 @@ class _DuplicateIdentifierScope<T extends AstNode> {
958972

959973
if (_scope[identifier.lexeme] case (var previousNode, var previousToken)) {
960974
_verifier._reportedTokens.add(identifier);
961-
962-
var code = getDiagnostic(previousNode, node);
963-
964975
_verifier._diagnosticReporter.reportError(
965976
_verifier._diagnosticFactory.duplicateDefinitionForNodes(
966977
_verifier._diagnosticReporter.source,
967-
code,
978+
getDiagnostic(previousNode, node),
968979
identifier,
969980
previousToken,
970981
[identifier.lexeme],
@@ -992,6 +1003,26 @@ class _FormalParameterDuplicateIdentifierScope
9921003
extends _DuplicateIdentifierScope<FormalParameter> {
9931004
_FormalParameterDuplicateIdentifierScope(super.verifier);
9941005

1006+
/// Given a private named parameter with [privateName] and corresponding
1007+
/// [publicName], checks that the public name doesn't collide with any other
1008+
/// parameter.
1009+
void checkPublicName({
1010+
required Token privateName,
1011+
required String publicName,
1012+
}) {
1013+
if (_scope[publicName] case (var _, var previousToken)) {
1014+
_verifier._diagnosticReporter.reportError(
1015+
_verifier._diagnosticFactory.duplicateDefinitionForNodes(
1016+
_verifier._diagnosticReporter.source,
1017+
diag.privateNamedParameterDuplicatePublicName,
1018+
privateName,
1019+
previousToken,
1020+
[publicName],
1021+
),
1022+
);
1023+
}
1024+
}
1025+
9951026
@override
9961027
DiagnosticCode getDiagnostic(
9971028
FormalParameter previous,

pkg/analyzer/messages.yaml

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14362,6 +14362,37 @@ CompileTimeErrorCode:
1436214362

1436314363
If you need both of the mixins, then rename the conflicting member in one
1436414364
of the two mixins.
14365+
PRIVATE_NAMED_PARAMETER_DUPLICATE_PUBLIC_NAME:
14366+
type: compileTimeError
14367+
parameters:
14368+
String publicName: the corresponding public name of private named parameter
14369+
problemMessage: "The corresponding public name '#publicName' is already the name of another parameter."
14370+
correctionMessage: "Try renaming one of the parameters."
14371+
hasPublishedDocs: false
14372+
documentation: |-
14373+
#### Description
14374+
14375+
The analyzer produces this diagnostic when a private named parameter's
14376+
corresponding public name with the leading `_` removed is the same as
14377+
another parameter in the parameter list.
14378+
14379+
#### Example
14380+
14381+
The following code produces this diagnostic because the private named
14382+
parameter `_x`'s public name is `x` and there is another parameter with
14383+
that name:
14384+
14385+
```dart
14386+
class C {
14387+
int? _x;
14388+
C({int? x, this.[!_x!]});
14389+
int? get x => _x;
14390+
}
14391+
```
14392+
14393+
#### Common fixes
14394+
14395+
Rename one of the two parameters.
1436514396
PRIVATE_NAMED_PARAMETER_WITHOUT_PUBLIC_NAME:
1436614397
type: compileTimeError
1436714398
parameters: none
@@ -22601,7 +22632,6 @@ ParserErrorCode:
2260122632
C({int x = 0});
2260222633
}
2260322634
```
22604-
2260522635
REPRESENTATION_FIELD_MODIFIER:
2260622636
type: syntacticError
2260722637
parameters: none
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
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(DuplicatePrivateNamedParameterTest);
13+
});
14+
}
15+
16+
@reflectiveTest
17+
class DuplicatePrivateNamedParameterTest extends PubPackageResolutionTest {
18+
test_initializingFormal_initializingFormal() async {
19+
await assertErrorsInCode(
20+
r'''
21+
class C {
22+
final String? _foo;
23+
C({required this._foo, required this._foo}) {}
24+
}
25+
''',
26+
[
27+
error(diag.unusedField, 26, 4),
28+
error(
29+
diag.duplicateFieldFormalParameter,
30+
71,
31+
4,
32+
contextMessages: [message(testFile, 51, 4)],
33+
),
34+
],
35+
);
36+
}
37+
38+
test_initializingFormal_privateNamed() async {
39+
await assertErrorsInCode(
40+
r'''
41+
class C {
42+
final String? _foo;
43+
C({required this._foo, String? _foo}) {}
44+
}
45+
''',
46+
[
47+
error(diag.unusedField, 26, 4),
48+
error(diag.privateNamedNonFieldParameter, 65, 4),
49+
error(
50+
diag.duplicateDefinition,
51+
65,
52+
4,
53+
contextMessages: [message(testFile, 51, 4)],
54+
),
55+
],
56+
);
57+
}
58+
59+
test_initializingFormal_publicNamed() async {
60+
await assertErrorsInCode(
61+
r'''
62+
class C {
63+
final String? _foo;
64+
C({required this._foo, String? foo}) {}
65+
}
66+
''',
67+
[
68+
error(diag.unusedField, 26, 4),
69+
error(
70+
diag.privateNamedParameterDuplicatePublicName,
71+
51,
72+
4,
73+
contextMessages: [message(testFile, 65, 3)],
74+
),
75+
],
76+
);
77+
}
78+
79+
test_privateNamed_initializingFormal() async {
80+
await assertErrorsInCode(
81+
r'''
82+
class C {
83+
final String? _foo;
84+
C({String? _foo, required this._foo}) {}
85+
}
86+
''',
87+
[
88+
error(diag.unusedField, 26, 4),
89+
error(diag.privateNamedNonFieldParameter, 45, 4),
90+
error(
91+
diag.duplicateDefinition,
92+
65,
93+
4,
94+
contextMessages: [message(testFile, 45, 4)],
95+
),
96+
],
97+
);
98+
}
99+
100+
test_privatePositional_initializingFormal() async {
101+
await assertErrorsInCode(
102+
r'''
103+
class C {
104+
final String? _foo;
105+
C(String _foo, {required this._foo}) {}
106+
}
107+
''',
108+
[
109+
error(diag.unusedField, 26, 4),
110+
error(
111+
diag.duplicateDefinition,
112+
64,
113+
4,
114+
contextMessages: [message(testFile, 43, 4)],
115+
),
116+
],
117+
);
118+
}
119+
120+
test_publicInitializingFormal_privateInitializingFormal() async {
121+
await assertErrorsInCode(
122+
r'''
123+
class C {
124+
final String? foo;
125+
final String? _foo;
126+
C({required this.foo, required this._foo}) {}
127+
}
128+
''',
129+
[
130+
error(diag.unusedField, 47, 4),
131+
error(
132+
diag.privateNamedParameterDuplicatePublicName,
133+
91,
134+
4,
135+
contextMessages: [message(testFile, 72, 3)],
136+
),
137+
],
138+
);
139+
}
140+
141+
test_publicNamed_initializingFormal() async {
142+
await assertErrorsInCode(
143+
r'''
144+
class C {
145+
final String? _foo;
146+
C({String? foo, required this._foo}) {}
147+
}
148+
''',
149+
[
150+
error(diag.unusedField, 26, 4),
151+
error(
152+
diag.privateNamedParameterDuplicatePublicName,
153+
64,
154+
4,
155+
contextMessages: [message(testFile, 45, 3)],
156+
),
157+
],
158+
);
159+
}
160+
161+
test_publicPositional_initializingFormal() async {
162+
await assertErrorsInCode(
163+
r'''
164+
class C {
165+
final String? _foo;
166+
C(String? foo, {required this._foo}) {}
167+
}
168+
''',
169+
[
170+
error(diag.unusedField, 26, 4),
171+
error(
172+
diag.privateNamedParameterDuplicatePublicName,
173+
64,
174+
4,
175+
contextMessages: [message(testFile, 44, 3)],
176+
),
177+
],
178+
);
179+
}
180+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ import 'duplicate_part_test.dart' as duplicate_part;
227227
import 'duplicate_pattern_assignment_variable_test.dart'
228228
as duplicate_pattern_assignment_variable;
229229
import 'duplicate_pattern_field_test.dart' as duplicate_pattern_field;
230+
import 'duplicate_private_named_parameter_test.dart'
231+
as duplicate_private_named_parameter;
230232
import 'duplicate_rest_element_in_pattern_test.dart'
231233
as duplicate_rest_element_in_pattern;
232234
import 'duplicate_shown_name_test.dart' as duplicate_shown_name;
@@ -1107,6 +1109,7 @@ main() {
11071109
duplicate_part.main();
11081110
duplicate_pattern_assignment_variable.main();
11091111
duplicate_pattern_field.main();
1112+
duplicate_private_named_parameter.main();
11101113
duplicate_rest_element_in_pattern.main();
11111114
duplicate_shown_name.main();
11121115
duplicate_variable_pattern.main();

0 commit comments

Comments
 (0)