Enable clang-tidy for pre-push (opt-out), exclude performance-unnecessary-value-param

.clang-tidy

@@ -1,6 +1,8 @@
-# Prefix check with "-" to ignore.
-# Note: Some of the checks here are used as errors selectively, see
-# //ci/lint.sh
+# Keep in sync with .clang-tidy-for-githooks.
+#
+# These checks are run by the CI presubmit script, but are not run by the
+# githooks presubmit script. The githooks presubmit script runs a subset of
+# these checks (often due to performance reasons, i.e. it taking forever).
 Checks: "bugprone-use-after-move,\
 bugprone-unchecked-optional-access,\
 clang-analyzer-*,\

.clang-tidy-for-githooks

@@ -0,0 +1,41 @@
+# Keep in sync with .clang-tidy.
+#
+# These checks are run by the git hooks presubmit script, but are not run by the
+# CI presubmit script. The CI presubmit script runs a superset of these checks
+# (it's OK if CI takes 10s of minutes, but not git push).
+Checks: "bugprone-use-after-move,\
+bugprone-unchecked-optional-access,\
+clang-analyzer-*,\
+clang-diagnostic-*,\
+darwin-*,\
+google-*,\
+modernize-use-default-member-init,\
+objc-*,\
+-objc-nsinvocation-argument-lifetime,\
+readability-identifier-naming,\
+-google-build-using-namespace,\
+-google-default-arguments,\
+-google-objc-global-variable-declaration,\
+-google-objc-avoid-throwing-exception,\
+-google-readability-casting,\
+-clang-analyzer-nullability.NullPassedToNonnull,\
+-clang-analyzer-nullability.NullablePassedToNonnull,\
+-clang-analyzer-nullability.NullReturnedFromNonnull,\
+-clang-analyzer-nullability.NullableReturnedFromNonnull,\
+performance-move-const-arg"
+
+CheckOptions:
+  - key:    modernize-use-default-member-init.UseAssignment
+    value:  true
+  - key:    readability-identifier-naming.EnumConstantCase
+    value:  'CamelCase'
+  - key:    readability-identifier-naming.EnumConstantPrefix
+    value:  'k'
+  - key:    readability-identifier-naming.GlobalConstantCase
+    value:  'CamelCase'
+  - key:    readability-identifier-naming.GlobalConstantPrefix
+    value:  'k'
+  - key:    readability-identifier-naming.PrivateMemberCase
+    value:  'lower_case'
+  - key:    readability-identifier-naming.PrivateMemberSuffix
+    value:  '_'

ci/licenses_golden/excluded_files

@@ -14,6 +14,7 @@
 ../../../flutter/.ci.yaml
 ../../../flutter/.clang-format
 ../../../flutter/.clang-tidy
+../../../flutter/.clang-tidy-for-githooks
 ../../../flutter/.git
 ../../../flutter/.gitattributes
 ../../../flutter/.github

tools/clang_tidy/lib/clang_tidy.dart

