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

Sign up
Appearance settings

Commit 2a4c954

Browse files
FMorschelCommit Queue
authored and
Commit Queue
committed
[DAS] Fixes Add field formal parameters for private named fields
Fixes: #61381 Change-Id: I02fd027e874325107c6b81663139cf6fd3189a38 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459502 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Auto-Submit: Felipe Morschel <git@fmorschel.dev> Reviewed-by: Samuel Rawlins <srawlins@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com>
1 parent 0a95e6f commit 2a4c954

File tree

3 files changed

+234
-75
lines changed

3 files changed

+234
-75
lines changed

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

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

14-
/// The boolean value indicates whether the field is required.
15-
/// The string value is the field/parameter name.
16-
typedef _FieldRecord = ({bool isRequired, String parameter});
17-
1814
class AddFieldFormalParameters extends ResolvedCorrectionProducer {
15+
static final _charAfterUnderscore = RegExp('[^_]');
16+
17+
static final _startsWithNumber = RegExp('^[0-9]');
18+
1919
final _Style _style;
2020

2121
@override
@@ -27,8 +27,7 @@ class AddFieldFormalParameters extends ResolvedCorrectionProducer {
2727

2828
AddFieldFormalParameters.requiredNamed({required super.context})
2929
: _style = _Style.requiredNamed,
30-
fixKind = DartFixKind.addInitializingFormalNamesParameters;
31-
30+
fixKind = DartFixKind.addInitializingFormalNamedParameters;
3231
@override
3332
CorrectionApplicability get applicability =>
3433
// TODO(applicability): comment on why.
@@ -61,96 +60,149 @@ class AddFieldFormalParameters extends ResolvedCorrectionProducer {
6160
.map((pair) => pair.1ドル)
6261
.toList();
6362

63+
if (fields.isEmpty) {
64+
assert(false, 'How can this trigger with no fields?');
65+
return;
66+
}
67+
6468
// Prepare the last required parameter.
65-
FormalParameter? lastRequiredParameter;
69+
FormalParameter? lastRequiredPositionalParameter;
6670
FormalParameter? firstNamedParameter;
71+
var containsOptionalPositional = false;
6772
for (var parameter in parameters) {
6873
if (parameter.isRequiredPositional) {
69-
lastRequiredParameter = parameter;
70-
} else if (_style == _Style.base) {
71-
break;
74+
lastRequiredPositionalParameter = parameter;
7275
} else if (parameter.isOptionalPositional) {
73-
// If there are optional positional parameters, we can't add required
74-
// named parameters.
75-
return;
76+
if (_style == _Style.requiredNamed) {
77+
// If there are optional positional parameters, we can't add required
78+
// named parameters.
79+
return;
80+
} else {
81+
containsOptionalPositional = true;
82+
}
7683
} else if (parameter.isNamed) {
7784
firstNamedParameter = parameter;
7885
break;
7986
}
8087
}
8188

82-
var fieldsRecords = fields.map(_parameterForField).toList();
83-
var requiredFirst = getCodeStyleOptions(
84-
unitResult.file,
85-
).requiredNamedParametersFirst;
86-
if (requiredFirst) {
87-
fieldsRecords.sort((a, b) {
88-
if (a.isRequired && !b.isRequired) {
89-
return -1;
90-
} else if (!a.isRequired && b.isRequired) {
91-
return 1;
92-
}
93-
return a.parameter.compareTo(b.parameter);
94-
});
95-
}
96-
var requiredParameters = fieldsRecords.where((r) => r.isRequired);
97-
var optionalParameters = fieldsRecords
98-
.where((r) => !r.isRequired)
99-
.map((r) => r.parameter);
100-
var fieldParametersCode = fieldsRecords.map((r) => r.parameter).join(', ');
10189
await builder.addDartFileEdit(file, (builder) {
102-
if (firstNamedParameter != null &&
103-
requiredFirst &&
104-
requiredParameters.isNotEmpty) {
105-
builder.addSimpleInsertion(
106-
firstNamedParameter.offset,
107-
'${requiredParameters.map((r) => r.parameter).join(', ')}, ',
108-
);
109-
if (optionalParameters.isNotEmpty) {
110-
fieldParametersCode = optionalParameters.join(', ');
90+
var insertOffset =
91+
(lastRequiredPositionalParameter ??
92+
constructor.parameters.leftParenthesis)
93+
.end;
94+
var mappedFields = fields.map(
95+
(field) => (field, isRequired: _isFieldRequired(field)),
96+
);
97+
var initializer = <({String publicName, FieldElement field})>[];
98+
var addCurlyBraces =
99+
firstNamedParameter == null && _style == _Style.requiredNamed;
100+
var parametersAtCurly = getCodeStyleOptions(
101+
unitResult.file,
102+
).requiredNamedParametersFirst;
103+
var curlyOpen = lastRequiredPositionalParameter == null ? '{' : ', {';
104+
if (addCurlyBraces && !parametersAtCurly) {
105+
builder.addSimpleInsertion(insertOffset, curlyOpen);
106+
}
107+
if (!addCurlyBraces && _style == _Style.requiredNamed) {
108+
if (parametersAtCurly) {
109+
insertOffset = firstNamedParameter!.offset;
111110
} else {
112-
return; // No optional parameters to add.
111+
insertOffset =
112+
parameters.lastOrNull?.end ??
113+
constructor.parameters.rightParenthesis.offset;
113114
}
114115
}
115-
if (_style == _Style.requiredNamed) {
116-
var lastParameter = parameters.lastOrNull;
117-
if (lastParameter != null) {
118-
var write = ', ';
119-
if (!lastParameter.isNamed) {
120-
write += '{$fieldParametersCode}';
116+
if (parametersAtCurly) {
117+
mappedFields.sorted((r1, r2) {
118+
return r1.isRequired == r2.isRequired ? 0 : (r1.isRequired ? -1 : 1);
119+
});
120+
}
121+
var requiredFirst =
122+
firstNamedParameter != null &&
123+
firstNamedParameter.isRequiredNamed &&
124+
parametersAtCurly;
125+
builder.addInsertion(insertOffset, (builder) {
126+
var addComma = _style == _Style.base
127+
? lastRequiredPositionalParameter != null
128+
: !parametersAtCurly && !addCurlyBraces;
129+
for (var (field, :isRequired) in mappedFields) {
130+
// If we have a required named parameter already, don't add
131+
// non-required parameters yet.
132+
if (requiredFirst && !isRequired) {
133+
continue;
134+
}
135+
if (addComma) {
136+
builder.write(', ');
137+
}
138+
addComma = true;
139+
if (isRequired) {
140+
builder.write('required ');
141+
}
142+
if (field.isPrivate) {
143+
var nameIndex = field.displayName.indexOf(_charAfterUnderscore);
144+
var publicName = field.displayName.substring(nameIndex);
145+
if (_startsWithNumber.hasMatch(publicName)) {
146+
// Like we do for closures suggesting p0, p1, etc.
147+
publicName = 'p$publicName';
148+
}
149+
builder.writeType(field.type);
150+
builder.write(' $publicName');
151+
initializer.add((field: field, publicName: publicName));
121152
} else {
122-
write+= fieldParametersCode;
153+
builder.write('this.${field.name}');
123154
}
124-
builder.addSimpleInsertion(parameters.last.end, write);
125-
} else {
126-
var offset = constructor.parameters.leftParenthesis.end;
127-
builder.addSimpleInsertion(offset, '{$fieldParametersCode}');
128155
}
129-
} else if (lastRequiredParameter != null) {
130-
return builder.addSimpleInsertion(
131-
lastRequiredParameter.end,
132-
', $fieldParametersCode',
133-
);
134-
} else {
135-
var offset = constructor.parameters.leftParenthesis.end;
136-
if (parameters.isNotEmpty) {
137-
fieldParametersCode += ', ';
156+
if (_style == _Style.base
157+
? containsOptionalPositional || firstNamedParameter != null
158+
: firstNamedParameter != null && parametersAtCurly) {
159+
builder.write(', ');
138160
}
139-
builder.addSimpleInsertion(offset, fieldParametersCode);
161+
});
162+
insertOffset =
163+
parameters.lastOrNull?.end ??
164+
constructor.parameters.rightParenthesis.offset;
165+
if (requiredFirst) {
166+
builder.addInsertion(insertOffset, (builder) {
167+
for (var (field, :isRequired) in mappedFields) {
168+
if (isRequired) {
169+
continue;
170+
}
171+
builder.write(', this.${field.name}');
172+
}
173+
});
174+
}
175+
if (addCurlyBraces) {
176+
builder.addSimpleInsertion(insertOffset, '}');
177+
}
178+
179+
if (initializer.isNotEmpty) {
180+
var colonOffset =
181+
constructor.separator?.end ??
182+
constructor.parameters.rightParenthesis.end;
183+
builder.addInsertion(colonOffset, (builder) {
184+
if (constructor.separator == null) {
185+
builder.write(' :');
186+
}
187+
var writeComma = false;
188+
for (var (:field, :publicName) in initializer) {
189+
if (writeComma) {
190+
builder.write(',');
191+
}
192+
writeComma = true;
193+
builder.write(' ${field.name} = $publicName');
194+
}
195+
if (constructor.initializers.isNotEmpty) {
196+
builder.write(',');
197+
}
198+
});
140199
}
141200
});
142201
}
143202

144-
_FieldRecord _parameterForField(FieldElement field) {
145-
var prefix = '';
146-
var isRequired = false;
147-
if (typeSystem.isPotentiallyNonNullable(field.type) &&
148-
_style == _Style.requiredNamed) {
149-
isRequired = true;
150-
prefix = 'required ';
151-
}
152-
return (isRequired: isRequired, parameter: '${prefix}this.${field.name}');
153-
}
203+
bool _isFieldRequired(FieldElement field) =>
204+
_style == _Style.requiredNamed &&
205+
typeSystem.isPotentiallyNonNullable(field.type);
154206
}
155207

156208
enum _Style { base, requiredNamed }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ abstract final class DartFixKind {
157157
DartFixKindPriority.standard + 20,
158158
'Add final initializing formal parameters',
159159
);
160-
static const addInitializingFormalNamesParameters = FixKind(
160+
static const addInitializingFormalNamedParameters = FixKind(
161161
'dart.fix.add.initializingFormalNamedParameters',
162162
DartFixKindPriority.standard + 21,
163163
'Add final initializing formal required named parameters',

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

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ void main() {
2020
@reflectiveTest
2121
class AddFieldFormalNamedParametersTest extends FixProcessorTest {
2222
@override
23-
FixKind get kind => DartFixKind.addInitializingFormalNamesParameters;
23+
FixKind get kind => DartFixKind.addInitializingFormalNamedParameters;
2424

2525
Future<void> test_enum() async {
2626
await resolveTestCode('''
@@ -221,6 +221,113 @@ class Test {
221221
}
222222
''');
223223
}
224+
225+
Future<void> test_privateField() async {
226+
await resolveTestCode('''
227+
class A {
228+
A({int? other});
229+
230+
final int _foo;
231+
}
232+
''');
233+
await assertHasFix(
234+
'''
235+
class A {
236+
A({int? other, required int foo}) : _foo = foo;
237+
238+
final int _foo;
239+
}
240+
''',
241+
filter: (diagnostic) =>
242+
diagnostic.diagnosticCode == diag.finalNotInitializedConstructor1,
243+
);
244+
}
245+
246+
Future<void> test_privateField_alphaNumeric() async {
247+
await resolveTestCode('''
248+
class C {
249+
C();
250+
251+
final int _1to1;
252+
}
253+
''');
254+
await assertHasFix(
255+
'''
256+
class C {
257+
C({required int p1to1}) : _1to1 = p1to1;
258+
259+
final int _1to1;
260+
}
261+
''',
262+
filter: (diagnostic) =>
263+
diagnostic.diagnosticCode == diag.finalNotInitializedConstructor1,
264+
);
265+
}
266+
267+
Future<void> test_privateField_initializer() async {
268+
await resolveTestCode('''
269+
class A {
270+
A({int? other}) : assert(true);
271+
272+
final int _foo;
273+
}
274+
''');
275+
await assertHasFix(
276+
'''
277+
class A {
278+
A({int? other, required int foo}) : _foo = foo, assert(true);
279+
280+
final int _foo;
281+
}
282+
''',
283+
filter: (diagnostic) =>
284+
diagnostic.diagnosticCode == diag.finalNotInitializedConstructor1,
285+
);
286+
}
287+
288+
Future<void> test_privateField_numeric() async {
289+
await resolveTestCode('''
290+
class C {
291+
C();
292+
293+
final int _3;
294+
}
295+
''');
296+
await assertHasFix(
297+
'''
298+
class C {
299+
C({required int p3}) : _3 = p3;
300+
301+
final int _3;
302+
}
303+
''',
304+
filter: (diagnostic) =>
305+
diagnostic.diagnosticCode == diag.finalNotInitializedConstructor1,
306+
);
307+
}
308+
309+
Future<void> test_privateFields() async {
310+
await resolveTestCode('''
311+
class A {
312+
A({int? other});
313+
314+
final int _foo;
315+
final String _bar;
316+
}
317+
''');
318+
await assertHasFix(
319+
'''
320+
class A {
321+
A({int? other, required int foo, required String bar}) : _foo = foo, _bar = bar;
322+
323+
final int _foo;
324+
final String _bar;
325+
}
326+
''',
327+
filter: (diagnostic) =>
328+
diagnostic.diagnosticCode == diag.finalNotInitializedConstructor2,
329+
);
330+
}
224331
}
225332

226333
@reflectiveTest

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /