diff --git a/rust/builder.go b/rust/builder.go index 8bc11bb8a..b0ab29743 100644 --- a/rust/builder.go +++ b/rust/builder.go @@ -46,7 +46,7 @@ var ( // 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 050a2593d..51d7180ca 100644 --- a/rust/compiler.go +++ b/rust/compiler.go @@ -28,10 +28,6 @@ func getEdition(compiler *baseCompiler) string { 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 @@ 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 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) featuresToFlags(features []string) []string { 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 1d30f826f..bcfac7c06 100644 --- a/rust/config/Android.bp +++ b/rust/config/Android.bp @@ -9,7 +9,7 @@ bootstrap_go_package { "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 c199ff269..000000000 --- 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 663514d46..e1b1775c9 100644 --- a/rust/config/global.go +++ b/rust/config/global.go @@ -32,8 +32,6 @@ var ( "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 000000000..529d0942f --- /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 +}