diff --git a/Android.bp b/Android.bp index f9ba2c22b..74d9c0ac8 100644 --- a/Android.bp +++ b/Android.bp @@ -95,6 +95,7 @@ bootstrap_go_package { srcs: [ "cc/config/clang.go", "cc/config/global.go", + "cc/config/tidy.go", "cc/config/toolchain.go", "cc/config/arm_device.go", @@ -108,6 +109,9 @@ bootstrap_go_package { "cc/config/x86_linux_host.go", "cc/config/x86_windows_host.go", ], + testSrcs: [ + "cc/config/tidy_test.go", + ], } bootstrap_go_package { @@ -134,6 +138,7 @@ bootstrap_go_package { "cc/sanitize.go", "cc/stl.go", "cc/strip.go", + "cc/tidy.go", "cc/util.go", "cc/compiler.go", diff --git a/android/config.go b/android/config.go index 4d7e8df55..74cb56e88 100644 --- a/android/config.go +++ b/android/config.go @@ -384,6 +384,17 @@ func (c *config) UseGoma() bool { return Bool(c.ProductVariables.UseGoma) } +func (c *config) ClangTidy() bool { + return Bool(c.ProductVariables.ClangTidy) +} + +func (c *config) TidyChecks() string { + if c.ProductVariables.TidyChecks == nil { + return "" + } + return *c.ProductVariables.TidyChecks +} + func (c *config) LibartImgHostBaseAddress() string { return "0x60000000" } diff --git a/android/variable.go b/android/variable.go index a1a2bf009..4aff26e31 100644 --- a/android/variable.go +++ b/android/variable.go @@ -112,6 +112,9 @@ type productVariables struct { UseGoma *bool `json:",omitempty"` Debuggable *bool `json:",omitempty"` + ClangTidy *bool `json:",omitempty"` + TidyChecks *string `json:",omitempty"` + DevicePrefer32BitExecutables *bool `json:",omitempty"` HostPrefer32BitExecutables *bool `json:",omitempty"` diff --git a/androidmk/cmd/androidmk/android.go b/androidmk/cmd/androidmk/android.go index 1fac4bf88..6387ff1b8 100644 --- a/androidmk/cmd/androidmk/android.go +++ b/androidmk/cmd/androidmk/android.go @@ -53,6 +53,9 @@ var standardProperties = map[string]struct { "LOCAL_EXPORT_SHARED_LIBRARY_HEADERS": {"export_shared_lib_headers", bpparser.ListType}, "LOCAL_EXPORT_STATIC_LIBRARY_HEADERS": {"export_static_lib_headers", bpparser.ListType}, "LOCAL_INIT_RC": {"init_rc", bpparser.ListType}, + "LOCAL_TIDY_FLAGS": {"tidy_flags", bpparser.ListType}, + // TODO: This is comma-seperated, not space-separated + "LOCAL_TIDY_CHECKS": {"tidy_checks", bpparser.ListType}, "LOCAL_JAVA_RESOURCE_DIRS": {"java_resource_dirs", bpparser.ListType}, "LOCAL_JAVACFLAGS": {"javacflags", bpparser.ListType}, @@ -73,6 +76,7 @@ var standardProperties = map[string]struct { "LOCAL_RTTI_FLAG": {"rtti", bpparser.BoolType}, "LOCAL_NO_STANDARD_LIBRARIES": {"no_standard_libraries", bpparser.BoolType}, "LOCAL_PACK_MODULE_RELOCATIONS": {"pack_relocations", bpparser.BoolType}, + "LOCAL_TIDY": {"tidy", bpparser.BoolType}, "LOCAL_EXPORT_PACKAGE_RESOURCES": {"export_package_resources", bpparser.BoolType}, } diff --git a/cc/binary.go b/cc/binary.go index 1e923a4f1..b02943947 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -282,6 +282,7 @@ func (binary *binaryDecorator) link(ctx ModuleContext, linkerDeps = append(linkerDeps, deps.SharedLibsDeps...) linkerDeps = append(linkerDeps, deps.LateSharedLibsDeps...) + linkerDeps = append(linkerDeps, objs.tidyFiles...) TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs, deps.StaticLibs, deps.LateStaticLibs, deps.WholeStaticLibs, linkerDeps, deps.CrtBegin, deps.CrtEnd, true, diff --git a/cc/builder.go b/cc/builder.go index 0006ed367..faa39d152 100644 --- a/cc/builder.go +++ b/cc/builder.go @@ -150,6 +150,14 @@ var ( Restat: true, }, "crossCompile") + + clangTidy = pctx.AndroidStaticRule("clangTidy", + blueprint.RuleParams{ + Command: "rm -f $out && ${config.ClangBin}/clang-tidy $tidyFlags $in -- $cFlags && touch $out", + CommandDeps: []string{"${config.ClangBin}/clang-tidy"}, + Description: "tidy $out", + }, + "cFlags", "tidyFlags") ) func init() { @@ -174,8 +182,10 @@ type builderFlags struct { libFlags string yaccFlags string protoFlags string + tidyFlags string toolchain config.Toolchain clang bool + tidy bool stripKeepSymbols bool stripKeepMiniDebugInfo bool @@ -183,18 +193,21 @@ type builderFlags struct { } type Objects struct { - objFiles android.Paths + objFiles android.Paths + tidyFiles android.Paths } func (a Objects) Copy() Objects { return Objects{ - objFiles: append(android.Paths{}, a.objFiles...), + objFiles: append(android.Paths{}, a.objFiles...), + tidyFiles: append(android.Paths{}, a.tidyFiles...), } } func (a Objects) Append(b Objects) Objects { return Objects{ - objFiles: append(a.objFiles, b.objFiles...), + objFiles: append(a.objFiles, b.objFiles...), + tidyFiles: append(a.tidyFiles, b.tidyFiles...), } } @@ -203,6 +216,10 @@ func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and flags builderFlags, deps android.Paths) Objects { objFiles := make(android.Paths, len(srcFiles)) + var tidyFiles android.Paths + if flags.tidy && flags.clang { + tidyFiles = make(android.Paths, 0, len(srcFiles)) + } cflags := flags.globalFlags + " " + flags.cFlags + " " + flags.conlyFlags cppflags := flags.globalFlags + " " + flags.cFlags + " " + flags.cppFlags @@ -223,11 +240,13 @@ func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and var moduleCflags string var ccCmd string + tidy := flags.tidy && flags.clang switch srcFile.Ext() { case ".S", ".s": ccCmd = "gcc" moduleCflags = asflags + tidy = false case ".c": ccCmd = "gcc" moduleCflags = cflags @@ -264,24 +283,45 @@ func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and "ccCmd": ccCmd, }, }) + + if tidy { + tidyFile := android.ObjPathWithExt(ctx, srcFile, subdir, "tidy") + tidyFiles = append(tidyFiles, tidyFile) + + ctx.ModuleBuild(pctx, android.ModuleBuildParams{ + Rule: clangTidy, + Output: tidyFile, + Input: srcFile, + // We must depend on objFile, since clang-tidy doesn't + // support exporting dependencies. + Implicit: objFile, + Args: map[string]string{ + "cFlags": moduleCflags, + "tidyFlags": flags.tidyFlags, + }, + }) + } + } return Objects{ - objFiles: objFiles, + objFiles: objFiles, + tidyFiles: tidyFiles, } } // Generate a rule for compiling multiple .o files to a static library (.a) func TransformObjToStaticLib(ctx android.ModuleContext, objFiles android.Paths, - flags builderFlags, outputFile android.ModuleOutPath) { + flags builderFlags, outputFile android.ModuleOutPath, deps android.Paths) { arCmd := gccCmd(flags.toolchain, "ar") arFlags := "crsPD" ctx.ModuleBuild(pctx, android.ModuleBuildParams{ - Rule: ar, - Output: outputFile, - Inputs: objFiles, + Rule: ar, + Output: outputFile, + Inputs: objFiles, + Implicits: deps, Args: map[string]string{ "arFlags": arFlags, "arCmd": arCmd, @@ -294,7 +334,7 @@ func TransformObjToStaticLib(ctx android.ModuleContext, objFiles android.Paths, // very small command line length limit, so we have to split the ar into multiple // steps, each appending to the previous one. func TransformDarwinObjToStaticLib(ctx android.ModuleContext, objFiles android.Paths, - flags builderFlags, outputPath android.ModuleOutPath) { + flags builderFlags, outputPath android.ModuleOutPath, deps android.Paths) { arFlags := "cqs" @@ -303,8 +343,9 @@ func TransformDarwinObjToStaticLib(ctx android.ModuleContext, objFiles android.P dummyAr := android.PathForModuleOut(ctx, "dummy"+staticLibraryExtension) ctx.ModuleBuild(pctx, android.ModuleBuildParams{ - Rule: emptyFile, - Output: dummy, + Rule: emptyFile, + Output: dummy, + Implicits: deps, }) ctx.ModuleBuild(pctx, android.ModuleBuildParams{ @@ -347,9 +388,10 @@ func TransformDarwinObjToStaticLib(ctx android.ModuleContext, objFiles android.P if in == "" { ctx.Build(pctx, blueprint.BuildParams{ - Rule: darwinAr, - Outputs: []string{out}, - Inputs: l, + Rule: darwinAr, + Outputs: []string{out}, + Inputs: l, + Implicits: deps.Strings(), Args: map[string]string{ "arFlags": arFlags, }, diff --git a/cc/cc.go b/cc/cc.go index c2884ace4..b6e98b1cc 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -100,9 +100,11 @@ type Flags struct { protoFlags []string // Flags that apply to proto source files LdFlags []string // Flags that apply to linker command lines libFlags []string // Flags to add libraries early to the link order + TidyFlags []string // Flags that apply to clang-tidy Toolchain config.Toolchain Clang bool + Tidy bool RequiredInstructionSet string DynamicLinker string @@ -368,6 +370,9 @@ func newBaseModule(hod android.HostOrDeviceSupported, multilib android.Multilib) func newModule(hod android.HostOrDeviceSupported, multilib android.Multilib) *Module { module := newBaseModule(hod, multilib) + module.features = []feature{ + &tidyFeature{}, + } module.stl = &stl{} module.sanitize = &sanitize{} return module @@ -948,6 +953,7 @@ func DefaultsFactory(props ...interface{}) (blueprint.Module, []interface{}) { &SanitizeProperties{}, &StripProperties{}, &InstallerProperties{}, + &TidyProperties{}, ) return android.InitDefaultsModule(module, module, props...) diff --git a/cc/check.go b/cc/check.go index 4e403a58b..340464e4b 100644 --- a/cc/check.go +++ b/cc/check.go @@ -105,3 +105,31 @@ func CheckBadHostLdlibs(ctx ModuleContext, prop string, flags []string) { } } } + +// Check for bad clang tidy flags +func CheckBadTidyFlags(ctx ModuleContext, prop string, flags []string) { + for _, flag := range flags { + flag = strings.TrimSpace(flag) + + if !strings.HasPrefix(flag, "-") { + ctx.PropertyErrorf(prop, "Flag `%s` must start with `-`", flag) + } else if strings.HasPrefix(flag, "-fix") { + ctx.PropertyErrorf(prop, "Flag `%s` is not allowed, since it could cause multiple writes to the same source file", flag) + } else if strings.HasPrefix(flag, "-checks=") { + ctx.PropertyErrorf(prop, "Flag `%s` is not allowed, use `tidy_checks` property instead", flag) + } else if strings.Contains(flag, " ") { + ctx.PropertyErrorf(prop, "Bad flag: `%s` is not an allowed multi-word flag. Should it be split into multiple flags?", flag) + } + } +} + +// Check for bad clang tidy checks +func CheckBadTidyChecks(ctx ModuleContext, prop string, checks []string) { + for _, check := range checks { + if strings.Contains(check, " ") { + ctx.PropertyErrorf("tidy_checks", "Check `%s` invalid, cannot contain spaces", check) + } else if strings.Contains(check, ",") { + ctx.PropertyErrorf("tidy_checks", "Check `%s` invalid, cannot contain commas. Split each entry into it's own string instead", check) + } + } +} diff --git a/cc/config/tidy.go b/cc/config/tidy.go new file mode 100644 index 000000000..dd3942167 --- /dev/null +++ b/cc/config/tidy.go @@ -0,0 +1,106 @@ +// Copyright 2016 Google Inc. All rights reserved. +// +// 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" +) + +func init() { + // Most Android source files are not clang-tidy clean yet. + // Global tidy checks include only google*, performance*, + // and misc-macro-parentheses, but not google-readability* + // or google-runtime-references. + pctx.StaticVariable("TidyDefaultGlobalChecks", strings.Join([]string{ + "-*", + "google*", + "misc-macro-parentheses", + "performance*", + "-google-readability*", + "-google-runtime-references", + }, ",")) + + // There are too many clang-tidy warnings in external and vendor projects. + // Enable only some google checks for these projects. + pctx.StaticVariable("TidyExternalVendorChecks", strings.Join([]string{ + "-*", + "google*", + "-google-build-using-namespace", + "-google-default-arguments", + "-google-explicit-constructor", + "-google-readability*", + "-google-runtime-int", + "-google-runtime-references", + }, ",")) + + // Give warnings to header files only in selected directories. + // Do not give warnings to external or vendor header files, which contain too + // many warnings. + pctx.StaticVariable("TidyDefaultHeaderDirs", strings.Join([]string{ + "art/", + "bionic/", + "bootable/", + "build/", + "cts/", + "dalvik/", + "developers/", + "development/", + "frameworks/", + "libcore/", + "libnativehelper/", + "system/", + }, "|")) +} + +type PathBasedTidyCheck struct { + PathPrefix string + Checks string +} + +const tidyDefault = "${config.TidyDefaultGlobalChecks}" +const tidyExternalVendor = "${config.TidyExternalVendorChecks}" + +// This is a map of local path prefixes to the set of default clang-tidy checks +// to be used. +// The last matched local_path_prefix should be the most specific to be used. +var DefaultLocalTidyChecks = []PathBasedTidyCheck{ + {"external/", tidyExternalVendor}, + {"external/google", tidyDefault}, + {"external/webrtc", tidyDefault}, + {"frameworks/compile/mclinker/", tidyExternalVendor}, + {"hardware/qcom", tidyExternalVendor}, + {"vendor/", tidyExternalVendor}, + {"vendor/google", tidyDefault}, + {"vendor/google_devices", tidyExternalVendor}, +} + +var reversedDefaultLocalTidyChecks = reverseTidyChecks(DefaultLocalTidyChecks) + +func reverseTidyChecks(in []PathBasedTidyCheck) []PathBasedTidyCheck { + ret := make([]PathBasedTidyCheck, len(in)) + for i, check := range in { + ret[len(in)-i-1] = check + } + return ret +} + +func TidyChecksForDir(dir string) string { + for _, pathCheck := range reversedDefaultLocalTidyChecks { + if strings.HasPrefix(dir, pathCheck.PathPrefix) { + return pathCheck.Checks + } + } + return tidyDefault +} diff --git a/cc/config/tidy_test.go b/cc/config/tidy_test.go new file mode 100644 index 000000000..4ed8b231d --- /dev/null +++ b/cc/config/tidy_test.go @@ -0,0 +1,41 @@ +// Copyright 2016 Google Inc. All rights reserved. +// +// 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 ( + "testing" +) + +func TestTidyChecksForDir(t *testing.T) { + testCases := []struct { + input string + expected string + }{ + {"foo/bar", tidyDefault}, + {"vendor/foo/bar", tidyExternalVendor}, + {"vendor/google", tidyDefault}, + {"vendor/google/foo", tidyDefault}, + {"vendor/google_devices/foo", tidyExternalVendor}, + } + + for _, testCase := range testCases { + t.Run(testCase.input, func(t *testing.T) { + output := TidyChecksForDir(testCase.input) + if output != testCase.expected { + t.Error("Output doesn't match expected", output, testCase.expected) + } + }) + } +} diff --git a/cc/library.go b/cc/library.go index db19d6658..dcfc077d2 100644 --- a/cc/library.go +++ b/cc/library.go @@ -347,9 +347,9 @@ func (library *libraryDecorator) linkStatic(ctx ModuleContext, ctx.ModuleName()+library.Properties.VariantName+staticLibraryExtension) if ctx.Darwin() { - TransformDarwinObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile) + TransformDarwinObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile, objs.tidyFiles) } else { - TransformObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile) + TransformObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile, objs.tidyFiles) } library.wholeStaticMissingDeps = ctx.GetMissingDependencies() @@ -452,6 +452,7 @@ func (library *libraryDecorator) linkShared(ctx ModuleContext, linkerDeps = append(linkerDeps, deps.SharedLibsDeps...) linkerDeps = append(linkerDeps, deps.LateSharedLibsDeps...) + linkerDeps = append(linkerDeps, objs.tidyFiles...) TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs, deps.StaticLibs, deps.LateStaticLibs, deps.WholeStaticLibs, diff --git a/cc/makevars.go b/cc/makevars.go index 770e1d0d9..7f1063f3e 100644 --- a/cc/makevars.go +++ b/cc/makevars.go @@ -36,6 +36,7 @@ func makeVarsProvider(ctx android.MakeVarsContext) { ctx.Strict("CLANG_CXX", "${config.ClangBin}/clang++") ctx.Strict("LLVM_AS", "${config.ClangBin}/llvm-as") ctx.Strict("LLVM_LINK", "${config.ClangBin}/llvm-link") + ctx.Strict("PATH_TO_CLANG_TIDY", "${config.ClangBin}/clang-tidy") ctx.StrictSorted("CLANG_CONFIG_UNKNOWN_CFLAGS", strings.Join(config.ClangUnknownCflags, " ")) ctx.Strict("GLOBAL_CFLAGS_NO_OVERRIDE", "${config.NoOverrideGlobalCflags}") @@ -52,6 +53,10 @@ func makeVarsProvider(ctx android.MakeVarsContext) { ctx.Strict("DEFAULT_CPP_STD_VERSION", config.CppStdVersion) ctx.Strict("DEFAULT_GCC_CPP_STD_VERSION", config.GccCppStdVersion) + ctx.Strict("DEFAULT_GLOBAL_TIDY_CHECKS", "${config.TidyDefaultGlobalChecks}") + ctx.Strict("DEFAULT_LOCAL_TIDY_CHECKS", joinLocalTidyChecks(config.DefaultLocalTidyChecks)) + ctx.Strict("DEFAULT_TIDY_HEADER_DIRS", "${config.TidyDefaultHeaderDirs}") + includeFlags, err := ctx.Eval("${config.CommonGlobalIncludes} ${config.CommonGlobalSystemIncludes}") if err != nil { panic(err) @@ -257,3 +262,11 @@ func splitSystemIncludes(ctx android.MakeVarsContext, val string) (includes, sys return includes, systemIncludes } + +func joinLocalTidyChecks(checks []config.PathBasedTidyCheck) string { + rets := make([]string, len(checks)) + for i, check := range config.DefaultLocalTidyChecks { + rets[i] = check.PathPrefix + ":" + check.Checks + } + return strings.Join(rets, " ") +} diff --git a/cc/tidy.go b/cc/tidy.go new file mode 100644 index 000000000..68380ec70 --- /dev/null +++ b/cc/tidy.go @@ -0,0 +1,93 @@ +// Copyright 2016 Google Inc. All rights reserved. +// +// 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 cc + +import ( + "strings" + + "github.com/google/blueprint/proptools" + + "android/soong/cc/config" +) + +type TidyProperties struct { + // whether to run clang-tidy over C-like sources. + Tidy *bool + + // Extra flags to pass to clang-tidy + Tidy_flags []string + + // Extra checks to enable or disable in clang-tidy + Tidy_checks []string +} + +type tidyFeature struct { + Properties TidyProperties +} + +func (tidy *tidyFeature) props() []interface{} { + return []interface{}{&tidy.Properties} +} + +func (tidy *tidyFeature) begin(ctx BaseModuleContext) { +} + +func (tidy *tidyFeature) deps(ctx BaseModuleContext, deps Deps) Deps { + return deps +} + +func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags { + // Check if tidy is explicitly disabled for this module + if tidy.Properties.Tidy != nil && !*tidy.Properties.Tidy { + return flags + } + + // If not explicitly set, check the global tidy flag + if tidy.Properties.Tidy == nil && !ctx.AConfig().ClangTidy() { + return flags + } + + // Clang-tidy requires clang + if !flags.Clang { + return flags + } + + flags.Tidy = true + + CheckBadTidyFlags(ctx, "tidy_flags", tidy.Properties.Tidy_flags) + + esc := proptools.NinjaAndShellEscape + + flags.TidyFlags = append(flags.TidyFlags, esc(tidy.Properties.Tidy_flags)...) + if len(flags.TidyFlags) == 0 { + headerFilter := "-header-filter=\"(" + ctx.ModuleDir() + "|${config.TidyDefaultHeaderDirs})\"" + flags.TidyFlags = append(flags.TidyFlags, headerFilter) + } + + tidyChecks := "-checks=" + if checks := ctx.AConfig().TidyChecks(); len(checks) > 0 { + tidyChecks += checks + } else { + tidyChecks += config.TidyChecksForDir(ctx.ModuleDir()) + } + if len(tidy.Properties.Tidy_checks) > 0 { + CheckBadTidyChecks(ctx, "tidy_checks", tidy.Properties.Tidy_checks) + + tidyChecks = tidyChecks + "," + strings.Join(esc(tidy.Properties.Tidy_checks), ",") + } + flags.TidyFlags = append(flags.TidyFlags, tidyChecks) + + return flags +} diff --git a/cc/util.go b/cc/util.go index 07296b9e9..31f0aec77 100644 --- a/cc/util.go +++ b/cc/util.go @@ -96,8 +96,10 @@ func flagsToBuilderFlags(in Flags) builderFlags { protoFlags: strings.Join(in.protoFlags, " "), ldFlags: strings.Join(in.LdFlags, " "), libFlags: strings.Join(in.libFlags, " "), + tidyFlags: strings.Join(in.TidyFlags, " "), toolchain: in.Toolchain, clang: in.Clang, + tidy: in.Tidy, } }