Merge "Explicitly define Rust default lints"

This commit is contained in:
Thiébaud Weksteen 2020-07-01 17:38:58 +00:00 committed by Gerrit Code Review
commit 6bbe5774a3
6 changed files with 156 additions and 92 deletions

View file

@ -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")

View file

@ -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)...)

View file

@ -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",

View file

@ -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
}

View file

@ -32,8 +32,6 @@ var (
"libtest",
}
DefaultDenyWarnings = true
GlobalRustFlags = []string{
"--remap-path-prefix $$(pwd)=",
"-C codegen-units=1",

150
rust/config/lints.go Normal file
View file

@ -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
}