Skip to content

Commit fae7752

Browse files
FMorschelCommit Queue
authored andcommitted
[DAS] Adds tests for fix priorities and new priority for Create class
The related issue asked for a new priority for the `Create class` fix that would be lower if the undefined name was lowercase, giving the `Create method` and `Create function` fixes a higher relative priority. This change also adds a new abstract class to test the relative priority between fix kinds. It is also used to test agains the merge combinators fixes. Fixes: #60523 Change-Id: I938f52a577ecf1b6bb8dd66c94fd45395a011ffa Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422321 Auto-Submit: Felipe Morschel <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent f0ad03c commit fae7752

File tree

8 files changed

+450
-149
lines changed

8 files changed

+450
-149
lines changed

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

Lines changed: 114 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,13 @@ import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dar
1111
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
1212
import 'package:analyzer_plugin/utilities/range_factory.dart';
1313

14-
class CreateClass extends ResolvedCorrectionProducer {
15-
String className = '';
14+
class CreateClass extends MultiCorrectionProducer {
15+
static final _lowerCaseRegex = RegExp(r'([_\$]||[_\$]+[0-9])*[a-z]');
1616

1717
CreateClass({required super.context});
1818

1919
@override
20-
CorrectionApplicability get applicability =>
21-
// TODO(applicability): comment on why.
22-
CorrectionApplicability.singleLocation;
23-
24-
@override
25-
List<String> get fixArguments => [className];
26-
27-
@override
28-
FixKind get fixKind => DartFixKind.CREATE_CLASS;
29-
30-
@override
31-
Future<void> compute(ChangeBuilder builder) async {
20+
Future<List<ResolvedCorrectionProducer>> get producers async {
3221
var targetNode = node;
3322
Element? prefixElement;
3423
ArgumentList? arguments;
@@ -41,7 +30,7 @@ class CreateClass extends ResolvedCorrectionProducer {
4130
if (name.element != null || arguments == null) {
4231
// TODO(brianwilkerson): Consider supporting creating a class when the
4332
// arguments are missing by also adding an empty argument list.
44-
return;
33+
return const [];
4534
}
4635
targetNode = name;
4736
requiresConstConstructor = true;
@@ -51,40 +40,132 @@ class CreateClass extends ResolvedCorrectionProducer {
5140
if (importPrefix != null) {
5241
prefixElement = importPrefix.element2;
5342
if (prefixElement == null) {
54-
return;
43+
return const [];
5544
}
5645
}
5746
className = targetNode.name.lexeme;
5847
requiresConstConstructor |= _requiresConstConstructor(targetNode);
5948
} else if (targetNode case SimpleIdentifier(
6049
:var parent,
6150
) when parent is! PropertyAccess && parent is! PrefixedIdentifier) {
62-
className = targetNode.nameOfType;
51+
className = targetNode.nameOfType ?? targetNode.name;
6352
requiresConstConstructor |= _requiresConstConstructor(targetNode);
6453
} else if (targetNode is PrefixedIdentifier) {
6554
prefixElement = targetNode.prefix.element;
6655
if (prefixElement == null) {
67-
return;
56+
return const [];
6857
}
69-
className = targetNode.identifier.nameOfType;
58+
className =
59+
targetNode.identifier.nameOfType ?? targetNode.identifier.name;
7060
} else {
71-
return;
61+
return const [];
7262
}
7363

74-
if (className == null) {
75-
return;
64+
if (className.isEmpty) {
65+
return const [];
7666
}
77-
this.className = className;
67+
// Lowercase class names are valid but not idiomatic so lower the priority.
68+
if (className.startsWith(_lowerCaseRegex)) {
69+
return [
70+
_CreateClass.lowercase(
71+
context: context,
72+
targetNode: targetNode,
73+
prefixElement: prefixElement,
74+
className: className,
75+
requiresConstConstructor: requiresConstConstructor,
76+
arguments: arguments,
77+
),
78+
];
79+
} else {
80+
return [
81+
_CreateClass.uppercase(
82+
context: context,
83+
targetNode: targetNode,
84+
prefixElement: prefixElement,
85+
className: className,
86+
requiresConstConstructor: requiresConstConstructor,
87+
arguments: arguments,
88+
),
89+
];
90+
}
91+
}
7892

93+
static bool _requiresConstConstructor(AstNode node) {
94+
var parent = node.parent;
95+
// TODO(scheglov): remove after NamedType refactoring.
96+
if (node is SimpleIdentifier && parent is NamedType) {
97+
return _requiresConstConstructor(parent);
98+
}
99+
if (node is SimpleIdentifier && parent is MethodInvocation) {
100+
return parent.inConstantContext;
101+
}
102+
if (node is NamedType && parent is ConstructorName) {
103+
return _requiresConstConstructor(parent);
104+
}
105+
if (node is ConstructorName && parent is InstanceCreationExpression) {
106+
return parent.isConst;
107+
}
108+
return false;
109+
}
110+
}
111+
112+
class _CreateClass extends ResolvedCorrectionProducer {
113+
final ArgumentList? _arguments;
114+
final bool _requiresConstConstructor;
115+
final AstNode _targetNode;
116+
final Element? _prefixElement;
117+
final String _className;
118+
119+
@override
120+
final FixKind fixKind;
121+
122+
_CreateClass.lowercase({
123+
required super.context,
124+
required ArgumentList? arguments,
125+
required bool requiresConstConstructor,
126+
required AstNode targetNode,
127+
required Element? prefixElement,
128+
required String className,
129+
}) : _className = className,
130+
_prefixElement = prefixElement,
131+
_targetNode = targetNode,
132+
_requiresConstConstructor = requiresConstConstructor,
133+
_arguments = arguments,
134+
fixKind = DartFixKind.CREATE_CLASS_LOWERCASE;
135+
136+
_CreateClass.uppercase({
137+
required super.context,
138+
required ArgumentList? arguments,
139+
required bool requiresConstConstructor,
140+
required AstNode targetNode,
141+
required Element? prefixElement,
142+
required String className,
143+
}) : _className = className,
144+
_prefixElement = prefixElement,
145+
_targetNode = targetNode,
146+
_requiresConstConstructor = requiresConstConstructor,
147+
_arguments = arguments,
148+
fixKind = DartFixKind.CREATE_CLASS_UPPERCASE;
149+
150+
@override
151+
CorrectionApplicability get applicability =>
152+
// TODO(applicability): comment on why.
153+
CorrectionApplicability.singleLocation;
154+
155+
@override
156+
List<String> get fixArguments => [_className];
157+
158+
@override
159+
Future<void> compute(ChangeBuilder builder) async {
79160
// prepare environment
80161
LibraryFragment targetUnit;
81162
var prefix = '';
82163
var suffix = '';
83164
var offset = -1;
84165
String? filePath;
85-
if (prefixElement == null) {
166+
if (_prefixElement == null) {
86167
targetUnit = unit.declaredFragment!;
87-
var enclosingMember = targetNode.thisOrAncestorMatching(
168+
var enclosingMember = _targetNode.thisOrAncestorMatching(
88169
(node) =>
89170
node is CompilationUnitMember && node.parent is CompilationUnit,
90171
);
@@ -96,8 +177,8 @@ class CreateClass extends ResolvedCorrectionProducer {
96177
prefix = '$eol$eol';
97178
} else {
98179
for (var import in libraryElement2.firstFragment.libraryImports2) {
99-
if (prefixElement is PrefixElement &&
100-
import.prefix2?.element == prefixElement) {
180+
if (_prefixElement is PrefixElement &&
181+
import.prefix2?.element == _prefixElement) {
101182
var library = import.importedLibrary2;
102183
if (library != null) {
103184
targetUnit = library.firstFragment;
@@ -120,11 +201,11 @@ class CreateClass extends ResolvedCorrectionProducer {
120201
return;
121202
}
122203

123-
var className2 = className;
204+
var className2 = _className;
124205
await builder.addDartFileEdit(filePath, (builder) {
125206
builder.addInsertion(offset, (builder) {
126207
builder.write(prefix);
127-
if (arguments == null && !requiresConstConstructor) {
208+
if (_arguments == null && !_requiresConstConstructor) {
128209
builder.writeClassDeclaration(className2, nameGroupName: 'NAME');
129210
} else {
130211
builder.writeClassDeclaration(
@@ -134,39 +215,21 @@ class CreateClass extends ResolvedCorrectionProducer {
134215
builder.write(' ');
135216
builder.writeConstructorDeclaration(
136217
className2,
137-
argumentList: arguments,
218+
argumentList: _arguments,
138219
classNameGroupName: 'NAME',
139-
isConst: requiresConstConstructor,
220+
isConst: _requiresConstConstructor,
140221
);
141222
builder.writeln();
142223
},
143224
);
144225
}
145226
builder.write(suffix);
146227
});
147-
if (prefixElement == null) {
148-
builder.addLinkedPosition(range.node(targetNode), 'NAME');
228+
if (_prefixElement == null) {
229+
builder.addLinkedPosition(range.node(_targetNode), 'NAME');
149230
}
150231
});
151232
}
152-
153-
static bool _requiresConstConstructor(AstNode node) {
154-
var parent = node.parent;
155-
// TODO(scheglov): remove after NamedType refactoring.
156-
if (node is SimpleIdentifier && parent is NamedType) {
157-
return _requiresConstConstructor(parent);
158-
}
159-
if (node is SimpleIdentifier && parent is MethodInvocation) {
160-
return parent.inConstantContext;
161-
}
162-
if (node is NamedType && parent is ConstructorName) {
163-
return _requiresConstConstructor(parent);
164-
}
165-
if (node is ConstructorName && parent is InstanceCreationExpression) {
166-
return parent.isConst;
167-
}
168-
return false;
169-
}
170233
}
171234

172235
extension on AstNode {

0 commit comments

Comments
 (0)