Skip to content

Commit 86d3d63

Browse files
stereotype441Commit Queue
authored and
Commit Queue
committed
[analyzer] Fix dead code range for do loop.
Prior to this change, a `do` loop such as this one: do { return; } while (true); would be flagged with two DEAD_CODE warnings: one covering the text `} while (true);` (which makes sense), and one covering the text `do {` (which doesn't make sense at all, since the top of the `do` loop is reachable). Digging through the code and git history, I've found no good reason for the DEAD_CODE warning at `do {` to exist. It looks like it was added as part of a bug fix from an external contributor and not spotted during code review (https://dart-review.googlesource.com/c/sdk/+/266389). With the unnecessary dead code range removed, some of the tests of the analysis server's `remove_dead_code` fix become simpler (they don't need to use an `errorFilter` to select which dead code range to act on). Others become unnecessary (since they were verifying that `remove_dead_code` properly handled the bogus dead code range). Others simply needed their `errorFilter` changed to select the appropriate dead code range. Making this fix paves the way for improving the `remove_dead_code` fix, which I want to do as part of shipping the `sound-flow-analysis` feature. Change-Id: Iee0bc187441bc4be76a8fa57198155dff3731bc8 Bug: #60438 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423421 Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 564e938 commit 86d3d63

File tree

3 files changed

+10
-99
lines changed

3 files changed

+10
-99
lines changed

pkg/analysis_server/test/src/services/correction/fix/remove_dead_code_test.dart

+7-75
Original file line numberDiff line numberDiff line change
@@ -117,75 +117,7 @@ void f(bool c) {
117117
print(c);
118118
return;
119119
}
120-
''', errorFilter: (error) => error.length == 4);
121-
}
122-
123-
Future<void> test_doWhile_atDo() async {
124-
await resolveTestCode('''
125-
void f(bool c) {
126-
do {
127-
print(c);
128-
return;
129-
} while (c);
130-
}
131-
''');
132-
await assertHasFix('''
133-
void f(bool c) {
134-
print(c);
135-
return;
136-
}
137-
''', errorFilter: (err) => err.problemMessage.length == 4);
138-
}
139-
140-
Future<void> test_doWhile_atDo_followed() async {
141-
await resolveTestCode('''
142-
void f(bool c) {
143-
do {
144-
print(c);
145-
return;
146-
} while (c);
147-
print('');
148-
}
149120
''');
150-
await assertHasFix('''
151-
void f(bool c) {
152-
print(c);
153-
return;
154-
print('');
155-
}
156-
''', errorFilter: (err) => err.problemMessage.length == 4);
157-
}
158-
159-
Future<void> test_doWhile_atDo_noBrackets() async {
160-
await resolveTestCode('''
161-
void f(bool c) {
162-
do
163-
return;
164-
while (c);
165-
}
166-
''');
167-
await assertHasFix('''
168-
void f(bool c) {
169-
return;
170-
}
171-
''', errorFilter: (err) => err.problemMessage.length == 2);
172-
}
173-
174-
Future<void> test_doWhile_atDo_noBrackets_followed() async {
175-
await resolveTestCode('''
176-
void f(bool c) {
177-
do
178-
return;
179-
while (c);
180-
print('');
181-
}
182-
''');
183-
await assertHasFix('''
184-
void f(bool c) {
185-
return;
186-
print('');
187-
}
188-
''', errorFilter: (err) => err.problemMessage.length == 2);
189121
}
190122

191123
Future<void> test_doWhile_atWhile() async {
@@ -202,7 +134,7 @@ void f(bool c) {
202134
print(c);
203135
return;
204136
}
205-
''', errorFilter: (err) => err.problemMessage.length == 12);
137+
''');
206138
}
207139

208140
Future<void> test_doWhile_atWhile_noBrackets() async {
@@ -217,7 +149,7 @@ void f(bool c) {
217149
void f(bool c) {
218150
return;
219151
}
220-
''', errorFilter: (err) => err.problemMessage.length == 10);
152+
''');
221153
}
222154

223155
Future<void> test_doWhile_break() async {
@@ -232,7 +164,7 @@ void f(bool c) {
232164
print('');
233165
}
234166
''');
235-
await assertNoFix(errorFilter: (error) => error.length == 4);
167+
await assertNoFix();
236168
}
237169

