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

Commit 2f851b8

Browse files
committed
Only run clang-tidy warning checks reported as errors
1 parent 80847c9 commit 2f851b8

File tree

5 files changed

+44
-17
lines changed

5 files changed

+44
-17
lines changed

.clang-tidy

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
Checks: "bugprone-use-after-move,\
33
clang-analyzer-*,\
44
clang-diagnostic-*,\
5-
google-*,\
5+
google-objc-*,\
6+
google-explicit-constructor" \
67
modernize-use-default-member-init,\
78
readability-identifier-naming,\
89
-google-objc-global-variable-declaration,\
@@ -14,18 +15,6 @@ readability-identifier-naming,\
1415
performance-move-const-arg,\
1516
performance-unnecessary-value-param"
1617

17-
# Only warnings treated as errors are reported
18-
# in the "ci/lint.sh" script and pre-push git hook.
19-
# Add checks when all warnings are fixed
20-
# to prevent new warnings being introduced.
21-
# https://github.com/flutter/flutter/issues/93279
22-
WarningsAsErrors: "bugprone-use-after-move,\
23-
clang-analyzer-*,\
24-
readability-identifier-naming,\
25-
clang-diagnostic-*,\
26-
google-objc-*,\
27-
google-explicit-constructor"
28-
2918
CheckOptions:
3019
- key: modernize-use-default-member-init.UseAssignment
3120
value: true

