Skip to content

Commit

Permalink
fix: respect kind mapping (#1158)
Browse files Browse the repository at this point in the history
When using the kind `gazelle:map_kind` directive, `gazelle` will
correctly generate the buildfile on the first pass (or if no target of
that type / name are present).

However, when running gazelle a second time (or if a target of the
mapped kind with the same name is present), `gazelle` will error out
saying that it kind create a target of the original kind because a
target of mapped kind is present and has the same name.

Ex:
Given the directive `# gazelle:map_kind py_test py_pytest_test
//src/bazel/rules/python:py_pytest_test.bzl`, `gazelle` will correctly
generate a `py_pytest_test` target where it would have generated a
`py_test` target.

But on a second invocation of `gazelle` (and subsequent invocations) it
will error our with:
```
gazelle: ERROR: failed to generate target "//test/python/common:common_test" of kind "py_test": a target of kind "py_pytest_test" with the same name already exists. Use the '# gazelle:python_test_naming_convention' directive to change the naming convention.
```
  • Loading branch information
OniOni authored Apr 8, 2023
1 parent 86eadf1 commit b80b8fd
Show file tree
Hide file tree
Showing 9 changed files with 134 additions and 8 deletions.
27 changes: 19 additions & 8 deletions gazelle/python/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ var (
buildFilenames = []string{"BUILD", "BUILD.bazel"}
)

func GetActualKindName(kind string, args language.GenerateArgs) string {
if kindOverride, ok := args.Config.KindMap[kind]; ok {
return kindOverride.KindName
}
return kind
}

// GenerateRules extracts build metadata from source files in a directory.
// GenerateRules is called in each directory where an update is requested
// in depth-first post-order.
Expand All @@ -70,6 +77,10 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
}
}

actualPyBinaryKind := GetActualKindName(pyBinaryKind, args)
actualPyLibraryKind := GetActualKindName(pyLibraryKind, args)
actualPyTestKind := GetActualKindName(pyTestKind, args)

pythonProjectRoot := cfg.PythonProjectRoot()

packageName := filepath.Base(args.Dir)
Expand Down Expand Up @@ -217,12 +228,12 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == pyLibraryTargetName && t.Kind() != pyLibraryKind {
if t.Name() == pyLibraryTargetName && t.Kind() != actualPyLibraryKind {
fqTarget := label.New("", args.Rel, pyLibraryTargetName)
err := fmt.Errorf("failed to generate target %q of kind %q: "+
"a target of kind %q with the same name already exists. "+
"Use the '# gazelle:%s' directive to change the naming convention.",
fqTarget.String(), pyLibraryKind, t.Kind(), pythonconfig.LibraryNamingConvention)
fqTarget.String(), actualPyLibraryKind, t.Kind(), pythonconfig.LibraryNamingConvention)
collisionErrors.Add(err)
}
}
Expand Down Expand Up @@ -253,12 +264,12 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == pyBinaryTargetName && t.Kind() != pyBinaryKind {
if t.Name() == pyBinaryTargetName && t.Kind() != actualPyBinaryKind {
fqTarget := label.New("", args.Rel, pyBinaryTargetName)
err := fmt.Errorf("failed to generate target %q of kind %q: "+
"a target of kind %q with the same name already exists. "+
"Use the '# gazelle:%s' directive to change the naming convention.",
fqTarget.String(), pyBinaryKind, t.Kind(), pythonconfig.BinaryNamingConvention)
fqTarget.String(), actualPyBinaryKind, t.Kind(), pythonconfig.BinaryNamingConvention)
collisionErrors.Add(err)
}
}
Expand Down Expand Up @@ -290,11 +301,11 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == conftestTargetname && t.Kind() != pyLibraryKind {
if t.Name() == conftestTargetname && t.Kind() != actualPyLibraryKind {
fqTarget := label.New("", args.Rel, conftestTargetname)
err := fmt.Errorf("failed to generate target %q of kind %q: "+
"a target of kind %q with the same name already exists.",
fqTarget.String(), pyLibraryKind, t.Kind())
fqTarget.String(), actualPyLibraryKind, t.Kind())
collisionErrors.Add(err)
}
}
Expand Down Expand Up @@ -325,12 +336,12 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == pyTestTargetName && t.Kind() != pyTestKind {
if t.Name() == pyTestTargetName && t.Kind() != actualPyTestKind {
fqTarget := label.New("", args.Rel, pyTestTargetName)
err := fmt.Errorf("failed to generate target %q of kind %q: "+
"a target of kind %q with the same name already exists. "+
"Use the '# gazelle:%s' directive to change the naming convention.",
fqTarget.String(), pyTestKind, t.Kind(), pythonconfig.TestNamingConvention)
fqTarget.String(), actualPyTestKind, t.Kind(), pythonconfig.TestNamingConvention)
collisionErrors.Add(err)
}
}
Expand Down
15 changes: 15 additions & 0 deletions gazelle/python/testdata/respect_kind_mapping/BUILD.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
load("@rules_python//python:defs.bzl", "py_library")

# gazelle:map_kind py_test my_test :mytest.bzl

py_library(
name = "respect_kind_mapping",
srcs = ["__init__.py"],
)

my_test(
name = "respect_kind_mapping_test",
srcs = ["__test__.py"],
main = "__test__.py",
deps = [":respect_kind_mapping"],
)
20 changes: 20 additions & 0 deletions gazelle/python/testdata/respect_kind_mapping/BUILD.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
load(":mytest.bzl", "my_test")
load("@rules_python//python:defs.bzl", "py_library")

# gazelle:map_kind py_test my_test :mytest.bzl

py_library(
name = "respect_kind_mapping",
srcs = [
"__init__.py",
"foo.py",
],
visibility = ["//:__subpackages__"],
)

my_test(
name = "respect_kind_mapping_test",
srcs = ["__test__.py"],
main = "__test__.py",
deps = [":respect_kind_mapping"],
)
3 changes: 3 additions & 0 deletions gazelle/python/testdata/respect_kind_mapping/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Respect Kind Mapping

This test case asserts that when using a kind mapping, gazelle will respect that mapping when parsing a BUILD file containing a mapped kind.
1 change: 1 addition & 0 deletions gazelle/python/testdata/respect_kind_mapping/WORKSPACE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# This is a Bazel workspace for the Gazelle test data.
17 changes: 17 additions & 0 deletions gazelle/python/testdata/respect_kind_mapping/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from foo import foo

_ = foo
26 changes: 26 additions & 0 deletions gazelle/python/testdata/respect_kind_mapping/__test__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import unittest

from __init__ import foo


class FooTest(unittest.TestCase):
def test_foo(self):
self.assertEqual("foo", foo())


if __name__ == "__main__":
unittest.main()
16 changes: 16 additions & 0 deletions gazelle/python/testdata/respect_kind_mapping/foo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

def foo():
return "foo"
17 changes: 17 additions & 0 deletions gazelle/python/testdata/respect_kind_mapping/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

---
expect:
exit_code: 0

0 comments on commit b80b8fd

Please sign in to comment.