Explicitly define Rust default lints
Add documentation on how lints are defined and used in Android. Merge
the deny_warnings attribute with a new attribute (no_lint) which can be
used to disable the default linting parameters.
Explicitly allow all lints for external/ and prebuilts/, which remove
any warning when building sysroot for the devices.
Test: cd external/rust/crates; mma
Test: add dummy internal Rust module; mma
Change-Id: I62be1c41aeda4068fb9e288038727c1de5ffe547
diff --git a/rust/builder.go b/rust/builder.go
index b191323..2d69f27 100644
--- a/rust/builder.go
+++ b/rust/builder.go
@@ -46,7 +46,7 @@
// Because clippy-driver uses rustc as backend, we need to have some output even during the linting.
// Use the metadata output as it has the smallest footprint.
"--emit metadata -o $out $in ${libFlags} " +
- "$clippyFlags $rustcFlags",
+ "$rustcFlags $clippyFlags",
CommandDeps: []string{"$clippyCmd"},
},
"rustcFlags", "libFlags", "clippyFlags")
diff --git a/rust/compiler.go b/rust/compiler.go
index efc1ce4..363fe5e 100644
--- a/rust/compiler.go
+++ b/rust/compiler.go
@@ -28,10 +28,6 @@
return proptools.StringDefault(compiler.Properties.Edition, config.DefaultEdition)
}
-func getDenyWarnings(compiler *baseCompiler) bool {
- return BoolDefault(compiler.Properties.Deny_warnings, config.DefaultDenyWarnings)
-}
-
func (compiler *baseCompiler) setNoStdlibs() {
compiler.Properties.No_stdlibs = proptools.BoolPtr(true)
}
@@ -56,8 +52,8 @@
// 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 pass "-D warnings" to rustc. Defaults to true.
- Deny_warnings *bool
+ // whether to suppress the standard lint flags - default to false
+ No_lint *bool
// flags to pass to rustc
Flags []string `android:"path,arch_variant"`
@@ -145,8 +141,8 @@
func (compiler *baseCompiler) compilerFlags(ctx ModuleContext, flags Flags) Flags {
- if getDenyWarnings(compiler) {
- flags.RustFlags = append(flags.RustFlags, "-D warnings")
+ if !Bool(compiler.Properties.No_lint) {
+ flags.RustFlags = append(flags.RustFlags, config.RustcLintsForDir(ctx.ModuleDir()))
}
flags.RustFlags = append(flags.RustFlags, compiler.Properties.Flags...)
flags.RustFlags = append(flags.RustFlags, compiler.featuresToFlags(compiler.Properties.Features)...)
diff --git a/rust/config/Android.bp b/rust/config/Android.bp
index 1d30f82..bcfac7c 100644
--- a/rust/config/Android.bp
+++ b/rust/config/Android.bp
@@ -9,7 +9,7 @@
"arm_device.go",
"arm64_device.go",
"global.go",
- "clippy.go",
+ "lints.go",
"toolchain.go",
"allowed_list.go",
"x86_darwin_host.go",
diff --git a/rust/config/clippy.go b/rust/config/clippy.go
deleted file mode 100644
index c199ff2..0000000
--- a/rust/config/clippy.go
+++ /dev/null
@@ -1,80 +0,0 @@
-// Copyright 2020 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package config
-
-import (
- "strings"
-
- "android/soong/android"
-)
-
-var (
- defaultLints = []string{
- "-D missing-docs",
- "-D clippy::missing-safety-doc",
- }
- defaultVendorLints = []string{
- "",
- }
-)
-
-func init() {
- // Default Rust lints. These apply to all Google-authored modules.
- pctx.VariableFunc("ClippyDefaultLints", func(ctx android.PackageVarContext) string {
- if override := ctx.Config().Getenv("CLIPPY_DEFAULT_LINTS"); override != "" {
- return override
- }
- return strings.Join(defaultLints, " ")
- })
-
- // Rust lints that only applies to external code.
- pctx.VariableFunc("ClippyVendorLints", func(ctx android.PackageVarContext) string {
- if override := ctx.Config().Getenv("CLIPPY_VENDOR_LINTS"); override != "" {
- return override
- }
- return strings.Join(defaultVendorLints, " ")
- })
-}
-
-type PathBasedClippyConfig struct {
- PathPrefix string
- Enabled bool
- ClippyConfig string
-}
-
-const clippyNone = ""
-const clippyDefault = "${config.ClippyDefaultLints}"
-const clippyVendor = "${config.ClippyVendorLints}"
-
-// This is a map of local path prefixes to a boolean indicating if the lint
-// rule should be generated and if so, the set of lints to use. The first entry
-// matching will be used. If no entry is matching, clippyDefault will be used.
-var DefaultLocalTidyChecks = []PathBasedClippyConfig{
- {"external/", false, clippyNone},
- {"hardware/", true, clippyVendor},
- {"prebuilts/", false, clippyNone},
- {"vendor/google", true, clippyDefault},
- {"vendor/", true, clippyVendor},
-}
-
-// ClippyLintsForDir returns the Clippy lints to be used for a repository.
-func ClippyLintsForDir(dir string) (bool, string) {
- for _, pathCheck := range DefaultLocalTidyChecks {
- if strings.HasPrefix(dir, pathCheck.PathPrefix) {
- return pathCheck.Enabled, pathCheck.ClippyConfig
- }
- }
- return true, clippyDefault
-}
diff --git a/rust/config/global.go b/rust/config/global.go
index 663514d..e1b1775 100644
--- a/rust/config/global.go
+++ b/rust/config/global.go
@@ -32,8 +32,6 @@
"libtest",
}
- DefaultDenyWarnings = true
-
GlobalRustFlags = []string{
"--remap-path-prefix $$(pwd)=",
"-C codegen-units=1",
diff --git a/rust/config/lints.go b/rust/config/lints.go
new file mode 100644
index 0000000..529d094
--- /dev/null
+++ b/rust/config/lints.go
@@ -0,0 +1,150 @@
+// Copyright 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config
+
+import (
+ "strings"
+
+ "android/soong/android"
+)
+
+// Overarching principles for Rust lints on Android:
+// 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).
+var (
+ // Default Rust lints that applies to Google-authored modules.
+ defaultRustcLints = []string{
+ "-A deprecated",
+ "-D missing-docs",
+ "-D warnings",
+ }
+ // Default Clippy lints. These are applied on top of defaultRustcLints.
+ // It should be assumed that any warning lint will be promoted to a
+ // deny.
+ defaultClippyLints = []string{
+ "-A clippy::type-complexity",
+ }
+
+ // Rust lints for vendor code.
+ defaultRustcVendorLints = []string{
+ "-A deprecated",
+ "-D warnings",
+ }
+ // Clippy lints for vendor source. These are applied on top of
+ // defaultRustcVendorLints. It should be assumed that any warning lint
+ // will be promoted to a deny.
+ defaultClippyVendorLints = []string{
+ "-A clippy::complexity",
+ "-A clippy::perf",
+ "-A clippy::style",
+ }
+
+ // For prebuilts/ and external/, no linting is expected. If a warning
+ // or a deny is reported, it should be fixed upstream.
+ allowAllLints = []string{
+ "--cap-lints allow",
+ }
+)
+
+func init() {
+ // Default Rust lints. These apply to all Google-authored modules.
+ pctx.VariableFunc("RustDefaultLints", func(ctx android.PackageVarContext) string {
+ if override := ctx.Config().Getenv("RUST_DEFAULT_LINTS"); override != "" {
+ return override
+ }
+ return strings.Join(defaultRustcLints, " ")
+ })
+ pctx.VariableFunc("ClippyDefaultLints", func(ctx android.PackageVarContext) string {
+ if override := ctx.Config().Getenv("CLIPPY_DEFAULT_LINTS"); override != "" {
+ return override
+ }
+ return strings.Join(defaultClippyLints, " ")
+ })
+
+ // Rust lints that only applies to external code.
+ pctx.VariableFunc("RustVendorLints", func(ctx android.PackageVarContext) string {
+ if override := ctx.Config().Getenv("RUST_VENDOR_LINTS"); override != "" {
+ return override
+ }
+ return strings.Join(defaultRustcVendorLints, " ")
+ })
+ pctx.VariableFunc("ClippyVendorLints", func(ctx android.PackageVarContext) string {
+ if override := ctx.Config().Getenv("CLIPPY_VENDOR_LINTS"); override != "" {
+ return override
+ }
+ return strings.Join(defaultClippyVendorLints, " ")
+ })
+ 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}"
+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},
+}
+
+// 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
+ }
+ }
+ return true, clippyDefault
+}
+
+// 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
+ }
+ }
+ return rustcDefault
+}