Allow engine variant to be passed into linter script

Author: jmagman

ci/lint.sh

@@ -41,6 +41,6 @@ cd "$SCRIPT_DIR"
 "$DART" \
   --disable-dart-dev \
   "$SRC_DIR/flutter/tools/clang_tidy/bin/main.dart" \
-  --compile-commands="$COMPILE_COMMANDS" \
   --repo="$SRC_DIR/flutter" \
+  --src-dir="$SRC_DIR" \
   "$@"

tools/clang_tidy/README.md

@@ -1,9 +1,15 @@
 # clang_tidy
 
-This is a Dart program/library that runs clang_tidy over modified files. It
-takes two mandatory arguments that point at a compile_commands.json command
-and the root of the Flutter engine repo:
+This is a Dart program/library that runs clang_tidy over modified files in the Flutter engine repo.
+
+By default the linter runs on the repo files changed contained in `src/out/host_debug/compile_commands.json` command.
+To check files other than in `host_debug` use `--target-variant android_debug_unopt`,
+`--target-variant ios_debug_sim_unopt`, etc.
+
+Alternatively, use `--compile-commands` to specify a path to a `compile_commands.json` file.
 
 ```
+$ bin/main.dart --target-variant <engine-variant> --repo <path-to-repo>
 $ bin/main.dart --compile-commands <compile_commands.json-path> --repo <path-to-repo>
+$ bin/main.dart --help
 ```

tools/clang_tidy/lib/src/options.dart

@@ -5,6 +5,10 @@
 import 'dart:io' as io show Directory, File, Platform, stderr;
 
 import 'package:args/args.dart';
+import 'package:path/path.dart' as path;
+
+// Path to root of the flutter/engine repository containing this script.
+final String _engineRoot = path.dirname(path.dirname(path.dirname(path.dirname(path.fromUri(io.Platform.script)))));
 
 /// A class for organizing the options to the Engine linter, and the files
 /// that it operates on.
