Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 2df836f

Browse files
authored
Require that FLUTTER_NOLINT include issue link (#21922)
This adds enforcement to the linter that all FLUTTER_NOLINT comments be of the form: // FLUTTER_NOLINT: https://github.com/flutter/flutter/issue/ID Every linter opt-out should have an associated bug describing what issue it works around so that others can work on eliminating it, or at least understanding the rationale and whether it's still relevant. This also reduces the likelihood of inadvertent copy-pasting into new files either because the author fails to spot it when copying the copyright block from another file, or assumes that it's necessary for some subcomponent of the engine. Bug: flutter/flutter#68273
1 parent 49c35b6 commit 2df836f

File tree

1 file changed

+73
-29
lines changed

1 file changed

+73
-29
lines changed

ci/bin/lint.dart

Lines changed: 73 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import 'package:args/args.dart';
1717
import 'package:path/path.dart' as path;
1818
import 'package:process_runner/process_runner.dart';
1919

20-
String _linterOutputHeader = '''
20+
const String _linterOutputHeader = '''
2121
┌──────────────────────────┐
2222
│ Engine Clang Tidy Linter │
2323
└──────────────────────────┘
@@ -26,6 +26,8 @@ more information on addressing these issues please see:
2626
https://github.com/flutter/flutter/wiki/Engine-Clang-Tidy-Linter
2727
''';
2828

29+
const String issueUrlPrefix = 'https://github.com/flutter/flutter/issues';
30+
2931
class Command {
3032
Directory directory = Directory('');
3133
String command = '';
@@ -109,23 +111,59 @@ File buildFileAsRepoFile(String buildFile, Directory repoPath) {
109111
return result;
110112
}
111113

112-
Future<String> shouldIgnoreFile(File file) async {
113-
if (path.split(file.path).contains('third_party')) {
114-
return 'third_party';
115-
} else {
116-
final RegExp exp = RegExp(r'//\s*FLUTTER_NOLINT');
117-
await for (String line
118-
in file.openRead().transform(utf8.decoder).transform(const LineSplitter())) {
119-
if (exp.hasMatch(line)) {
120-
return 'FLUTTER_NOLINT';
121-
} else if (line.isNotEmpty && line[0] != '\n' && line[0] != '/') {
122-
// Quick out once we find a line that isn't empty or a comment. The
123-
// FLUTTER_NOLINT must show up before the first real code.
124-
return '';
125-
}
114+
/// Lint actions to apply to a file.
115+
enum LintAction {
116+
/// Run the linter over the file.
117+
lint,
118+
119+
/// Ignore files under third_party/.
120+
skipThirdParty,
121+
122+
/// Ignore due to a well-formed FLUTTER_NOLINT comment.
123+
skipNoLint,
124+
125+
/// Fail due to a malformed FLUTTER_NOLINT comment.
126+
failMalformedNoLint,
127+
}
128+
129+
bool isThirdPartyFile(File file) {
130+
return path.split(file.path).contains('third_party');
131+
}
132+
133+
Future<LintAction> getLintAction(File file) async {
134+
if (isThirdPartyFile(file)) {
135+
return LintAction.skipThirdParty;
136+
}
137+
138+
// Check for FlUTTER_NOLINT at top of file.
139+
final RegExp exp = RegExp('\/\/\\s*FLUTTER_NOLINT(: $issueUrlPrefix/\\d+)?');
140+
final Stream<String> lines = file.openRead()
141+
.transform(utf8.decoder)
142+
.transform(const LineSplitter());
143+
await for (String line in lines) {
144+
final RegExpMatch match = exp.firstMatch(line);
145+
if (match != null) {
146+
return match.group(1) != null
147+
? LintAction.skipNoLint
148+
: LintAction.failMalformedNoLint;
149+
} else if (line.isNotEmpty && line[0] != '\n' && line[0] != '/') {
150+
// Quick out once we find a line that isn't empty or a comment. The
151+
// FLUTTER_NOLINT must show up before the first real code.
152+
return LintAction.lint;
126153
}
127-
return '';
128154
}
155+
return LintAction.lint;
156+
}
157+
158+
WorkerJob createLintJob(Command command, String checks, String tidyPath) {
159+
final String tidyArgs = calcTidyArgs(command);
160+
final List<String> args = <String>[command.file.path, checks, '--'];
161+
args.addAll(tidyArgs?.split(' ') ?? <String>[]);
162+
return WorkerJob(
163+
<String>[tidyPath, ...args],
164+
workingDirectory: command.directory,
165+
name: 'clang-tidy on ${command.file.path}',
166+
);
129167
}
130168

131169
void _usage(ArgParser parser, {int exitCode = 1}) {
@@ -226,19 +264,23 @@ void main(List<String> arguments) async {
226264
final List<WorkerJob> jobs = <WorkerJob>[];
227265
for (Command command in changedFileBuildCommands) {
228266
final String relativePath = path.relative(command.file.path, from: repoPath.parent.path);
229-
final String ignoreReason = await shouldIgnoreFile(command.file);
230-
if (ignoreReason.isEmpty) {
231-
final String tidyArgs = calcTidyArgs(command);
232-
final List<String> args = <String>[command.file.path, checks, '--'];
233-
args.addAll(tidyArgs?.split(' ') ?? <String>[]);
234-
print('🔶 linting $relativePath');
235-
jobs.add(WorkerJob(
236-
<String>[tidyPath, ...args],
237-
workingDirectory: command.directory,
238-
name: 'clang-tidy on ${command.file.path}',
239-
));
240-
} else {
241-
print('🔷 ignoring $relativePath ($ignoreReason)');
267+
final LintAction action = await getLintAction(command.file);
268+
switch (action) {
269+
case LintAction.skipNoLint:
270+
print('🔷 ignoring $relativePath (FLUTTER_NOLINT)');
271+
break;
272+
case LintAction.failMalformedNoLint:
273+
print('❌ malformed opt-out $relativePath');
274+
print(' Required format: // FLUTTER_NOLINT: $issueUrlPrefix/ISSUE_ID');
275+
exitCode = 1;
276+
break;
277+
case LintAction.lint:
278+
print('🔶 linting $relativePath');
279+
jobs.add(createLintJob(command, checks, tidyPath));
280+
break;
281+
case LintAction.skipThirdParty:
282+
print('🔷 ignoring $relativePath (third_party)');
283+
break;
242284
}
243285
}
244286
final ProcessPool pool = ProcessPool();
@@ -260,5 +302,7 @@ void main(List<String> arguments) async {
260302
print('\n');
261303
if (exitCode == 0) {
262304
print('No lint problems found.');
305+
} else {
306+
print('Lint problems found.');
263307
}
264308
}

0 commit comments

Comments
 (0)