From 9fc356529eae4d00234cadcc902d804810702395 Mon Sep 17 00:00:00 2001 From: Charles Guo Date: Sun, 13 Jul 2025 14:25:22 -0400 Subject: [PATCH 1/5] Add gazelle:python_proto_naming_convention directive, and a bunch of tests --- gazelle/python/configure.go | 3 +++ gazelle/python/generate.go | 6 +++--- .../README.md | 10 +++++++++ .../WORKSPACE | 1 + .../test.yaml | 3 +++ .../BUILD.in | 17 +++++++++++++++ .../BUILD.out | 17 +++++++++++++++ .../foo.proto | 7 +++++++ .../BUILD.in | 9 ++++++++ .../BUILD.out | 16 ++++++++++++++ .../foo.proto | 7 +++++++ .../BUILD.in | 10 +++++++++ .../BUILD.out | 17 +++++++++++++++ .../foo.proto | 7 +++++++ gazelle/pythonconfig/pythonconfig.go | 21 +++++++++++++++++++ 15 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/README.md create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/WORKSPACE create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test.yaml create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.in create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/BUILD.out create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test1_python_generation_disabled_does_nothing/foo.proto create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.in create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/BUILD.out create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test2_python_generation_enabled_uses_default/foo.proto create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.in create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/BUILD.out create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test3_python_generation_enabled_uses_value/foo.proto diff --git a/gazelle/python/configure.go b/gazelle/python/configure.go index 7131be283d..079f1d84d4 100644 --- a/gazelle/python/configure.go +++ b/gazelle/python/configure.go @@ -63,6 +63,7 @@ func (py *Configurer) KnownDirectives() []string { pythonconfig.LibraryNamingConvention, pythonconfig.BinaryNamingConvention, pythonconfig.TestNamingConvention, + pythonconfig.ProtoNamingConvention, pythonconfig.DefaultVisibilty, pythonconfig.Visibility, pythonconfig.TestFilePattern, @@ -179,6 +180,8 @@ func (py *Configurer) Configure(c *config.Config, rel string, f *rule.File) { 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 343743559f..6fb00fa805 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -227,7 +227,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes result.Gen = make([]*rule.Rule, 0) if cfg.GenerateProto() { - generateProtoLibraries(args, pythonProjectRoot, visibility, &result) + generateProtoLibraries(args, cfg, pythonProjectRoot, visibility, &result) } collisionErrors := singlylinkedlist.New() @@ -556,7 +556,7 @@ func ensureNoCollision(file *rule.File, targetName, kind string) error { 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 { @@ -580,7 +580,7 @@ func generateProtoLibraries(args language.GenerateArgs, pythonProjectRoot string 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) 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 0000000000..4901574a59 --- /dev/null +++ b/gazelle/python/testdata/directive_python_proto_naming_convention/README.md @@ -0,0 +1,10 @@ +# 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. +2. Uses the provided naming convention when proto generation is on and `python_proto_naming_convention` is set. + +[gh-3081]: https://github.com/bazel-contrib/rules_python/issues/3081 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 0000000000..faff6af87a --- /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 0000000000..36dd656b39 --- /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 0000000000..2171d877f4 --- /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 0000000000..2171d877f4 --- /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 0000000000..022e29ae69 --- /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 0000000000..4713404b19 --- /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 0000000000..686252f27c --- /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 0000000000..fe2af27aa6 --- /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 0000000000..b68a9937dc --- /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 0000000000..f432e9a0c3 --- /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 0000000000..fe2af27aa6 --- /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/pythonconfig/pythonconfig.go b/gazelle/pythonconfig/pythonconfig.go index b76e1f92ec..001fd334a4 100644 --- a/gazelle/pythonconfig/pythonconfig.go +++ b/gazelle/pythonconfig/pythonconfig.go @@ -74,6 +74,12 @@ const ( // 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 ( const ( packageNameNamingConventionSubstitution = "$package_name$" + protoNameNamingConventionSubstitution = "$proto_name$" distributionNameLabelConventionSubstitution = "$distribution_name$" ) @@ -182,6 +189,7 @@ type Config struct { libraryNamingConvention string binaryNamingConvention string testNamingConvention string + protoNamingConvention string defaultVisibility []string visibility []string testFilePattern []string @@ -220,6 +228,7 @@ func New( 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 @@ func (c *Config) NewChild() *Config { 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 @@ func (c *Config) RenderTestName(packageName string) string { 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) From b0e51b629549b1cffe8409f3230a687c02a8c5a7 Mon Sep 17 00:00:00 2001 From: Charles OuGuo Date: Mon, 14 Jul 2025 21:25:01 -0400 Subject: [PATCH 2/5] Update README.md, CHANGELOG.md --- CHANGELOG.md | 2 ++ gazelle/README.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e74f14b1db..3194f9356b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,8 @@ END_UNRELEASED_TEMPLATE * 3.14.0b3 * (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. See `gazelle/README.md`. {#v0-0-0-removed} ### Removed diff --git a/gazelle/README.md b/gazelle/README.md index 35a1e4f701..ae1ae25376 100644 --- a/gazelle/README.md +++ b/gazelle/README.md @@ -208,6 +208,8 @@ Python-specific directives are as follows: | 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` | `$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) | | From 118aca0c8381326b1cb9482a0273a8d04f214ef1 Mon Sep 17 00:00:00 2001 From: Charles Guo Date: Tue, 15 Jul 2025 01:27:42 -0400 Subject: [PATCH 3/5] Fixup documentation --- CHANGELOG.md | 2 +- gazelle/README.md | 29 +++++++++++++++++-- .../README.md | 4 +-- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9caeb3dd8..26c5c710e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,7 +114,7 @@ END_UNRELEASED_TEMPLATE * (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. See `gazelle/README.md`. + naming of `py_proto_library` rules. {#v0-0-0-removed} ### Removed diff --git a/gazelle/README.md b/gazelle/README.md index ebd2657a28..126689b7a1 100644 --- a/gazelle/README.md +++ b/gazelle/README.md @@ -208,8 +208,8 @@ Python-specific directives are as follows: | 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` | `$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: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) | | @@ -264,6 +264,31 @@ py_libary( [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. 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`. + +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. #### Directive: `python_default_visibility`: diff --git a/gazelle/python/testdata/directive_python_proto_naming_convention/README.md b/gazelle/python/testdata/directive_python_proto_naming_convention/README.md index 4901574a59..6983d2eefe 100644 --- a/gazelle/python/testdata/directive_python_proto_naming_convention/README.md +++ b/gazelle/python/testdata/directive_python_proto_naming_convention/README.md @@ -5,6 +5,4 @@ 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. -2. Uses the provided naming convention when proto generation is on and `python_proto_naming_convention` is set. - -[gh-3081]: https://github.com/bazel-contrib/rules_python/issues/3081 +3. Uses the provided naming convention when proto generation is on and `python_proto_naming_convention` is set. From 7dafe83d4d274f1bf0f5ae49d85fa7a2a78669cd Mon Sep 17 00:00:00 2001 From: Charles Guo Date: Tue, 15 Jul 2025 01:36:48 -0400 Subject: [PATCH 4/5] Link to github issue --- gazelle/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gazelle/README.md b/gazelle/README.md index 126689b7a1..967c6215b4 100644 --- a/gazelle/README.md +++ b/gazelle/README.md @@ -288,7 +288,7 @@ py_proto_library( The default naming convention is `$proto_name$_pb2_py`, so by default in the above example Gazelle would generate `foo_pb2_py`. -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. +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`: From d8b3f5dac36920fb3e7874f941560256dfb65deb Mon Sep 17 00:00:00 2001 From: Charles Guo Date: Tue, 15 Jul 2025 02:03:45 -0400 Subject: [PATCH 5/5] Add test ensuring that pre-existing targets are kept intact when they violate the naming convention, and adjust behaviour to match --- gazelle/README.md | 4 ++-- gazelle/python/generate.go | 11 +++++++++++ .../README.md | 1 + .../BUILD.in | 16 ++++++++++++++++ .../BUILD.out | 17 +++++++++++++++++ .../foo.proto | 7 +++++++ 6 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.in create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/BUILD.out create mode 100644 gazelle/python/testdata/directive_python_proto_naming_convention/test4_python_generation_enabled_with_preexisting_keeps_intact/foo.proto diff --git a/gazelle/README.md b/gazelle/README.md index 967c6215b4..222c1171ab 100644 --- a/gazelle/README.md +++ b/gazelle/README.md @@ -266,7 +266,7 @@ py_libary( #### Directive: `python_proto_naming_convention`: -Set this directive to a string pattern to control how the generated `py_proto_library` targets are named. Gazelle will replace `$proto_name$` in the pattern with the name of the `proto_library` rule, stripping out a trailing `_proto`. For example: +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 @@ -286,7 +286,7 @@ py_proto_library( ) ``` -The default naming convention is `$proto_name$_pb2_py`, so by default in the above example Gazelle would generate `foo_pb2_py`. +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). diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index 8503a1c47a..5b6ba79d69 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -582,10 +582,16 @@ func generateProtoLibraries(args language.GenerateArgs, cfg *pythonconfig.Config // 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() + } } } } @@ -594,6 +600,11 @@ func generateProtoLibraries(args language.GenerateArgs, cfg *pythonconfig.Config // Generate a py_proto_library for each proto_library. for _, protoRuleName := range protoRuleNames { 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 index 6983d2eefe..594379cdfc 100644 --- a/gazelle/python/testdata/directive_python_proto_naming_convention/README.md +++ b/gazelle/python/testdata/directive_python_proto_naming_convention/README.md @@ -6,3 +6,4 @@ 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/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 0000000000..cc7d120a7e --- /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 0000000000..080b83f1fb --- /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 0000000000..fe2af27aa6 --- /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; +}