From 43b920e707a61717887a728e868289aa4bda1c56 Mon Sep 17 00:00:00 2001
From: Chih-Hung Hsieh <chh@google.com>
Date: Thu, 9 Jun 2022 17:58:41 -0700
Subject: [PATCH] Disable tidy checks in TidyGlobalNoChecks

* Save repeated strings in TidyFlags of build.ninja rules.
* Some of these checks were disabled to upgrade clang-tidy.
  They could later be moved to TidyDefaultGlobalChecks and
  TidyExternalVendorChecks if not breaking the build.
  Some projects can then enable those checks locally.

Test: WITH_TIDY=1 make; make tidy-soong_subset
Change-Id: I70e4218c929e3c88f766f2c68c56c51356110e72
---
 cc/config/tidy.go | 31 +++++++++++++++++++++++++++++++
 cc/tidy.go        | 21 ++-------------------
 cc/tidy_test.go   |  2 +-
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/cc/config/tidy.go b/cc/config/tidy.go
index 826197a7e..f96f3ed41 100644
--- a/cc/config/tidy.go
+++ b/cc/config/tidy.go
@@ -94,6 +94,31 @@ func init() {
 		}, ",")
 	})
 
+	// Some clang-tidy checks have bugs or not work for Android.
+	// They are disabled here, overriding any locally selected checks.
+	pctx.VariableFunc("TidyGlobalNoChecks", func(ctx android.PackageVarContext) string {
+		return strings.Join([]string{
+			// https://b.corp.google.com/issues/153464409
+			// many local projects enable cert-* checks, which
+			// trigger bugprone-reserved-identifier.
+			"-bugprone-reserved-identifier*,-cert-dcl51-cpp,-cert-dcl37-c",
+			// http://b/153757728
+			"-readability-qualified-auto",
+			// http://b/155034563
+			"-bugprone-signed-char-misuse",
+			// http://b/155034972
+			"-bugprone-branch-clone",
+			// http://b/193716442
+			"-bugprone-implicit-widening-of-multiplication-result",
+			// Too many existing functions trigger this rule, and fixing it requires large code
+			// refactoring. The cost of maintaining this tidy rule outweighs the benefit it brings.
+			"-bugprone-easily-swappable-parameters",
+			// http://b/216364337 - TODO: Follow-up after compiler update to
+			// disable or fix individual instances.
+			"-cert-err33-c",
+		}, ",")
+	})
+
 	// To reduce duplicate warnings from the same header files,
 	// header-filter will contain only the module directory and
 	// those specified by DEFAULT_TIDY_HEADER_DIRS.
@@ -115,6 +140,7 @@ type PathBasedTidyCheck struct {
 const tidyDefault = "${config.TidyDefaultGlobalChecks}"
 const tidyExternalVendor = "${config.TidyExternalVendorChecks}"
 const tidyDefaultNoAnalyzer = "${config.TidyDefaultGlobalChecks},-clang-analyzer-*"
+const tidyGlobalNoChecks = "${config.TidyGlobalNoChecks}"
 
 // This is a map of local path prefixes to the set of default clang-tidy checks
 // to be used.
@@ -152,6 +178,11 @@ func TidyChecksForDir(dir string) string {
 	return tidyDefault
 }
 
+// Returns a globally disabled tidy checks, overriding locally selected checks.
+func TidyGlobalNoChecks() string {
+	return tidyGlobalNoChecks
+}
+
 func TidyFlagsForSrcFile(srcFile android.Path, flags string) string {
 	// Disable clang-analyzer-* checks globally for generated source files
 	// because some of them are too huge. Local .bp files can add wanted
diff --git a/cc/tidy.go b/cc/tidy.go
index e8e17831c..d465fbdb5 100644
--- a/cc/tidy.go
+++ b/cc/tidy.go
@@ -160,32 +160,15 @@ func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags {
 			tidyChecks = tidyChecks + "," + strings.Join(esc(ctx, "tidy_checks",
 				config.ClangRewriteTidyChecks(localChecks)), ",")
 		}
-
 	}
+	tidyChecks = tidyChecks + "," + config.TidyGlobalNoChecks()
 	if ctx.Windows() {
 		// https://b.corp.google.com/issues/120614316
 		// mingw32 has cert-dcl16-c warning in NO_ERROR,
 		// which is used in many Android files.
 		tidyChecks = tidyChecks + ",-cert-dcl16-c"
 	}
-	// https://b.corp.google.com/issues/153464409
-	// many local projects enable cert-* checks, which
-	// trigger bugprone-reserved-identifier.
-	tidyChecks = tidyChecks + ",-bugprone-reserved-identifier*,-cert-dcl51-cpp,-cert-dcl37-c"
-	// http://b/153757728
-	tidyChecks = tidyChecks + ",-readability-qualified-auto"
-	// http://b/155034563
-	tidyChecks = tidyChecks + ",-bugprone-signed-char-misuse"
-	// http://b/155034972
-	tidyChecks = tidyChecks + ",-bugprone-branch-clone"
-	// http://b/193716442
-	tidyChecks = tidyChecks + ",-bugprone-implicit-widening-of-multiplication-result"
-	// Too many existing functions trigger this rule, and fixing it requires large code
-	// refactoring. The cost of maintaining this tidy rule outweighs the benefit it brings.
-	tidyChecks = tidyChecks + ",-bugprone-easily-swappable-parameters"
-	// http://b/216364337 - TODO: Follow-up after compiler update to
-	// disable or fix individual instances.
-	tidyChecks = tidyChecks + ",-cert-err33-c"
+
 	flags.TidyFlags = append(flags.TidyFlags, tidyChecks)
 
 	// Embedding -warnings-as-errors in tidy_flags is error-prone.
diff --git a/cc/tidy_test.go b/cc/tidy_test.go
index 5863a6c17..a6b8a55d7 100644
--- a/cc/tidy_test.go
+++ b/cc/tidy_test.go
@@ -122,7 +122,7 @@ func TestTidyChecks(t *testing.T) {
 	firstXyzChecks := "-checks='-*','xyz-*',"
 	localXyzChecks := "'-*','xyz-*'"
 	localAbcChecks := "'-abc*','xyz-*',mycheck"
-	extraGlobalChecks := ",-bugprone-easily-swappable-parameters,"
+	extraGlobalChecks := ",${config.TidyGlobalNoChecks}"
 	testCases := []struct {
 		libNumber int      // 1,2,3,...
 		checks    []string // must have substrings in -checks