diff --git a/cmd/gazelle/BUILD.bazel b/cmd/gazelle/BUILD.bazel index c269a3b7c..2c0ea1785 100644 --- a/cmd/gazelle/BUILD.bazel +++ b/cmd/gazelle/BUILD.bazel @@ -26,6 +26,7 @@ go_library( deps = [ "//config", "//flag", + "//internal/mapkind", "//internal/wspace", "//label", "//language", @@ -36,7 +37,6 @@ 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 c0c8a72ff..69b63be15 100644 --- a/cmd/gazelle/fix-update.go +++ b/cmd/gazelle/fix-update.go @@ -28,10 +28,9 @@ import ( "strings" "syscall" - "github.com/bazelbuild/buildtools/build" - "github.com/bazelbuild/bazel-gazelle/config" gzflag "github.com/bazelbuild/bazel-gazelle/flag" + "github.com/bazelbuild/bazel-gazelle/internal/mapkind" "github.com/bazelbuild/bazel-gazelle/internal/wspace" "github.com/bazelbuild/bazel-gazelle/label" "github.com/bazelbuild/bazel-gazelle/language" @@ -245,9 +244,8 @@ type visitRecord struct { // file is the build file being processed. file *rule.File - // mappedKinds are mapped kinds used during this visit. - mappedKinds []config.MappedKind - mappedKindInfo map[string]rule.KindInfo + // mappedResult contains all mapped kinds and kindInfos used during this visit. + mappedResult *mapkind.MappedResult } var genericLoads = []rule.LoadInfo{ @@ -275,13 +273,13 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { } mrslv := newMetaResolver() - kinds := make(map[string]rule.KindInfo) + kindInfos := make(map[string]rule.KindInfo) loads := genericLoads exts := make([]interface{}, 0, len(languages)) for _, lang := range languages { for kind, info := range lang.Kinds() { mrslv.AddBuiltin(kind, lang) - kinds[kind] = info + kindInfos[kind] = info } if moduleAwareLang, ok := lang.(language.ModuleAwareLanguage); ok { loads = append(loads, moduleAwareLang.ApparentLoads(c.ModuleToApparentName)...) @@ -362,68 +360,14 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { return } - // Apply and record relevant kind mappings. - var ( - mappedKinds []config.MappedKind - mappedKindInfo = make(map[string]rule.KindInfo) - ) - // We apply map_kind to all rules, including pre-existing ones. - var allRules []*rule.Rule - allRules = append(allRules, gen...) - if f != nil { - allRules = append(allRules, f.Rules...) - } - - maybeRecordReplacement := func(ruleKind string) (*string, error) { - var repl *config.MappedKind - repl, err = lookupMapKindReplacement(c.KindMap, ruleKind) - if err != nil { - return nil, err - } - if repl != nil { - mappedKindInfo[repl.KindName] = kinds[ruleKind] - mappedKinds = append(mappedKinds, *repl) - mrslv.MappedKind(rel, *repl) - return &repl.KindName, nil - } - return nil, nil + // Apply map_kind to all rules including pre-existing rules, and record relevant kind mappings. + mappedResult, err := mapkind.ApplyOnRules(c, kindInfos, f, gen, empty) + if err != nil { + errorsFromWalk = append(errorsFromWalk, fmt.Errorf("error applying mapped kinds: %v", err)) } - 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() { - // Only check the first arg - this supports the maybe(java_library, ...) pattern, - // but avoids potential false positives from other uses of symbols. - if i != 0 { - break - } - if ident, ok := arg.(*build.Ident); ok { - // Don't allow re-mapping symbols that aren't known loads of a plugin. - if _, knownKind := kinds[ident.Name]; !knownKind { - continue - } - if replacementName, err := maybeRecordReplacement(ident.Name); err != nil { - errorsFromWalk = append(errorsFromWalk, fmt.Errorf("looking up mapped kind: %w", err)) - } else if replacementName != nil { - if err := r.UpdateArg(i, &build.Ident{Name: *replacementName}); err != nil { - log.Panicf("%s: %v", rel, err) - } - } - } - } - } - for _, r := range empty { - if repl, ok := c.KindMap[r.Kind()]; ok { - mappedKindInfo[repl.KindName] = kinds[r.Kind()] - mappedKinds = append(mappedKinds, repl) - mrslv.MappedKind(rel, repl) - r.SetKind(repl.KindName) - } + for _, mappedKind := range mappedResult.MappedKinds { + mrslv.MappedKind(rel, mappedKind) } // Insert or merge rules into the build file. @@ -433,18 +377,16 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { r.Insert(f) } } else { - merger.MergeFile(f, empty, gen, merger.PreResolve, - unionKindInfoMaps(kinds, mappedKindInfo)) + merger.MergeFile(f, empty, gen, merger.PreResolve, mappedResult.Kinds) } visits = append(visits, visitRecord{ - pkgRel: rel, - c: c, - rules: gen, - imports: imports, - empty: empty, - file: f, - mappedKinds: mappedKinds, - mappedKindInfo: mappedKindInfo, + pkgRel: rel, + c: c, + rules: gen, + imports: imports, + empty: empty, + file: f, + mappedResult: mappedResult, }) // Add library rules to the dependency resolution table. @@ -494,8 +436,7 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { rslv.Resolve(v.c, ruleIndex, rc, r, v.imports[i], from) } } - merger.MergeFile(v.file, v.empty, v.rules, merger.PostResolve, - unionKindInfoMaps(kinds, v.mappedKindInfo)) + merger.MergeFile(v.file, v.empty, v.rules, merger.PostResolve, v.mappedResult.Kinds) } for _, lang := range languages { if life, ok := lang.(language.LifecycleManager); ok { @@ -506,7 +447,7 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { // Emit merged files. var exit error for _, v := range visits { - merger.FixLoads(v.file, applyKindMappings(v.mappedKinds, loads)) + merger.FixLoads(v.file, v.mappedResult.ApplyOnLoads(loads)) if err := uc.emit(v.c, v.file); err != nil { if err == errExit { exit = err @@ -524,37 +465,6 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) { return exit } -// lookupMapKindReplacement finds a mapped replacement for rule kind `kind`, resolving transitively. -// i.e. if go_library is mapped to custom_go_library, and custom_go_library is mapped to other_go_library, -// looking up go_library will return other_go_library. -// It returns an error on a loop, and may return nil if no remapping should be performed. -func lookupMapKindReplacement(kindMap map[string]config.MappedKind, kind string) (*config.MappedKind, error) { - var mapped *config.MappedKind - seenKinds := make(map[string]struct{}) - seenKindPath := []string{kind} - for { - replacement, ok := kindMap[kind] - if !ok { - break - } - - seenKindPath = append(seenKindPath, replacement.KindName) - if _, alreadySeen := seenKinds[replacement.KindName]; alreadySeen { - return nil, fmt.Errorf("found loop of map_kind replacements: %s", strings.Join(seenKindPath, " -> ")) - } - - seenKinds[replacement.KindName] = struct{}{} - mapped = &replacement - if kind == replacement.KindName { - break - } - - kind = replacement.KindName - } - - return mapped, nil -} - func newFixUpdateConfiguration(wd string, cmd command, args []string, cexts []config.Configurer) (*config.Config, error) { c := config.New() c.WorkDir = wd @@ -746,54 +656,6 @@ func maybePopulateRemoteCacheFromGoMod(c *config.Config, rc *repo.RemoteCache) e return rc.PopulateFromGoMod(goModPath) } -func unionKindInfoMaps(a, b map[string]rule.KindInfo) map[string]rule.KindInfo { - if len(a) == 0 { - return b - } - if len(b) == 0 { - return a - } - result := make(map[string]rule.KindInfo, len(a)+len(b)) - for _, m := range []map[string]rule.KindInfo{a, b} { - for k, v := range m { - result[k] = v - } - } - return result -} - -// applyKindMappings returns a copy of LoadInfo that includes c.KindMap. -func applyKindMappings(mappedKinds []config.MappedKind, loads []rule.LoadInfo) []rule.LoadInfo { - if len(mappedKinds) == 0 { - return loads - } - - // Add new RuleInfos or replace existing ones with merged ones. - mappedLoads := make([]rule.LoadInfo, len(loads)) - copy(mappedLoads, loads) - for _, mappedKind := range mappedKinds { - mappedLoads = appendOrMergeKindMapping(mappedLoads, mappedKind) - } - return mappedLoads -} - -// appendOrMergeKindMapping adds LoadInfo for the given replacement. -func appendOrMergeKindMapping(mappedLoads []rule.LoadInfo, mappedKind config.MappedKind) []rule.LoadInfo { - // If mappedKind.KindLoad already exists in the list, create a merged copy. - for i, load := range mappedLoads { - if load.Name == mappedKind.KindLoad { - mappedLoads[i].Symbols = append(load.Symbols, mappedKind.KindName) - return mappedLoads - } - } - - // Add a new LoadInfo. - return append(mappedLoads, rule.LoadInfo{ - Name: mappedKind.KindLoad, - Symbols: []string{mappedKind.KindName}, - }) -} - func isDirErr(err error) bool { if err == nil { return false diff --git a/internal/BUILD.bazel b/internal/BUILD.bazel index b3c5d1017..02d49bdef 100644 --- a/internal/BUILD.bazel +++ b/internal/BUILD.bazel @@ -55,6 +55,7 @@ filegroup( "//internal/gazellebinarytest:all_files", "//internal/generationtest:all_files", "//internal/language:all_files", + "//internal/mapkind:all_files", "//internal/module:all_files", "//internal/version:all_files", "//internal/wspace:all_files", diff --git a/internal/go_repository_tools_srcs.bzl b/internal/go_repository_tools_srcs.bzl index fa45db0d6..d4ff0e7dd 100644 --- a/internal/go_repository_tools_srcs.bzl +++ b/internal/go_repository_tools_srcs.bzl @@ -48,6 +48,11 @@ GO_REPOSITORY_TOOLS_SRCS = [ Label("//internal/language/test_loads_from_flag:BUILD.bazel"), Label("//internal/language/test_loads_from_flag:lang.go"), Label("//internal:list_repository_tools_srcs.go"), + Label("//internal/mapkind:BUILD.bazel"), + Label("//internal/mapkind:apply.go"), + Label("//internal/mapkind:result.go"), + Label("//internal/mapkind:rules_iterator.go"), + Label("//internal/mapkind:rules_iterator_compat.go"), Label("//internal/module:BUILD.bazel"), Label("//internal/module:module.go"), Label("//internal/version:BUILD.bazel"), diff --git a/internal/mapkind/BUILD.bazel b/internal/mapkind/BUILD.bazel new file mode 100644 index 000000000..bb6819320 --- /dev/null +++ b/internal/mapkind/BUILD.bazel @@ -0,0 +1,53 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "mapkind", + srcs = [ + "apply.go", + "result.go", + "rules_iterator.go", + "rules_iterator_compat.go", + ], + importpath = "github.com/bazelbuild/bazel-gazelle/internal/mapkind", + visibility = ["//visibility:public"], + deps = [ + "//config", + "//rule", + "@com_github_bazelbuild_buildtools//build", + ], +) + +alias( + name = "go_default_library", + actual = ":mapkind", + visibility = ["//:__subpackages__"], +) + +filegroup( + name = "all_files", + testonly = True, + srcs = [ + "BUILD.bazel", + "apply.go", + "apply_test.go", + "result.go", + "result_test.go", + "rules_iterator.go", + "rules_iterator_compat.go", + ], + visibility = ["//visibility:public"], +) + +go_test( + name = "mapkind_test", + srcs = [ + "apply_test.go", + "result_test.go", + ], + embed = [":mapkind"], + deps = [ + "//config", + "//rule", + "@com_github_google_go_cmp//cmp", + ], +) diff --git a/internal/mapkind/apply.go b/internal/mapkind/apply.go new file mode 100644 index 000000000..7f8073556 --- /dev/null +++ b/internal/mapkind/apply.go @@ -0,0 +1,125 @@ +package mapkind + +import ( + "fmt" + "log" + "strings" + + "github.com/bazelbuild/buildtools/build" + + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +// ApplyOnRules returns all mapped kind rules of rules of the given file and of all the given rules list. +func ApplyOnRules(c *config.Config, knownKinds map[string]rule.KindInfo, f *rule.File, extraRuleLists ...[]*rule.Rule) (*MappedResult, error) { + kindInfos := make(map[string]rule.KindInfo, len(knownKinds)) + for k, v := range knownKinds { + kindInfos[k] = v + } + + res := MappedResult{ + Kinds: kindInfos, + } + + for _, r := range allRules(f, extraRuleLists...) { + if err := applyOnRuleKind(&res, c, r); err != nil { + return &res, err + } + + if err := applyOnFirstRuleArg(&res, c, r); err != nil { + return &res, err + } + } + + return &res, nil +} + +// applyOnRuleKind applies all kind replacements on the kind of the given rule. +func applyOnRuleKind(res *MappedResult, c *config.Config, r *rule.Rule) error { + ruleKind := r.Kind() + + repl, err := lookupReplacement(c.KindMap, ruleKind) + if err != nil { + return fmt.Errorf("looking up mapped kind: %v", err) + } + + if repl != nil { + r.SetKind(repl.KindName) + + res.add(repl, ruleKind) + } + + return nil +} + +// applyOnFirstRuleArg applies all kind replacements on the first argument of the fiven rule. +// This supports the maybe(java_library, ...) pattern, but checks only the first arg to +// avoids potential false positives from other uses of symbols. +func applyOnFirstRuleArg(res *MappedResult, c *config.Config, r *rule.Rule) error { + ruleArgs := r.Args() + + if len(ruleArgs) == 0 { + return nil + } + + firstRuleArg := ruleArgs[0] + + ident, ok := firstRuleArg.(*build.Ident) + if !ok { + return nil + } + + ruleKind := ident.Name + + // Don't allow re-mapping symbols that aren't known loads of a plugin. + if _, found := res.Kinds[ruleKind]; !found { + return nil + } + + repl, err := lookupReplacement(c.KindMap, ruleKind) + if err != nil { + return fmt.Errorf("looking up mapped kind: %v", err) + } + + if repl != nil { + if err := r.UpdateArg(0, &build.Ident{Name: repl.KindName}); err != nil { + log.Fatal(err) // Should never happen beacuse index 0 is always available. + } + + res.add(repl, ruleKind) + } + + return nil +} + +// lookupReplacement finds a mapped replacement for rule kind `kind`, resolving transitively. +// i.e. if go_library is mapped to custom_go_library, and custom_go_library is mapped to other_go_library, +// looking up go_library will return other_go_library. +// It returns an error on a loop, and may return nil if no remapping should be performed. +func lookupReplacement(kindMap map[string]config.MappedKind, kind string) (*config.MappedKind, error) { + var mapped *config.MappedKind + seenKinds := make(map[string]struct{}) + seenKindPath := []string{kind} + for { + replacement, ok := kindMap[kind] + if !ok { + break + } + + seenKindPath = append(seenKindPath, replacement.KindName) + if _, alreadySeen := seenKinds[replacement.KindName]; alreadySeen { + return nil, fmt.Errorf("found loop of map_kind replacements: %s", strings.Join(seenKindPath, " -> ")) + } + + seenKinds[replacement.KindName] = struct{}{} + mapped = &replacement + if kind == replacement.KindName { + break + } + + kind = replacement.KindName + } + + return mapped, nil +} diff --git a/internal/mapkind/apply_test.go b/internal/mapkind/apply_test.go new file mode 100644 index 000000000..923cc0ce2 --- /dev/null +++ b/internal/mapkind/apply_test.go @@ -0,0 +1,146 @@ +package mapkind + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +var testDataBuildFile = []byte(`load("//my:defs.bzl", "my_rule") +load("@other_rules//:defs.bzl", "something") + +maybe( + my_rule, + something, + name = "maybe", + first_arg = 1, + second_arg = "x", +) + +my_rule( + name = "always", + first_arg = 2, + second_arg = "y", +) +`) + +func TestApplyOnRules(t *testing.T) { + f, err := rule.LoadData("test/file.BUILD.bazel", "", testDataBuildFile) + if err != nil { + t.Fatalf("can't load nominal build file: %v", err) + } + + knownKinds := map[string]rule.KindInfo{ + "my_rule": { + MatchAny: false, + MergeableAttrs: map[string]bool{ + "first_arg": true, + "seconf_arg": true, + }, + }, + } + + for _, tt := range []struct { + TestName string + NewConfig func() *config.Config + ExpectedResult *MappedResult + ExpectedError error + }{ + { + TestName: "no_mapping", + NewConfig: config.New, + ExpectedResult: &MappedResult{ + Kinds: knownKinds, + }, + }, + { + TestName: "same_name", + NewConfig: func() *config.Config { + c := config.New() + c.KindMap = map[string]config.MappedKind{ + "my_rule": { + FromKind: "my_rule", + KindName: "my_rule", + KindLoad: "//other:defs.bzl", + }, + } + return c + }, + ExpectedResult: &MappedResult{ + MappedKinds: []config.MappedKind{ + { + FromKind: "my_rule", + KindName: "my_rule", + KindLoad: "//other:defs.bzl", + }, + }, + Kinds: knownKinds, + }, + }, + { + TestName: "different_name", + NewConfig: func() *config.Config { + c := config.New() + c.KindMap = map[string]config.MappedKind{ + "my_rule": { + FromKind: "my_rule", + KindName: "other_rule", + KindLoad: "//other:defs.bzl", + }, + } + return c + }, + ExpectedResult: &MappedResult{ + MappedKinds: []config.MappedKind{ + { + FromKind: "my_rule", + KindName: "other_rule", + KindLoad: "//other:defs.bzl", + }, + }, + Kinds: map[string]rule.KindInfo{ + "my_rule": knownKinds["my_rule"], + "other_rule": knownKinds["my_rule"], + }, + }, + }, + { + TestName: "no_first_arg", + NewConfig: func() *config.Config { + c := config.New() + c.KindMap = map[string]config.MappedKind{ + "something": { + FromKind: "something", + KindName: "my_something", + KindLoad: "//other:defs.bzl", + }, + } + return c + }, + ExpectedResult: &MappedResult{ + Kinds: knownKinds, + }, + }, + } { + t.Run(tt.TestName, func(t *testing.T) { + c := tt.NewConfig() + + result, err := ApplyOnRules(c, knownKinds, f) + + if !cmp.Equal(tt.ExpectedError, err) { + if err != nil { + t.Errorf("An unexpected error is returned: %v", err) + } else { + t.Errorf("An error was expected but no error has been returned, expected error: %v", tt.ExpectedError) + } + } + + if diff := cmp.Diff(tt.ExpectedResult, result); diff != "" { + t.Errorf("(-want, +got):\n%s", diff) + } + }) + } +} diff --git a/internal/mapkind/result.go b/internal/mapkind/result.go new file mode 100644 index 000000000..157bf38e9 --- /dev/null +++ b/internal/mapkind/result.go @@ -0,0 +1,70 @@ +package mapkind + +import ( + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +// MappedResult provides information related to all mapped kind rules. +type MappedResult struct { + // MappedKinds is the list of kind replacements used in all rules. + MappedKinds []config.MappedKind + + // Kinds is the list of all known kind of rules, including all mapped kinds. + Kinds map[string]rule.KindInfo +} + +// add registers the given kind replacement used in a rule. +func (mr *MappedResult) add(mappedKind *config.MappedKind, ruleKind string) { + var found bool + for i := range mr.MappedKinds { + k := &mr.MappedKinds[i] + if k.FromKind == mappedKind.FromKind { + k.KindLoad = mappedKind.KindLoad + k.KindName = mappedKind.KindName + found = true + break + } + } + + if !found { + mr.MappedKinds = append(mr.MappedKinds, *mappedKind) + } + + if ki, ok := mr.Kinds[ruleKind]; ok { + mr.Kinds[mappedKind.KindName] = ki + } +} + +// ApplyOnLoads applies all kind replacements on the given list of load instructions. +func (mr *MappedResult) ApplyOnLoads(loads []rule.LoadInfo) []rule.LoadInfo { + if len(mr.MappedKinds) == 0 { + return loads + } + + // Add new RuleInfos or replace existing ones with merged ones. + mappedLoads := make([]rule.LoadInfo, len(loads)) + copy(mappedLoads, loads) + for _, mappedKind := range mr.MappedKinds { + mappedLoads = appendOrMergeKindMapping(mappedLoads, mappedKind) + } + + return mappedLoads +} + +// appendOrMergeKindMapping adds LoadInfo for the given replacement. +func appendOrMergeKindMapping(mappedLoads []rule.LoadInfo, mappedKind config.MappedKind) []rule.LoadInfo { + // If mappedKind.KindLoad already exists in the list, create a merged copy. + for i, load := range mappedLoads { + if load.Name == mappedKind.KindLoad { + mappedLoads[i].Symbols = append(load.Symbols, mappedKind.KindName) + return mappedLoads + } + } + + // Add a new LoadInfo. + return append(mappedLoads, rule.LoadInfo{ + Name: mappedKind.KindLoad, + Symbols: []string{mappedKind.KindName}, + }) +} diff --git a/internal/mapkind/result_test.go b/internal/mapkind/result_test.go new file mode 100644 index 000000000..702ca26f6 --- /dev/null +++ b/internal/mapkind/result_test.go @@ -0,0 +1,115 @@ +package mapkind + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +func TestResult_ApplyOnLoads(t *testing.T) { + loads := []rule.LoadInfo{ + { + Name: "//my:defs.bzl", + Symbols: []string{ + "my_rule", + "my_second_rule", + }, + }, + } + + t.Run("no_mapping", func(t *testing.T) { + mr := MappedResult{} + + // Copy to ensure ApplyOnLoads don't modify the given data. + expectedLoads := make([]rule.LoadInfo, len(loads)) + copy(expectedLoads, loads) + + result := mr.ApplyOnLoads(loads) + + if diff := cmp.Diff(expectedLoads, result); diff != "" { + t.Errorf("(-want, +got):\n%s", diff) + } + }) + + t.Run("with_mapping", func(t *testing.T) { + mr := MappedResult{ + MappedKinds: []config.MappedKind{ + { + FromKind: "my_rule", + KindName: "other_rule", + KindLoad: "//other:defs.bzl", + }, + }, + } + + expectedLoads := []rule.LoadInfo{ + { + Name: "//my:defs.bzl", + Symbols: []string{ + "my_rule", + "my_second_rule", + }, + }, + { + Name: "//other:defs.bzl", + Symbols: []string{ + "other_rule", + }, + }, + } + + result := mr.ApplyOnLoads(loads) + + if diff := cmp.Diff(expectedLoads, result); diff != "" { + t.Errorf("(-want, +got):\n%s", diff) + } + }) + + t.Run("with_recursive_mapping", func(t *testing.T) { + mr := MappedResult{ + MappedKinds: []config.MappedKind{ + { + FromKind: "my_rule", + KindName: "other_rule", + KindLoad: "//other:defs.bzl", + }, + { + FromKind: "other_rule", + KindName: "other_other_rule", + KindLoad: "//other/other:defs.bzl", + }, + }, + } + + expectedLoads := []rule.LoadInfo{ + { + Name: "//my:defs.bzl", + Symbols: []string{ + "my_rule", + "my_second_rule", + }, + }, + { + Name: "//other:defs.bzl", + Symbols: []string{ + "other_rule", + }, + }, + { + Name: "//other/other:defs.bzl", + Symbols: []string{ + "other_other_rule", + }, + }, + } + + result := mr.ApplyOnLoads(loads) + + if diff := cmp.Diff(expectedLoads, result); diff != "" { + t.Errorf("(-want, +got):\n%s", diff) + } + }) +} diff --git a/internal/mapkind/rules_iterator.go b/internal/mapkind/rules_iterator.go new file mode 100644 index 000000000..cc6322887 --- /dev/null +++ b/internal/mapkind/rules_iterator.go @@ -0,0 +1,34 @@ +//go:build go1.23 +// +build go1.23 + +package mapkind + +import ( + "iter" + + "github.com/bazelbuild/bazel-gazelle/rule" +) + +func allRules(f *rule.File, extraRuleLists ...[]*rule.Rule) iter.Seq2[int, *rule.Rule] { + return func(yield func(int, *rule.Rule) bool) { + idx := 0 + + if f != nil { + for _, r := range f.Rules { + if !yield(idx, r) { + return + } + idx++ + } + } + + for _, extraRuleList := range extraRuleLists { + for _, r := range extraRuleList { + if !yield(idx, r) { + return + } + idx++ + } + } + } +} diff --git a/internal/mapkind/rules_iterator_compat.go b/internal/mapkind/rules_iterator_compat.go new file mode 100644 index 000000000..7efacb331 --- /dev/null +++ b/internal/mapkind/rules_iterator_compat.go @@ -0,0 +1,28 @@ +//go:build !go1.23 +// +build !go1.23 + +package mapkind + +import ( + "github.com/bazelbuild/bazel-gazelle/rule" +) + +func allRules(f *rule.File, extraRuleLists ...[]*rule.Rule) []*rule.Rule { + count := 0 + if f != nil { + count += len(f.Rules) + } + for _, extraRuleList := range extraRuleLists { + count += len(extraRuleList) + } + + rules := make([]*rule.Rule, 0, count) + if f != nil { + rules = append(rules, f.Rules...) + } + for _, extraRuleList := range extraRuleLists { + rules = append(rules, extraRuleList...) + } + + return rules +} diff --git a/language/go/BUILD.bazel b/language/go/BUILD.bazel index 4e41f68bc..24d0c2e5a 100644 --- a/language/go/BUILD.bazel +++ b/language/go/BUILD.bazel @@ -75,6 +75,7 @@ go_test( embed = [":go"], deps = [ "//config", + "//internal/mapkind", "//label", "//language", "//language/proto", diff --git a/language/go/config.go b/language/go/config.go index e23ed5437..8771f1b1a 100644 --- a/language/go/config.go +++ b/language/go/config.go @@ -787,7 +787,7 @@ func detectNamingConvention(c *config.Config, rootFile *rule.File) namingConvent // kind of go library. The old version of go_proto_library used this // name, and it's possible with map_kind as well. return goDefaultLibraryNamingConvention - } else if isGoLibrary(kind) && name == path.Base(r.AttrString("importpath")) { + } else if isGoLibrary(c, r) && name == path.Base(r.AttrString("importpath")) { return importNamingConvention } } diff --git a/language/go/fix.go b/language/go/fix.go index 6d31add74..593752027 100644 --- a/language/go/fix.go +++ b/language/go/fix.go @@ -69,11 +69,11 @@ func migrateNamingConvention(c *config.Config, f *rule.File) { switch { case r.Name() == libName: haveLib = true - case r.Kind() == "go_library" && r.Name() == migrateLibName && r.AttrString("importpath") == importPath: + case isRuleKind(c, r, "go_library") && r.Name() == migrateLibName && r.AttrString("importpath") == importPath: haveMigrateLib = true case r.Name() == testName: haveTest = true - case r.Kind() == "go_test" && r.Name() == migrateTestName && strListAttrContains(r, "embed", ":"+migrateLibName): + case isRuleKind(c, r, "go_test") && r.Name() == migrateTestName && strListAttrContains(r, "embed", ":"+migrateLibName): haveMigrateTest = true } } @@ -88,18 +88,17 @@ func migrateNamingConvention(c *config.Config, f *rule.File) { // Rename the targets and stuff in the same file that refers to them. for _, r := range f.Rules { - // TODO(jayconrod): support map_kind directive. // We'll need to move the metaresolver from resolve.RuleIndex to config.Config so we can access it from here. - switch r.Kind() { - case "go_binary": + switch { + case isRuleKind(c, r, "go_binary"): if haveMigrateLib && shouldMigrateLib { replaceInStrListAttr(r, "embed", ":"+migrateLibName, ":"+libName) } - case "go_library": + case isRuleKind(c, r, "go_library"): if r.Name() == migrateLibName && shouldMigrateLib { r.SetName(libName) } - case "go_test": + case isRuleKind(c, r, "go_test"): if r.Name() == migrateTestName && shouldMigrateTest { r.SetName(testName) } @@ -116,16 +115,9 @@ func fileContainsGoBinary(c *config.Config, f *rule.File) bool { return false } for _, r := range f.Rules { - kind := r.Kind() - if kind == "go_binary" { + if isRuleKind(c, r, "go_binary") { return true } - - if mappedKind, ok := c.KindMap["go_binary"]; ok { - if mappedKind.KindName == kind { - return true - } - } } return false } @@ -159,7 +151,7 @@ func strListAttrContains(r *rule.Rule, attr, s string) bool { // no keep comment on "library" and no existing "embed" attribute. func migrateLibraryEmbed(c *config.Config, f *rule.File) { for _, r := range f.Rules { - if !isGoRule(r.Kind()) { + if !isGoRule(c, r) { continue } libExpr := r.Attr("library") @@ -175,7 +167,7 @@ func migrateLibraryEmbed(c *config.Config, f *rule.File) { // rules with a "compilers" attribute. func migrateGrpcCompilers(c *config.Config, f *rule.File) { for _, r := range f.Rules { - if r.Kind() != "go_grpc_library" || r.ShouldKeep() || r.Attr("compilers") != nil { + if !isRuleKind(c, r, "go_grpc_library") || r.ShouldKeep() || r.Attr("compilers") != nil { continue } r.SetKind("go_proto_library") @@ -194,7 +186,7 @@ func squashCgoLibrary(c *config.Config, f *rule.File) { // Find the default cgo_library and go_library rules. var cgoLibrary, goLibrary *rule.Rule for _, r := range f.Rules { - if r.Kind() == "cgo_library" && r.Name() == "cgo_default_library" && !r.ShouldKeep() { + if isRuleKind(c, r, "cgo_library") && r.Name() == "cgo_default_library" && !r.ShouldKeep() { if cgoLibrary != nil { log.Printf("%s: when fixing existing file, multiple cgo_library rules with default name found", f.Path) continue @@ -202,7 +194,7 @@ func squashCgoLibrary(c *config.Config, f *rule.File) { cgoLibrary = r continue } - if r.Kind() == "go_library" && r.Name() == defaultLibName { + if isRuleKind(c, r, "go_library") && r.Name() == defaultLibName { if goLibrary != nil { log.Printf("%s: when fixing existing file, multiple go_library rules with default name referencing cgo_library found", f.Path) } @@ -243,7 +235,7 @@ func squashXtest(c *config.Config, f *rule.File) { // Search for internal and external tests. var itest, xtest *rule.Rule for _, r := range f.Rules { - if r.Kind() != "go_test" { + if !isRuleKind(c, r, "go_test") { continue } if r.Name() == defaultTestName { @@ -286,7 +278,7 @@ func squashXtest(c *config.Config, f *rule.File) { // duplicate expressions. func flattenSrcs(c *config.Config, f *rule.File) { for _, r := range f.Rules { - if !isGoRule(r.Kind()) { + if !isGoRule(c, r) { continue } oldSrcs := r.Attr("srcs") @@ -323,7 +315,7 @@ func removeLegacyProto(c *config.Config, f *rule.File) { if r.Kind() == "filegroup" && r.Name() == legacyProtoFilegroupName { protoFilegroups = append(protoFilegroups, r) } - if r.Kind() == "go_proto_library" { + if isRuleKind(c, r, "go_proto_library") { protoRules = append(protoRules, r) } } @@ -364,10 +356,10 @@ func removeLegacyGazelle(c *config.Config, f *rule.File) { } } -func isGoRule(kind string) bool { - return kind == "go_library" || - kind == "go_binary" || - kind == "go_test" || - kind == "go_proto_library" || - kind == "go_grpc_library" +func isGoRule(c *config.Config, r *rule.Rule) bool { + return isRuleKind(c, r, "go_library") || + isRuleKind(c, r, "go_binary") || + isRuleKind(c, r, "go_test") || + isRuleKind(c, r, "go_proto_library") || + isRuleKind(c, r, "go_grpc_library") } diff --git a/language/go/generate.go b/language/go/generate.go index e0cc1c286..093136e43 100644 --- a/language/go/generate.go +++ b/language/go/generate.go @@ -47,7 +47,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes protoPackages := make(map[string]proto.Package) protoFileInfo := make(map[string]proto.FileInfo) for _, r := range args.OtherGen { - if r.Kind() == "go_proto_library" { + if isRuleKind(c, r, "go_proto_library") { if proto := r.AttrString("proto"); proto != "" { goProtoRules[proto] = struct{}{} } @@ -58,7 +58,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes } } - if r.Kind() != "proto_library" { + if !isRuleKind(c, r, "proto_library") { continue } pkg := r.PrivateAttr(proto.PackageKey).(proto.Package) @@ -71,7 +71,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes sort.Strings(protoRuleNames) var emptyProtoRuleNames []string for _, r := range args.OtherEmpty { - if r.Kind() == "proto_library" { + if isRuleKind(c, r, "proto_library") { emptyProtoRuleNames = append(emptyProtoRuleNames, r.Name()) } } @@ -234,7 +234,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes protoEmbed, rs = g.generateProto(pcMode, protoTargets, importPath) if protoEmbed != "" { // check if rs is non-empty and that the first rule is a go_proto_library with the same importPath - if len(rs) > 0 && rs[0].Kind() == "go_proto_library" && rs[0].AttrString("importpath") == pkg.importPath { + if len(rs) > 0 && isRuleKind(c, rs[0], "go_proto_library") && rs[0].AttrString("importpath") == pkg.importPath { protoEmbeds = append(protoEmbeds, protoEmbed) } } @@ -259,7 +259,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes protoEmbed, rs = g.generateProto(pcMode, []protoTarget{pkg.proto}, pkg.importPath) if protoEmbed != "" { // check if rs is non-empty and that the first rule is a go_proto_library with the same importPath - if len(rs) > 0 && rs[0].Kind() == "go_proto_library" && rs[0].AttrString("importpath") == pkg.importPath { + if len(rs) > 0 && isRuleKind(c, rs[0], "go_proto_library") && rs[0].AttrString("importpath") == pkg.importPath { protoEmbeds = append(protoEmbeds, protoEmbed) } } diff --git a/language/go/generate_test.go b/language/go/generate_test.go index 60d775cc9..5747e4168 100644 --- a/language/go/generate_test.go +++ b/language/go/generate_test.go @@ -24,6 +24,7 @@ import ( "testing" "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/internal/mapkind" "github.com/bazelbuild/bazel-gazelle/language" "github.com/bazelbuild/bazel-gazelle/language/proto" "github.com/bazelbuild/bazel-gazelle/merger" @@ -87,6 +88,11 @@ func TestGenerateRules(t *testing.T) { } var testsFound int walk.Walk(c, cexts, []string{testdataDir}, walk.VisitAllUpdateSubdirsMode, func(dir, rel string, c *config.Config, update bool, oldFile *rule.File, subdirs, regularFiles, genFiles []string) { + // Ignore testdata root dir + if rel == "" { + return + } + t.Run(rel, func(t *testing.T) { var empty, gen []*rule.Rule for _, lang := range langs { @@ -117,12 +123,16 @@ func TestGenerateRules(t *testing.T) { return } testsFound += 1 + mappedResult, err := mapkind.ApplyOnRules(c, nil, oldFile, gen, empty) + if err != nil { + t.Fatalf("error applying mapped kinds: %v", err) + } f := rule.EmptyFile("test", "") for _, r := range gen { r.Insert(f) } convertImportsAttrs(f) - merger.FixLoads(f, loads) + merger.FixLoads(f, mappedResult.ApplyOnLoads(loads)) f.Sync() got := string(bzl.Format(f.File)) wantPath := filepath.Join(dir, "BUILD.want") diff --git a/language/go/kinds.go b/language/go/kinds.go index 0f465f0c3..e2da7546c 100644 --- a/language/go/kinds.go +++ b/language/go/kinds.go @@ -17,6 +17,8 @@ package golang import ( "fmt" + + "github.com/bazelbuild/bazel-gazelle/config" "github.com/bazelbuild/bazel-gazelle/rule" ) @@ -217,3 +219,22 @@ func apparentLoads(moduleToApparentName func(string) string) []rule.LoadInfo { } var goLoadsForTesting = apparentLoads(func(string) string { return "" }) + +func isRuleKind(c *config.Config, r *rule.Rule, expectedKind string) bool { + kind := r.Kind() + if kind == expectedKind { + return true + } + + if c == nil { + return false + } + + if mappedKind, ok := c.KindMap[expectedKind]; ok { + if mappedKind.KindName == kind { + return true + } + } + + return false +} diff --git a/language/go/resolve.go b/language/go/resolve.go index 7921bc25d..1088ada01 100644 --- a/language/go/resolve.go +++ b/language/go/resolve.go @@ -31,8 +31,8 @@ import ( "github.com/bazelbuild/bazel-gazelle/rule" ) -func (*goLang) Imports(_ *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { - if !isGoLibrary(r.Kind()) || isExtraLibrary(r) { +func (*goLang) Imports(c *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { + if !isGoLibrary(c, r) || isExtraLibrary(r) { return nil } if importPath := r.AttrString("importpath"); importPath == "" { @@ -47,7 +47,7 @@ func (*goLang) Imports(_ *config.Config, r *rule.Rule, f *rule.File) []resolve.I func (*goLang) Embeds(r *rule.Rule, from label.Label) []label.Label { embedStrings := r.AttrStrings("embed") - if isGoProtoLibrary(r.Kind()) { + if isGoProtoLibrary(nil, r) { embedStrings = append(embedStrings, r.AttrString("proto")) embedStrings = append(embedStrings, r.AttrStrings("protos")...) } @@ -71,8 +71,8 @@ func (gl *goLang) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.Remo imports := importsRaw.(rule.PlatformStrings) r.DelAttr("deps") var resolve func(*config.Config, *resolve.RuleIndex, *repo.RemoteCache, string, label.Label) (label.Label, error) - switch r.Kind() { - case "go_proto_library": + switch { + case isRuleKind(c, r, "go_proto_library"): resolve = resolveProto default: resolve = ResolveGo @@ -96,7 +96,7 @@ func (gl *goLang) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.Remo log.Print(err) } if !deps.IsEmpty() { - if r.Kind() == "go_proto_library" { + if isRuleKind(c, r, "go_proto_library") { // protos may import the same library multiple times by different names, // so we need to de-duplicate them. Protos are not platform-specific, // so it's safe to just flatten them. @@ -369,12 +369,12 @@ func resolveWithIndexProto(c *config.Config, ix *resolve.RuleIndex, imp string, return matches[0].Label, nil } -func isGoLibrary(kind string) bool { - return kind == "go_library" || isGoProtoLibrary(kind) +func isGoLibrary(c *config.Config, r *rule.Rule) bool { + return isRuleKind(c, r, "go_library") || isGoProtoLibrary(c, r) } -func isGoProtoLibrary(kind string) bool { - return kind == "go_proto_library" || kind == "go_grpc_library" +func isGoProtoLibrary(c *config.Config, r *rule.Rule) bool { + return isRuleKind(c, r, "go_proto_library") || isRuleKind(c, r, "go_grpc_library") } // isExtraLibrary returns true if this rule is one of a handful of proto diff --git a/language/go/testdata/mapping/BUILD.old b/language/go/testdata/mapping/BUILD.old new file mode 100644 index 000000000..ff965e881 --- /dev/null +++ b/language/go/testdata/mapping/BUILD.old @@ -0,0 +1,5 @@ +# gazelle:map_kind go_binary my_go_binary //my/go:defs.bzl +# gazelle:map_kind go_library my_go_library //my/go:defs.bzl +# gazelle:map_kind go_test my_go_test //my/go:defs.bzl +# gazelle:map_kind proto_library my_proto_library //my/proto:defs.bzl +# gazelle:map_kind go_proto_library my_go_proto_library //my/proto:defs.bzl diff --git a/language/go/testdata/mapping/BUILD.want b/language/go/testdata/mapping/BUILD.want new file mode 100644 index 000000000..e69de29bb diff --git a/language/go/testdata/mapping/bin/added/BUILD.want b/language/go/testdata/mapping/bin/added/BUILD.want new file mode 100644 index 000000000..f35a4e75b --- /dev/null +++ b/language/go/testdata/mapping/bin/added/BUILD.want @@ -0,0 +1,16 @@ +load("//my/go:defs.bzl", "my_go_binary", "my_go_library") + +my_go_library( + name = "added_lib", + srcs = ["main.go"], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/bin/added", + visibility = ["//visibility:private"], +) + +my_go_binary( + name = "added", + _gazelle_imports = [], + embed = [":added_lib"], + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/bin/added/main.go b/language/go/testdata/mapping/bin/added/main.go new file mode 100644 index 000000000..38dd16da6 --- /dev/null +++ b/language/go/testdata/mapping/bin/added/main.go @@ -0,0 +1,3 @@ +package main + +func main() {} diff --git a/language/go/testdata/mapping/bin/removed/BUILD.old b/language/go/testdata/mapping/bin/removed/BUILD.old new file mode 100644 index 000000000..b19860240 --- /dev/null +++ b/language/go/testdata/mapping/bin/removed/BUILD.old @@ -0,0 +1,16 @@ +load("//my/go:defs.bzl", "my_go_binary", "my_go_library") + +my_go_library( + name = "removed_lib", + srcs = ["main.go"], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/bin/removed", + visibility = ["//visibility:private"], +) + +my_go_binary( + name = "removed", + _gazelle_imports = [], + embed = [":removed_lib"], + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/bin/removed/BUILD.want b/language/go/testdata/mapping/bin/removed/BUILD.want new file mode 100644 index 000000000..e69de29bb diff --git a/language/go/testdata/mapping/bin/updated/BUILD.old b/language/go/testdata/mapping/bin/updated/BUILD.old new file mode 100644 index 000000000..9aba9396e --- /dev/null +++ b/language/go/testdata/mapping/bin/updated/BUILD.old @@ -0,0 +1,16 @@ +load("//my/go:defs.bzl", "my_go_binary", "my_go_library") + +my_go_library( + name = "updated_lib", + srcs = ["main.go"], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/bin/updated", + visibility = ["//visibility:private"], +) + +my_go_binary( + name = "updated", + _gazelle_imports = [], + embed = [":updated_lib"], + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/bin/updated/BUILD.want b/language/go/testdata/mapping/bin/updated/BUILD.want new file mode 100644 index 000000000..0c2dffa7d --- /dev/null +++ b/language/go/testdata/mapping/bin/updated/BUILD.want @@ -0,0 +1,19 @@ +load("//my/go:defs.bzl", "my_go_binary", "my_go_library") + +my_go_library( + name = "updated_lib", + srcs = [ + "main.go", + "other.go", + ], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/bin/updated", + visibility = ["//visibility:private"], +) + +my_go_binary( + name = "updated", + _gazelle_imports = [], + embed = [":updated_lib"], + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/bin/updated/main.go b/language/go/testdata/mapping/bin/updated/main.go new file mode 100644 index 000000000..38dd16da6 --- /dev/null +++ b/language/go/testdata/mapping/bin/updated/main.go @@ -0,0 +1,3 @@ +package main + +func main() {} diff --git a/language/go/testdata/mapping/bin/updated/other.go b/language/go/testdata/mapping/bin/updated/other.go new file mode 100644 index 000000000..06ab7d0f9 --- /dev/null +++ b/language/go/testdata/mapping/bin/updated/other.go @@ -0,0 +1 @@ +package main diff --git a/language/go/testdata/mapping/lib/added/BUILD.want b/language/go/testdata/mapping/lib/added/BUILD.want new file mode 100644 index 000000000..786bcc4e5 --- /dev/null +++ b/language/go/testdata/mapping/lib/added/BUILD.want @@ -0,0 +1,16 @@ +load("//my/go:defs.bzl", "my_go_library", "my_go_test") + +my_go_library( + name = "added", + srcs = ["foo.go"], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/lib/added", + visibility = ["//visibility:public"], +) + +my_go_test( + name = "added_test", + srcs = ["foo_test.go"], + _gazelle_imports = [], + embed = [":added"], +) diff --git a/language/go/testdata/mapping/lib/added/foo.go b/language/go/testdata/mapping/lib/added/foo.go new file mode 100644 index 000000000..74b71d9ad --- /dev/null +++ b/language/go/testdata/mapping/lib/added/foo.go @@ -0,0 +1 @@ +package added diff --git a/language/go/testdata/mapping/lib/added/foo_test.go b/language/go/testdata/mapping/lib/added/foo_test.go new file mode 100644 index 000000000..74b71d9ad --- /dev/null +++ b/language/go/testdata/mapping/lib/added/foo_test.go @@ -0,0 +1 @@ +package added diff --git a/language/go/testdata/mapping/lib/removed/BUILD.old b/language/go/testdata/mapping/lib/removed/BUILD.old new file mode 100644 index 000000000..c3c752ea4 --- /dev/null +++ b/language/go/testdata/mapping/lib/removed/BUILD.old @@ -0,0 +1,16 @@ +load("//my/go:defs.bzl", "my_go_library", "my_go_test") + +my_go_library( + name = "removed", + srcs = ["foo.go"], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/lib/removed", + visibility = ["//visibility:public"], +) + +my_go_test( + name = "removed_test", + srcs = ["foo_test.go"], + _gazelle_imports = [], + embed = [":removed"], +) diff --git a/language/go/testdata/mapping/lib/removed/BUILD.want b/language/go/testdata/mapping/lib/removed/BUILD.want new file mode 100644 index 000000000..e69de29bb diff --git a/language/go/testdata/mapping/lib/updated/BUILD.old b/language/go/testdata/mapping/lib/updated/BUILD.old new file mode 100644 index 000000000..71567da3b --- /dev/null +++ b/language/go/testdata/mapping/lib/updated/BUILD.old @@ -0,0 +1,16 @@ +load("//my/go:defs.bzl", "my_go_library", "my_go_test") + +my_go_library( + name = "updated", + srcs = ["foo.go"], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/lib/updated", + visibility = ["//visibility:public"], +) + +my_go_test( + name = "updated_test", + srcs = ["foo_test.go"], + _gazelle_imports = [], + embed = [":updated"], +) diff --git a/language/go/testdata/mapping/lib/updated/BUILD.want b/language/go/testdata/mapping/lib/updated/BUILD.want new file mode 100644 index 000000000..97ee67555 --- /dev/null +++ b/language/go/testdata/mapping/lib/updated/BUILD.want @@ -0,0 +1,22 @@ +load("//my/go:defs.bzl", "my_go_library", "my_go_test") + +my_go_library( + name = "updated", + srcs = [ + "bar.go", + "foo.go", + ], + _gazelle_imports = [], + importpath = "example.com/repo/mapping/lib/updated", + visibility = ["//visibility:public"], +) + +my_go_test( + name = "updated_test", + srcs = [ + "bar_test.go", + "foo_test.go", + ], + _gazelle_imports = [], + embed = [":updated"], +) diff --git a/language/go/testdata/mapping/lib/updated/bar.go b/language/go/testdata/mapping/lib/updated/bar.go new file mode 100644 index 000000000..e71da67b0 --- /dev/null +++ b/language/go/testdata/mapping/lib/updated/bar.go @@ -0,0 +1 @@ +package updated diff --git a/language/go/testdata/mapping/lib/updated/bar_test.go b/language/go/testdata/mapping/lib/updated/bar_test.go new file mode 100644 index 000000000..e71da67b0 --- /dev/null +++ b/language/go/testdata/mapping/lib/updated/bar_test.go @@ -0,0 +1 @@ +package updated diff --git a/language/go/testdata/mapping/lib/updated/foo.go b/language/go/testdata/mapping/lib/updated/foo.go new file mode 100644 index 000000000..e71da67b0 --- /dev/null +++ b/language/go/testdata/mapping/lib/updated/foo.go @@ -0,0 +1 @@ +package updated diff --git a/language/go/testdata/mapping/lib/updated/foo_test.go b/language/go/testdata/mapping/lib/updated/foo_test.go new file mode 100644 index 000000000..e71da67b0 --- /dev/null +++ b/language/go/testdata/mapping/lib/updated/foo_test.go @@ -0,0 +1 @@ +package updated diff --git a/language/go/testdata/mapping/proto/added/BUILD.want b/language/go/testdata/mapping/proto/added/BUILD.want new file mode 100644 index 000000000..9fc843aac --- /dev/null +++ b/language/go/testdata/mapping/proto/added/BUILD.want @@ -0,0 +1,26 @@ +load("//my/go:defs.bzl", "my_go_library") +load("//my/proto:defs.bzl", "my_go_proto_library", "my_proto_library") + +my_proto_library( + name = "added_proto", + srcs = ["foo.proto"], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) + +my_go_proto_library( + name = "added_go_proto", + _gazelle_imports = [], + importpath = "example.com/repo/mapping/proto/added", + proto = ":added_proto", + visibility = ["//visibility:public"], +) + +my_go_library( + name = "added", + srcs = ["extra.go"], + _gazelle_imports = [], + embed = [":added_go_proto"], + importpath = "example.com/repo/mapping/proto/added", + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/proto/added/extra.go b/language/go/testdata/mapping/proto/added/extra.go new file mode 100644 index 000000000..cb5a98900 --- /dev/null +++ b/language/go/testdata/mapping/proto/added/extra.go @@ -0,0 +1 @@ +package protos diff --git a/language/go/testdata/mapping/proto/added/foo.proto b/language/go/testdata/mapping/proto/added/foo.proto new file mode 100644 index 000000000..ab3f96ef1 --- /dev/null +++ b/language/go/testdata/mapping/proto/added/foo.proto @@ -0,0 +1,3 @@ +syntax = "proto2"; + +option go_package = "example.com/repo/mapping/proto/added"; diff --git a/language/go/testdata/mapping/proto/removed/BUILD.old b/language/go/testdata/mapping/proto/removed/BUILD.old new file mode 100644 index 000000000..eeb230e69 --- /dev/null +++ b/language/go/testdata/mapping/proto/removed/BUILD.old @@ -0,0 +1,26 @@ +load("//my/go:defs.bzl", "my_go_library") +load("//my/proto:defs.bzl", "my_go_proto_library", "my_proto_library") + +my_proto_library( + name = "removed_proto", + srcs = ["foo.proto"], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) + +my_go_proto_library( + name = "removed_go_proto", + _gazelle_imports = [], + importpath = "example.com/repo/mapping/proto/removed", + proto = ":removed_proto", + visibility = ["//visibility:public"], +) + +my_go_library( + name = "removed", + srcs = ["extra.go"], + _gazelle_imports = [], + embed = [":removed_go_proto"], + importpath = "example.com/repo/mapping/proto/removed", + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/proto/removed/BUILD.want b/language/go/testdata/mapping/proto/removed/BUILD.want new file mode 100644 index 000000000..e69de29bb diff --git a/language/go/testdata/mapping/proto/updated/BUILD.old b/language/go/testdata/mapping/proto/updated/BUILD.old new file mode 100644 index 000000000..db1d48053 --- /dev/null +++ b/language/go/testdata/mapping/proto/updated/BUILD.old @@ -0,0 +1,26 @@ +load("//my/go:defs.bzl", "my_go_library") +load("//my/proto:defs.bzl", "my_go_proto_library", "my_proto_library") + +my_proto_library( + name = "updated_proto", + srcs = ["foo.proto"], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) + +my_go_proto_library( + name = "updated_go_proto", + _gazelle_imports = [], + importpath = "example.com/repo/mapping/proto/updated", + proto = ":updated_proto", + visibility = ["//visibility:public"], +) + +my_go_library( + name = "updated", + srcs = ["extra.go"], + _gazelle_imports = [], + embed = [":updated_go_proto"], + importpath = "example.com/repo/mapping/proto/updated", + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/proto/updated/BUILD.want b/language/go/testdata/mapping/proto/updated/BUILD.want new file mode 100644 index 000000000..c5ea6c12c --- /dev/null +++ b/language/go/testdata/mapping/proto/updated/BUILD.want @@ -0,0 +1,32 @@ +load("//my/go:defs.bzl", "my_go_library") +load("//my/proto:defs.bzl", "my_go_proto_library", "my_proto_library") + +my_proto_library( + name = "updated_proto", + srcs = [ + "bar.proto", + "foo.proto", + ], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) + +my_go_proto_library( + name = "updated_go_proto", + _gazelle_imports = [], + importpath = "example.com/repo/mapping/proto/updated", + proto = ":updated_proto", + visibility = ["//visibility:public"], +) + +my_go_library( + name = "updated", + srcs = [ + "extra.go", + "other_extra.go", + ], + _gazelle_imports = [], + embed = [":updated_go_proto"], + importpath = "example.com/repo/mapping/proto/updated", + visibility = ["//visibility:public"], +) diff --git a/language/go/testdata/mapping/proto/updated/bar.proto b/language/go/testdata/mapping/proto/updated/bar.proto new file mode 100644 index 000000000..e09421732 --- /dev/null +++ b/language/go/testdata/mapping/proto/updated/bar.proto @@ -0,0 +1,3 @@ +syntax = "proto2"; + +option go_package = "example.com/repo/mapping/proto/updated"; diff --git a/language/go/testdata/mapping/proto/updated/extra.go b/language/go/testdata/mapping/proto/updated/extra.go new file mode 100644 index 000000000..cb5a98900 --- /dev/null +++ b/language/go/testdata/mapping/proto/updated/extra.go @@ -0,0 +1 @@ +package protos diff --git a/language/go/testdata/mapping/proto/updated/foo.proto b/language/go/testdata/mapping/proto/updated/foo.proto new file mode 100644 index 000000000..e09421732 --- /dev/null +++ b/language/go/testdata/mapping/proto/updated/foo.proto @@ -0,0 +1,3 @@ +syntax = "proto2"; + +option go_package = "example.com/repo/mapping/proto/updated"; diff --git a/language/go/testdata/mapping/proto/updated/other_extra.go b/language/go/testdata/mapping/proto/updated/other_extra.go new file mode 100644 index 000000000..cb5a98900 --- /dev/null +++ b/language/go/testdata/mapping/proto/updated/other_extra.go @@ -0,0 +1 @@ +package protos diff --git a/language/proto/BUILD.bazel b/language/proto/BUILD.bazel index 6be680000..282236a17 100644 --- a/language/proto/BUILD.bazel +++ b/language/proto/BUILD.bazel @@ -78,6 +78,7 @@ go_test( embed = [":proto"], deps = [ "//config", + "//internal/mapkind", "//label", "//language", "//merger", diff --git a/language/proto/generate.go b/language/proto/generate.go index 725597e38..059595fee 100644 --- a/language/proto/generate.go +++ b/language/proto/generate.go @@ -47,7 +47,7 @@ func (*protoLang) GenerateRules(args language.GenerateArgs) language.GenerateRes // Some of the generated files may have been consumed by other rules consumedFileSet := make(map[string]bool) for _, r := range args.OtherGen { - if r.Kind() != "proto_library" { + if !isRuleKind(c, r, "proto_library") { continue } for _, f := range r.AttrStrings("srcs") { @@ -86,7 +86,7 @@ func (*protoLang) GenerateRules(args language.GenerateArgs) language.GenerateRes for i, r := range res.Gen { res.Imports[i] = r.PrivateAttr(config.GazelleImportsKey) } - res.Empty = append(res.Empty, generateEmpty(args.File, regularProtoFiles, genProtoFiles)...) + res.Empty = append(res.Empty, generateEmpty(c, args.File, regularProtoFiles, genProtoFiles)...) return res } @@ -278,7 +278,7 @@ func getPrefix(pc *ProtoConfig, rel string) string { // generateEmpty generates a list of proto_library rules that may be deleted. // This is generated from existing proto_library rules with srcs lists that // don't match any static or generated files. -func generateEmpty(f *rule.File, regularFiles, genFiles []string) []*rule.Rule { +func generateEmpty(cfg *config.Config, f *rule.File, regularFiles, genFiles []string) []*rule.Rule { if f == nil { return nil } @@ -292,7 +292,7 @@ func generateEmpty(f *rule.File, regularFiles, genFiles []string) []*rule.Rule { var empty []*rule.Rule outer: for _, r := range f.Rules { - if r.Kind() != "proto_library" { + if !isRuleKind(cfg, r, "proto_library") { continue } srcs := r.AttrStrings("srcs") diff --git a/language/proto/generate_test.go b/language/proto/generate_test.go index e973b65dd..ee93cd038 100644 --- a/language/proto/generate_test.go +++ b/language/proto/generate_test.go @@ -24,13 +24,13 @@ import ( "testing" "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/internal/mapkind" "github.com/bazelbuild/bazel-gazelle/language" "github.com/bazelbuild/bazel-gazelle/merger" "github.com/bazelbuild/bazel-gazelle/resolve" "github.com/bazelbuild/bazel-gazelle/rule" "github.com/bazelbuild/bazel-gazelle/testtools" "github.com/bazelbuild/bazel-gazelle/walk" - bzl "github.com/bazelbuild/buildtools/build" ) @@ -47,6 +47,11 @@ func TestGenerateRules(t *testing.T) { c, lang, _ := testConfig(t, "testdata") walk.Walk(c, []config.Configurer{lang}, []string{"testdata"}, walk.VisitAllUpdateSubdirsMode, func(dir, rel string, c *config.Config, update bool, oldFile *rule.File, subdirs, regularFiles, genFiles []string) { + // Ignore testdata root dir + if rel == "" { + return + } + isTest := false for _, name := range regularFiles { if name == "BUILD.want" { @@ -57,6 +62,7 @@ func TestGenerateRules(t *testing.T) { if !isTest { return } + loads := lang.(language.ModuleAwareLanguage).ApparentLoads(func(string) string { return "" }) t.Run(rel, func(t *testing.T) { res := lang.GenerateRules(language.GenerateArgs{ Config: c, @@ -70,12 +76,16 @@ func TestGenerateRules(t *testing.T) { if len(res.Empty) > 0 { t.Errorf("got %d empty rules; want 0", len(res.Empty)) } + mappedResult, err := mapkind.ApplyOnRules(c, nil, oldFile, res.Gen, res.Empty) + if err != nil { + t.Fatalf("error applying mapped kinds: %v", err) + } f := rule.EmptyFile("test", "") for _, r := range res.Gen { r.Insert(f) } convertImportsAttrs(f) - merger.FixLoads(f, lang.(language.ModuleAwareLanguage).ApparentLoads(func(string) string { return "" })) + merger.FixLoads(f, mappedResult.ApplyOnLoads(loads)) f.Sync() got := string(bzl.Format(f.File)) wantPath := filepath.Join(dir, "BUILD.want") @@ -175,7 +185,7 @@ func TestGeneratePackage(t *testing.T) { "protos/sub/sub.proto", }, HasServices: true, - Services: []string{"Quux"}, + Services: []string{"Quux"}, }, }, Imports: map[string]bool{ @@ -236,7 +246,7 @@ func TestFileModeImports(t *testing.T) { Path: filepath.Join(dir, "foo.proto"), Name: "foo.proto", PackageName: "file_mode", - Messages: []string{"Foo"}, + Messages: []string{"Foo"}, }, }, Imports: map[string]bool{}, diff --git a/language/proto/kinds.go b/language/proto/kinds.go index 31473b8d5..c4852a8c2 100644 --- a/language/proto/kinds.go +++ b/language/proto/kinds.go @@ -18,6 +18,7 @@ package proto import ( "fmt" + "github.com/bazelbuild/bazel-gazelle/config" "github.com/bazelbuild/bazel-gazelle/rule" ) @@ -54,3 +55,22 @@ func (*protoLang) ApparentLoads(moduleToApparentName func(string) string) []rule }, } } + +func isRuleKind(c *config.Config, r *rule.Rule, expectedKind string) bool { + kind := r.Kind() + if kind == expectedKind { + return true + } + + if c == nil { + return false + } + + if mappedKind, ok := c.KindMap[expectedKind]; ok { + if mappedKind.KindName == kind { + return true + } + } + + return false +} diff --git a/language/proto/testdata/mapping/BUILD.old b/language/proto/testdata/mapping/BUILD.old new file mode 100644 index 000000000..90be8d633 --- /dev/null +++ b/language/proto/testdata/mapping/BUILD.old @@ -0,0 +1 @@ +# gazelle:map_kind proto_library my_proto_library //my/proto:defs.bzl diff --git a/language/proto/testdata/mapping/BUILD.want b/language/proto/testdata/mapping/BUILD.want new file mode 100644 index 000000000..e69de29bb diff --git a/language/proto/testdata/mapping/added/BUILD.want b/language/proto/testdata/mapping/added/BUILD.want new file mode 100644 index 000000000..080b1056a --- /dev/null +++ b/language/proto/testdata/mapping/added/BUILD.want @@ -0,0 +1,8 @@ +load("//my/proto:defs.bzl", "my_proto_library") + +my_proto_library( + name = "added_proto", + srcs = ["foo.proto"], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) diff --git a/language/proto/testdata/mapping/added/foo.proto b/language/proto/testdata/mapping/added/foo.proto new file mode 100644 index 000000000..fddb20b36 --- /dev/null +++ b/language/proto/testdata/mapping/added/foo.proto @@ -0,0 +1 @@ +syntax = "proto2"; diff --git a/language/proto/testdata/mapping/removed/BUILD.old b/language/proto/testdata/mapping/removed/BUILD.old new file mode 100644 index 000000000..f3ed70ddb --- /dev/null +++ b/language/proto/testdata/mapping/removed/BUILD.old @@ -0,0 +1,8 @@ +load("//my/proto:defs.bzl", "my_proto_library") + +my_proto_library( + name = "removed_proto", + srcs = ["foo.proto"], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) diff --git a/language/proto/testdata/mapping/removed/BUILD.want b/language/proto/testdata/mapping/removed/BUILD.want new file mode 100644 index 000000000..e69de29bb diff --git a/language/proto/testdata/mapping/updated/BUILD.old b/language/proto/testdata/mapping/updated/BUILD.old new file mode 100644 index 000000000..61c4443ae --- /dev/null +++ b/language/proto/testdata/mapping/updated/BUILD.old @@ -0,0 +1,8 @@ +load("//my/proto:defs.bzl", "my_proto_library") + +my_proto_library( + name = "updated_proto", + srcs = ["foo.proto"], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) diff --git a/language/proto/testdata/mapping/updated/BUILD.want b/language/proto/testdata/mapping/updated/BUILD.want new file mode 100644 index 000000000..964fdc6f4 --- /dev/null +++ b/language/proto/testdata/mapping/updated/BUILD.want @@ -0,0 +1,11 @@ +load("//my/proto:defs.bzl", "my_proto_library") + +my_proto_library( + name = "updated_proto", + srcs = [ + "bar.proto", + "foo.proto", + ], + _gazelle_imports = [], + visibility = ["//visibility:public"], +) diff --git a/language/proto/testdata/mapping/updated/bar.proto b/language/proto/testdata/mapping/updated/bar.proto new file mode 100644 index 000000000..e09421732 --- /dev/null +++ b/language/proto/testdata/mapping/updated/bar.proto @@ -0,0 +1,3 @@ +syntax = "proto2"; + +option go_package = "example.com/repo/mapping/proto/updated"; diff --git a/language/proto/testdata/mapping/updated/foo.proto b/language/proto/testdata/mapping/updated/foo.proto new file mode 100644 index 000000000..e09421732 --- /dev/null +++ b/language/proto/testdata/mapping/updated/foo.proto @@ -0,0 +1,3 @@ +syntax = "proto2"; + +option go_package = "example.com/repo/mapping/proto/updated";