tools/clang_tidy/lib/clang_tidy.dart

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class ClangTidy {
4646
ClangTidy({
4747
required io.File buildCommandsPath,
4848
String checksArg = '',
49+
bool warningsAsErrors = false,
4950
bool lintAll = false,
5051
bool lintHead = false,
5152
bool fix = false,
@@ -55,6 +56,7 @@ class ClangTidy {
5556
options = Options(
5657
buildCommandsPath: buildCommandsPath,
5758
checksArg: checksArg,
59+
warningsAsErrors: warningsAsErrors,
5860
lintAll: lintAll,
5961
lintHead: lintHead,
6062
fix: fix,
@@ -99,6 +101,9 @@ class ClangTidy {
99101
if (options.checksArg.isNotEmpty) {
100102
_outSink.writeln('Checking for specific checks: ${options.checks}.');
101103
}
104+
if (options.warningsAsErrors) {
105+
_outSink.writeln('Treating warnings as errors.');
106+
}
102107
final int changedFilesCount = changedFiles.length;
103108
if (options.lintAll) {
104109
_outSink.writeln('Checking all $changedFilesCount files the repo dir.');
@@ -136,6 +141,7 @@ class ClangTidy {
136141
changedFileBuildCommands,
137142
options.repoPath,
138143
options.checks,
144+
options.warningsAsErrors,
139145
);
140146
final int computeResult = computeJobsResult.sawMalformed ? 1 : 0;
141147
final List<WorkerJob> jobs = computeJobsResult.jobs;
@@ -195,6 +201,7 @@ class ClangTidy {
195201
List<Command> commands,
196202
io.Directory repoPath,
197203
String? checks,
204+
bool warningsAsErrors,
198205
) async {
199206
bool sawMalformed = false;
200207
final List<WorkerJob> jobs = <WorkerJob>[];
@@ -217,7 +224,11 @@ class ClangTidy {
217224
break;
218225
case LintAction.lint:
219226
_outSink.writeln('🔶 linting $relativePath');
220-
jobs.add(command.createLintJob(checks, options.fix));
227+
jobs.add(command.createLintJob(
228+
checks: checks,
229+
warningsAsErrors: warningsAsErrors,
230+
fix: options.fix,
231+
));
221232
break;
222233
case LintAction.skipThirdParty:
223234
_outSink.writeln('🔷 ignoring $relativePath (third_party)');

tools/clang_tidy/lib/src/command.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,19 @@ class Command {
129129
}
130130

131131
/// The job for the process runner for the lint needed for this command.
132-
WorkerJob createLintJob(String? checks, bool fix) {
132+
WorkerJob createLintJob({
133+
String? checks,
134+
bool warningsAsErrors = false,
135+
bool fix = false,
136+
}) {
133137
final List<String> args = <String>[
134138
filePath,
135139
if (checks != null)
136140
checks,
141+
if (warningsAsErrors) ...<String>[
142+
'--warnings-as-errors',
143+
'*',
144+
],
137145
if (fix) ...<String>[
138146
'--fix',
139147
'--format-style=file',

tools/clang_tidy/lib/src/options.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class Options {
1919
this.help = false,
2020
this.verbose = false,
2121
this.checksArg = '',
22+
this.warningsAsErrors = false,
2223
this.lintAll = false,
2324
this.lintHead = false,
2425
this.fix = false,
@@ -59,6 +60,7 @@ class Options {
5960
verbose: options['verbose'] as bool,
6061
buildCommandsPath: buildCommandsPath,
6162
checksArg: options.wasParsed('checks') ? options['checks'] as String : '',
63+
warningsAsErrors: options['warnings-as-errors'] as bool? ?? false,
6264
lintAll: io.Platform.environment['FLUTTER_LINT_ALL'] != null ||
6365
options['lint-all'] as bool,
6466
lintHead: options['lint-head'] as bool,
@@ -120,6 +122,11 @@ class Options {
120122
'verbose',
121123
help: 'Print verbose output.',
122124
)
125+
..addFlag(
126+
'warnings-as-errors',
127+
help: 'Treat all warnings as errors.',
128+
defaultsTo: true,
129+
)
123130
..addOption(
124131
'compile-commands',
125132
help: 'Use the given path as the source of compile_commands.json. This '
@@ -166,6 +173,9 @@ class Options {
166173
/// Check arguments to plumb through to clang-tidy.
167174
final String? checks;
168175

176+
/// Surface all warnings as errors.
177+
final bool warningsAsErrors;
178+
169179
/// Whether all files should be linted.
170180
final bool lintAll;
171181

tools/clang_tidy/test/clang_tidy_test.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,21 +228,30 @@ Future<int> main(List<String> args) async {
228228
expect(commands, isNotEmpty);
229229
final Command command = commands.first;
230230
expect(command.tidyPath, contains('clang/bin/clang-tidy'));
231-
final WorkerJob jobNoFix = command.createLintJob(null, false);
231+
final WorkerJob jobNoFix = command.createLintJob();
232232
expect(jobNoFix.command[0], endsWith('../../buildtools/mac-x64/clang/bin/clang-tidy'));
233233
expect(jobNoFix.command[1], endsWith(filePath.replaceAll('/', io.Platform.pathSeparator)));
234234
expect(jobNoFix.command[2], '--');
235235
expect(jobNoFix.command[3], '');
236236
expect(jobNoFix.command[4], endsWith(filePath));
237237

238-
final WorkerJob jobWithFix = command.createLintJob(null, true);
238+
final WorkerJob jobWithFix = command.createLintJob(fix: true);
239239
expect(jobWithFix.command[0], endsWith('../../buildtools/mac-x64/clang/bin/clang-tidy'));
240240
expect(jobWithFix.command[1], endsWith(filePath.replaceAll('/', io.Platform.pathSeparator)));
241241
expect(jobWithFix.command[2], '--fix');
242242
expect(jobWithFix.command[3], '--format-style=file');
243243
expect(jobWithFix.command[4], '--');
244244
expect(jobWithFix.command[5], '');
245245
expect(jobWithFix.command[6], endsWith(filePath));
246+
247+
final WorkerJob jobWithWarning = command.createLintJob(warningsAsErrors: true);
248+
expect(jobWithWarning.command[0], endsWith('../../buildtools/mac-x64/clang/bin/clang-tidy'));
249+
expect(jobWithWarning.command[1], endsWith(filePath.replaceAll('/', io.Platform.pathSeparator)));
250+
expect(jobWithWarning.command[2], '--warnings-as-errors');
251+
expect(jobWithWarning.command[3], '*');
252+
expect(jobWithWarning.command[4], '--');
253+
expect(jobWithWarning.command[5], '');
254+
expect(jobWithWarning.command[6], endsWith(filePath));
246255
});
247256

248257
test('Command getLintAction flags third_party files', () async {

0 commit comments

Comments
 (0)