From e3a4a46ff62b8c7e28539ae690d8d5f2097f8368 Mon Sep 17 00:00:00 2001
From: hvd <hvadehra@gmail.com>
Date: Mon, 4 Mar 2024 15:14:25 +0100
Subject: [PATCH] Update warnings for java rules

This is in preparation for the native java rules being moved out of Bazel.
---
 WARNINGS.md                      | 95 +++++++++++++++++++++++++++++---
 buildifier/config/config_test.go | 18 +++++-
 buildifier/integration_test.sh   |  9 ++-
 tables/tables.go                 | 18 +-----
 warn/docs/warnings.textproto     | 59 +++++++++++++++++---
 warn/warn.go                     | 18 +++++-
 warn/warn_bazel_api.go           | 38 ++++++++++++-
 warn/warn_bazel_api_test.go      | 50 ++++++++---------
 8 files changed, 240 insertions(+), 65 deletions(-)

diff --git a/WARNINGS.md b/WARNINGS.md
index 0d6270449..6e64d0315 100644
--- a/WARNINGS.md
+++ b/WARNINGS.md
@@ -40,7 +40,14 @@ Warning categories supported by buildifier's linter:
   * [`native-android`](#native-android)
   * [`native-build`](#native-build)
   * [`native-cc`](#native-cc)
-  * [`native-java`](#native-java)
+  * [`native-java-binary`](#native-java-binary)
+  * [`native-java-import`](#native-java-import)
+  * [`native-java-library`](#native-java-library)
+  * [`native-java-package-config`](#native-java-package-config)
+  * [`native-java-plugin`](#native-java-plugin)
+  * [`native-java-runtime`](#native-java-runtime)
+  * [`native-java-test`](#native-java-test)
+  * [`native-java-toolchain`](#native-java-toolchain)
   * [`native-package`](#native-package)
   * [`native-proto`](#native-proto)
   * [`native-py`](#native-py)
@@ -692,19 +699,91 @@ at the moment it's not required to load Starlark rules.
 
 --------------------------------------------------------------------------------
 
-## <a name="native-java"></a>All Java build rules should be loaded from Starlark
+## <a name="native-java-binary"></a>All Java build rules should be loaded from Starlark
 
-  * Category name: `native-java`
-  * Flag in Bazel: [`--incompatible_load_java_rules_from_bzl`](https://github.com/bazelbuild/bazel/issues/8746)
+  * Category name: `native-java-binary`
   * Automatic fix: yes
   * [Disabled by default](buildifier/README.md#linter)
-  * [Suppress the warning](#suppress): `# buildifier: disable=native-java`
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-binary`
 
 The Java build rules should be loaded from Starlark.
 
-Update: the plans for disabling native rules
-[have been postponed](https://groups.google.com/g/bazel-discuss/c/XNvpWcge4AE/m/aJ-aQzszAwAJ),
-at the moment it's not required to load Starlark rules.
+--------------------------------------------------------------------------------
+
+## <a name="native-java-import"></a>All Java build rules should be loaded from Starlark
+
+  * Category name: `native-java-import`
+  * Automatic fix: yes
+  * [Disabled by default](buildifier/README.md#linter)
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-import`
+
+The Java build rules should be loaded from Starlark.
+
+--------------------------------------------------------------------------------
+
+## <a name="native-java-library"></a>All Java build rules should be loaded from Starlark
+
+  * Category name: `native-java-library`
+  * Automatic fix: yes
+  * [Disabled by default](buildifier/README.md#linter)
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-library`
+
+The Java build rules should be loaded from Starlark.
+
+--------------------------------------------------------------------------------
+
+## <a name="native-java-package-config"></a>All Java build rules should be loaded from Starlark
+
+  * Category name: `native-java-package-config`
+  * Automatic fix: yes
+  * [Disabled by default](buildifier/README.md#linter)
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-package-config`
+
+The Java build rules should be loaded from Starlark.
+
+--------------------------------------------------------------------------------
+
+## <a name="native-java-plugin"></a>All Java build rules should be loaded from Starlark
+
+  * Category name: `native-java-plugin`
+  * Automatic fix: yes
+  * [Disabled by default](buildifier/README.md#linter)
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-plugin`
+
+The Java build rules should be loaded from Starlark.
+
+--------------------------------------------------------------------------------
+
+## <a name="native-java-runtime"></a>All Java build rules should be loaded from Starlark
+
+  * Category name: `native-java-runtime`
+  * Automatic fix: yes
+  * [Disabled by default](buildifier/README.md#linter)
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-runtime`
+
+The Java build rules should be loaded from Starlark.
+
+--------------------------------------------------------------------------------
+
+## <a name="native-java-test"></a>All Java build rules should be loaded from Starlark
+
+  * Category name: `native-java-test`
+  * Automatic fix: yes
+  * [Disabled by default](buildifier/README.md#linter)
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-test`
+
+The Java build rules should be loaded from Starlark.
+
+--------------------------------------------------------------------------------
+
+## <a name="native-java-toolchain"></a>All Java build rules should be loaded from Starlark
+
+  * Category name: `native-java-toolchain`
+  * Automatic fix: yes
+  * [Disabled by default](buildifier/README.md#linter)
+  * [Suppress the warning](#suppress): `# buildifier: disable=native-java-toolchain`
+
+The Java build rules should be loaded from Starlark.
 
 --------------------------------------------------------------------------------
 
diff --git a/buildifier/config/config_test.go b/buildifier/config/config_test.go
index 741dc05ec..b26c8c4f2 100644
--- a/buildifier/config/config_test.go
+++ b/buildifier/config/config_test.go
@@ -81,7 +81,14 @@ func ExampleExample() {
 	//     "native-android",
 	//     "native-build",
 	//     "native-cc",
-	//     "native-java",
+	//     "native-java-binary",
+	//     "native-java-import",
+	//     "native-java-library",
+	//     "native-java-package-config",
+	//     "native-java-plugin",
+	//     "native-java-runtime",
+	//     "native-java-test",
+	//     "native-java-toolchain",
 	//     "native-package",
 	//     "native-proto",
 	//     "native-py",
@@ -265,7 +272,14 @@ func TestValidate(t *testing.T) {
 			"native-android",
 			"native-build",
 			"native-cc",
-			"native-java",
+			"native-java-binary",
+			"native-java-import",
+			"native-java-library",
+			"native-java-package-config",
+			"native-java-plugin",
+			"native-java-runtime",
+			"native-java-test",
+			"native-java-toolchain",
 			"native-package",
 			"native-proto",
 			"native-py",
diff --git a/buildifier/integration_test.sh b/buildifier/integration_test.sh
index 87cf9caf5..5b352adff 100755
--- a/buildifier/integration_test.sh
+++ b/buildifier/integration_test.sh
@@ -285,7 +285,14 @@ cat > golden/.buildifier.example.json <<EOF
     "native-android",
     "native-build",
     "native-cc",
-    "native-java",
+    "native-java-binary",
+    "native-java-import",
+    "native-java-library",
+    "native-java-package-config",
+    "native-java-plugin",
+    "native-java-runtime",
+    "native-java-test",
+    "native-java-toolchain",
     "native-package",
     "native-proto",
     "native-py",
diff --git a/tables/tables.go b/tables/tables.go
index 720487edb..e41ff96cf 100644
--- a/tables/tables.go
+++ b/tables/tables.go
@@ -246,22 +246,8 @@ var CcNativeRules = []string{
 // CcLoadPath is the load path for the Starlark C++ Rules.
 var CcLoadPath = "@rules_cc//cc:defs.bzl"
 
-// JavaNativeRules lists all Java rules that are being migrated from Native to Starlark.
-var JavaNativeRules = []string{
-	"java_binary",
-	"java_import",
-	"java_library",
-	"java_lite_proto_library",
-	"java_proto_library",
-	"java_test",
-	"java_package_configuration",
-	"java_plugin",
-	"java_runtime",
-	"java_toolchain",
-}
-
-// JavaLoadPath is the load path for the Starlark Java Rules.
-var JavaLoadPath = "@rules_java//java:defs.bzl"
+// JavaRulesLoadPrefix is the prefix for the Java rules load paths.
+var JavaRulesLoadPrefix = "@rules_java//java"
 
 // PyNativeRules lists all Python rules that are being migrated from Native to Starlark.
 var PyNativeRules = []string{
diff --git a/warn/docs/warnings.textproto b/warn/docs/warnings.textproto
index e37df9c77..a17821940 100644
--- a/warn/docs/warnings.textproto
+++ b/warn/docs/warnings.textproto
@@ -490,15 +490,58 @@ warnings: {
 }
 
 warnings: {
-  name: "native-java"
+  name: "native-java-binary"
   header: "All Java build rules should be loaded from Starlark"
-  description:
-    "The Java build rules should be loaded from Starlark.\n\n"
-    "Update: the plans for disabling native rules\n"
-    "[have been postponed](https://groups.google.com/g/bazel-discuss/c/XNvpWcge4AE/m/aJ-aQzszAwAJ),\n"
-    "at the moment it's not required to load Starlark rules."
-  bazel_flag: "--incompatible_load_java_rules_from_bzl"
-  bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/8746"
+  description: "The Java build rules should be loaded from Starlark."
+  autofix: true
+}
+
+warnings: {
+  name: "native-java-import"
+  header: "All Java build rules should be loaded from Starlark"
+  description: "The Java build rules should be loaded from Starlark."
+  autofix: true
+}
+
+warnings: {
+  name: "native-java-library"
+  header: "All Java build rules should be loaded from Starlark"
+  description: "The Java build rules should be loaded from Starlark."
+  autofix: true
+}
+
+warnings: {
+  name: "native-java-package-config"
+  header: "All Java build rules should be loaded from Starlark"
+  description: "The Java build rules should be loaded from Starlark."
+  autofix: true
+}
+
+warnings: {
+  name: "native-java-plugin"
+  header: "All Java build rules should be loaded from Starlark"
+  description: "The Java build rules should be loaded from Starlark."
+  autofix: true
+}
+
+warnings: {
+  name: "native-java-runtime"
+  header: "All Java build rules should be loaded from Starlark"
+  description: "The Java build rules should be loaded from Starlark."
+  autofix: true
+}
+
+warnings: {
+  name: "native-java-test"
+  header: "All Java build rules should be loaded from Starlark"
+  description: "The Java build rules should be loaded from Starlark."
+  autofix: true
+}
+
+warnings: {
+  name: "native-java-toolchain"
+  header: "All Java build rules should be loaded from Starlark"
+  description: "The Java build rules should be loaded from Starlark."
   autofix: true
 }
 
diff --git a/warn/warn.go b/warn/warn.go
index 346d9eceb..1c7c088e2 100644
--- a/warn/warn.go
+++ b/warn/warn.go
@@ -154,7 +154,14 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
 	"native-android":            nativeAndroidRulesWarning,
 	"native-build":              nativeInBuildFilesWarning,
 	"native-cc":                 nativeCcRulesWarning,
-	"native-java":               nativeJavaRulesWarning,
+	"native-java-binary":         nativeJavaBinaryWarning,
+	"native-java-import":         nativeJavaImportWarning,
+	"native-java-library":        nativeJavaLibraryWarning,
+	"native-java-package-config": nativeJavaPackageConfigWarning,
+	"native-java-plugin":         nativeJavaPluginWarning,
+	"native-java-runtime":        nativeJavaRuntimeWarning,
+	"native-java-test":           nativeJavaTestWarning,
+	"native-java-toolchain":      nativeJavaToolchainWarning,
 	"native-package":            nativePackageWarning,
 	"native-proto":              nativeProtoRulesWarning,
 	"native-py":                 nativePyRulesWarning,
@@ -192,7 +199,14 @@ var nonDefaultWarnings = map[string]bool{
 	"unsorted-dict-items": true, // dict items should be sorted
 	"native-android":      true, // disables native android rules
 	"native-cc":           true, // disables native cc rules
-	"native-java":         true, // disables native java rules
+	"native-java-binary":         true, // disables native java_binary rule
+	"native-java-import":         true, // disables native java_import rule
+	"native-java-library":        true, // disables native java_library rule
+	"native-java-package-config": true, // disables native java_package_configuration rule
+	"native-java-plugin":         true, // disables native java_plugin rule
+	"native-java-runtime":        true, // disables native java_runtime rule
+	"native-java-test":           true, // disables native java_test rule
+	"native-java-toolchain":      true, // disables native java_toolchain rule
 	"native-proto":        true, // disables native proto rules
 	"native-py":           true, // disables native python rules
 }
diff --git a/warn/warn_bazel_api.go b/warn/warn_bazel_api.go
index ecce12fa4..e169b88ad 100644
--- a/warn/warn_bazel_api.go
+++ b/warn/warn_bazel_api.go
@@ -695,11 +695,43 @@ func nativeCcRulesWarning(f *build.File) []*LinterFinding {
 	return NotLoadedFunctionUsageCheck(f, tables.CcNativeRules, tables.CcLoadPath)
 }
 
-func nativeJavaRulesWarning(f *build.File) []*LinterFinding {
-	if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
+func nativeJavaRulesWarning(f *build.File, symbol string, loadPath string) []*LinterFinding {
+	if f.Type != build.TypeBzl {
 		return nil
 	}
-	return NotLoadedFunctionUsageCheck(f, tables.JavaNativeRules, tables.JavaLoadPath)
+	return NotLoadedFunctionUsageCheck(f, []string{symbol}, tables.JavaRulesLoadPrefix+loadPath)
+}
+
+func nativeJavaBinaryWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_binary", ":java_binary.bzl")
+}
+
+func nativeJavaImportWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_import", ":java_import.bzl")
+}
+
+func nativeJavaLibraryWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_library", ":java_library.bzl")
+}
+
+func nativeJavaPackageConfigWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_package_configuration", "/toolchains:java_package_configuration.bzl")
+}
+
+func nativeJavaPluginWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_plugin", ":java_plugin.bzl")
+}
+
+func nativeJavaRuntimeWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_runtime", "/toolchains:java_runtime.bzl")
+}
+
+func nativeJavaTestWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_test", ":java_test.bzl")
+}
+
+func nativeJavaToolchainWarning(f *build.File) []*LinterFinding {
+	return nativeJavaRulesWarning(f, "java_toolchain", "/toolchains:java_toolchain.bzl")
 }
 
 func nativePyRulesWarning(f *build.File) []*LinterFinding {
diff --git a/warn/warn_bazel_api_test.go b/warn/warn_bazel_api_test.go
index b23f970ce..71f5d50ff 100644
--- a/warn/warn_bazel_api_test.go
+++ b/warn/warn_bazel_api_test.go
@@ -596,38 +596,38 @@ cc_import()
 		scopeBzl|scopeBuild)
 }
 
-func TestNativeJavaWarning(t *testing.T) {
-	checkFindingsAndFix(t, "native-java", `
+func TestNativeJavaRulesWarnings(t *testing.T) {
+  for category, config := range map[string][]string {
+    "native-java-binary" : []string{"java_binary", ""},
+    "native-java-import" : []string{"java_import", ""},
+    "native-java-library" : []string{"java_library", ""},
+    "native-java-package-config" : []string{"java_package_configuration", "/toolchains"},
+    "native-java-plugin" : []string{"java_plugin", ""},
+    "native-java-runtime" : []string{"java_runtime", "/toolchains"},
+    "native-java-test" : []string{"java_test", ""},
+    "native-java-toolchain" : []string{"java_toolchain", "/toolchains"},
+  } {
+    var rule = config[0]
+    var loadPath = tables.JavaRulesLoadPrefix + config[1] + ":" + rule + ".bzl"
+    checkFindingsAndFix(t, category, fmt.Sprintf(`
 """My file"""
 
 def macro():
-    java_import()
-    java_library()
-    native.java_library()
-    native.java_binary()
-
-java_test()
-`, fmt.Sprintf(`
+    native.%s()
+  `, rule), fmt.Sprintf(`
 """My file"""
 
-load(%q, "java_binary", "java_import", "java_library", "java_test")
+load(%q, "%s")
 
 def macro():
-    java_import()
-    java_library()
-    java_library()
-    java_binary()
-
-java_test()
-`, tables.JavaLoadPath),
-		[]string{
-			fmt.Sprintf(`:4: Function "java_import" is not global anymore and needs to be loaded from "%s".`, tables.JavaLoadPath),
-			fmt.Sprintf(`:5: Function "java_library" is not global anymore and needs to be loaded from "%s".`, tables.JavaLoadPath),
-			fmt.Sprintf(`:6: Function "java_library" is not global anymore and needs to be loaded from "%s".`, tables.JavaLoadPath),
-			fmt.Sprintf(`:7: Function "java_binary" is not global anymore and needs to be loaded from "%s".`, tables.JavaLoadPath),
-			fmt.Sprintf(`:9: Function "java_test" is not global anymore and needs to be loaded from "%s".`, tables.JavaLoadPath),
-		},
-		scopeBzl|scopeBuild)
+    %s()
+  `, loadPath, rule, rule),
+      []string{
+        fmt.Sprintf(`:4: Function "%s" is not global anymore and needs to be loaded from "%s".`,
+        rule, loadPath),
+      },
+      scopeBzl)
+  }
 }
 
 func TestNativePyWarning(t *testing.T) {