@@ -49,12 +53,13 @@ class Options {
   /// Builds an [Options] instance with an [ArgResults] instance.
   factory Options._fromArgResults(
     ArgResults options, {
+    required io.File buildCommandsPath,
     StringSink? errSink,
   }) {
     return Options(
       help: options['help'] as bool,
       verbose: options['verbose'] as bool,
-      buildCommandsPath: io.File(options['compile-commands'] as String),
+      buildCommandsPath: buildCommandsPath,
       repoPath: io.Directory(options['repo'] as String),
       checksArg: options.wasParsed('checks') ? options['checks'] as String : '',
       lintAll: io.Platform.environment['FLUTTER_LINT_ALL'] != null ||
@@ -70,7 +75,17 @@ class Options {
     StringSink? errSink,
   }) {
     final ArgResults argResults = _argParser.parse(arguments);
-    final String? message = _checkArguments(argResults);
+
+    String? buildCommandsPath = argResults['compile-commands'] as String?;
+    // path/to/engine/src/out/variant/compile_commands.json
+    buildCommandsPath ??= path.join(
+      argResults['src-dir'] as String,
+      'out',
+      argResults['target-variant'] as String,
+      'compile_commands.json',
+    );
+    final io.File buildCommands = io.File(buildCommandsPath);
+    final String? message = _checkArguments(argResults, buildCommands);
     if (message != null) {
       return Options._error(message, errSink: errSink);
     }
@@ -79,14 +94,17 @@ class Options {
     }
     return Options._fromArgResults(
       argResults,
+      buildCommandsPath: buildCommands,
       errSink: errSink,
     );
   }
 
   static final ArgParser _argParser = ArgParser()
     ..addFlag(
       'help',
+      abbr: 'h',
       help: 'Print help.',
+      negatable: false,
     )
     ..addFlag(
       'lint-all',
@@ -112,6 +130,20 @@ class Options {
       help: 'Use the given path as the source of compile_commands.json. This '
             'file is created by running tools/gn',
     )
+    ..addOption(
+      'target-variant',
+      aliases: <String>['variant'],
+      help: 'The engine variant directory containing compile_commands.json '
+            'created by running tools/gn. Ignored if --compile-commands is also passed.',
+      valueHelp: 'host_debug|android_debug_unopt|ios_debug|ios_debug_sim_unopt',
+      defaultsTo: 'host_debug',
+    )
+    ..addOption(
+      'src-dir',
+      help: 'Path to the engine src directory. Ignored if --compile-commands is also passed.',
+      valueHelp: 'path/to/engine/src',
+      defaultsTo: path.dirname(_engineRoot),
+    )
     ..addOption(
       'checks',
       help: 'Perform the given checks on the code. Defaults to the empty '
@@ -156,26 +188,21 @@ class Options {
       _errSink.writeln(message);
     }
     _errSink.writeln(
-      'Usage: bin/main.dart [--help] [--lint-all] [--fix] [--verbose] [--diff-branch]',
+      'Usage: bin/main.dart [--help] [--lint-all] [--fix] [--verbose] [--diff-branch] [--target-variant variant] [--src-dir path/to/engine/src]',
     );
     _errSink.writeln(_argParser.usage);
   }
 
   /// Command line argument validation.
-  static String? _checkArguments(ArgResults argResults) {
+  static String? _checkArguments(ArgResults argResults, io.File buildCommandsPath) {
     if (argResults.wasParsed('help')) {
       return null;
     }
 
-    if (!argResults.wasParsed('compile-commands')) {
-      return 'ERROR: The --compile-commands argument is required.';
-    }
-
     if (!argResults.wasParsed('repo')) {
       return 'ERROR: The --repo argument is required.';
     }
 
-    final io.File buildCommandsPath = io.File(argResults['compile-commands'] as String);
     if (!buildCommandsPath.existsSync()) {
       return "ERROR: Build commands path ${buildCommandsPath.absolute.path} doesn't exist.";
     }

tools/clang_tidy/test/clang_tidy_test.dart

@@ -12,7 +12,7 @@ import 'package:process_runner/process_runner.dart';
 Future<int> main(List<String> args) async {
   if (args.length < 2) {
     io.stderr.writeln(
-      'Usage: clang_tidy_test.dart [build commands] [repo root]',
+      'Usage: clang_tidy_test.dart [path/to/compile_commands.json] [repo root]',
     );
     return 1;
   }
@@ -37,11 +37,14 @@ Future<int> main(List<String> args) async {
     expect(errBuffer.toString(), contains('Usage: '));
   });
 
-  test('Error when --compile-commands is missing', () async {
+  test('Error when --repo is missing', () async {
     final StringBuffer outBuffer = StringBuffer();
     final StringBuffer errBuffer = StringBuffer();
     final ClangTidy clangTidy = ClangTidy.fromCommandLine(
-      <String>[],
+      <String>[
+        '--compile-commands',
+        '/unused',
+      ],
       outSink: outBuffer,
       errSink: errBuffer,
     );
@@ -51,16 +54,18 @@ Future<int> main(List<String> args) async {
     expect(clangTidy.options.help, isFalse);
     expect(result, equals(1));
     expect(errBuffer.toString(), contains(
-      'ERROR: The --compile-commands argument is required.',
+      'ERROR: The --repo argument is required.',
     ));
   });
 
-  test('Error when --repo is missing', () async {
+  test('Error when --compile-commands path does not exist', () async {
     final StringBuffer outBuffer = StringBuffer();
     final StringBuffer errBuffer = StringBuffer();
     final ClangTidy clangTidy = ClangTidy.fromCommandLine(
       <String>[
         '--compile-commands',
+        '/does/not/exist',
+        '--repo',
         '/unused',
       ],
       outSink: outBuffer,
@@ -72,17 +77,19 @@ Future<int> main(List<String> args) async {
     expect(clangTidy.options.help, isFalse);
     expect(result, equals(1));
     expect(errBuffer.toString(), contains(
-      'ERROR: The --repo argument is required.',
+      "ERROR: Build commands path /does/not/exist doesn't exist.",
     ));
   });
 
-  test('Error when --compile-commands path does not exist', () async {
+  test('Error when --src-dir path does not exist, uses target variant in path', () async {
     final StringBuffer outBuffer = StringBuffer();
     final StringBuffer errBuffer = StringBuffer();
     final ClangTidy clangTidy = ClangTidy.fromCommandLine(
       <String>[
-        '--compile-commands',
+        '--src-dir',
         '/does/not/exist',
+        '--target-variant',
+        'ios_debug_unopt',
         '--repo',
         '/unused',
       ],
@@ -95,7 +102,7 @@ Future<int> main(List<String> args) async {
     expect(clangTidy.options.help, isFalse);
     expect(result, equals(1));
     expect(errBuffer.toString(), contains(
-      "ERROR: Build commands path /does/not/exist doesn't exist.",
+      "ERROR: Build commands path /does/not/exist/out/ios_debug_unopt/compile_commands.json doesn't exist.",
     ));
   });