diff --git a/CHANGELOG.md b/CHANGELOG.md index b36ceafa27..d54d3a8881 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,6 +116,8 @@ END_UNRELEASED_TEMPLATE 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 cf91461e39..222c1171ab 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`](#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 @@ 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. 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 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 279bee6af7..5b6ba79d69 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() @@ -569,7 +569,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 { @@ -582,10 +582,16 @@ func generateProtoLibraries(args language.GenerateArgs, pythonProjectRoot string // 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 @@ 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) + 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 0000000000..594379cdfc --- /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 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/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; +} 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)