diff --git a/cc/tidy.go b/cc/tidy.go index e8e17831c..ff49c64bf 100644 --- a/cc/tidy.go +++ b/cc/tidy.go @@ -188,14 +188,31 @@ func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags { 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 5863a6c17..14b33b239 100644 --- a/cc/tidy_test.go +++ b/cc/tidy_test.go @@ -22,76 +22,6 @@ import ( "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