Revert "Disallow -warnings-as-errors in tidy_flags"
This reverts commit 2d481842b1be6f5bd14331258e10b7cee6d9e2e5.
Reason for revert: b/235569235#comment4
Change-Id: I5c87b1d5a1bdf0bad7f436c397688fe3fc20d04e
diff --git a/cc/tidy.go b/cc/tidy.go
index e8e1783..ff49c64 100644
--- a/cc/tidy.go
+++ b/cc/tidy.go
@@ -188,14 +188,31 @@
tidyChecks = tidyChecks + ",-cert-err33-c"
flags.TidyFlags = append(flags.TidyFlags, tidyChecks)
- // Embedding -warnings-as-errors in tidy_flags is error-prone.
- // It should be replaced with the tidy_checks_as_errors list.
- for _, s := range flags.TidyFlags {
- if strings.Contains(s, "-warnings-as-errors=") {
- ctx.PropertyErrorf("tidy_flags", "should not contain -warnings-as-errors, use tidy_checks_as_errors instead")
+ if ctx.Config().IsEnvTrue("WITH_TIDY") {
+ // WITH_TIDY=1 enables clang-tidy globally. There could be many unexpected
+ // warnings from new checks and many local tidy_checks_as_errors and
+ // -warnings-as-errors can break a global build.
+ // So allow all clang-tidy warnings.
+ inserted := false
+ for i, s := range flags.TidyFlags {
+ if strings.Contains(s, "-warnings-as-errors=") {
+ // clang-tidy accepts only one -warnings-as-errors
+ // replace the old one
+ re := regexp.MustCompile(`'?-?-warnings-as-errors=[^ ]* *`)
+ newFlag := re.ReplaceAllString(s, "")
+ if newFlag == "" {
+ flags.TidyFlags[i] = "-warnings-as-errors=-*"
+ } else {
+ flags.TidyFlags[i] = newFlag + " -warnings-as-errors=-*"
+ }
+ inserted = true
+ break
+ }
}
- }
- if len(tidy.Properties.Tidy_checks_as_errors) > 0 {
+ if !inserted {
+ flags.TidyFlags = append(flags.TidyFlags, "-warnings-as-errors=-*")
+ }
+ } else if len(tidy.Properties.Tidy_checks_as_errors) > 0 {
tidyChecksAsErrors := "-warnings-as-errors=" + strings.Join(esc(ctx, "tidy_checks_as_errors", tidy.Properties.Tidy_checks_as_errors), ",")
flags.TidyFlags = append(flags.TidyFlags, tidyChecksAsErrors)
}
diff --git a/cc/tidy_test.go b/cc/tidy_test.go
index 5863a6c..14b33b2 100644
--- a/cc/tidy_test.go
+++ b/cc/tidy_test.go
@@ -22,76 +22,6 @@
"android/soong/android"
)
-func TestTidyFlagsWarningsAsErrors(t *testing.T) {
- // The "tidy_flags" property should not contain -warnings-as-errors.
- testCases := []struct {
- libName, bp string
- errorMsg string // a negative test; must have error message
- flags []string // must have substrings in tidyFlags
- noFlags []string // must not have substrings in tidyFlags
- }{
- {
- "libfoo1",
- `cc_library_shared { // no warnings-as-errors, good tidy_flags
- name: "libfoo1",
- srcs: ["foo.c"],
- tidy_flags: ["-header-filter=dir1/"],
- }`,
- "",
- []string{"-header-filter=dir1/"},
- []string{"-warnings-as-errors"},
- },
- {
- "libfoo2",
- `cc_library_shared { // good use of tidy_checks_as_errors
- name: "libfoo2",
- srcs: ["foo.c"],
- tidy_checks_as_errors: ["xyz-*", "abc"],
- tidy_flags: ["-header-filter=dir2/"],
- }`,
- "",
- []string{"-header-filter=dir2/", "-warnings-as-errors='xyz-*',abc"},
- []string{},
- },
- {
- "libfoo3",
- `cc_library_shared { // bad use of -warnings-as-errors in tidy_flags
- name: "libfoo3",
- srcs: ["foo.c"],
- tidy_flags: [
- "-header-filters=.*",
- "-warnings-as-errors=xyz-*",
- ],
- }`,
- `module "libfoo3" .*: tidy_flags: should not contain -warnings-as-errors,` +
- ` use tidy_checks_as_errors instead`,
- []string{},
- []string{},
- },
- }
- for _, test := range testCases {
- if test.errorMsg != "" {
- testCcError(t, test.errorMsg, test.bp)
- continue
- }
- variant := "android_arm64_armv8-a_shared"
- ctx := testCc(t, test.bp)
- t.Run("caseTidyFlags", func(t *testing.T) {
- flags := ctx.ModuleForTests(test.libName, variant).Rule("clangTidy").Args["tidyFlags"]
- for _, flag := range test.flags {
- if !strings.Contains(flags, flag) {
- t.Errorf("tidyFlags for %s does not contain %s.", test.libName, flag)
- }
- }
- for _, flag := range test.noFlags {
- if strings.Contains(flags, flag) {
- t.Errorf("tidyFlags for %s should not contain %s.", test.libName, flag)
- }
- }
- })
- }
-}
-
func TestTidyChecks(t *testing.T) {
// The "tidy_checks" property defines additional checks appended
// to global default. But there are some checks disabled after