@@ -47,6 +47,7 @@ class ClangTidy {
   /// an instance of [ClangTidy].
   ///
   /// `buildCommandsPath` is the path to the build_commands.json file.
+  /// `configPath` is a path to a `.clang-tidy` config file.
   /// `repoPath` is the path to the Engine repo.
   /// `checksArg` are specific checks for clang-tidy to do.
   /// `lintAll` when true indicates that all files should be linted.
@@ -56,6 +57,7 @@ class ClangTidy {
   /// will otherwise go to stderr.
   ClangTidy({
     required io.File buildCommandsPath,
+    io.File? configPath,
     String checksArg = '',
     bool lintAll = false,
     bool lintHead = false,
@@ -65,6 +67,7 @@ class ClangTidy {
   }) :
     options = Options(
       buildCommandsPath: buildCommandsPath,
+      configPath: configPath,
       checksArg: checksArg,
       lintAll: lintAll,
       lintHead: lintHead,

tools/clang_tidy/lib/src/command.dart

@@ -135,6 +135,8 @@ class Command {
     final List<String> args = <String>[
       filePath,
       '--warnings-as-errors=${options.warningsAsErrors ?? '*'}',
+      if (options.configPath != null)
+        '--config-file=${options.configPath}',
       if (options.checks != null)
         options.checks!,
       if (options.fix) ...<String>[

tools/clang_tidy/lib/src/options.dart

@@ -28,6 +28,7 @@ class Options {
     required this.buildCommandsPath,
     this.help = false,
     this.verbose = false,
+    this.configPath,
     this.checksArg = '',
     this.lintAll = false,
     this.lintHead = false,
@@ -70,10 +71,21 @@ class Options {
     required List<io.File> shardCommandsPaths,
     int? shardId,
   }) {
+    io.File? configPath;
+    if (options.wasParsed('config-file')) {
+      configPath = io.File(options['config-file'] as String);
+      if (!configPath.existsSync()) {
+        return Options._error(
+          'ERROR: Config file ${configPath.absolute.path} does not exist.',
+          errSink: errSink,
+        );
+      }
+    }
     return Options(
       help: options['help'] as bool,
       verbose: options['verbose'] as bool,
       buildCommandsPath: buildCommandsPath,
+      configPath: configPath,
       checksArg: options.wasParsed('checks') ? options['checks'] as String : '',
       lintAll: io.Platform.environment['FLUTTER_LINT_ALL'] != null ||
                options['lint-all'] as bool,
@@ -197,6 +209,11 @@ class Options {
             'string, indicating all checks should be performed.',
       defaultsTo: '',
     )
+    ..addOption(
+      'config-file',
+      help: 'Path to a .clang-tidy configuration file.',
+      valueHelp: 'path/to/.clang-tidy',
+    )
     ..addFlag(
       'enable-check-profile',
       help: 'Enable per-check timing profiles and print a report to stderr.',
@@ -212,6 +229,9 @@ class Options {
   /// The location of the compile_commands.json file.
   final io.File buildCommandsPath;
 
+  /// A location of a `.clang-tidy` configuration file.
+  final io.File? configPath;
+
   /// The location of shard compile_commands.json files.
   final List<io.File> shardCommandsPaths;
 

tools/clang_tidy/test/clang_tidy_test.dart

@@ -148,6 +148,44 @@ Future<int> main(List<String> args) async {
     print(outBuffer);
   });
 
+  test('Accepts --config-file', () async {
+    // If buildCommands is in "$ENGINE/src/out/host_debug", then the config
+    // file should be in "$ENGINE/src/flutter/.clang-tidy-for-githooks".
+    late final String flutterRoot;
+
+    // Find the 'src' directory and append 'flutter/.clang-tidy-for-githooks'.
+    {
+      final List<String> buildCommandParts = path.split(path.absolute(buildCommands));
+      for (int i = 0; i < buildCommandParts.length; ++i) {
+        if (buildCommandParts[i] == 'src') {
+          flutterRoot = path.joinAll(<String>[
+            ...buildCommandParts.sublist(0, i + 1),
+            'flutter',
+          ]);
+          break;
+        }
+      }
+    }
+
+    final StringBuffer outBuffer = StringBuffer();
+    final StringBuffer errBuffer = StringBuffer();
+    final ClangTidy clangTidy = ClangTidy.fromCommandLine(
+      <String>[
+        '--compile-commands',
+        buildCommands,
+        '--config-file=${path.join(flutterRoot, '.clang-tidy-for-githooks')}',
+      ],
+      outSink: outBuffer,
+      errSink: errBuffer,
+    );
+
+    final int result = await clangTidy.run();
+
+    expect(result, equals(0));
+    expect(clangTidy.options.configPath?.path, endsWith('.clang-tidy-for-githooks'));
+    print(outBuffer);
+  });
+
   test('shard-id valid', () async {
     _withTempFile('shard-id-valid', (String path) {
       final Options options = Options.fromCommandLine( <String>[

tools/githooks/lib/src/pre_push_command.dart

@@ -24,8 +24,9 @@ class PrePushCommand extends Command<bool> {
     final String flutterRoot = globalResults!['flutter']! as String;
 
     if (!enableClangTidy) {
-      print('The clang-tidy check is disabled. To enable set the environment '
-            'variable PRE_PUSH_CLANG_TIDY to any value.');
+      print(
+        'The clang-tidy check was explicitly disabled. To enable clear '
+        'the environment variable PRE_PUSH_CLANG_TIDY or set it to true.');
     }
 
     final List<bool> checkResults = <bool>[
@@ -38,36 +39,31 @@ class PrePushCommand extends Command<bool> {
     return !checkResults.contains(false);
   }
 
+  static const List<String> _checkForHostTargets = <String>[
+    'host_debug_unopt_arm64',
+    'host_debug_arm64',
+    'host_debug_unopt',
+    'host_debug',
+  ];
+
   Future<bool> _runClangTidy(String flutterRoot, bool verbose) async {
     io.stdout.writeln('Starting clang-tidy checks.');
     final Stopwatch sw = Stopwatch()..start();
-    // First ensure that out/host_debug/compile_commands.json exists by running
-    // //flutter/tools/gn.
-    io.File compileCommands = io.File(path.join(
-      flutterRoot,
-      '..',
-      'out',
-      'host_debug',
-      'compile_commands.json',
-    ));
-    if (!compileCommands.existsSync()) {
-      compileCommands = io.File(path.join(
-        flutterRoot,
-        '..',
-        'out',
-        'host_debug_unopt',
-        'compile_commands.json',
-      ));
-      if (!compileCommands.existsSync()) {
-        io.stderr.writeln('clang-tidy requires a fully built host_debug or '
-                          'host_debug_unopt build directory');
-        return false;
-      }
+    // First ensure that out/host_{{flags}}/compile_commands.json exists by running
+    // //flutter/tools/gn. See _checkForHostTargets above for supported targets.
+    final io.File? compileCommands = _findCompileCommands(flutterRoot);
+    if (compileCommands == null) {
+      io.stderr.writeln(
+        'clang-tidy requires a fully built host directory, such as: '
+        '${_checkForHostTargets.join(', ')}.'
+      );
+      return false;
     }
     final StringBuffer outBuffer = StringBuffer();
     final StringBuffer errBuffer = StringBuffer();
     final ClangTidy clangTidy = ClangTidy(
       buildCommandsPath: compileCommands,
+      configPath: io.File(path.join(flutterRoot, '.clang-tidy-for-githooks')),
       outSink: outBuffer,
       errSink: errBuffer,
     );
@@ -81,6 +77,22 @@ class PrePushCommand extends Command<bool> {
     return true;
   }
 
+  io.File? _findCompileCommands(String flutterRoot) {
+    for (final String dir in _checkForHostTargets) {
+      final io.File file = io.File(path.join(
+        flutterRoot,
+        '..',
+        'out',
+        dir,
+        'compile_commands.json',
+      ));
+      if (file.existsSync()) {
+        return file;
+      }
+    }
+    return null;
+  }
+
   Future<bool> _runFormatter(String flutterRoot, bool verbose) async {
     io.stdout.writeln('Starting formatting checks.');
     final Stopwatch sw = Stopwatch()..start();

tools/githooks/pre-push

@@ -23,7 +23,7 @@ def Main(argv):
     FLUTTER_DIR,
   ]
 
-  if ENABLE_CLANG_TIDY is not None:
+  if ENABLE_CLANG_TIDY is not False:
     githook_args += [
       '--enable-clang-tidy',
     ]

tools/licenses/lib/paths.dart

@@ -20,6 +20,7 @@ final Set<String> skippedPaths = <String>{
   r'build', // only used by build
   r'build_overrides', // only used by build
   r'buildtools', // only used by build
+  r'flutter/.clang-tidy-for-githooks', // nearly identical to .clang-tidy
   r'flutter/build',
   r'flutter/ci',
   r'flutter/docs',