Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions tools/clang_tidy/lib/clang_tidy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class ClangTidy {
final List<dynamic> buildCommandsData = jsonDecode(
options.buildCommandsPath.readAsStringSync(),
) as List<dynamic>;
final List<Command> changedFileBuildCommands = getLintCommandsForChangedFiles(
final List<Command> changedFileBuildCommands = await getLintCommandsForChangedFiles(
buildCommandsData,
changedFiles,
);
Expand Down Expand Up @@ -165,20 +165,20 @@ class ClangTidy {
/// Given a build commands json file, and the files with local changes,
/// compute the lint commands to run.
@visibleForTesting
List<Command> getLintCommandsForChangedFiles(
Future<List<Command>> getLintCommandsForChangedFiles(
List<dynamic> buildCommandsData,
List<io.File> changedFiles,
) {
final List<Command> buildCommands = <Command>[
for (final dynamic c in buildCommandsData)
Command.fromMap(c as Map<String, dynamic>),
];

return <Command>[
for (final Command c in buildCommands)
if (c.containsAny(changedFiles))
c,
];
) async {
final List<Command> buildCommands = <Command>[];
for (final dynamic data in buildCommandsData) {
final Command command = Command.fromMap(data as Map<String, dynamic>);
final LintAction lintAction = await command.lintAction;
// Short-circuit the expensive containsAny call for the many third_party files.
if (lintAction != LintAction.skipThirdParty && command.containsAny(changedFiles)) {
buildCommands.add(command);
}
}
return buildCommands;
}

Future<_ComputeJobsResult> _computeJobs(
Expand Down Expand Up @@ -212,6 +212,9 @@ class ClangTidy {
case LintAction.skipThirdParty:
_outSink.writeln('🔷 ignoring $relativePath (third_party)');
break;
case LintAction.skipMissing:
_outSink.writeln('🔷 ignoring $relativePath (missing)');
break;
}
}
return _ComputeJobsResult(jobs, sawMalformed);
Expand Down Expand Up @@ -239,5 +242,3 @@ class ClangTidy {
return result;
}
}


15 changes: 9 additions & 6 deletions tools/clang_tidy/lib/src/command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ enum LintAction {

/// Fail due to a malformed FLUTTER_NOLINT comment.
failMalformedNoLint,

/// Ignore because the file doesn't exist locally.
skipMissing,
}

/// A compilation command and methods to generate the lint command and job for
Expand Down Expand Up @@ -86,20 +89,20 @@ class Command {
r'//\s*FLUTTER_NOLINT(: https://github.com/flutter/flutter/issues/\d+)?',
);

LintAction? _lintAction;

/// The type of lint that is appropriate for this command.
Future<LintAction> get lintAction async =>
_lintAction ??= await getLintAction(filePath);
late final Future<LintAction> lintAction = getLintAction(filePath);

/// Determine the lint action for the file at `path`.
@visibleForTesting
static Future<LintAction> getLintAction(String filePath) {
static Future<LintAction> getLintAction(String filePath) async {
if (path.split(filePath).contains('third_party')) {
return Future<LintAction>.value(LintAction.skipThirdParty);
return LintAction.skipThirdParty;
}

final io.File file = io.File(filePath);
if (!file.existsSync()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this check reading the lines crashes

final Stream<String> lines = file.openRead()

return LintAction.skipMissing;
}
final Stream<String> lines = file.openRead()
.transform(utf8.decoder)
.transform(const LineSplitter());
Expand Down
18 changes: 14 additions & 4 deletions tools/clang_tidy/test/clang_tidy_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ Future<int> main(List<String> args) async {
<String>[
'--compile-commands',
// This just has to exist.
io.Platform.executable,
io.Platform.script.path,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

io.Platform.executable crashed, this file is actually parsed now and needs to be utf8.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the comment on the line above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

'--repo',
'/does/not/exist',
],
Expand Down Expand Up @@ -168,7 +168,7 @@ Future<int> main(List<String> args) async {
'file': filePath,
},
];
final List<Command> commands = clangTidy.getLintCommandsForChangedFiles(
final List<Command> commands = await clangTidy.getLintCommandsForChangedFiles(
buildCommandsData,
<io.File>[],
);
Expand All @@ -186,15 +186,17 @@ Future<int> main(List<String> args) async {
outSink: outBuffer,
errSink: errBuffer,
);
const String filePath = '/path/to/a/source_file.cc';

// This just has to exist.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

final String filePath = io.Platform.script.path;
final List<dynamic> buildCommandsData = <Map<String, dynamic>>[
<String, dynamic>{
'directory': '/unused',
'command': '../../buildtools/mac-x64/clang/bin/clang $filePath',
'file': filePath,
},
];
final List<Command> commands = clangTidy.getLintCommandsForChangedFiles(
final List<Command> commands = await clangTidy.getLintCommandsForChangedFiles(
buildCommandsData,
<io.File>[io.File(filePath)],
);
Expand All @@ -220,6 +222,14 @@ Future<int> main(List<String> args) async {
expect(lintAction, equals(LintAction.skipThirdParty));
});

test('Command getLintAction flags missing files', () async {
final LintAction lintAction = await Command.getLintAction(
'does/not/exist',
);

expect(lintAction, equals(LintAction.skipMissing));
});

test('Command getLintActionFromContents flags FLUTTER_NOLINT', () async {
final LintAction lintAction = await Command.lintActionFromContents(
Stream<String>.fromIterable(<String>[
Expand Down