Add native-android fix to buildifier (#638)

WARNINGS.md

@@ -28,6 +28,7 @@ Warning categories supported by buildifier's linter:
   * [load-on-top](#load-on-top)
   * [module-docstring](#module-docstring)
   * [name-conventions](#name-conventions)
+  * [native-android] (#native-android)
   * [native-build](#native-build)
   * [native-package](#native-package)
   * [no-effect](#no-effect)
@@ -456,6 +457,16 @@ UPPER_SNAKE_CASE, and providers should be UpperCamelCase ending with `Info`.
 
 --------------------------------------------------------------------------------
 
+## <a name="native-android"></a>All Android build rules should be loaded from Starlark
+
+  * Category name: `native-android`
+  * Automatic fix: yes
+
+The Android build rules should be loaded from Starlark. The native rules [will be
+disabled](https://github.com/bazelbuild/bazel/issues/8391).
+
+--------------------------------------------------------------------------------
+
 ## <a name="native-build"></a>The `native` module shouldn't be used in BUILD files
 
   * Category name: `native-build`

tables/tables.go

@@ -208,6 +208,21 @@ var StripLabelLeadingSlashes = false
 
 var ShortenAbsoluteLabelsToRelative = false
 
+// AndroidNativeRules lists all Android rules that are being migrated from Native to Starlark.
+var AndroidNativeRules = []string{
+	"aar_import",
+	"android_binary",
+	"android_device",
+	"android_instrumentation_test",
+	"android_library",
+	"android_local_test",
+	"android_ndk_respository",
+	"android_sdk_repository",
+}
+
+// AndroidLoadPath is the load path for the Starlark Android Rules.
+var AndroidLoadPath = "@rules_android//android:rules.bzl"
+
 // OverrideTables allows a user of the build package to override the special-case rules. The user-provided tables replace the built-in tables.
 func OverrideTables(labelArg, blacklist, listArg, sortableListArg, sortBlacklist, sortWhitelist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) {
 	IsLabelArg = labelArg

warn/warn.go

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

warn/warn_bazel_api.go

@@ -4,9 +4,12 @@ package warn
 
 import (
 	"fmt"
+	"sort"
+
 	"github.com/bazelbuild/buildtools/build"
 	"github.com/bazelbuild/buildtools/bzlenv"
 	"github.com/bazelbuild/buildtools/edit"
+	"github.com/bazelbuild/buildtools/tables"
 )
 
 // Bazel API-specific warnings
@@ -123,16 +126,11 @@ func globalVariableUsageCheck(f *build.File, category, global, alternative strin
 	return findings
 }
 
-// notLoadedFunctionUsageCheck checks whether there's a usage of a given not imported  function in the file
+// notLoadedFunctionUsageCheck checks whether there's a usage of a given not imported function in the file
 // and adds a load statement if necessary.
 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) {
@@ -154,7 +152,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()
@@ -168,7 +166,59 @@ 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.Strings(loads)
+		f.Stmt = edit.InsertLoad(f.Stmt, loadFrom, loads, loads)
+	}
+
+	return findings
+}
+
+// notLoadedNativeFunctionUsageCheck checks whether there's a usage of a given not
+// import function in the file and adds a load statement if necessary.
+func notLoadedNativeFunctionUsageCheck(f *build.File, category string, globals []string, loadFrom string, fix bool) []*Finding {
+	findings := []*Finding{}
+	toLoad := make(map[string]bool)
+
+	build.Edit(f, func(expr build.Expr, stack []build.Expr) build.Expr {
+
+		dot, ok := expr.(*build.DotExpr)
+		if !ok {
+			return nil
+		}
+		ident, ok := dot.X.(*build.Ident)
+		if !ok || ident.Name != "native" {
+			return nil
+		}
+		for _, global := range globals {
+			if dot.Name == global {
+				if fix {
+					toLoad[global] = true
+					start, _ := dot.Span()
+					return &build.Ident{
+						Name:    dot.Name,
+						NamePos: start,
+					}
+				}
+				start, end := dot.Span()
+				findings = append(findings,
+					makeFinding(f, start, end, category,
+						fmt.Sprintf(`Native function "%s" is not global anymore and needs to be loaded from "%s".`, global, loadFrom), true, nil))
+			}
+		}
+		return nil
+	})
+
+	if fix && len(toLoad) > 0 {
+		loads := []string{}
+		for k := range toLoad {
+			loads = append(loads, k)
+		}
+		sort.Strings(loads)
+		f.Stmt = edit.InsertLoad(f.Stmt, loadFrom, loads, loads)
 	}
 
 	return findings
@@ -483,13 +533,29 @@ 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 append(
+		notLoadedFunctionUsageCheck(f, "native-android", tables.AndroidNativeRules, tables.AndroidLoadPath, fix),
+		notLoadedNativeFunctionUsageCheck(f, "native-android", tables.AndroidNativeRules, tables.AndroidLoadPath, fix)...)
+}
+
 func contextArgsAPIWarning(f *build.File, fix bool) []*Finding {
 	findings := []*Finding{}
 

warn/warn_bazel_api_test.go

@@ -457,3 +457,37 @@ 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()
+    native.android_library()
+    native.android_local_test()
+
+android_binary()
+`, `
+"""My file"""
+
+load("@rules_android//android:rules.bzl", "aar_import", "android_binary", "android_library", "android_local_test")
+
+def macro():
+    aar_import()
+    android_library()
+    android_library()
+    android_local_test()
+
+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".`,
+			`:6: Native function "android_library" is not global anymore and needs to be loaded from "@rules_android//android:rules.bzl".`,
+			`:7: Native function "android_local_test" is not global anymore and needs to be loaded from "@rules_android//android:rules.bzl".`,
+			`:9: Function "android_binary" is not global anymore and needs to be loaded from "@rules_android//android:rules.bzl".`,
+		},
+		scopeBzl|scopeBuild)
+}