Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add native-android fix to buildifier #638

Merged
merged 9 commits into from
May 21, 2019
Merged
1 change: 1 addition & 0 deletions warn/warn.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ var FileWarningMap = map[string]func(f *build.File, fix bool) []*Finding{
"return-value": missingReturnValueWarning,
"module-docstring": moduleDocstringWarning,
"name-conventions": nameConventionsWarning,
"native-android": nativeAndroidRulesWarning,
"native-build": nativeInBuildFilesWarning,
"native-package": nativePackageWarning,
"no-effect": noEffectWarning,
Expand Down
58 changes: 43 additions & 15 deletions warn/warn_bazel_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,35 @@ package warn

import (
"fmt"
"sort"

"github.com/bazelbuild/buildtools/build"
"github.com/bazelbuild/buildtools/bzlenv"
"github.com/bazelbuild/buildtools/edit"
)

// Bazel API-specific warnings

var functionsWithPositionalArguments = map[string]bool{
"distribs": true,
"exports_files": true,
"licenses": true,
"print": true,
"register_toolchains": true,
"vardef": true,
}
var (
functionsWithPositionalArguments = map[string]bool{
"distribs": true,
"exports_files": true,
"licenses": true,
"print": true,
"register_toolchains": true,
"vardef": true,
}
androidNativeRules = []string{
"aar_import",
"android_binary",
"android_device",
"android_instrumentation_test",
"android_library",
"android_local_test",
"android_ndk_respository",
"android_sdk_repository",
}
)

// negateExpression returns an expression which is a negation of the input.
// If it's a boolean literal (true or false), just return the opposite literal.
Expand Down Expand Up @@ -128,11 +142,7 @@ func globalVariableUsageCheck(f *build.File, category, global, alternative strin
func notLoadedFunctionUsageCheck(f *build.File, category string, globals []string, loadFrom string, fix bool) []*Finding {
findings := []*Finding{}

if f.Type != build.TypeBzl {
return findings
}

toLoad := []string{}
toLoad := make(map[string]bool)

var walk func(e *build.Expr, env *bzlenv.Environment)
walk = func(e *build.Expr, env *bzlenv.Environment) {
Expand All @@ -154,7 +164,7 @@ func notLoadedFunctionUsageCheck(f *build.File, category string, globals []strin
for _, global := range globals {
if ident.Name == global {
if fix {
toLoad = append(toLoad, global)
toLoad[global] = true
return
}
start, end := call.Span()
Expand All @@ -168,7 +178,12 @@ func notLoadedFunctionUsageCheck(f *build.File, category string, globals []strin
walk(&expr, bzlenv.NewEnvironment())

if fix && len(toLoad) > 0 {
f.Stmt = edit.InsertLoad(f.Stmt, loadFrom, toLoad, toLoad)
loads := []string{}
for k := range toLoad {
loads = append(loads, k)
}
sort.Slice(loads, func(i, j int) bool { return loads[i] < loads[j] })
timpeut marked this conversation as resolved.
Show resolved Hide resolved
f.Stmt = edit.InsertLoad(f.Stmt, loadFrom, loads, loads)
}

return findings
Expand Down Expand Up @@ -483,13 +498,26 @@ func outputGroupWarning(f *build.File, fix bool) []*Finding {
}

func nativeGitRepositoryWarning(f *build.File, fix bool) []*Finding {
if f.Type != build.TypeBzl {
return []*Finding{}
}
return notLoadedFunctionUsageCheck(f, "git-repository", []string{"git_repository", "new_git_repository"}, "@bazel_tools//tools/build_defs/repo:git.bzl", fix)
}

func nativeHTTPArchiveWarning(f *build.File, fix bool) []*Finding {
if f.Type != build.TypeBzl {
return []*Finding{}
}
return notLoadedFunctionUsageCheck(f, "http-archive", []string{"http_archive"}, "@bazel_tools//tools/build_defs/repo:http.bzl", fix)
}

func nativeAndroidRulesWarning(f *build.File, fix bool) []*Finding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return []*Finding{}
}
return notLoadedFunctionUsageCheck(f, "native-android", androidNativeRules, "@rules_android//android:rules.bzl", fix)
}

func contextArgsAPIWarning(f *build.File, fix bool) []*Finding {
findings := []*Finding{}

Expand Down
28 changes: 28 additions & 0 deletions warn/warn_bazel_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,3 +457,31 @@ rule() # no parameters
rule(foo = bar) # no matching parameters
`, []string{}, scopeBzl)
}

func TestNativeAndroidWarning(t *testing.T) {
checkFindingsAndFix(t, "native-android", `
"""My file"""

def macro():
aar_import()
android_library()

android_binary()
`, `
"""My file"""

load("@rules_android//android:rules.bzl", "aar_import", "android_binary", "android_library")

def macro():
aar_import()
android_library()

android_binary()
`,
[]string{
`:4: Function "aar_import" is not global anymore and needs to be loaded from "@rules_android//android:rules.bzl".`,
`:5: Function "android_library" is not global anymore and needs to be loaded from "@rules_android//android:rules.bzl".`,
`:7: Function "android_binary" is not global anymore and needs to be loaded from "@rules_android//android:rules.bzl".`,
},
scopeBzl|scopeBuild)
}