From 80e3e03aa9b575d4d314ecbee148bcdd5b0d4e23 Mon Sep 17 00:00:00 2001
From: Chih-Hung Hsieh <chh@google.com>
Date: Thu, 2 Jun 2022 19:55:15 -0700
Subject: [PATCH] Shorten the -checks flag in clang-tidy rules

* If a module defines tidy_checks with "-*",
  pass only "-*" and checks after it to clang-tidy.

Test: make tidy-soong_subset
Change-Id: I2a4a6111f67b934bc29e4e4fe8596a8dce4e7031
---
 cc/tidy.go      | 19 +++++++++++--
 cc/tidy_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/cc/tidy.go b/cc/tidy.go
index ac1521ba1..ff49c64bf 100644
--- a/cc/tidy.go
+++ b/cc/tidy.go
@@ -144,8 +144,23 @@ func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags {
 		tidyChecks += config.TidyChecksForDir(ctx.ModuleDir())
 	}
 	if len(tidy.Properties.Tidy_checks) > 0 {
-		tidyChecks = tidyChecks + "," + strings.Join(esc(ctx, "tidy_checks",
-			config.ClangRewriteTidyChecks(tidy.Properties.Tidy_checks)), ",")
+		// If Tidy_checks contains "-*", ignore all checks before "-*".
+		localChecks := tidy.Properties.Tidy_checks
+		ignoreGlobalChecks := false
+		for n, check := range tidy.Properties.Tidy_checks {
+			if check == "-*" {
+				ignoreGlobalChecks = true
+				localChecks = tidy.Properties.Tidy_checks[n:]
+			}
+		}
+		if ignoreGlobalChecks {
+			tidyChecks = "-checks=" + strings.Join(esc(ctx, "tidy_checks",
+				config.ClangRewriteTidyChecks(localChecks)), ",")
+		} else {
+			tidyChecks = tidyChecks + "," + strings.Join(esc(ctx, "tidy_checks",
+				config.ClangRewriteTidyChecks(localChecks)), ",")
+		}
+
 	}
 	if ctx.Windows() {
 		// https://b.corp.google.com/issues/120614316
diff --git a/cc/tidy_test.go b/cc/tidy_test.go
index 339b30281..14b33b239 100644
--- a/cc/tidy_test.go
+++ b/cc/tidy_test.go
@@ -16,11 +16,83 @@ package cc
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"android/soong/android"
 )
 
+func TestTidyChecks(t *testing.T) {
+	// The "tidy_checks" property defines additional checks appended
+	// to global default. But there are some checks disabled after
+	// the local tidy_checks.
+	bp := `
+		cc_library_shared { // has global checks + extraGlobalChecks
+			name: "libfoo_1",
+			srcs: ["foo.c"],
+		}
+		cc_library_shared { // has only local checks + extraGlobalChecks
+			name: "libfoo_2",
+			srcs: ["foo.c"],
+			tidy_checks: ["-*", "xyz-*"],
+		}
+		cc_library_shared { // has global checks + local checks + extraGlobalChecks
+			name: "libfoo_3",
+			srcs: ["foo.c"],
+			tidy_checks: ["-abc*", "xyz-*", "mycheck"],
+		}
+		cc_library_shared { // has only local checks after "-*" + extraGlobalChecks
+			name: "libfoo_4",
+			srcs: ["foo.c"],
+			tidy_checks: ["-abc*", "xyz-*", "mycheck", "-*", "xyz-*"],
+		}`
+	ctx := testCc(t, bp)
+
+	globalChecks := "-checks=${config.TidyDefaultGlobalChecks},"
+	firstXyzChecks := "-checks='-*','xyz-*',"
+	localXyzChecks := "'-*','xyz-*'"
+	localAbcChecks := "'-abc*','xyz-*',mycheck"
+	extraGlobalChecks := ",-bugprone-easily-swappable-parameters,"
+	testCases := []struct {
+		libNumber int      // 1,2,3,...
+		checks    []string // must have substrings in -checks
+		noChecks  []string // must not have substrings in -checks
+	}{
+		{1, []string{globalChecks, extraGlobalChecks}, []string{localXyzChecks, localAbcChecks}},
+		{2, []string{firstXyzChecks, extraGlobalChecks}, []string{globalChecks, localAbcChecks}},
+		{3, []string{globalChecks, localAbcChecks, extraGlobalChecks}, []string{localXyzChecks}},
+		{4, []string{firstXyzChecks, extraGlobalChecks}, []string{globalChecks, localAbcChecks}},
+	}
+	t.Run("caseTidyChecks", func(t *testing.T) {
+		variant := "android_arm64_armv8-a_shared"
+		for _, test := range testCases {
+			libName := fmt.Sprintf("libfoo_%d", test.libNumber)
+			flags := ctx.ModuleForTests(libName, variant).Rule("clangTidy").Args["tidyFlags"]
+			splitFlags := strings.Split(flags, " ")
+			foundCheckFlag := false
+			for _, flag := range splitFlags {
+				if strings.HasPrefix(flag, "-checks=") {
+					foundCheckFlag = true
+					for _, check := range test.checks {
+						if !strings.Contains(flag, check) {
+							t.Errorf("tidyFlags for %s does not contain %s.", libName, check)
+						}
+					}
+					for _, check := range test.noChecks {
+						if strings.Contains(flag, check) {
+							t.Errorf("tidyFlags for %s should not contain %s.", libName, check)
+						}
+					}
+					break
+				}
+			}
+			if !foundCheckFlag {
+				t.Errorf("tidyFlags for %s does not contain -checks=.", libName)
+			}
+		}
+	})
+}
+
 func TestWithTidy(t *testing.T) {
 	// When WITH_TIDY=1 or (ALLOW_LOCAL_TIDY_TRUE=1 and local tidy:true)
 	// a C++ library should depend on .tidy files.