Skip to content

Commit

Permalink
Add an alias_kind directive to configure wrapper macros (#1986)
Browse files Browse the repository at this point in the history 
**What type of PR is this?**

Feature


**What package or component does this PR mostly affect?**

cmd/gazelle

**What does this PR do? Why is it needed?**

Introduces an `alias_kind` directive to instruct gazelle for how to
handle wrapper macros. Wrapper macros are starlark macros around rules
that gazelle knows how to understand. A wrapper macro will have the same
attrs that the underlying rule kind has, but likely does additional
things like setting up common deps or generating additional targets.

A common pattern for creating wrapper macros that gazelle should
understand is the custom `rule.verb` usecase described here -
https://bazel.build/rules/verbs-tutorial.

Gazelle should still be able to understand how to index and update a
wrapper macro as long as it is told what underlying rule the wrapper
macro should be considered as.

`alias_kind` is unique from `map_kind` in that it does not force a rule
into the kind that is specified in the `alias_kind` directive. In the
above "verbs" example, someone may configure a wrapper macro around a
`py_binary` target, while still wanting to keep other py_binary targets
around in their BUILD file. `map_kind` would not allow this, as it would
force ALL py_binary targets into the mapped kind.

**Which issues(s) does this PR fix?**

Fixes #18

**Other notes for review**

Getting this functionality required a few changes that I'll discuss here
since the map_kind and wrapper macro functionality is similar, yet
subtly different:

1. We now pass the wrapper macro config into the rule merger so that it
can match an existing wrapper macro against the newly generated rule,
even though their kinds are not strictly the same. The mapped_kind
functionality did not need to do this, as the kind mapping logic in
fix-update.go handles updating the generated rule to match the new kind
prior to build file merging. We cannot do something similar here, as the
wrapper macro pattern should not force all rules to exist in the new
type. Instead, it should instruct the merger that the newly generated
rule can match the wrapper macro.
2. Update the metaresolver to perform a lookup in the wrapper macro
configuration. When we come across a wrapper macro, we need to use the
inverseKindMapper so that we correctly update the existing rule's kind
prior to sending it off to a language extension to be indexed.
  • Loading branch information
@alex-torok
alex-torok authored Dec 11, 2024
1 parent 2898dda commit 46d1f66
Show file tree
Hide file tree
Showing 8 changed files with 454 additions and 31 deletions.
23 changes: 23 additions & 0 deletions README.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Gazelle build file generator
.. _rules_swift_package_manager: https://github.com/cgrindel/rules_swift_package_manager
.. _gazelle_rust: https://github.com/Calsign/gazelle_rust
.. _rules_rust: https://github.com/bazelbuild/rules_rust
.. _Verbs Tutorial: https://bazel.build/rules/verbs-tutorial

.. role:: cmd(code)
.. role:: flag(code)
Expand Down Expand Up @@ -881,6 +882,28 @@ The following directives are recognized:
| |
| Existing rules of the old kind will be ignored. To switch your codebase from a builtin |
| kind to a mapped kind, use `buildozer`_. |
+------------------------------------------------------------+-------------------------------+
| :direc:`# gazelle:alias_kind macro_name wrapped_kind` | n/a |
+------------------------------------------------------------+-------------------------------+
| Denotes that a macro aliases / wraps a given rule. |
| |
| If you have a wrapper macro around a rule that gazelle knows how to update the attrs for, |
| the alias_kind directive will instruct gazelle that it should treat the particular marco |
| like the underlying wrapped kind. |
| |
| ``alias_kind`` is different from the ``map_kind`` directive in that it does not force the |
| rule to be generated as the wrapped kind. Instead, it just instructs gazelle that it |
| should index and update the attrs for rules that match the macro. |
| |
| For example, if you use ``# gazelle:alias_kind my_foo_binary foo_binary``, Gazelle will |
| still generate ``foo_binary`` targets when generating new targets from new source files. |
| It is up to a person to update the ``foo_binary`` targets to ``my_foo_binary`` targets. |
| Once this manual step is done, Gazelle will continue to update the ``my_foo_binary`` |
| targets as if they were ``foo_binary`` targets. |
| |
| Wrapper macros are commonly used to handle common boilerplate or to add deploy/release |
| verbs, as described in the bazel `Verbs Tutorial`_. |
| |
+---------------------------------------------------+----------------------------------------+
| :direc:`# gazelle:prefix path` | n/a |
+---------------------------------------------------+----------------------------------------+
Expand Down
9 changes: 7 additions & 2 deletions cmd/gazelle/fix-update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {

var errorsFromWalk []error
walk.Walk(c, cexts, uc.dirs, uc.walkMode, func(dir, rel string, c *config.Config, update bool, f *rule.File, subdirs, regularFiles, genFiles []string) {
mrslv.AliasedKinds(rel, c.AliasMap)
// If this file is ignored or if Gazelle was not asked to update this
// directory, just index the build file and move on.
if !update {
Expand Down Expand Up @@ -434,7 +435,9 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {
}
} else {
merger.MergeFile(f, empty, gen, merger.PreResolve,
unionKindInfoMaps(kinds, mappedKindInfo))
unionKindInfoMaps(kinds, mappedKindInfo),
c.AliasMap,
)
}
visits = append(visits, visitRecord{
pkgRel: rel,
Expand Down Expand Up @@ -495,7 +498,9 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {
}
}
merger.MergeFile(v.file, v.empty, v.rules, merger.PostResolve,
unionKindInfoMaps(kinds, v.mappedKindInfo))
unionKindInfoMaps(kinds, v.mappedKindInfo),
v.c.AliasMap,
)
}
for _, lang := range languages {
if life, ok := lang.(language.LifecycleManager); ok {
Expand Down
269 changes: 269 additions & 0 deletions cmd/gazelle/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2476,6 +2476,275 @@ go_library(
})
}

func TestAliasKind(t *testing.T) {
for name, tc := range map[string]struct {
before []testtools.FileSpec
after []testtools.FileSpec
index bool
}{
"existing aliased kind with matching name has deps updated": {
index: false,
before: []testtools.FileSpec{
{
Path: "WORKSPACE",
},
{
Path: "BUILD.bazel",
Content: `
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:alias_kind my_custom_macro go_library
load("//custom:def.bzl", "my_custom_macro")
my_custom_macro(
name = "go_default_library",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
)
`,
},
{
Path: "file.go",
Content: `
package aliaskind
import (
_ "example.com/aliaskind/foo"
_ "github.com/external"
)
`,
},
},
after: []testtools.FileSpec{
{
Path: "BUILD.bazel",
Content: `
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:alias_kind my_custom_macro go_library
load("//custom:def.bzl", "my_custom_macro")
my_custom_macro(
name = "go_default_library",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
deps = [
"//foo:go_default_library",
"//vendor/github.com/external:go_default_library",
],
)
`,
},
},
},
"existing aliased kind with different name has deps updated": {
index: false,
before: []testtools.FileSpec{
{
Path: "WORKSPACE",
},
{
Path: "BUILD.bazel",
Content: `
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:alias_kind my_custom_macro go_library
load("//custom:def.bzl", "my_custom_macro")
my_custom_macro(
name = "custom_lib",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
)
`,
},
{
Path: "file.go",
Content: `
package aliaskind
import (
_ "example.com/aliaskind/foo"
_ "github.com/external"
)
`,
},
},
after: []testtools.FileSpec{
{
Path: "BUILD.bazel",
Content: `
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:alias_kind my_custom_macro go_library
load("//custom:def.bzl", "my_custom_macro")
my_custom_macro(
name = "custom_lib",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
deps = [
"//foo:go_default_library",
"//vendor/github.com/external:go_default_library",
],
)
`,
},
},
},
"existing aliased kind is indexed for deps": {
index: true,
before: []testtools.FileSpec{
{
Path: "WORKSPACE",
},
{
Path: "BUILD.bazel",
Content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:alias_kind my_custom_macro go_library
go_library(
name = "go_default_library",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
)
`,
},
{
Path: "file.go",
Content: `
package aliaskind
import (
_ "example.com/aliaskind/foo"
_ "github.com/external"
)
`,
},
{
Path: "foo/BUILD.bazel",
Content: `
load("//custom:def.bzl", "my_custom_macro")
my_custom_macro(
name = "go_default_library",
srcs = ["foo.go"],
importpath = "example.com/aliaskind/foo",
visibility = ["//visibility:public"],
)
`,
},
{
Path: "foo/foo.go",
Content: "package foo",
},
},
after: []testtools.FileSpec{
{
Path: "BUILD.bazel",
Content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:alias_kind my_custom_macro go_library
go_library(
name = "go_default_library",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
deps = [
"//foo:go_default_library",
"//vendor/github.com/external:go_default_library",
],
)
`,
},
},
},
"alias_kind around a mapped_kind": {
index: false,
before: []testtools.FileSpec{
{
Path: "WORKSPACE",
},
{
Path: "BUILD.bazel",
Content: `
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:map_kind go_library my_library //tools:go:def.bzl
# gazelle:alias_kind my_custom_macro my_library
load("//custom:def.bzl", "my_custom_macro")
my_custom_macro(
name = "go_default_library",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
)
`,
},
{
Path: "file.go",
Content: `
package aliaskind
import (
_ "example.com/aliaskind/foo"
_ "github.com/external"
)
`,
},
},
after: []testtools.FileSpec{
{
Path: "BUILD.bazel",
Content: `
# gazelle:prefix example.com/aliaskind
# gazelle:go_naming_convention go_default_library
# gazelle:map_kind go_library my_library //tools:go:def.bzl
# gazelle:alias_kind my_custom_macro my_library
load("//custom:def.bzl", "my_custom_macro")
my_custom_macro(
name = "go_default_library",
srcs = ["file.go"],
importpath = "example.com/aliaskind",
visibility = ["//visibility:public"],
deps = [
"//foo:go_default_library",
"//vendor/github.com/external:go_default_library",
],
)
`,
},
},
},
} {
t.Run(name, func(t *testing.T) {
dir, cleanup := testtools.CreateFiles(t, tc.before)
t.Cleanup(cleanup)
args := []string{"-external=vendored"}
if !tc.index {
args = append(args, "-index=false")
}
if err := runGazelle(dir, args); err != nil {
t.Fatal(err)
}
testtools.CheckFiles(t, dir, tc.after)
})
}
}

func TestMapKindEdgeCases(t *testing.T) {
for name, tc := range map[string]struct {
before []testtools.FileSpec
Expand Down
Loading

0 comments on commit 46d1f66

Please sign in to comment.