238170
Future<void> test_doWhile_break_doLabel() async {
@@ -248,7 +180,7 @@ void f(bool c) {
248180
print('');
249181
}
250182
''');
251-
await assertNoFix(errorFilter: (error) => error.length == 4);
183+
await assertNoFix();
252184
}
253185

254186
Future<void> test_doWhile_break_inner() async {
@@ -271,7 +203,7 @@ void f(bool c) {
271203
return;
272204
print('');
273205
}
274-
''', errorFilter: (error) => error.length == 4);
206+
''', errorFilter: (error) => error.length == 12);
275207
}
276208

277209
Future<void> test_doWhile_break_outerDoLabel() async {
@@ -302,7 +234,7 @@ void f(bool c) {
302234
} while (c);
303235
print('');
304236
}
305-
''', errorFilter: (error) => error.length == 4);
237+
''', errorFilter: (error) => error.length == 12);
306238
}
307239

308240
Future<void> test_doWhile_break_outerLabel() async {
@@ -329,7 +261,7 @@ void f(bool c) {
329261
print('');
330262
}
331263
}
332-
''', errorFilter: (error) => error.length == 4);
264+
''', errorFilter: (error) => error.length == 12);
333265
}
334266

335267
Future<void> test_emptyStatement() async {

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

-8
Original file line numberDiff line numberDiff line change
@@ -285,20 +285,12 @@ class NullSafetyDeadCodeVerifier {
285285
);
286286
offset = node.end;
287287
} else if (parent is DoStatement) {
288-
var doOffset = parent.doKeyword.offset;
289-
var doEnd = parent.doKeyword.end;
290288
var whileOffset = parent.whileKeyword.offset;
291289
var whileEnd = parent.semicolon.end;
292290
var body = parent.body;
293291
if (body is Block) {
294-
doEnd = body.leftBracket.end;
295292
whileOffset = body.rightBracket.offset;
296293
}
297-
_errorReporter.atOffset(
298-
offset: doOffset,
299-
length: doEnd - doOffset,
300-
errorCode: WarningCode.DEAD_CODE,
301-
);
302294
_errorReporter.atOffset(
303295
offset: whileOffset,
304296
length: whileEnd - whileOffset,

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

+3-16
Original file line numberDiff line numberDiff line change
@@ -950,10 +950,7 @@ void f(bool c) {
950950
} while (c);
951951
}
952952
''',
953-
[
954-
error(WarningCode.DEAD_CODE, 19, 4),
955-
error(WarningCode.DEAD_CODE, 52, 12),
956-
],
953+
[error(WarningCode.DEAD_CODE, 52, 12)],
957954
);
958955
}
959956

@@ -970,10 +967,7 @@ void f(bool c) {
970967
print('');
971968
}
972969
''',
973-
[
974-
error(WarningCode.DEAD_CODE, 19, 4),
975-
error(WarningCode.DEAD_CODE, 69, 12),
976-
],
970+
[error(WarningCode.DEAD_CODE, 69, 12)],
977971
);
978972
}
979973

@@ -991,10 +985,7 @@ void f(bool c) {
991985
print('');
992986
}
993987
''',
994-
[
995-
error(WarningCode.DEAD_CODE, 28, 4),
996-
error(WarningCode.DEAD_CODE, 85, 12),
997-
],
988+
[error(WarningCode.DEAD_CODE, 85, 12)],
998989
);
999990
}
1000991

@@ -1012,7 +1003,6 @@ void f(bool c) {
10121003
}
10131004
''',
10141005
[
1015-
error(WarningCode.DEAD_CODE, 19, 4),
10161006
error(WarningCode.DEAD_CODE, 73, 12),
10171007
error(WarningCode.DEAD_CODE, 88, 10),
10181008
],
@@ -1037,7 +1027,6 @@ void f(bool c) {
10371027
}
10381028
''',
10391029
[
1040-
error(WarningCode.DEAD_CODE, 37, 4),
10411030
error(WarningCode.DEAD_CODE, 104, 12),
10421031
error(WarningCode.DEAD_CODE, 121, 38),
10431032
],
@@ -1060,7 +1049,6 @@ void f(bool c) {
10601049
}
10611050
''',
10621051
[
1063-
error(WarningCode.DEAD_CODE, 32, 4),
10641052
error(WarningCode.DEAD_CODE, 98, 12),
10651053
error(WarningCode.DEAD_CODE, 115, 14),
10661054
],
@@ -1079,7 +1067,6 @@ void f(bool c) {
10791067
}
10801068
''',
10811069
[
1082-
error(WarningCode.DEAD_CODE, 19, 4),
10831070
error(WarningCode.DEAD_CODE, 52, 12),
10841071
error(WarningCode.DEAD_CODE, 67, 11),
10851072
],

0 commit comments

Comments
 (0)