Disallow -warnings-as-errors in tidy_flags (2d481842) · Commits · e / os / android_build_soong

cc/tidy.go

+7 −24

Original line number	Diff line number	Diff line
		@@ -188,31 +188,14 @@ func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags {
		tidyChecks = tidyChecks + ",-cert-err33-c"
		flags.TidyFlags = append(flags.TidyFlags, tidyChecks)

		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 {
		// 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=") {
		// 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
		}
		ctx.PropertyErrorf("tidy_flags", "should not contain -warnings-as-errors, use tidy_checks_as_errors instead")
		}
		if !inserted {
		flags.TidyFlags = append(flags.TidyFlags, "-warnings-as-errors=-*")
		}
		} else if len(tidy.Properties.Tidy_checks_as_errors) > 0 {
		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)
		}

cc/tidy_test.go

+70 −0

Original line number	Diff line number	Diff line
		@@ -22,6 +22,76 @@ 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