Skip to content

Commit

Permalink
Require proto rules to be loaded by buildifier (#1310)
Browse files Browse the repository at this point in the history 
* Require proto rules to be loaded by buildifier

Because the rules are moved into the repository the loads become mandatory with Bazel@HEAD.

Add new warning for each rule and symbol that needs to be loaded. Buildifier doesn't handle well loading from different bzl files in a single warning.

Enable all proto rules warning by default.

* Format with gofmt

* Add missing quote

* Add and regenerated docs

* Fix tests

* Remove \n\n
  • Loading branch information
@comius
comius authored Nov 29, 2024
1 parent be1c24c commit a0444eb
Show file tree
Hide file tree
Showing 9 changed files with 323 additions and 126 deletions.
93 changes: 86 additions & 7 deletions WARNINGS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,16 @@ Warning categories supported by buildifier's linter:
* [`native-android`](#native-android)
* [`native-build`](#native-build)
* [`native-cc`](#native-cc)
* [`native-cc-proto`](#native-cc-proto)
* [`native-java`](#native-java)
* [`native-java-lite-proto`](#native-java-lite-proto)
* [`native-java-proto`](#native-java-proto)
* [`native-package`](#native-package)
* [`native-proto`](#native-proto)
* [`native-proto-common`](#native-proto-common)
* [`native-proto-info`](#native-proto-info)
* [`native-proto-lang-toolchain`](#native-proto-lang-toolchain)
* [`native-proto-lang-toolchain-info`](#native-proto-lang-toolchain-info)
* [`native-py`](#native-py)
* [`no-effect`](#no-effect)
* [`out-of-order-load`](#out-of-order-load)
Expand Down Expand Up @@ -695,6 +702,17 @@ at the moment it's not required to load Starlark rules.

--------------------------------------------------------------------------------

## <a name="native-cc-proto"></a>cc_proto_library rule should be loaded from Starlark

* Category name: `native-cc-proto`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-cc-proto`

The cc_proto_library rule should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-java"></a>All Java build rules should be loaded from Starlark

* Category name: `native-java`
Expand All @@ -711,6 +729,28 @@ at the moment it's not required to load Starlark rules.

--------------------------------------------------------------------------------

## <a name="native-java-lite-proto"></a>java_lite_proto_library rule should be loaded from Starlark

* Category name: `native-java-lite-proto`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-java-lite-proto`

The java_lite_proto_library rule should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-java-proto"></a>java_proto_library rule should be loaded from Starlark

* Category name: `native-java-proto`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-java-proto`

The java_proto_library rule should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-package"></a>`native.package()` shouldn't be used in .bzl files

* Category name: `native-package`
Expand All @@ -722,19 +762,58 @@ It can silently modify the semantics of a BUILD file and makes it hard to mainta

--------------------------------------------------------------------------------

## <a name="native-proto"></a>All Proto build rules and symbols should be loaded from Starlark
## <a name="native-proto"></a>proto_library rule should be loaded from Starlark

* Category name: `native-proto`
* Flag in Bazel: [`--incompatible_load_proto_rules_from_bzl`](https://github.com/bazelbuild/bazel/issues/8922)
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Disabled by default](buildifier/README.md#linter)
* [Suppress the warning](#suppress): `# buildifier: disable=native-proto`

The Proto build rules should be loaded from Starlark.
The proto_library rule 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-proto-common"></a>proto_common module should be loaded from Starlark

* Category name: `native-proto-common`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-proto-common`

The proto_common module should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-proto-info"></a>ProtoInfo provider should be loaded from Starlark

* Category name: `native-proto-info`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-proto-info`

The ProtoInfo provider should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-proto-lang-toolchain"></a>proto_lang_toolchain rule should be loaded from Starlark

* Category name: `native-proto-lang-toolchain`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-proto-lang-toolchain`

The proto_lang_toolchain rule should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-proto-lang-toolchain-info"></a>ProtoLangToolchainInfo provider should be loaded from Starlark

* Category name: `native-proto-lang-toolchain-info`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-proto-lang-toolchain-info`

The ProtoLangToolchainInfo provider should be loaded from Starlark.

--------------------------------------------------------------------------------

Expand Down
32 changes: 30 additions & 2 deletions buildifier/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,16 @@ func ExampleExample() {
// "native-android",
// "native-build",
// "native-cc",
// "native-cc-proto",
// "native-java",
// "native-java-lite-proto",
// "native-java-proto",
// "native-package",
// "native-proto",
// "native-proto-common",
// "native-proto-info",
// "native-proto-lang-toolchain",
// "native-proto-lang-toolchain-info",
// "native-py",
// "no-effect",
// "output-group",
Expand Down Expand Up @@ -260,9 +267,16 @@ func TestValidate(t *testing.T) {
"native-android",
"native-build",
"native-cc",
"native-cc-proto",
"native-java",
"native-java-lite-proto",
"native-java-proto",
"native-package",
"native-proto",
"native-proto-common",
"native-proto-info",
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
"native-py",
"no-effect",
"output-group",
Expand Down Expand Up @@ -322,9 +336,16 @@ func TestValidate(t *testing.T) {
// "native-android",
"native-build",
// "native-cc",
"native-cc-proto",
// "native-java",
"native-java-lite-proto",
"native-java-proto",
"native-package",
// "native-proto",
"native-proto",
"native-proto-common",
"native-proto-info",
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
// "native-py",
"no-effect",
"output-group",
Expand Down Expand Up @@ -383,9 +404,16 @@ func TestValidate(t *testing.T) {
"name-conventions",
// "native-android",
"native-build",
"native-cc-proto",
// "native-java",
"native-java-lite-proto",
"native-java-proto",
"native-package",
// "native-proto",
"native-proto",
"native-proto-common",
"native-proto-info",
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
// "native-py",
"no-effect",
"output-group",
Expand Down
7 changes: 7 additions & 0 deletions buildifier/integration_test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,16 @@ cat > golden/.buildifier.example.json <<EOF
"native-android",
"native-build",
"native-cc",
"native-cc-proto",
"native-java",
"native-java-lite-proto",
"native-java-proto",
"native-package",
"native-proto",
"native-proto-common",
"native-proto-info",
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
"native-py",
"no-effect",
"output-group",
Expand Down
19 changes: 2 additions & 17 deletions tables/tables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ var CcNativeRules = []string{
"cc_test",
"cc_library",
"cc_import",
"cc_proto_library",
"fdo_prefetch_hints",
"fdo_profile",
"cc_toolchain",
Expand All @@ -251,8 +250,6 @@ var JavaNativeRules = []string{
"java_binary",
"java_import",
"java_library",
"java_lite_proto_library",
"java_proto_library",
"java_test",
"java_package_configuration",
"java_plugin",
Expand All @@ -274,20 +271,8 @@ var PyNativeRules = []string{
// PyLoadPath is the load path for the Starlark Python Rules.
var PyLoadPath = "@rules_python//python:defs.bzl"

// ProtoNativeRules lists all Proto rules that are being migrated from Native to Starlark.
var ProtoNativeRules = []string{
"proto_lang_toolchain",
"proto_library",
}

// ProtoNativeSymbols lists all Proto symbols that are being migrated from Native to Starlark.
var ProtoNativeSymbols = []string{
"ProtoInfo",
"proto_common",
}

// ProtoLoadPath is the load path for the Starlark Proto Rules.
var ProtoLoadPath = "@rules_proto//proto:defs.bzl"
// ProtoLoadPathPrefix is the load path prefix for the Starlark Proto Rules.
var ProtoLoadPathPrefix = "@com_google_protobuf//bazel"

// IsModuleOverride contains the names of all Bzlmod module overrides available in MODULE.bazel.
var IsModuleOverride = map[string]bool{
Expand Down
81 changes: 74 additions & 7 deletions warn/docs/warnings.textproto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,14 +515,81 @@ warnings: {

warnings: {
name: "native-proto"
header: "All Proto build rules and symbols should be loaded from Starlark"
header: "proto_library rule should be loaded from Starlark"
description:
"The Proto 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_proto_rules_from_bzl"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/8922"
"The proto_library rule should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

warnings: {
name: "native-java-proto"
header: "java_proto_library rule should be loaded from Starlark"
description:
"The java_proto_library rule should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

warnings: {
name: "native-java-lite-proto"
header: "java_lite_proto_library rule should be loaded from Starlark"
description:
"The java_lite_proto_library rule should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

warnings: {
name: "native-cc-proto"
header: "cc_proto_library rule should be loaded from Starlark"
description:
"The cc_proto_library rule should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

warnings: {
name: "native-proto-lang-toolchain"
header: "proto_lang_toolchain rule should be loaded from Starlark"
description:
"The proto_lang_toolchain rule should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

warnings: {
name: "native-proto-info"
header: "ProtoInfo provider should be loaded from Starlark"
description:
"The ProtoInfo provider should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

warnings: {
name: "native-proto-lang-toolchain-info"
header: "ProtoLangToolchainInfo provider should be loaded from Starlark"
description:
"The ProtoLangToolchainInfo provider should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

warnings: {
name: "native-proto-common"
header: "proto_common module should be loaded from Starlark"
description:
"The proto_common module should be loaded from Starlark."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

Expand Down
Loading

0 comments on commit a0444eb

Please sign in to comment.