Skip to content
This repository was archived by the owner on Aug 28, 2024. It is now read-only.

Commit 19f7189

Browse files
kturneykturney
authored
Ignore enhancements (#454)
* typo and case fixes * allow omitting space between // and coverage * allow text after coverage:ignore comments * handle unbalanced ignore ranges instead of bailing * add changelog entry * update title in changelog * throw FormatException when encountering unbalanced ignore comments * add file path to err message and test err message * refactor test
1 parent 3dcefbc commit 19f7189

File tree

4 files changed

+126
-23
lines changed

4 files changed

+126
-23
lines changed

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
## 1.6.4-wip.
2+
3+
- allow omitting space between `//` and `coverage` in coverage ignore comments
4+
- allow text after coverage ignore comments
5+
- throw FormatException when encountering unbalanced ignore comments instead of silently erroring
6+
17
## 1.6.3
28

39
- Require Dart 2.18

‎lib/src/hitmap.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class HitMap {
7676
final path = resolver.resolve(source);
7777
if (path != null) {
7878
final lines = loader.loadSync(path) ?? [];
79-
ignoredLinesList = getIgnoredLines(lines);
79+
ignoredLinesList = getIgnoredLines(path, lines);
8080

8181
// Ignore the whole file.
8282
if (ignoredLinesList.length == 1 &&

‎lib/src/util.dart‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,13 @@ Future<int> getOpenPort() async {
7474
}
7575
}
7676

77-
final muliLineIgnoreStart = RegExp(r'// coverage:ignore-start\s*$');
78-
final muliLineIgnoreEnd = RegExp(r'// coverage:ignore-end\s*$');
79-
final singleLineIgnore = RegExp(r'// coverage:ignore-line\s*$');
80-
final ignoreFile = RegExp(r'// coverage:ignore-file\s*$');
77+
final muliLineIgnoreStart = RegExp(r'//\s*coverage:ignore-start[\w\d\s]*$');
78+
final muliLineIgnoreEnd = RegExp(r'//\s*coverage:ignore-end[\w\d\s]*$');
79+
final singleLineIgnore = RegExp(r'//\s*coverage:ignore-line[\w\d\s]*$');
80+
final ignoreFile = RegExp(r'//\s*coverage:ignore-file[\w\d\s]*$');
8181

8282
/// Return list containing inclusive range of lines to be ignored by coverage.
83-
/// If there is a error in balancing the statements it will ignore nothing,
83+
/// If there is a error in balancing the statements it will throw a FormatException,
8484
/// unless `coverage:ignore-file` is found.
8585
/// Return [0, lines.length] if the whole file is ignored.
8686
///
@@ -100,44 +100,64 @@ final ignoreFile = RegExp(r'// coverage:ignore-file\s*$');
100100
/// ]
101101
/// ```
102102
///
103-
List<List<int>> getIgnoredLines(List<String>? lines) {
103+
List<List<int>> getIgnoredLines(String filePath, List<String>? lines) {
104104
final ignoredLines = <List<int>>[];
105105
if (lines == null) return ignoredLines;
106106

107107
final allLines = [
108108
[0, lines.length]
109109
];
110110

111-
var isError = false;
111+
FormatException? err;
112112
var i = 0;
113113
while (i < lines.length) {
114114
if (lines[i].contains(ignoreFile)) return allLines;
115115

116-
if (lines[i].contains(muliLineIgnoreEnd)) isError = true;
116+
if (lines[i].contains(muliLineIgnoreEnd)) {
117+
err ??= FormatException(
118+
'unmatched coverage:ignore-end found at $filePath:${i + 1}',
119+
);
120+
}
117121

118122
if (lines[i].contains(singleLineIgnore)) ignoredLines.add([i + 1, i + 1]);
119123

120124
if (lines[i].contains(muliLineIgnoreStart)) {
121125
final start = i;
126+
var isUnmatched = true;
122127
++i;
123128
while (i < lines.length) {
124129
if (lines[i].contains(ignoreFile)) return allLines;
125130
if (lines[i].contains(muliLineIgnoreStart)) {
126-
isError = true;
131+
err ??= FormatException(
132+
'coverage:ignore-start found at $filePath:${i + 1}'
133+
' before previous coverage:ignore-start ended',
134+
);
127135
break;
128136
}
129137

130138
if (lines[i].contains(muliLineIgnoreEnd)) {
131139
ignoredLines.add([start + 1, i + 1]);
140+
isUnmatched = false;
132141
break;
133142
}
134143
++i;
135144
}
145+
146+
if (isUnmatched) {
147+
err ??= FormatException(
148+
'coverage:ignore-start found at $filePath:${start + 1}'
149+
' has no matching coverage:ignore-end',
150+
);
151+
}
136152
}
137153
++i;
138154
}
139155

140-
return isError ? [] : ignoredLines;
156+
if (err == null) {
157+
return ignoredLines;
158+
}
159+
160+
throw err;
141161
}
142162

143163
extension StandardOutExtension on Stream<List<int>> {

‎test/util_test.dart‎

Lines changed: 89 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,53 @@ void main() {
176176
''',
177177
];
178178

179-
test('returns empty when the annotations are not balanced', () {
180-
for (final content in invalidSources) {
181-
expect(getIgnoredLines(content.split('\n')), isEmpty);
179+
test('throws FormatException when the annotations are not balanced', () {
180+
void runTest(int index, String errMsg) {
181+
final content = invalidSources[index].split('\n');
182+
expect(
183+
() => getIgnoredLines('content-$index.dart', content),
184+
throwsA(
185+
allOf(
186+
isFormatException,
187+
predicate((FormatException e) => e.message == errMsg),
188+
),
189+
),
190+
reason: 'expected FormatException with message "$errMsg"',
191+
);
182192
}
193+
194+
runTest(
195+
0,
196+
'coverage:ignore-start found at content-0.dart:3 before previous coverage:ignore-start ended',
197+
);
198+
runTest(
199+
1,
200+
'coverage:ignore-start found at content-1.dart:3 before previous coverage:ignore-start ended',
201+
);
202+
runTest(
203+
2,
204+
'unmatched coverage:ignore-end found at content-2.dart:5',
205+
);
206+
runTest(
207+
3,
208+
'unmatched coverage:ignore-end found at content-3.dart:1',
209+
);
210+
runTest(
211+
4,
212+
'unmatched coverage:ignore-end found at content-4.dart:1',
213+
);
214+
runTest(
215+
5,
216+
'unmatched coverage:ignore-end found at content-5.dart:1',
217+
);
218+
runTest(
219+
6,
220+
'unmatched coverage:ignore-end found at content-6.dart:1',
221+
);
222+
runTest(
223+
7,
224+
'coverage:ignore-start found at content-7.dart:1 has no matching coverage:ignore-end',
225+
);
183226
});
184227

185228
test(
@@ -188,20 +231,20 @@ void main() {
188231
for (final content in invalidSources) {
189232
final lines = content.split('\n');
190233
lines.add(' // coverage:ignore-file');
191-
expect(getIgnoredLines(lines), [
234+
expect(getIgnoredLines('', lines), [
192235
[0, lines.length]
193236
]);
194237
}
195238
});
196239

197-
test('Returns [[0,lines.length]] when the whole file is ignored', () {
240+
test('returns [[0,lines.length]] when the whole file is ignored', () {
198241
final lines = '''final str = ''; // coverage:ignore-start
199242
final str = ''; // coverage:ignore-end
200243
final str = ''; // coverage:ignore-file
201244
'''
202245
.split('\n');
203246

204-
expect(getIgnoredLines(lines), [
247+
expect(getIgnoredLines('', lines), [
205248
[0, lines.length]
206249
]);
207250
});
@@ -217,7 +260,7 @@ void main() {
217260
'''
218261
.split('\n');
219262

220-
expect(getIgnoredLines(lines), [
263+
expect(getIgnoredLines('', lines), [
221264
[1, 3],
222265
[4, 6],
223266
]);
@@ -231,14 +274,14 @@ void main() {
231274
'''
232275
.split('\n');
233276

234-
expect(getIgnoredLines(lines), [
277+
expect(getIgnoredLines('', lines), [
235278
[1, 1],
236279
[2, 2],
237280
[3, 3],
238281
]);
239282
});
240283

241-
test('Ingore comments have no effect inside string literals', () {
284+
test('ignore comments have no effect inside string literals', () {
242285
final lines = '''
243286
final str = '// coverage:ignore-file';
244287
final str = '// coverage:ignore-line';
@@ -248,12 +291,12 @@ void main() {
248291
'''
249292
.split('\n');
250293

251-
expect(getIgnoredLines(lines), [
294+
expect(getIgnoredLines('', lines), [
252295
[3, 3],
253296
]);
254297
});
255298

256-
test('Allow white-space after ignore comments', () {
299+
test('allow white-space after ignore comments', () {
257300
// Using multiple strings, rather than splitting a multi-line string,
258301
// because many code editors remove trailing white-space.
259302
final lines = [
@@ -265,7 +308,41 @@ void main() {
265308
"final str = ''; // coverage:ignore-end \t \t ",
266309
];
267310

268-
expect(getIgnoredLines(lines), [
311+
expect(getIgnoredLines('', lines), [
312+
[1, 3],
313+
[4, 4],
314+
[5, 6],
315+
]);
316+
});
317+
318+
test('allow omitting space after //', () {
319+
final lines = [
320+
"final str = ''; //coverage:ignore-start",
321+
"final str = ''; //coverage:ignore-line",
322+
"final str = ''; //coverage:ignore-end",
323+
"final str = ''; //coverage:ignore-line",
324+
"final str = ''; //coverage:ignore-start",
325+
"final str = ''; //coverage:ignore-end",
326+
];
327+
328+
expect(getIgnoredLines('', lines), [
329+
[1, 3],
330+
[4, 4],
331+
[5, 6],
332+
]);
333+
});
334+
335+
test('allow text after ignore comments', () {
336+
final lines = [
337+
"final str = ''; // coverage:ignore-start due to XYZ",
338+
"final str = ''; // coverage:ignore-line",
339+
"final str = ''; // coverage:ignore-end due to XYZ",
340+
"final str = ''; // coverage:ignore-line due to 123",
341+
"final str = ''; // coverage:ignore-start",
342+
"final str = ''; // coverage:ignore-end it is done",
343+
];
344+
345+
expect(getIgnoredLines('', lines), [
269346
[1, 3],
270347
[4, 4],
271348
[5, 6],

0 commit comments

Comments
 (0)