Skip to content

Commit 91aea3f

Browse files
fshcheglovdart-scoped@luci-project-accounts.iam.gserviceaccount.com
authored andcommitted
Check for const and factory mismatch between constructors in Class, Enum, and Extension Type.
Change-Id: Ib34306676b8555a49bb15896c60f1065e69c3035 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/517622 Reviewed-by: Paul Berry <paulberry@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
1 parent 38078e7 commit 91aea3f

4 files changed

Lines changed: 180 additions & 49 deletions

File tree

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

Lines changed: 76 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
671671
var element = fragment.element;
672672

673673
_checkAugmentationWithoutDeclaration(node.augmentKeyword, fragment);
674+
_checkForConstructorAugmentationModifierMismatch(node, fragment);
674675
_checkForAugmentationFormalParameters(
675676
executableFragment: fragment,
676677
formalParameterList: node.parameters,
@@ -3128,6 +3129,32 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
31283129
}
31293130
}
31303131

3132+
void _checkForAugmentationModifierMismatch({
3133+
required Token augmentKeyword,
3134+
required bool inAugmentation,
3135+
required bool inIntroductory,
3136+
required Token? modifierToken,
3137+
required String modifierName,
3138+
}) {
3139+
if (inAugmentation != inIntroductory) {
3140+
if (inAugmentation) {
3141+
if (modifierToken != null) {
3142+
diagnosticReporter.report(
3143+
diag.augmentationModifierExtra
3144+
.withArguments(modifier: modifierName)
3145+
.at(modifierToken),
3146+
);
3147+
}
3148+
} else {
3149+
diagnosticReporter.report(
3150+
diag.augmentationModifierMissing
3151+
.withArguments(modifier: modifierName)
3152+
.at(augmentKeyword),
3153+
);
3154+
}
3155+
}
3156+
}
3157+
31313158
void _checkForAugmentationReturnTypeMismatch({
31323159
required ExecutableFragmentImpl fragment,
31333160
required TypeAnnotation? returnTypeNode,
@@ -3372,62 +3399,43 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
33723399
return;
33733400
}
33743401

3375-
void checkModifier({
3376-
required bool inAugmentation,
3377-
required bool inIntroductory,
3378-
required Token? modifierToken,
3379-
required String modifierName,
3380-
}) {
3381-
if (inAugmentation != inIntroductory) {
3382-
if (inAugmentation) {
3383-
if (modifierToken != null) {
3384-
diagnosticReporter.report(
3385-
diag.augmentationModifierExtra
3386-
.withArguments(modifier: modifierName)
3387-
.at(modifierToken),
3388-
);
3389-
}
3390-
} else {
3391-
diagnosticReporter.report(
3392-
diag.augmentationModifierMissing
3393-
.withArguments(modifier: modifierName)
3394-
.at(augmentKeyword),
3395-
);
3396-
}
3397-
}
3398-
}
3399-
3400-
checkModifier(
3402+
_checkForAugmentationModifierMismatch(
3403+
augmentKeyword: augmentKeyword,
34013404
inAugmentation: declaredFragment.isAbstract,
34023405
inIntroductory: firstFragment.isAbstract,
34033406
modifierToken: node.abstractKeyword,
34043407
modifierName: 'abstract',
34053408
);
3406-
checkModifier(
3409+
_checkForAugmentationModifierMismatch(
3410+
augmentKeyword: augmentKeyword,
34073411
inAugmentation: declaredFragment.isBase,
34083412
inIntroductory: firstFragment.isBase,
34093413
modifierToken: node.baseKeyword,
34103414
modifierName: 'base',
34113415
);
3412-
checkModifier(
3416+
_checkForAugmentationModifierMismatch(
3417+
augmentKeyword: augmentKeyword,
34133418
inAugmentation: declaredFragment.isFinal,
34143419
inIntroductory: firstFragment.isFinal,
34153420
modifierToken: node.finalKeyword,
34163421
modifierName: 'final',
34173422
);
3418-
checkModifier(
3423+
_checkForAugmentationModifierMismatch(
3424+
augmentKeyword: augmentKeyword,
34193425
inAugmentation: declaredFragment.isInterface,
34203426
inIntroductory: firstFragment.isInterface,
34213427
modifierToken: node.interfaceKeyword,
34223428
modifierName: 'interface',
34233429
);
3424-
checkModifier(
3430+
_checkForAugmentationModifierMismatch(
3431+
augmentKeyword: augmentKeyword,
34253432
inAugmentation: declaredFragment.isSealed,
34263433
inIntroductory: firstFragment.isSealed,
34273434
modifierToken: node.sealedKeyword,
34283435
modifierName: 'sealed',
34293436
);
3430-
checkModifier(
3437+
_checkForAugmentationModifierMismatch(
3438+
augmentKeyword: augmentKeyword,
34313439
inAugmentation: declaredFragment.isMixinClass,
34323440
inIntroductory: firstFragment.isMixinClass,
34333441
modifierToken: node.mixinKeyword,
@@ -4200,6 +4208,36 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
42004208
}
42014209
}
42024210

4211+
void _checkForConstructorAugmentationModifierMismatch(
4212+
ConstructorDeclarationImpl node,
4213+
ConstructorFragmentImpl declaredFragment,
4214+
) {
4215+
var augmentKeyword = node.augmentKeyword;
4216+
if (augmentKeyword == null) {
4217+
return;
4218+
}
4219+
4220+
var firstFragment = declaredFragment.element.firstFragment;
4221+
if (identical(declaredFragment, firstFragment)) {
4222+
return;
4223+
}
4224+
4225+
_checkForAugmentationModifierMismatch(
4226+
augmentKeyword: augmentKeyword,
4227+
inAugmentation: declaredFragment.isConst,
4228+
inIntroductory: firstFragment.isConst,
4229+
modifierToken: node.constKeyword,
4230+
modifierName: 'const',
4231+
);
4232+
_checkForAugmentationModifierMismatch(
4233+
augmentKeyword: augmentKeyword,
4234+
inAugmentation: declaredFragment.isFactory,
4235+
inIntroductory: firstFragment.isFactory,
4236+
modifierToken: node.factoryKeyword,
4237+
modifierName: 'factory',
4238+
);
4239+
}
4240+
42034241
bool _checkForConstVariableAugmentation({
42044242
required Token errorToken,
42054243
required PropertyInducingFragmentImpl fragment,
@@ -5912,23 +5950,13 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
59125950
return;
59135951
}
59145952

5915-
if (declaredFragment.isBase != firstFragment.isBase) {
5916-
if (declaredFragment.isBase) {
5917-
if (node.baseKeyword case var baseKeyword?) {
5918-
diagnosticReporter.report(
5919-
diag.augmentationModifierExtra
5920-
.withArguments(modifier: 'base')
5921-
.at(baseKeyword),
5922-
);
5923-
}
5924-
} else {
5925-
diagnosticReporter.report(
5926-
diag.augmentationModifierMissing
5927-
.withArguments(modifier: 'base')
5928-
.at(augmentKeyword),
5929-
);
5930-
}
5931-
}
5953+
_checkForAugmentationModifierMismatch(
5954+
augmentKeyword: augmentKeyword,
5955+
inAugmentation: declaredFragment.isBase,
5956+
inIntroductory: firstFragment.isBase,
5957+
modifierToken: node.baseKeyword,
5958+
modifierName: 'base',
5959+
);
59325960
}
59335961

59345962
/// Verify that mixin classes must have 'Object' as their superclass and that

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,28 @@ augment abstract base class A {}
4141
''');
4242
}
4343

44+
test_class_constructor_secondary_nothing_const() async {
45+
await resolveTestCodeWithDiagnostics(r'''
46+
class A {
47+
A();
48+
augment const A();
49+
// ^^^^^
50+
// [diag.augmentationModifierExtra] The augmentation has the 'const' modifier that the declaration doesn't have.
51+
}
52+
''');
53+
}
54+
55+
test_class_constructor_secondary_nothing_factory() async {
56+
await resolveTestCodeWithDiagnostics(r'''
57+
class A {
58+
A();
59+
augment factory A();
60+
// ^^^^^^^
61+
// [diag.augmentationModifierExtra] The augmentation has the 'factory' modifier that the declaration doesn't have.
62+
}
63+
''');
64+
}
65+
4466
test_class_nothing_abstract() async {
4567
await resolveTestCodeWithDiagnostics(r'''
4668
class A {}
@@ -116,6 +138,35 @@ augment sealed class A {}
116138
''');
117139
}
118140

141+
test_enum_constructor_primary_const_factory() async {
142+
await resolveTestCodeWithDiagnostics(r'''
143+
enum E.named() {
144+
v.named();
145+
}
146+
147+
augment enum E {
148+
;
149+
augment factory E.named();
150+
//^^^^^^^
151+
// [diag.augmentationModifierMissing] The augmentation is missing the 'const' modifier that the declaration has.
152+
// ^^^^^^^
153+
// [diag.augmentationModifierExtra] The augmentation has the 'factory' modifier that the declaration doesn't have.
154+
}
155+
''');
156+
}
157+
158+
test_extensionType_constructor_primary_nothing_factory() async {
159+
await resolveTestCodeWithDiagnostics(r'''
160+
extension type E(int it);
161+
162+
augment extension type E {
163+
augment factory E(int it);
164+
// ^^^^^^^
165+
// [diag.augmentationModifierExtra] The augmentation has the 'factory' modifier that the declaration doesn't have.
166+
}
167+
''');
168+
}
169+
119170
test_mixin_base_base() async {
120171
await resolveTestCodeWithDiagnostics(r'''
121172
base mixin A {}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,29 @@ augment class A {}
7373
''');
7474
}
7575

76+
test_class_constructor_secondary_const_nothing() async {
77+
await resolveTestCodeWithDiagnostics(r'''
78+
class A {
79+
const A();
80+
augment A();
81+
//^^^^^^^
82+
// [diag.augmentationModifierMissing] The augmentation is missing the 'const' modifier that the declaration has.
83+
}
84+
''');
85+
}
86+
87+
test_class_constructor_secondary_factory_nothing() async {
88+
await resolveTestCodeWithDiagnostics(r'''
89+
class A {
90+
A._();
91+
factory A() = A._;
92+
augment A();
93+
//^^^^^^^
94+
// [diag.augmentationModifierMissing] The augmentation is missing the 'factory' modifier that the declaration has.
95+
}
96+
''');
97+
}
98+
7699
test_class_final_nothing() async {
77100
await resolveTestCodeWithDiagnostics(r'''
78101
final class A {}
@@ -105,6 +128,35 @@ augment class A {}
105128
''');
106129
}
107130

131+
test_enum_constructor_secondary_factory_nothing() async {
132+
await resolveTestCodeWithDiagnostics(r'''
133+
enum E {
134+
v;
135+
136+
factory E.named() => v;
137+
}
138+
139+
augment enum E {
140+
;
141+
augment E.named();
142+
//^^^^^^^
143+
// [diag.augmentationModifierMissing] The augmentation is missing the 'factory' modifier that the declaration has.
144+
}
145+
''');
146+
}
147+
148+
test_extensionType_constructor_primary_const_nothing() async {
149+
await resolveTestCodeWithDiagnostics(r'''
150+
extension type const E(int it);
151+
152+
augment extension type E {
153+
augment E(int it);
154+
//^^^^^^^
155+
// [diag.augmentationModifierMissing] The augmentation is missing the 'const' modifier that the declaration has.
156+
}
157+
''');
158+
}
159+
108160
test_mixin_base_base() async {
109161
await resolveTestCodeWithDiagnostics(r'''
110162
base mixin A {}

pkg/linter/test/rules/use_key_in_widget_constructors_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import 'package:flutter/widgets.dart';
2929
part 'test.dart';
3030
3131
class W extends StatelessWidget {
32-
W();
32+
const W();
3333
3434
@override
3535
Widget build(BuildContext context) => Container();

0 commit comments

Comments
 (0)