From c27159d790c604ad9ba85f47a43f56c51fc816fb Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Mon, 22 Jan 2024 17:07:39 +0000 Subject: [PATCH] Apply map_kind to args as well as rule kinds https://github.com/bazelbuild/bazel-gazelle/pull/1310 added support for adding args from Gazelle, so you can express things like: ``` maybe( java_test, ... ) ``` But `map_kind` doesn't currently get applied to these args. This PR adds that application. --- cmd/gazelle/BUILD.bazel | 1 + cmd/gazelle/fix-update.go | 32 +++++++-- cmd/gazelle/fix_test.go | 136 ++++++++++++++++++++++++++++++++++++++ rule/rule.go | 9 +++ rule/rule_test.go | 3 + 5 files changed, 175 insertions(+), 6 deletions(-) diff --git a/cmd/gazelle/BUILD.bazel b/cmd/gazelle/BUILD.bazel index e14a1af47..15efc904e 100644 --- a/cmd/gazelle/BUILD.bazel +++ b/cmd/gazelle/BUILD.bazel @@ -37,6 +37,7 @@ go_library( "//resolve", "//rule", "//walk", + "@com_github_bazelbuild_buildtools//build", "@com_github_pmezard_go_difflib//difflib", ], ) diff --git a/cmd/gazelle/fix-update.go b/cmd/gazelle/fix-update.go index fe8fee57f..1f2c81e2e 100644 --- a/cmd/gazelle/fix-update.go +++ b/cmd/gazelle/fix-update.go @@ -39,6 +39,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/resolve" "github.com/bazelbuild/bazel-gazelle/rule" "github.com/bazelbuild/bazel-gazelle/walk" + "github.com/bazelbuild/buildtools/build" ) // updateConfig holds configuration information needed to run the fix and @@ -380,17 +381,36 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { if f != nil { allRules = append(allRules, f.Rules...) } - for _, r := range allRules { - repl, err := lookupMapKindReplacement(c.KindMap, r.Kind()) + + maybeRecordReplacement := func(ruleKind string) (*string, error) { + repl, err := lookupMapKindReplacement(c.KindMap, ruleKind) if err != nil { - errorsFromWalk = append(errorsFromWalk, fmt.Errorf("looking up mapped kind: %w", err)) - continue + return nil, err } if repl != nil { - mappedKindInfo[repl.KindName] = kinds[r.Kind()] + mappedKindInfo[repl.KindName] = kinds[ruleKind] mappedKinds = append(mappedKinds, *repl) mrslv.MappedKind(rel, *repl) - r.SetKind(repl.KindName) + return &repl.KindName, nil + } + return nil, nil + } + + for _, r := range allRules { + if replacementName, err := maybeRecordReplacement(r.Kind()); err != nil { + errorsFromWalk = append(errorsFromWalk, fmt.Errorf("looking up mapped kind: %w", err)) + } else if replacementName != nil { + r.SetKind(*replacementName) + } + + for i, arg := range r.Args() { + if ident, ok := arg.(*build.Ident); ok { + if replacementName, err := maybeRecordReplacement(ident.Name); err != nil { + errorsFromWalk = append(errorsFromWalk, fmt.Errorf("looking up mapped kind: %w", err)) + } else if replacementName != nil { + r.UpdateArg(i, &build.Ident{Name: *replacementName}) + } + } } } for _, r := range empty { diff --git a/cmd/gazelle/fix_test.go b/cmd/gazelle/fix_test.go index 0437721be..761284a5c 100644 --- a/cmd/gazelle/fix_test.go +++ b/cmd/gazelle/fix_test.go @@ -407,3 +407,139 @@ go_library( testtools.CheckFiles(t, dir, fixture) } + +func TestFix_MapKind_Argument_SameName(t *testing.T) { + before := []testtools.FileSpec{ + { + Path: "BUILD.bazel", + Content: ` +load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") + +# gazelle:map_kind go_binary go_binary //my:custom.bzl + +maybe( + go_binary, + name = "nofix", + library = ":go_default_library", + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + srcs = ["some.go"], + importpath = "example.com/repo", + visibility = ["//visibility:public"], +)`, + }, + { + Path: "some.go", + Content: `package some`, + }, + } + + after := []testtools.FileSpec{ + { + Path: "BUILD.bazel", + Content: ` +load("//my:custom.bzl", "go_binary") +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +# gazelle:map_kind go_binary go_binary //my:custom.bzl + +maybe( + go_binary, + name = "nofix", + library = ":go_default_library", + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + srcs = ["some.go"], + importpath = "example.com/repo", + visibility = ["//visibility:public"], +)`, + }, + } + + dir, cleanup := testtools.CreateFiles(t, before) + defer cleanup() + + if err := run(dir, []string{ + "-repo_root", dir, + "-go_prefix", "example.com/repo", + dir, + }); err != nil { + t.Fatalf("run failed: %v", err) + } + + testtools.CheckFiles(t, dir, after) +} + +func TestFix_MapKind_Argument_DifferentName(t *testing.T) { + before := []testtools.FileSpec{ + { + Path: "BUILD.bazel", + Content: ` +load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") + +# gazelle:map_kind go_binary custom_go_binary //my:custom.bzl + +maybe( + go_binary, + name = "nofix", + library = ":go_default_library", + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + srcs = ["some.go"], + importpath = "example.com/repo", + visibility = ["//visibility:public"], +)`, + }, + { + Path: "some.go", + Content: `package some`, + }, + } + + after := []testtools.FileSpec{ + { + Path: "BUILD.bazel", + Content: ` +load("//my:custom.bzl", "custom_go_binary") +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +# gazelle:map_kind go_binary custom_go_binary //my:custom.bzl + +maybe( + custom_go_binary, + name = "nofix", + library = ":go_default_library", + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + srcs = ["some.go"], + importpath = "example.com/repo", + visibility = ["//visibility:public"], +)`, + }, + } + + dir, cleanup := testtools.CreateFiles(t, before) + defer cleanup() + + if err := run(dir, []string{ + "-repo_root", dir, + "-go_prefix", "example.com/repo", + dir, + }); err != nil { + t.Fatalf("run failed: %v", err) + } + + testtools.CheckFiles(t, dir, after) +} diff --git a/rule/rule.go b/rule/rule.go index fb40952c9..3ddc90e68 100644 --- a/rule/rule.go +++ b/rule/rule.go @@ -1016,6 +1016,15 @@ func (r *Rule) AddArg(value bzl.Expr) { r.updated = true } +// UpdateArg replaces an existing arg with a new value. +func (r *Rule) UpdateArg(index int, value bzl.Expr) { + if len(r.args) < index { + panic("Can't update argument that doesn't exist") + } + r.args[index] = value + r.updated = true +} + // SortedAttrs returns the keys of attributes whose values will be sorted func (r *Rule) SortedAttrs() []string { return r.sortedAttrs diff --git a/rule/rule_test.go b/rule/rule_test.go index 5574ef4b3..c8e70c0ab 100644 --- a/rule/rule_test.go +++ b/rule/rule_test.go @@ -63,6 +63,8 @@ y_library(name = "bar") loadMaybe.Insert(f, 0) baz := NewRule("maybe", "baz") baz.AddArg(&bzl.LiteralExpr{Token: "z"}) + baz.AddArg(&bzl.LiteralExpr{Token: "z"}) + baz.UpdateArg(0, &bzl.LiteralExpr{Token: "z0"}) baz.SetAttr("srcs", GlobValue{ Patterns: []string{"**"}, Excludes: []string{"*.pem"}, @@ -85,6 +87,7 @@ y_library( ) maybe( + z0, z, name = "baz", srcs = glob(