Skip to content

Commit 96ab3c1

Browse files
FMorschelCommit Queue
FMorschel
authored and
Commit Queue
committed
[DAS] Consider two lint rules when converting forEach to a for-in loop.
[email protected] Fixes #56876 Change-Id: I7188b3c8fee661623ea57158cc3cd0bf601b9fd5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389380 Reviewed-by: Samuel Rawlins <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Auto-Submit: Felipe Morschel <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
1 parent 30dc2de commit 96ab3c1

File tree

4 files changed

+216
-4
lines changed

4 files changed

+216
-4
lines changed

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

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import 'package:analysis_server/src/services/correction/fix.dart';
66
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
77
import 'package:analyzer/dart/ast/ast.dart';
8+
import 'package:analyzer/dart/ast/token.dart';
89
import 'package:analyzer/dart/ast/visitor.dart';
910
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
1011
import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
@@ -34,22 +35,26 @@ class ConvertForEachToForLoop extends ResolvedCorrectionProducer {
3435
if (statement is! ExpressionStatement) {
3536
return;
3637
}
37-
var argument = invocation.argumentList.arguments[0];
38+
var argument = invocation.argumentList.arguments.first;
3839
if (argument is! FunctionExpression) {
3940
return;
4041
}
4142
var parameters = argument.parameters?.parameters;
4243
if (parameters == null || parameters.length != 1) {
4344
return;
4445
}
45-
var parameter = parameters[0];
46+
var parameter = parameters.first;
4647
if (parameter is! NormalFormalParameter) {
4748
return;
4849
}
4950
var loopVariableName = parameter.name?.lexeme;
5051
if (loopVariableName == null) {
5152
return;
5253
}
54+
var codeStyleOptions = getCodeStyleOptions(unitResult.file);
55+
var specifyTypes = codeStyleOptions.specifyTypes;
56+
var preferFinal = codeStyleOptions.finalInForEach;
57+
var type = parameter.declaredFragment?.element.type;
5358
var target = utils.getNodeText(invocation.target!);
5459
var body = argument.body;
5560
if (body.isAsynchronous || body.isGenerator) {
@@ -58,7 +63,19 @@ class ConvertForEachToForLoop extends ResolvedCorrectionProducer {
5863
if (body is BlockFunctionBody) {
5964
await builder.addDartFileEdit(file, (builder) {
6065
builder.addReplacement(range.startStart(invocation, body), (builder) {
61-
builder.write('for (var ');
66+
builder.write('for (');
67+
if (preferFinal) {
68+
builder.write(Keyword.FINAL.lexeme);
69+
if (specifyTypes) {
70+
builder.write(' ');
71+
builder.writeType(type);
72+
}
73+
} else if (specifyTypes) {
74+
builder.writeType(type);
75+
} else {
76+
builder.write(Keyword.VAR.lexeme);
77+
}
78+
builder.write(' ');
6279
builder.write(loopVariableName);
6380
builder.write(' in ');
6481
builder.write(target);
@@ -76,7 +93,19 @@ class ConvertForEachToForLoop extends ResolvedCorrectionProducer {
7693
await builder.addDartFileEdit(file, (builder) {
7794
builder.addReplacement(range.startStart(invocation, expression),
7895
(builder) {
79-
builder.write('for (var ');
96+
builder.write('for (');
97+
if (preferFinal) {
98+
builder.write(Keyword.FINAL.lexeme);
99+
if (specifyTypes) {
100+
builder.write(' ');
101+
builder.writeType(type);
102+
}
103+
} else if (specifyTypes) {
104+
builder.writeType(type);
105+
} else {
106+
builder.write(Keyword.VAR.lexeme);
107+
}
108+
builder.write(' ');
80109
builder.write(loopVariableName);
81110
builder.write(' in ');
82111
builder.write(target);

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

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,112 @@ void f(List<String> list) {
152152
await assertNoFix();
153153
}
154154

155+
Future<void> test_blockBody_preferFinal() async {
156+
createAnalysisOptionsFile(
157+
lints: [
158+
LintNames.avoid_function_literals_in_foreach_calls,
159+
LintNames.prefer_final_in_for_each,
160+
],
161+
);
162+
await resolveTestCode('''
163+
void f(List<String> list) {
164+
list.forEach((e) {
165+
e.substring(3, 7);
166+
});
167+
}
168+
''');
169+
await assertHasFix('''
170+
void f(List<String> list) {
171+
for (final e in list) {
172+
e.substring(3, 7);
173+
}
174+
}
175+
''');
176+
}
177+
178+
Future<void> test_blockBody_preferFinal_specifyTypes() async {
179+
createAnalysisOptionsFile(
180+
lints: [
181+
LintNames.avoid_function_literals_in_foreach_calls,
182+
LintNames.prefer_final_in_for_each,
183+
LintNames.always_specify_types,
184+
],
185+
);
186+
await resolveTestCode('''
187+
void f(List<String> list) {
188+
list.forEach((e) {
189+
e.substring(3, 7);
190+
});
191+
}
192+
''');
193+
await assertHasFix('''
194+
void f(List<String> list) {
195+
for (final String e in list) {
196+
e.substring(3, 7);
197+
}
198+
}
199+
''',
200+
errorFilter: (error) =>
201+
error.errorCode.name ==
202+
LintNames.avoid_function_literals_in_foreach_calls);
203+
}
204+
205+
Future<void> test_blockBody_specifyTypes() async {
206+
createAnalysisOptionsFile(
207+
lints: [
208+
LintNames.avoid_function_literals_in_foreach_calls,
209+
LintNames.always_specify_types,
210+
],
211+
);
212+
await resolveTestCode('''
213+
void f(List<String> list) {
214+
list.forEach((e) {
215+
e.substring(3, 7);
216+
});
217+
}
218+
''');
219+
await assertHasFix('''
220+
void f(List<String> list) {
221+
for (String e in list) {
222+
e.substring(3, 7);
223+
}
224+
}
225+
''',
226+
errorFilter: (error) =>
227+
error.errorCode.name ==
228+
LintNames.avoid_function_literals_in_foreach_calls);
229+
}
230+
231+
Future<void> test_blockBody_specifyTypes_prefixed() async {
232+
createAnalysisOptionsFile(
233+
lints: [
234+
LintNames.avoid_function_literals_in_foreach_calls,
235+
LintNames.always_specify_types,
236+
],
237+
);
238+
await resolveTestCode('''
239+
import 'dart:core' as core;
240+
241+
void f(core.List<core.Set<core.String>> list) {
242+
list.forEach((e) {
243+
e.map((s) => s.substring(3, 7));
244+
});
245+
}
246+
''');
247+
await assertHasFix('''
248+
import 'dart:core' as core;
249+
250+
void f(core.List<core.Set<core.String>> list) {
251+
for (core.Set<core.String> e in list) {
252+
e.map((s) => s.substring(3, 7));
253+
}
254+
}
255+
''',
256+
errorFilter: (error) =>
257+
error.errorCode.name ==
258+
LintNames.avoid_function_literals_in_foreach_calls);
259+
}
260+
155261
Future<void> test_blockBody_syncStar() async {
156262
await resolveTestCode('''
157263
void f(List<String> list) {
@@ -199,6 +305,76 @@ void f(List<String> list) {
199305
LintNames.avoid_function_literals_in_foreach_calls);
200306
}
201307

308+
Future<void> test_expressionBody_preferFinal() async {
309+
createAnalysisOptionsFile(
310+
lints: [
311+
LintNames.avoid_function_literals_in_foreach_calls,
312+
LintNames.prefer_final_in_for_each,
313+
],
314+
);
315+
await resolveTestCode('''
316+
void f(List<String> list) {
317+
list.forEach((e) => e.substring(3, 7));
318+
}
319+
''');
320+
await assertHasFix('''
321+
void f(List<String> list) {
322+
for (final e in list) {
323+
e.substring(3, 7);
324+
}
325+
}
326+
''');
327+
}
328+
329+
Future<void> test_expressionBody_preferFinal_specifyTypes() async {
330+
createAnalysisOptionsFile(
331+
lints: [
332+
LintNames.avoid_function_literals_in_foreach_calls,
333+
LintNames.prefer_final_in_for_each,
334+
LintNames.always_specify_types,
335+
],
336+
);
337+
await resolveTestCode('''
338+
void f(List<String> list) {
339+
list.forEach((e) => e.substring(3, 7));
340+
}
341+
''');
342+
await assertHasFix('''
343+
void f(List<String> list) {
344+
for (final String e in list) {
345+
e.substring(3, 7);
346+
}
347+
}
348+
''',
349+
errorFilter: (error) =>
350+
error.errorCode.name ==
351+
LintNames.avoid_function_literals_in_foreach_calls);
352+
}
353+
354+
Future<void> test_expressionBody_specifyTypes() async {
355+
createAnalysisOptionsFile(
356+
lints: [
357+
LintNames.avoid_function_literals_in_foreach_calls,
358+
LintNames.always_specify_types,
359+
],
360+
);
361+
await resolveTestCode('''
362+
void f(List<String> list) {
363+
list.forEach((e) => e.substring(3, 7));
364+
}
365+
''');
366+
await assertHasFix('''
367+
void f(List<String> list) {
368+
for (String e in list) {
369+
e.substring(3, 7);
370+
}
371+
}
372+
''',
373+
errorFilter: (error) =>
374+
error.errorCode.name ==
375+
LintNames.avoid_function_literals_in_foreach_calls);
376+
}
377+
202378
Future<void> test_expressionBody_syncStar() async {
203379
await resolveTestCode('''
204380
void f(List<String> list) {

‎pkg/analyzer/lib/dart/analysis/code_style_options.dart‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ abstract class CodeStyleOptions {
1313
/// commas should be inserted in function calls and declarations.
1414
bool get addTrailingCommas;
1515

16+
/// Return `true` if local variables should be `final` inside 'for' in
17+
/// iterable.
18+
bool get finalInForEach;
19+
1620
/// Return `true` if local variables should be `final` whenever possible.
1721
bool get makeLocalsFinal;
1822

‎pkg/analyzer/lib/src/analysis_options/code_style_options.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ class CodeStyleOptionsImpl implements CodeStyleOptions {
1919
@override
2020
bool get addTrailingCommas => _isLintEnabled('require_trailing_commas');
2121

22+
@override
23+
bool get finalInForEach => _isLintEnabled('prefer_final_in_for_each');
24+
2225
@override
2326
bool get makeLocalsFinal => _isLintEnabled('prefer_final_locals');
2427

0 commit comments

Comments
 (0)