feat(gazelle): `python_proto_naming_convention` directive controls `py_proto_library` naming (#3093)
Closes https://github.com/bazel-contrib/rules_python/issues/3081.
This adds support in the Gazelle plugin for controlling how the
generated `py_proto_library` rules are named; support for these was
originally added in
https://github.com/bazel-contrib/rules_python/pull/3057. We do this via
a new Gazelle directive, `python_proto_naming_convention`, which is
similar to `python_library_naming_convention` and the like, except it
interpolates `$proto_name$`, which is the `proto_library` rule minus any
trailing `_proto`. We default to `$proto_name$_py_pb2`.
For instance, for a `proto_library` named `foo_proto`, the default value
would generate `foo_py_pb2`, aligning with [the convention stated in the
Bazel
docs.](https://bazel.build/reference/be/protocol-buffer#py_proto_library)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b36ceaf..d54d3a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -116,6 +116,8 @@
dep is not added to the {obj}`py_test` target.
* (gazelle) New directive `gazelle:python_generate_proto`; when `true`,
Gazelle generates `py_proto_library` rules for `proto_library`. `false` by default.
+* (gazelle) New directive `gazelle:python_proto_naming_convention`; controls
+ naming of `py_proto_library` rules.
{#v0-0-0-removed}
### Removed
diff --git a/gazelle/README.md b/gazelle/README.md
index cf91461..222c117 100644
--- a/gazelle/README.md
+++ b/gazelle/README.md
@@ -208,6 +208,8 @@
| Controls the `py_binary` naming convention. Follows the same interpolation rules as `python_library_naming_convention`. | |
| `# gazelle:python_test_naming_convention` | `$package_name$_test` |
| Controls the `py_test` naming convention. Follows the same interpolation rules as `python_library_naming_convention`. | |
+| [`# gazelle:python_proto_naming_convention`](#directive-python_proto_naming_convention) | `$proto_name$_py_pb2` |
+| Controls the `py_proto_library` naming convention. It interpolates `$proto_name$` with the proto_library rule name, minus any trailing _proto. E.g. if the proto_library name is `foo_proto`, setting this to `$proto_name$_my_lib` would render to `foo_my_lib`. | |
| `# gazelle:resolve py ...` | n/a |
| Instructs the plugin what target to add as a dependency to satisfy a given import statement. The syntax is `# gazelle:resolve py import-string label` where `import-string` is the symbol in the python `import` statement, and `label` is the Bazel label that Gazelle should write in `deps`. | |
| [`# gazelle:python_default_visibility labels`](#directive-python_default_visibility) | |
@@ -262,6 +264,31 @@
[python-packaging-user-guide]: https://github.com/pypa/packaging.python.org/blob/4c86169a/source/tutorials/packaging-projects.rst
+#### Directive: `python_proto_naming_convention`:
+
+Set this directive to a string pattern to control how the generated `py_proto_library` targets are named. When generating new `py_proto_library` rules, Gazelle will replace `$proto_name$` in the pattern with the name of the `proto_library` rule, stripping out a trailing `_proto`. For example:
+
+```starlark
+# gazelle:python_generate_proto true
+# gazelle:python_proto_naming_convention my_custom_$proto_name$_pattern
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+)
+```
+
+produces the following `py_proto_library` rule:
+```starlark
+py_proto_library(
+ name = "my_custom_foo_pattern",
+ deps = [":foo_proto"],
+)
+```
+
+The default naming convention is `$proto_name$_pb2_py`, so by default in the above example Gazelle would generate `foo_pb2_py`. Any pre-existing rules are left in place and not renamed.
+
+Note that the Python library will always be imported as `foo_pb2` in Python code, regardless of the naming convention. Also note that Gazelle is currently not able to map said imports, e.g. `import foo_pb2`, to fill in `py_proto_library` targets as dependencies of other rules. See [this issue](https://github.com/bazel-contrib/rules_python/issues/1703).
#### Directive: `python_default_visibility`:
diff --git a/gazelle/python/configure.go b/gazelle/python/configure.go
index 7131be2..079f1d8 100644
--- a/gazelle/python/configure.go
+++ b/gazelle/python/configure.go
@@ -63,6 +63,7 @@
pythonconfig.LibraryNamingConvention,
pythonconfig.BinaryNamingConvention,
pythonconfig.TestNamingConvention,
+ pythonconfig.ProtoNamingConvention,
pythonconfig.DefaultVisibilty,
pythonconfig.Visibility,
pythonconfig.TestFilePattern,
@@ -179,6 +180,8 @@
config.SetBinaryNamingConvention(strings.TrimSpace(d.Value))
case pythonconfig.TestNamingConvention:
config.SetTestNamingConvention(strings.TrimSpace(d.Value))
+ case pythonconfig.ProtoNamingConvention:
+ config.SetProtoNamingConvention(strings.TrimSpace(d.Value))
case pythonconfig.DefaultVisibilty:
switch directiveArg := strings.TrimSpace(d.Value); directiveArg {
case "NONE":
diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go
index 279bee6..5b6ba79 100644
--- a/gazelle/python/generate.go
+++ b/gazelle/python/generate.go
@@ -227,7 +227,7 @@
result.Gen = make([]*rule.Rule, 0)
if cfg.GenerateProto() {
- generateProtoLibraries(args, pythonProjectRoot, visibility, &result)
+ generateProtoLibraries(args, cfg, pythonProjectRoot, visibility, &result)
}
collisionErrors := singlylinkedlist.New()
@@ -569,7 +569,7 @@
return nil
}
-func generateProtoLibraries(args language.GenerateArgs, pythonProjectRoot string, visibility []string, res *language.GenerateResult) {
+func generateProtoLibraries(args language.GenerateArgs, cfg *pythonconfig.Config, pythonProjectRoot string, visibility []string, res *language.GenerateResult) {
// First, enumerate all the proto_library in this package.
var protoRuleNames []string
for _, r := range args.OtherGen {
@@ -582,10 +582,16 @@
// Next, enumerate all the pre-existing py_proto_library in this package, so we can delete unnecessary rules later.
pyProtoRules := map[string]bool{}
+ pyProtoRulesForProto := map[string]string{}
if args.File != nil {
for _, r := range args.File.Rules {
if r.Kind() == "py_proto_library" {
pyProtoRules[r.Name()] = false
+
+ protos := r.AttrStrings("deps")
+ for _, proto := range protos {
+ pyProtoRulesForProto[strings.TrimPrefix(proto, ":")] = r.Name()
+ }
}
}
}
@@ -593,7 +599,12 @@
emptySiblings := treeset.Set{}
// Generate a py_proto_library for each proto_library.
for _, protoRuleName := range protoRuleNames {
- pyProtoLibraryName := strings.TrimSuffix(protoRuleName, "_proto") + "_py_pb2"
+ pyProtoLibraryName := cfg.RenderProtoName(protoRuleName)
+ if ruleName, ok := pyProtoRulesForProto[protoRuleName]; ok {
+ // There exists a pre-existing py_proto_library for this proto. Keep this name.
+ pyProtoLibraryName = ruleName
+ }
+
pyProtoLibrary := newTargetBuilder(pyProtoLibraryKind, pyProtoLibraryName, pythonProjectRoot, args.Rel, &emptySiblings).
addVisibility(visibility).
addResolvedDependency(":" + protoRuleName).
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/README.md b/gazelle/python/testdata/directive_python_proto_naming_convention/README.md
new file mode 100644
index 0000000..594379c
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/README.md
@@ -0,0 +1,9 @@
+# Directive: `python_proto_naming_convention`
+
+This test case asserts that the `# gazelle:python_proto_naming_convention` directive
+correctly:
+
+1. Has no effect on pre-existing `py_proto_library` when `gazelle:python_generate_proto` is disabled.
+2. Uses the default value when proto generation is on and `python_proto_naming_convention` is not set.
+3. Uses the provided naming convention when proto generation is on and `python_proto_naming_convention` is set.
+4. With a pre-existing `py_proto_library` not following a given naming convention, keeps it intact and does not rename it.
\ No newline at end of file
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/WORKSPACE b/gazelle/python/testdata/directive_python_proto_naming_convention/WORKSPACE
new file mode 100644
index 0000000..faff6af
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/WORKSPACE
@@ -0,0 +1 @@
+# This is a Bazel workspace for the Gazelle test data.
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test.yaml b/gazelle/python/testdata/directive_python_proto_naming_convention/test.yaml
new file mode 100644
index 0000000..36dd656
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test.yaml
@@ -0,0 +1,3 @@
+---
+expect:
+ exit_code: 0
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.in b/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.in
new file mode 100644
index 0000000..2171d87
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.in
@@ -0,0 +1,17 @@
+load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto false
+# gazelle:python_proto_naming_convention some_$proto_name$_value
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_proto_library(
+ name = "foo_proto_custom_name",
+ visibility = ["//:__subpackages__"],
+ deps = [":foo_proto"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.out b/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.out
new file mode 100644
index 0000000..2171d87
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.out
@@ -0,0 +1,17 @@
+load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto false
+# gazelle:python_proto_naming_convention some_$proto_name$_value
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_proto_library(
+ name = "foo_proto_custom_name",
+ visibility = ["//:__subpackages__"],
+ deps = [":foo_proto"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/foo.proto b/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/foo.proto
new file mode 100644
index 0000000..022e29a
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/foo.proto
@@ -0,0 +1,7 @@
+syntax = "proto3";
+
+package foo.bar;
+
+message Foo {
+ string bar = 1;
+}
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.in b/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.in
new file mode 100644
index 0000000..4713404
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.in
@@ -0,0 +1,9 @@
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto true
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.out b/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.out
new file mode 100644
index 0000000..686252f
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.out
@@ -0,0 +1,16 @@
+load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto true
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_proto_library(
+ name = "foo_py_pb2",
+ visibility = ["//:__subpackages__"],
+ deps = [":foo_proto"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/foo.proto b/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/foo.proto
new file mode 100644
index 0000000..fe2af27
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/foo.proto
@@ -0,0 +1,7 @@
+syntax = "proto3";
+
+package foo;
+
+message Foo {
+ string bar = 1;
+}
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.in b/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.in
new file mode 100644
index 0000000..b68a993
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.in
@@ -0,0 +1,10 @@
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto true
+# gazelle:python_proto_naming_convention some_$proto_name$_value
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.out b/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.out
new file mode 100644
index 0000000..f432e9a
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.out
@@ -0,0 +1,17 @@
+load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto true
+# gazelle:python_proto_naming_convention some_$proto_name$_value
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_proto_library(
+ name = "some_foo_value",
+ visibility = ["//:__subpackages__"],
+ deps = [":foo_proto"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/foo.proto b/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/foo.proto
new file mode 100644
index 0000000..fe2af27
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/foo.proto
@@ -0,0 +1,7 @@
+syntax = "proto3";
+
+package foo;
+
+message Foo {
+ string bar = 1;
+}
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.in b/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.in
new file mode 100644
index 0000000..cc7d120
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.in
@@ -0,0 +1,16 @@
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto true
+# gazelle:python_proto_naming_convention $proto_name$_bar
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_proto_library(
+ name = "foo_py_proto",
+ visibility = ["//:__subpackages__"],
+ deps = [":foo_proto"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.out b/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.out
new file mode 100644
index 0000000..080b83f
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.out
@@ -0,0 +1,17 @@
+load("@com_google_protobuf//bazel:py_proto_library.bzl", "py_proto_library")
+load("@rules_proto//proto:defs.bzl", "proto_library")
+
+# gazelle:python_generate_proto true
+# gazelle:python_proto_naming_convention $proto_name$_bar
+
+proto_library(
+ name = "foo_proto",
+ srcs = ["foo.proto"],
+ visibility = ["//:__subpackages__"],
+)
+
+py_proto_library(
+ name = "foo_py_proto",
+ visibility = ["//:__subpackages__"],
+ deps = [":foo_proto"],
+)
diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/foo.proto b/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/foo.proto
new file mode 100644
index 0000000..fe2af27
--- /dev/null
+++ b/gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/foo.proto
@@ -0,0 +1,7 @@
+syntax = "proto3";
+
+package foo;
+
+message Foo {
+ string bar = 1;
+}
diff --git a/gazelle/pythonconfig/pythonconfig.go b/gazelle/pythonconfig/pythonconfig.go
index b76e1f9..001fd33 100644
--- a/gazelle/pythonconfig/pythonconfig.go
+++ b/gazelle/pythonconfig/pythonconfig.go
@@ -74,6 +74,12 @@
// naming convention. See python_library_naming_convention for more info on
// the package name interpolation.
TestNamingConvention = "python_test_naming_convention"
+ // ProtoNamingConvention represents the directive that controls the
+ // py_proto_library naming convention. It interpolates $proto_name$ with
+ // the proto_library rule name, minus any trailing _proto. E.g. if the
+ // proto_library name is `foo_proto`, setting this to `$proto_name$_my_lib`
+ // would render to `foo_my_lib`.
+ ProtoNamingConvention = "python_proto_naming_convention"
// DefaultVisibilty represents the directive that controls what visibility
// labels are added to generated python targets.
DefaultVisibilty = "python_default_visibility"
@@ -121,6 +127,7 @@
const (
packageNameNamingConventionSubstitution = "$package_name$"
+ protoNameNamingConventionSubstitution = "$proto_name$"
distributionNameLabelConventionSubstitution = "$distribution_name$"
)
@@ -182,6 +189,7 @@
libraryNamingConvention string
binaryNamingConvention string
testNamingConvention string
+ protoNamingConvention string
defaultVisibility []string
visibility []string
testFilePattern []string
@@ -220,6 +228,7 @@
libraryNamingConvention: packageNameNamingConventionSubstitution,
binaryNamingConvention: fmt.Sprintf("%s_bin", packageNameNamingConventionSubstitution),
testNamingConvention: fmt.Sprintf("%s_test", packageNameNamingConventionSubstitution),
+ protoNamingConvention: fmt.Sprintf("%s_py_pb2", protoNameNamingConventionSubstitution),
defaultVisibility: []string{fmt.Sprintf(DefaultVisibilityFmtString, "")},
visibility: []string{},
testFilePattern: strings.Split(DefaultTestFilePatternString, ","),
@@ -255,6 +264,7 @@
libraryNamingConvention: c.libraryNamingConvention,
binaryNamingConvention: c.binaryNamingConvention,
testNamingConvention: c.testNamingConvention,
+ protoNamingConvention: c.protoNamingConvention,
defaultVisibility: c.defaultVisibility,
visibility: c.visibility,
testFilePattern: c.testFilePattern,
@@ -489,6 +499,17 @@
return strings.ReplaceAll(c.testNamingConvention, packageNameNamingConventionSubstitution, packageName)
}
+// SetProtoNamingConvention sets the py_proto_library target naming convention.
+func (c *Config) SetProtoNamingConvention(protoNamingConvention string) {
+ c.protoNamingConvention = protoNamingConvention
+}
+
+// RenderProtoName returns the py_proto_library target name by performing all
+// substitutions.
+func (c *Config) RenderProtoName(protoName string) string {
+ return strings.ReplaceAll(c.protoNamingConvention, protoNameNamingConventionSubstitution, strings.TrimSuffix(protoName, "_proto"))
+}
+
// AppendVisibility adds additional items to the target's visibility.
func (c *Config) AppendVisibility(visibility string) {
c.visibility = append(c.visibility, visibility)