Merge "rust: modify linting properties" (2346b014) · Commits · e / os / android_build_soong

rust/clippy.go

+11 −5

Original line number	Diff line number	Diff line
		@@ -19,8 +19,14 @@ import (
		)

		type ClippyProperties struct {
		// whether to run clippy.
		Clippy *bool
		// name of the lint set that should be used to validate this module.
		//
		// Possible values are "default" (for using a sensible set of lints
		// depending on the module's location), "android" (for the strictest
		// lint set that applies to all Android platform code), "vendor" (for a
		// relaxed set) and "none" (to disable the execution of clippy). The
		// default value is "default". See also the `lints` property.
		Clippy_lints *string
		}

		type clippy struct {
		@@ -32,10 +38,10 @@ func (c *clippy) props() []interface{} {
		}

		func (c *clippy) flags(ctx ModuleContext, flags Flags, deps PathDeps) (Flags, PathDeps) {
		if c.Properties.Clippy != nil && !*c.Properties.Clippy {
		return flags, deps
		enabled, lints, err := config.ClippyLintsForDir(ctx.ModuleDir(), c.Properties.Clippy_lints)
		if err != nil {
		ctx.PropertyErrorf("clippy_lints", err.Error())
		}
		enabled, lints := config.ClippyLintsForDir(ctx.ModuleDir())
		flags.Clippy = enabled
		flags.ClippyFlags = append(flags.ClippyFlags, lints)
		return flags, deps

rust/clippy_test.go

+57 −11

Original line number	Diff line number	Diff line
		@@ -16,31 +16,77 @@ package rust

		import (
		"testing"

		"android/soong/android"
		)

		func TestClippy(t *testing.T) {
		ctx := testRust(t, `

		bp := `
		// foo uses the default value of clippy_lints
		rust_library {
		name: "libfoo",
		srcs: ["foo.rs"],
		crate_name: "foo",
		}
		// bar forces the use of the "android" lint set
		rust_library {
		name: "libbar",
		srcs: ["foo.rs"],
		crate_name: "bar",
		clippy_lints: "android",
		}
		// foobar explicitly disable clippy
		rust_library {
		name: "libfoobar",
		srcs: ["foo.rs"],
		crate_name: "foobar",
		clippy: false,
		}`)
		clippy_lints: "none",
		}`

		bp = bp + GatherRequiredDepsForTest()

		fs := map[string][]byte{
		// Reuse the same blueprint file for subdirectories.
		"external/Android.bp": []byte(bp),
		"hardware/Android.bp": []byte(bp),
		}

		var clippyLintTests = []struct {
		modulePath string
		fooFlags string
		}{
		{"", "${config.ClippyDefaultLints}"},
		{"external/", ""},
		{"hardware/", "${config.ClippyVendorLints}"},
		}

		for _, tc := range clippyLintTests {
		t.Run("path="+tc.modulePath, func(t *testing.T) {

		config := android.TestArchConfig(buildDir, nil, bp, fs)
		ctx := CreateTestContext()
		ctx.Register(config)
		_, errs := ctx.ParseFileList(".", []string{tc.modulePath + "Android.bp"})
		android.FailIfErrored(t, errs)
		_, errs = ctx.PrepareBuildActions(config)
		android.FailIfErrored(t, errs)

		r := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib").MaybeRule("clippy")
		if r.Args["clippyFlags"] != tc.fooFlags {
		t.Errorf("Incorrect flags for libfoo: %q, want %q", r.Args["clippyFlags"], tc.fooFlags)
		}

		r = ctx.ModuleForTests("libbar", "android_arm64_armv8-a_dylib").MaybeRule("clippy")
		if r.Args["clippyFlags"] != "${config.ClippyDefaultLints}" {
		t.Errorf("Incorrect flags for libbar: %q, want %q", r.Args["clippyFlags"], "${config.ClippyDefaultLints}")
		}

		ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib").Output("libfoo.dylib.so")
		fooClippy := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib").MaybeRule("clippy")
		if fooClippy.Rule.String() != "android/soong/rust.clippy" {
		t.Errorf("Clippy output (default) for libfoo was not generated: %+v", fooClippy)
		r = ctx.ModuleForTests("libfoobar", "android_arm64_armv8-a_dylib").MaybeRule("clippy")
		if r.Rule != nil {
		t.Errorf("libfoobar is setup to use clippy when explicitly disabled: clippyFlags=%q", r.Args["clippyFlags"])
		}

		ctx.ModuleForTests("libfoobar", "android_arm64_armv8-a_dylib").Output("libfoobar.dylib.so")
		foobarClippy := ctx.ModuleForTests("libfoobar", "android_arm64_armv8-a_dylib").MaybeRule("clippy")
		if foobarClippy.Rule != nil {
		t.Errorf("Clippy output for libfoobar is not empty")
		})
		}
		}

rust/compiler.go

+14 −8

Original line number	Diff line number	Diff line
		@@ -32,8 +32,8 @@ func (compiler *baseCompiler) setNoStdlibs() {
		compiler.Properties.No_stdlibs = proptools.BoolPtr(true)
		}

		func (compiler *baseCompiler) setNoLint() {
		compiler.Properties.No_lint = proptools.BoolPtr(true)
		func (compiler *baseCompiler) disableLints() {
		compiler.Properties.Lints = proptools.StringPtr("none")
		}

		func NewBaseCompiler(dir, dir64 string, location installLocation) *baseCompiler {
		@@ -58,8 +58,14 @@ type BaseCompilerProperties struct {
		// path to the source file that is the main entry point of the program (e.g. main.rs or lib.rs)
		Srcs []string `android:"path,arch_variant"`

		// whether to suppress the standard lint flags - default to false
		No_lint *bool
		// name of the lint set that should be used to validate this module.
		//
		// Possible values are "default" (for using a sensible set of lints
		// depending on the module's location), "android" (for the strictest
		// lint set that applies to all Android platform code), "vendor" (for
		// a relaxed set) and "none" (for ignoring all lint warnings and
		// errors). The default value is "default".
		Lints *string

		// flags to pass to rustc
		Flags []string `android:"path,arch_variant"`
		@@ -159,11 +165,11 @@ func (compiler *baseCompiler) featuresToFlags(features []string) []string {

		func (compiler *baseCompiler) compilerFlags(ctx ModuleContext, flags Flags) Flags {

		if Bool(compiler.Properties.No_lint) {
		flags.RustFlags = append(flags.RustFlags, config.AllowAllLints)
		} else {
		flags.RustFlags = append(flags.RustFlags, config.RustcLintsForDir(ctx.ModuleDir()))
		lintFlags, err := config.RustcLintsForDir(ctx.ModuleDir(), compiler.Properties.Lints)
		if err != nil {
		ctx.PropertyErrorf("lints", err.Error())
		}
		flags.RustFlags = append(flags.RustFlags, lintFlags)
		flags.RustFlags = append(flags.RustFlags, compiler.Properties.Flags...)
		flags.RustFlags = append(flags.RustFlags, compiler.featuresToFlags(compiler.Properties.Features)...)
		flags.RustFlags = append(flags.RustFlags, "--edition="+compiler.edition())

rust/compiler_test.go

+73 −0

Original line number	Diff line number	Diff line
		@@ -17,6 +17,8 @@ package rust
		import (
		"strings"
		"testing"

		"android/soong/android"
		)

		// Test that feature flags are being correctly generated.
		@@ -104,3 +106,74 @@ func TestInstallDir(t *testing.T) {
		t.Fatalf("unexpected install path for binary: %#v", install_path_bin)
		}
		}

		func TestLints(t *testing.T) {

		bp := `
		// foo uses the default value of lints
		rust_library {
		name: "libfoo",
		srcs: ["foo.rs"],
		crate_name: "foo",
		}
		// bar forces the use of the "android" lint set
		rust_library {
		name: "libbar",
		srcs: ["foo.rs"],
		crate_name: "bar",
		lints: "android",
		}
		// foobar explicitly disable all lints
		rust_library {
		name: "libfoobar",
		srcs: ["foo.rs"],
		crate_name: "foobar",
		lints: "none",
		}`

		bp = bp + GatherRequiredDepsForTest()

		fs := map[string][]byte{
		// Reuse the same blueprint file for subdirectories.
		"external/Android.bp": []byte(bp),
		"hardware/Android.bp": []byte(bp),
		}

		var lintTests = []struct {
		modulePath string
		fooFlags string
		}{
		{"", "${config.RustDefaultLints}"},
		{"external/", "${config.RustAllowAllLints}"},
		{"hardware/", "${config.RustVendorLints}"},
		}

		for _, tc := range lintTests {
		t.Run("path="+tc.modulePath, func(t *testing.T) {

		config := android.TestArchConfig(buildDir, nil, bp, fs)
		ctx := CreateTestContext()
		ctx.Register(config)
		_, errs := ctx.ParseFileList(".", []string{tc.modulePath + "Android.bp"})
		android.FailIfErrored(t, errs)
		_, errs = ctx.PrepareBuildActions(config)
		android.FailIfErrored(t, errs)

		r := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib").MaybeRule("rustc")
		if !strings.Contains(r.Args["rustcFlags"], tc.fooFlags) {
		t.Errorf("Incorrect flags for libfoo: %q, want %q", r.Args["rustcFlags"], tc.fooFlags)
		}

		r = ctx.ModuleForTests("libbar", "android_arm64_armv8-a_dylib").MaybeRule("rustc")
		if !strings.Contains(r.Args["rustcFlags"], "${config.RustDefaultLints}") {
		t.Errorf("Incorrect flags for libbar: %q, want %q", r.Args["rustcFlags"], "${config.RustDefaultLints}")
		}

		r = ctx.ModuleForTests("libfoobar", "android_arm64_armv8-a_dylib").MaybeRule("rustc")
		if !strings.Contains(r.Args["rustcFlags"], "${config.RustAllowAllLints}") {
		t.Errorf("Incorrect flags for libfoobar: %q, want %q", r.Args["rustcFlags"], "${config.RustAllowAllLints}")
		}

		})
		}
		}

rust/config/lints.go

+76 −37

Original line number	Diff line number	Diff line
		@@ -15,6 +15,7 @@
		package config

		import (
		"fmt"
		"strings"

		"android/soong/android"
		@@ -24,18 +25,21 @@ import (
		// The Android build system tries to avoid reporting warnings during the build.
		// Therefore, by default, we upgrade warnings to denials. For some of these
		// lints, an allow exception is setup, using the variables below.
		// There are different default lints depending on the repository location (see
		// DefaultLocalClippyChecks).
		//
		// The lints are split into two categories. The first one contains the built-in
		// lints (https://doc.rust-lang.org/rustc/lints/index.html). The second is
		// specific to Clippy lints (https://rust-lang.github.io/rust-clippy/master/).
		//
		// When developing a module, it is possible to use the `no_lint` property to
		// disable the built-in lints configuration. It is also possible to set
		// `clippy` to false to disable the clippy verification. Expect some
		// questioning during review if you enable one of these options. For external/
		// code, if you need to use them, it is likely a bug. Otherwise, it may be
		// useful to add an exception (that is, move a lint from deny to allow).
		// For both categories, there are 3 levels of linting possible:
		// - "android", for the strictest lints that applies to all Android platform code.
		// - "vendor", for relaxed rules.
		// - "none", to disable the linting.
		// There is a fourth option ("default") which automatically selects the linting level
		// based on the module's location. See defaultLintSetForPath.
		//
		// When developing a module, you may set `lints = "none"` and `clippy_lints =
		// "none"` to disable all the linting. Expect some questioning during code review
		// if you enable one of these options.
		var (
		// Default Rust lints that applies to Google-authored modules.
		defaultRustcLints = []string{
		@@ -102,13 +106,6 @@ func init() {
		pctx.StaticVariable("RustAllowAllLints", strings.Join(allowAllLints, " "))
		}

		type PathBasedClippyConfig struct {
		PathPrefix string
		RustcConfig string
		ClippyEnabled bool
		ClippyConfig string
		}

		const noLint = ""
		const rustcDefault = "${config.RustDefaultLints}"
		const rustcVendor = "${config.RustVendorLints}"
		@@ -116,36 +113,78 @@ const rustcAllowAll = "${config.RustAllowAllLints}"
		const clippyDefault = "${config.ClippyDefaultLints}"
		const clippyVendor = "${config.ClippyVendorLints}"

		// This is a map of local path prefixes to a set of parameters for the linting:
		// - a string for the lints to apply to rustc.
		// - a boolean to indicate if clippy should be executed.
		// - a string for the lints to apply to clippy.
		// The first entry matching will be used.
		var DefaultLocalClippyChecks = []PathBasedClippyConfig{
		{"external/", rustcAllowAll, false, noLint},
		{"hardware/", rustcVendor, true, clippyVendor},
		{"prebuilts/", rustcAllowAll, false, noLint},
		{"vendor/google", rustcDefault, true, clippyDefault},
		{"vendor/", rustcVendor, true, clippyVendor},
		// lintConfig defines a set of lints and clippy configuration.
		type lintConfig struct {
		rustcConfig string // for the lints to apply to rustc.
		clippyEnabled bool // to indicate if clippy should be executed.
		clippyConfig string // for the lints to apply to clippy.
		}

		const (
		androidLints = "android"
		vendorLints = "vendor"
		noneLints = "none"
		)

		// lintSets defines the categories of linting for Android and their mapping to lintConfigs.
		var lintSets = map[string]lintConfig{
		androidLints: {rustcDefault, true, clippyDefault},
		vendorLints: {rustcVendor, true, clippyVendor},
		noneLints: {rustcAllowAll, false, noLint},
		}

		type pathLintSet struct {
		prefix string
		set string
		}

		// This is a map of local path prefixes to a lint set. The first entry
		// matching will be used. If no entry matches, androidLints ("android") will be
		// used.
		var defaultLintSetForPath = []pathLintSet{
		{"external", noneLints},
		{"hardware", vendorLints},
		{"prebuilts", noneLints},
		{"vendor/google", androidLints},
		{"vendor", vendorLints},
		}
		var AllowAllLints = rustcAllowAll

		// ClippyLintsForDir returns a boolean if Clippy should be executed and if so, the lints to be used.
		func ClippyLintsForDir(dir string) (bool, string) {
		for _, pathCheck := range DefaultLocalClippyChecks {
		if strings.HasPrefix(dir, pathCheck.PathPrefix) {
		return pathCheck.ClippyEnabled, pathCheck.ClippyConfig
		func ClippyLintsForDir(dir string, clippyLintsProperty *string) (bool, string, error) {
		if clippyLintsProperty != nil {
		set, ok := lintSets[*clippyLintsProperty]
		if ok {
		return set.clippyEnabled, set.clippyConfig, nil
		}
		if *clippyLintsProperty != "default" {
		return false, "", fmt.Errorf("unknown value for `clippy_lints`: %v, valid options are: default, android, vendor or none", *clippyLintsProperty)
		}
		}
		for _, p := range defaultLintSetForPath {
		if strings.HasPrefix(dir, p.prefix) {
		setConfig := lintSets[p.set]
		return setConfig.clippyEnabled, setConfig.clippyConfig, nil
		}
		}
		return true, clippyDefault
		return true, clippyDefault, nil
		}

		// RustcLintsForDir returns the standard lints to be used for a repository.
		func RustcLintsForDir(dir string) string {
		for _, pathCheck := range DefaultLocalClippyChecks {
		if strings.HasPrefix(dir, pathCheck.PathPrefix) {
		return pathCheck.RustcConfig
		func RustcLintsForDir(dir string, lintProperty *string) (string, error) {
		if lintProperty != nil {
		set, ok := lintSets[*lintProperty]
		if ok {
		return set.rustcConfig, nil
		}
		if *lintProperty != "default" {
		return "", fmt.Errorf("unknown value for `lints`: %v, valid options are: default, android, vendor or none", *lintProperty)
		}

		}
		for _, p := range defaultLintSetForPath {
		if strings.HasPrefix(dir, p.prefix) {
		return lintSets[p.set].rustcConfig, nil
		}
		}
		return rustcDefault
		return rustcDefault, nil
		}