From 3232eedc31801e357305fd7e9343d5423a263f32 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Fri, 17 Jul 2020 15:40:58 -0400 Subject: [PATCH 1/3] language/go: several naming convention fixes plus auto-detection * For external dependency resolution, if we don't know what naming convention the external repository uses (for example, because there's no known repository), we'll now use goDefaultLibraryNamingConvention. This avoids assuming that repositories with build files fetched with http_archive have been updated. * In migrateNamingConvention, print a warning and avoid renaming if there's already a target with the new name. * Library, test, and alias actuals are now generated based on import path instead of package name. * Added unknownNamingConvention, a new zero value. * Added detectNamingConvention. It reads the root build file and build files in subdirectories one level deep to infer the naming convention used if one is not specified in the root build file. Defaults to importNamingConvention. * Fixed tests. For #5 --- cmd/gazelle/diff_test.go | 6 +- cmd/gazelle/fix_test.go | 12 +-- cmd/gazelle/integration_test.go | 8 +- .../gazellebinarytest/gazellebinary_test.go | 2 +- language/go/config.go | 91 ++++++++++++++++++- language/go/fix.go | 55 ++++++++--- language/go/fix_test.go | 43 +++++++++ language/go/generate.go | 10 +- language/go/generate_test.go | 12 +-- language/go/package.go | 7 -- language/go/resolve.go | 17 ++-- language/go/resolve_test.go | 54 +++++++---- language/go/testdata/allcgolib/BUILD.want | 6 +- language/go/testdata/bin/BUILD.want | 4 +- .../go/testdata/bin_with_tests/BUILD.want | 8 +- language/go/testdata/cgolib/BUILD.want | 6 +- .../cgolib_with_build_tags/BUILD.want | 6 +- .../go/testdata/default_visibility/BUILD.want | 6 +- .../default_visibility/cmd/BUILD.want | 4 +- .../go/testdata/gen_and_exclude/BUILD.want | 2 +- language/go/testdata/importmap/BUILD.want | 6 +- language/go/testdata/lib/BUILD.want | 6 +- .../go/testdata/lib/internal/deep/BUILD.want | 2 +- .../lib/internal/go_visibility/BUILD.want | 2 +- .../testdata/lib/relativeimporter/BUILD.want | 2 +- .../go/testdata/main_test_only/BUILD.want | 2 +- .../naming_convention/import_alias/lib/lib.go | 4 +- .../import_alias/lib/lib_test.go | 2 +- language/go/testdata/platforms/BUILD.want | 6 +- .../proto_package_mode_extras/BUILD.want | 6 +- language/go/testdata/protos/BUILD.want | 2 +- .../protos_explicit_default/BUILD.want | 2 +- language/go/testdata/protos_gogo/BUILD.want | 2 +- .../protos_gogo_subdir_reset/BUILD.want | 2 +- .../protos_gogo_subdir_reset/sub/BUILD.want | 2 +- language/go/testdata/service/BUILD.want | 2 +- language/go/testdata/service_gogo/BUILD.want | 2 +- .../service_gogo_subdir_reset/BUILD.want | 2 +- .../service_gogo_subdir_reset/sub/BUILD.want | 2 +- .../testdata/tests_import_testdata/BUILD.want | 2 +- .../testdata/tests_with_testdata/BUILD.want | 2 +- 41 files changed, 296 insertions(+), 123 deletions(-) diff --git a/cmd/gazelle/diff_test.go b/cmd/gazelle/diff_test.go index 6a6b58886..ed5a755ea 100644 --- a/cmd/gazelle/diff_test.go +++ b/cmd/gazelle/diff_test.go @@ -55,7 +55,7 @@ func TestDiffExisting(t *testing.T) { # gazelle:prefix example.com/hello +go_library( -+ name = "go_default_library", ++ name = "hello", + srcs = ["hello.go"], + importpath = "example.com/hello", + visibility = ["//visibility:public"], @@ -91,7 +91,7 @@ func TestDiffNew(t *testing.T) { +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( -+ name = "go_default_library", ++ name = "hello", + srcs = ["hello.go"], + importpath = "example.com/hello", + visibility = ["//visibility:public"], @@ -138,7 +138,7 @@ func TestDiffReadWriteDir(t *testing.T) { # gazelle:prefix example.com/hello + +go_library( -+ name = "go_default_library", ++ name = "hello", + srcs = ["hello.go"], + importpath = "example.com/hello", + visibility = ["//visibility:public"], diff --git a/cmd/gazelle/fix_test.go b/cmd/gazelle/fix_test.go index 10482dda2..ecd7e2371 100644 --- a/cmd/gazelle/fix_test.go +++ b/cmd/gazelle/fix_test.go @@ -257,13 +257,13 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") go_binary( name = "hello", - embed = [":go_default_library"], + embed = [":repo_lib"], pure = "on", visibility = ["//visibility:public"], ) go_library( - name = "go_default_library", + name = "repo_lib", srcs = ["hello.go"], importpath = "example.com/repo", visibility = ["//visibility:private"], @@ -291,7 +291,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") # src build file go_library( - name = "go_default_library", + name = "repo_lib", srcs = ["hello.go"], importpath = "example.com/repo", visibility = ["//visibility:private"], @@ -299,7 +299,7 @@ go_library( go_binary( name = "repo", - embed = [":go_default_library"], + embed = [":repo_lib"], visibility = ["//visibility:public"], ) `, @@ -324,13 +324,13 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") go_binary( name = "hello", - embed = [":go_default_library"], + embed = [":repo_lib"], pure = "on", visibility = ["//visibility:public"], ) go_library( - name = "go_default_library", + name = "repo_lib", srcs = ["hello.go"], importpath = "example.com/repo", visibility = ["//visibility:private"], diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index 90b2d81d4..3aa47632d 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -524,11 +524,11 @@ import _ "golang.org/x/baz" Content: `load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "foo", srcs = ["a.go"], importpath = "example.com/foo", visibility = ["//visibility:public"], - deps = ["//vendor/golang.org/x/bar:go_default_library"], + deps = ["//vendor/golang.org/x/bar"], ) `, }, { @@ -536,12 +536,12 @@ go_library( Content: `load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "bar", srcs = ["bar.go"], importmap = "example.com/foo/vendor/golang.org/x/bar", importpath = "golang.org/x/bar", visibility = ["//visibility:public"], - deps = ["//vendor/golang.org/x/baz:go_default_library"], + deps = ["//vendor/golang.org/x/baz"], ) `, }, diff --git a/internal/gazellebinarytest/gazellebinary_test.go b/internal/gazellebinarytest/gazellebinary_test.go index c0cbb4867..6f5688f4f 100644 --- a/internal/gazellebinarytest/gazellebinary_test.go +++ b/internal/gazellebinarytest/gazellebinary_test.go @@ -55,7 +55,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") # gazelle:prefix example.com/test go_library( - name = "go_default_library", + name = "test", srcs = ["foo.go"], importpath = "example.com/test", visibility = ["//visibility:public"], diff --git a/language/go/config.go b/language/go/config.go index 04c12ac22..248146f3f 100644 --- a/language/go/config.go +++ b/language/go/config.go @@ -268,8 +268,11 @@ func (f *namingConventionFlag) String() string { type namingConvention int const ( + // Try to detect the naming convention in use. + unknownNamingConvention namingConvention = iota + // 'go_default_library' and 'go_default_test' - goDefaultLibraryNamingConvention = iota + goDefaultLibraryNamingConvention // For an import path that ends with foo, the go_library rules target is // named 'foo', the go_test is named 'foo_test'. @@ -296,6 +299,8 @@ func (nc namingConvention) String() string { func namingConventionFromString(s string) (namingConvention, error) { switch s { + case "": + return unknownNamingConvention, nil case "go_default_library": return goDefaultLibraryNamingConvention, nil case "import": @@ -303,7 +308,7 @@ func namingConventionFromString(s string) (namingConvention, error) { case "import_alias": return importAliasNamingConvention, nil default: - return goDefaultLibraryNamingConvention, fmt.Errorf("unknown naming convention %q", s) + return unknownNamingConvention, fmt.Errorf("unknown naming convention %q", s) } } @@ -456,7 +461,12 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.` for _, repo := range c.Repos { if repo.Kind() == "go_repository" { if attr := repo.AttrString("build_naming_convention"); attr == "" { - repoNamingConvention[repo.Name()] = goDefaultLibraryNamingConvention // default for go_repository + // No naming convention specified. + // go_repsitory uses importAliasNamingConvention by default, so we + // could use whichever name. + // resolveExternal should take that as a signal to follow the current + // naming convention to avoid churn. + repoNamingConvention[repo.Name()] = importAliasNamingConvention } else if nc, err := namingConventionFromString(attr); err != nil { log.Printf("in go_repository named %q: %v", repo.Name(), err) } else { @@ -543,6 +553,7 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.` setPrefix(d.Value) } } + if !gc.prefixSet { for _, r := range f.Rules { switch r.Kind() { @@ -565,6 +576,10 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.` } } } + + if gc.goNamingConvention == unknownNamingConvention { + gc.goNamingConvention = detectNamingConvention(c, f) + } } // checkPrefix checks that a string may be used as a prefix. We forbid local @@ -615,3 +630,73 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.` } var errRulesGoRepoNotFound = errors.New(config.RulesGoRepoName + " external repository not found") + +// detectNamingConvention attempts to detect the naming convention in use by +// reading build files in subdirectories of the repository root directory. +// If no build files are found (for example, in a new project), or if multiple +// naming conventions are found, importNamingConvention is returned. +func detectNamingConvention(c *config.Config, rootFile *rule.File) namingConvention { + detectInFile := func(f *rule.File) namingConvention { + for _, r := range f.Rules { + if r.Kind() != "go_library" { + continue + } + if name := r.Name(); name == defaultLibName { + return goDefaultLibraryNamingConvention + } else if name == path.Base(r.AttrString("importpath")) { + return importNamingConvention + } + } + return unknownNamingConvention + } + + detectInDir := func(dir, rel string) namingConvention { + var f *rule.File + for _, name := range c.ValidBuildFileNames { + fpath := filepath.Join(dir, name) + data, err := ioutil.ReadFile(fpath) + if err != nil { + continue + } + f, err = rule.LoadData(fpath, rel, data) + if err != nil { + continue + } + } + if f == nil { + return unknownNamingConvention + } + return detectInFile(f) + } + + nc := unknownNamingConvention + if rootFile != nil { + if rootNC := detectInFile(rootFile); rootNC != unknownNamingConvention { + return rootNC + } + } + + infos, err := ioutil.ReadDir(c.RepoRoot) + if err != nil { + return importNamingConvention + } + for _, info := range infos { + if !info.IsDir() { + continue + } + dirName := info.Name() + dirNC := detectInDir(filepath.Join(c.RepoRoot, dirName), dirName) + if dirNC == unknownNamingConvention { + continue + } + if nc != unknownNamingConvention && dirNC != nc { + // Subdirectories use different conventions. Return the default. + return importNamingConvention + } + nc = dirNC + } + if nc == unknownNamingConvention { + return importNamingConvention + } + return nc +} diff --git a/language/go/fix.go b/language/go/fix.go index b00c75f62..fbb1bdc46 100644 --- a/language/go/fix.go +++ b/language/go/fix.go @@ -30,17 +30,18 @@ func (_ *goLang) Fix(c *config.Config, f *rule.File) { flattenSrcs(c, f) squashCgoLibrary(c, f) squashXtest(c, f) - migrateNamingConvention(c, f) removeLegacyProto(c, f) removeLegacyGazelle(c, f) + migrateNamingConvention(c, f) } -// migrateNamingConvention renames rules according to go_naming_convention directives. +// migrateNamingConvention renames rules according to go_naming_convention +// directives. func migrateNamingConvention(c *config.Config, f *rule.File) { + // Determine old and new names for go_library and go_test. nc := getGoConfig(c).goNamingConvention - - binName := binName(f) - importPath := importPath(f) + binName := findBinName(f) + importPath := findImportPath(f) if importPath == "" { return } @@ -58,27 +59,56 @@ func migrateNamingConvention(c *config.Config, f *rule.File) { return } + // Check whether the new names are in use. If there are rules with both old + // and new names, there will be a conflict. + var haveLib, haveMigrateLib, haveTest, haveMigrateTest bool + for _, r := range f.Rules { + switch { + case r.Name() == libName: + haveLib = true + case r.Kind() == "go_library" && r.Name() == migrateLibName: + haveMigrateLib = true + case r.Name() == testName: + haveTest = true + case r.Kind() == "go_test" && r.Name() == migrateTestName: + haveMigrateTest = true + } + } + haveLibConflict := haveLib && haveMigrateLib + haveTestConflict := haveTest && haveMigrateTest + if haveLibConflict { + log.Printf("%[1]s: Tried to rename %[2]s to %[3]s, but %[3]s already exists.", f.Path, migrateLibName, libName) + } + if haveTestConflict { + log.Printf("%[1]s: Tried to rename %[2]s to %[3]s, but %[3]s already exists.", f.Path, migrateTestName, testName) + } + + // 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": - replaceInStrListAttr(r, "embed", ":"+migrateLibName, ":"+libName) + if !haveLibConflict { + replaceInStrListAttr(r, "embed", ":"+migrateLibName, ":"+libName) + } case "go_library": - if r.Name() == migrateLibName { + if r.Name() == migrateLibName && !haveLibConflict { r.SetName(libName) } case "go_test": - if r.Name() == migrateTestName { + if r.Name() == migrateTestName && !haveTestConflict { r.SetName(testName) + } + if !haveLibConflict { replaceInStrListAttr(r, "embed", ":"+migrateLibName, ":"+libName) } } } } -// binName returns the name of a go_binary rule if one can be found. -func binName(f *rule.File) string { +// findBinName returns the name of a go_binary rule if one can be found. +func findBinName(f *rule.File) string { for _, r := range f.Rules { if r.Kind() == "go_binary" { return r.Name() @@ -87,8 +117,9 @@ func binName(f *rule.File) string { return "" } -// import path returns the existing import path from the first encountered Go rule with the attribute set. -func importPath(f *rule.File) string { +// findImportPath returns the existing import path from the first encountered Go +// rule with the attribute set. +func findImportPath(f *rule.File) string { for _, r := range f.Rules { switch r.Kind() { case "go_binary", "go_library", "go_test": diff --git a/language/go/fix_test.go b/language/go/fix_test.go index ed7a48e84..48f91b514 100644 --- a/language/go/fix_test.go +++ b/language/go/fix_test.go @@ -103,6 +103,49 @@ go_test( srcs = ["foo_test.go"], embed = [":foo_lib"], ) +`, + }, { + desc: "go_naming_convention=go_default_library -> import conflict", + namingConvention: importNamingConvention, + old: `load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +load(":build_defs.bzl", "x_binary") + +go_library( + name = "go_default_library", + srcs = ["foo.go"], + importpath = "foo", + visibility = ["//visibility:private"], +) + +x_binary( + name = "foo", +) + +go_test( + name = "go_default_test", + srcs = ["foo_test.go"], + embed = [":go_default_library"], +) +`, + want: `load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +load(":build_defs.bzl", "x_binary") + +go_library( + name = "go_default_library", + srcs = ["foo.go"], + importpath = "foo", + visibility = ["//visibility:private"], +) + +x_binary( + name = "foo", +) + +go_test( + name = "foo_test", + srcs = ["foo_test.go"], + embed = [":go_default_library"], +) `, }, { desc: "go_naming_convention=import -> go_default_library for lib", diff --git a/language/go/generate.go b/language/go/generate.go index 301f06aa4..34f70d3f3 100644 --- a/language/go/generate.go +++ b/language/go/generate.go @@ -262,7 +262,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes libName = lib.Name() } rules = append(rules, lib) - if r := g.generateAlias(pkg, libName); r != nil { + if r := g.maybeGenerateAlias(pkg, libName); r != nil { rules = append(rules, r) } rules = append(rules, @@ -451,7 +451,7 @@ func (g *generator) generateLib(pkg *goPackage, binName, embed string) *rule.Rul if pkg.isCommand() { bn = binName } - name := libNameByConvention(getGoConfig(g.c).goNamingConvention, bn, pkg.name) + name := libNameByConvention(getGoConfig(g.c).goNamingConvention, bn, pkg.importPath) goLibrary := rule.NewRule("go_library", name) if !pkg.library.sources.hasGo() && embed == "" { return goLibrary // empty @@ -468,7 +468,7 @@ func (g *generator) generateLib(pkg *goPackage, binName, embed string) *rule.Rul return goLibrary } -func (g *generator) generateAlias(pkg *goPackage, libName string) *rule.Rule { +func (g *generator) maybeGenerateAlias(pkg *goPackage, libName string) *rule.Rule { if pkg.isCommand() || libName == "" { return nil } @@ -479,7 +479,7 @@ func (g *generator) generateAlias(pkg *goPackage, libName string) *rule.Rule { alias := rule.NewRule("alias", defaultLibName) alias.SetAttr("visibility", g.commonVisibility(pkg.importPath)) if gc.goNamingConvention == importAliasNamingConvention { - alias.SetAttr("actual", ":" + libName) + alias.SetAttr("actual", ":"+libName) } return alias } @@ -499,7 +499,7 @@ func (g *generator) generateTest(pkg *goPackage, binName, library string) *rule. if pkg.isCommand() { bn = binName } - name := testNameByConvention(getGoConfig(g.c).goNamingConvention, bn, pkg.name) + name := testNameByConvention(getGoConfig(g.c).goNamingConvention, bn, pkg.importPath) goTest := rule.NewRule("go_test", name) if !pkg.test.sources.hasGo() { return goTest // empty diff --git a/language/go/generate_test.go b/language/go/generate_test.go index a65b9e173..ab9d10ad8 100644 --- a/language/go/generate_test.go +++ b/language/go/generate_test.go @@ -105,7 +105,7 @@ func TestGenerateRules(t *testing.T) { } func TestGenerateRulesEmpty(t *testing.T) { - c, langs, _ := testConfig(t) + c, langs, _ := testConfig(t, "-go_prefix=example.com/repo") goLang := langs[1].(*goLang) res := goLang.GenerateRules(language.GenerateArgs{ Config: c, @@ -125,11 +125,11 @@ filegroup(name = "go_default_library_protos") go_proto_library(name = "foo_go_proto") -go_library(name = "go_default_library") +go_library(name = "foo") go_binary(name = "foo") -go_test(name = "go_default_test") +go_test(name = "foo_test") `) if got != want { t.Errorf("got:\n%s\nwant:\n%s", got, want) @@ -151,7 +151,7 @@ func TestGenerateRulesEmptyLegacyProto(t *testing.T) { } func TestGenerateRulesEmptyPackageProto(t *testing.T) { - c, langs, _ := testConfig(t, "-proto=package") + c, langs, _ := testConfig(t, "-proto=package", "-go_prefix=example.com/repo") oldContent := []byte(` proto_library( name = "dead_proto", @@ -187,11 +187,11 @@ filegroup(name = "go_default_library_protos") go_proto_library(name = "foo_go_proto") -go_library(name = "go_default_library") +go_library(name = "foo") go_binary(name = "foo") -go_test(name = "go_default_test") +go_test(name = "foo_test") `) if got != want { t.Errorf("got:\n%s\nwant:\n%s", got, want) diff --git a/language/go/package.go b/language/go/package.go index 2b4575ff2..356d973d5 100644 --- a/language/go/package.go +++ b/language/go/package.go @@ -159,13 +159,6 @@ func (pkg *goPackage) inferImportPath(c *config.Config) error { return fmt.Errorf("%s: go prefix is not set, so importpath can't be determined for rules. Set a prefix with a '# gazelle:prefix' comment or with -go_prefix on the command line", pkg.dir) } pkg.importPath = InferImportPath(c, pkg.rel) - - if pkg.rel == gc.prefixRel { - pkg.importPath = gc.prefix - } else { - fromPrefixRel := strings.TrimPrefix(pkg.rel, gc.prefixRel+"/") - pkg.importPath = path.Join(gc.prefix, fromPrefixRel) - } return nil } diff --git a/language/go/resolve.go b/language/go/resolve.go index 1d74c7268..23b5c1b9c 100644 --- a/language/go/resolve.go +++ b/language/go/resolve.go @@ -264,12 +264,17 @@ func resolveExternal(c *config.Config, rc *repo.RemoteCache, imp string) (label. pkg = pathtools.TrimPrefix(impWithoutSemver, prefix) } - // Determine what naming convention is used by the go_repository rule. - // If none is specified with "build_naming_convention", the either naming convention - // can be used, so we'll default to whatever's used in the current workspace to avoid churn. - nc := gc.goNamingConvention - if rnc, ok := gc.repoNamingConvention[repo]; ok { - nc = rnc + // Determine what naming convention is used by the repository. + // If there is no known repository, it's probably declared in an http_archive + // somewhere like go_rules_dependencies. Use the old naming convention. + nc, ok := gc.repoNamingConvention[repo] + if !ok { + nc = goDefaultLibraryNamingConvention + } + if nc == importAliasNamingConvention { + // If the repository is compatible with either naming convention, use + // whichever is preferred in this directory to avoid churn. + nc = gc.goNamingConvention } name := libNameByConvention(nc, "", imp) diff --git a/language/go/resolve_test.go b/language/go/resolve_test.go index a72e9388f..b6b5e27d2 100644 --- a/language/go/resolve_test.go +++ b/language/go/resolve_test.go @@ -462,8 +462,8 @@ go_binary( go_binary( name = "bin", deps = [ - ":go_default_library", - "//sub:go_default_library", + ":resolve", + "//sub", ], ) `, @@ -532,7 +532,7 @@ go_binary( want: ` go_binary( name = "bin", - deps = ["//vendor/example.com/outside/prefix:go_default_library"], + deps = ["//vendor/example.com/outside/prefix"], ) `, }, { @@ -575,7 +575,7 @@ go_binary( want: ` go_binary( name = "bin", - deps = ["//vendor/example.com/foo:go_default_library"], + deps = ["//vendor/example.com/foo"], )`, }, { desc: "proto_override", @@ -1084,31 +1084,45 @@ func TestResolveExternal(t *testing.T) { ix.Finish() gl := langs[1].(*goLang) for _, tc := range []struct { - desc, importpath string - repos []repo.Repo - moduleMode bool - namingConvention namingConvention + desc, importpath string + repos []repo.Repo + moduleMode bool + namingConvention namingConvention repoNamingConvention map[string]namingConvention - want string + want string }{ { desc: "top", importpath: "example.com/repo", want: "@com_example_repo//:go_default_library", + }, { + desc: "top_import_naming_convention", + namingConvention: goDefaultLibraryNamingConvention, + repoNamingConvention: map[string]namingConvention{ + "com_example_repo": importNamingConvention, + }, + importpath: "example.com/repo", + want: "@com_example_repo//:repo", + }, { + desc: "top_import_alias_naming_convention", + namingConvention: goDefaultLibraryNamingConvention, + repoNamingConvention: map[string]namingConvention{ + "com_example_repo": importAliasNamingConvention, + }, + importpath: "example.com/repo", + want: "@com_example_repo//:go_default_library", }, { desc: "sub", importpath: "example.com/repo/lib", want: "@com_example_repo//lib:go_default_library", }, { - desc: "top go_naming_convention=import", - namingConvention: importNamingConvention, - importpath: "example.com/repo", - want: "@com_example_repo//:repo", - }, { - desc: "sub go_naming_convention=import", + desc: "sub_import_alias_naming_convention", namingConvention: importNamingConvention, - importpath: "example.com/repo/lib", - want: "@com_example_repo//lib", + repoNamingConvention: map[string]namingConvention{ + "com_example_repo": importAliasNamingConvention, + }, + importpath: "example.com/repo/lib", + want: "@com_example_repo//lib", }, { desc: "custom_repo", repos: []repo.Repo{{ @@ -1118,10 +1132,10 @@ func TestResolveExternal(t *testing.T) { importpath: "example.com/repo/lib", want: "@custom_repo_name//lib:go_default_library", }, { - desc: "custom_repo go_naming_convention=import", + desc: "custom_repo_import_naming_convention", repos: []repo.Repo{{ - Name: "custom_repo_name", - GoPrefix: "example.com/repo", + Name: "custom_repo_name", + GoPrefix: "example.com/repo", }}, repoNamingConvention: map[string]namingConvention{ "custom_repo_name": importNamingConvention, diff --git a/language/go/testdata/allcgolib/BUILD.want b/language/go/testdata/allcgolib/BUILD.want index 29f9dd58a..730e3789d 100644 --- a/language/go/testdata/allcgolib/BUILD.want +++ b/language/go/testdata/allcgolib/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( - name = "go_default_library", + name = "allcgolib", srcs = [ "foo.c", "foo.go", @@ -16,8 +16,8 @@ go_library( ) go_test( - name = "go_default_test", + name = "allcgolib_test", srcs = ["foo_test.go"], _gazelle_imports = ["testing"], - embed = [":go_default_library"], + embed = [":allcgolib"], ) diff --git a/language/go/testdata/bin/BUILD.want b/language/go/testdata/bin/BUILD.want index 6f03dfca5..51ae87700 100644 --- a/language/go/testdata/bin/BUILD.want +++ b/language/go/testdata/bin/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") go_library( - name = "go_default_library", + name = "bin_lib", srcs = ["main.go"], _gazelle_imports = [ "example.com/repo/lib", @@ -14,6 +14,6 @@ go_library( go_binary( name = "bin", _gazelle_imports = [], - embed = [":go_default_library"], + embed = [":bin_lib"], visibility = ["//visibility:public"], ) diff --git a/language/go/testdata/bin_with_tests/BUILD.want b/language/go/testdata/bin_with_tests/BUILD.want index decc3eeeb..2dd826d1d 100644 --- a/language/go/testdata/bin_with_tests/BUILD.want +++ b/language/go/testdata/bin_with_tests/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test") go_library( - name = "go_default_library", + name = "bin_with_tests_lib", srcs = ["main.go"], _gazelle_imports = [ "example.com/repo/lib", @@ -14,13 +14,13 @@ go_library( go_binary( name = "bin_with_tests", _gazelle_imports = [], - embed = [":go_default_library"], + embed = [":bin_with_tests_lib"], visibility = ["//visibility:public"], ) go_test( - name = "go_default_test", + name = "bin_with_tests_test", srcs = ["bin_test.go"], _gazelle_imports = ["testing"], - embed = [":go_default_library"], + embed = [":bin_with_tests_lib"], ) diff --git a/language/go/testdata/cgolib/BUILD.want b/language/go/testdata/cgolib/BUILD.want index fdfce5336..85f192c54 100644 --- a/language/go/testdata/cgolib/BUILD.want +++ b/language/go/testdata/cgolib/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( - name = "go_default_library", + name = "cgolib", srcs = [ "asm.S", "foo.c", @@ -25,8 +25,8 @@ go_library( ) go_test( - name = "go_default_test", + name = "cgolib_test", srcs = ["foo_test.go"], _gazelle_imports = ["testing"], - embed = [":go_default_library"], + embed = [":cgolib"], ) diff --git a/language/go/testdata/cgolib_with_build_tags/BUILD.want b/language/go/testdata/cgolib_with_build_tags/BUILD.want index ea09c8e13..ca21cdf92 100644 --- a/language/go/testdata/cgolib_with_build_tags/BUILD.want +++ b/language/go/testdata/cgolib_with_build_tags/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( - name = "go_default_library", + name = "cgolib_with_build_tags", srcs = [ "asm_linux.S", "asm_other.S", @@ -90,8 +90,8 @@ go_library( ) go_test( - name = "go_default_test", + name = "cgolib_with_build_tags_test", srcs = ["foo_test.go"], _gazelle_imports = ["testing"], - embed = [":go_default_library"], + embed = [":cgolib_with_build_tags"], ) diff --git a/language/go/testdata/default_visibility/BUILD.want b/language/go/testdata/default_visibility/BUILD.want index 324a907f9..199f1e6df 100644 --- a/language/go/testdata/default_visibility/BUILD.want +++ b/language/go/testdata/default_visibility/BUILD.want @@ -16,7 +16,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "default_visibility", srcs = ["lib.go"], _gazelle_imports = [], embed = [":default_visibility_go_proto"], @@ -24,8 +24,8 @@ go_library( ) go_test( - name = "go_default_test", + name = "default_visibility_test", srcs = ["a_test.go"], _gazelle_imports = [], - embed = [":go_default_library"], + embed = [":default_visibility"], ) diff --git a/language/go/testdata/default_visibility/cmd/BUILD.want b/language/go/testdata/default_visibility/cmd/BUILD.want index 0844ac38d..cd846f144 100644 --- a/language/go/testdata/default_visibility/cmd/BUILD.want +++ b/language/go/testdata/default_visibility/cmd/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") go_library( - name = "go_default_library", + name = "cmd_lib", srcs = ["main.go"], _gazelle_imports = [], importpath = "example.com/repo/default_visibility/cmd", @@ -10,5 +10,5 @@ go_library( go_binary( name = "cmd", _gazelle_imports = [], - embed = [":go_default_library"], + embed = [":cmd_lib"], ) diff --git a/language/go/testdata/gen_and_exclude/BUILD.want b/language/go/testdata/gen_and_exclude/BUILD.want index 0cfb1a0ae..490a47ed6 100644 --- a/language/go/testdata/gen_and_exclude/BUILD.want +++ b/language/go/testdata/gen_and_exclude/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "gen_and_exclude", srcs = [ "gen.go", "gen_linux.go", diff --git a/language/go/testdata/importmap/BUILD.want b/language/go/testdata/importmap/BUILD.want index 32c02e68e..356e7e5fa 100644 --- a/language/go/testdata/importmap/BUILD.want +++ b/language/go/testdata/importmap/BUILD.want @@ -19,7 +19,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "importmap", srcs = ["extra.go"], _gazelle_imports = [], embed = [":hello_go_proto"], @@ -29,8 +29,8 @@ go_library( ) go_test( - name = "go_default_test", + name = "importmap_test", srcs = ["extra_test.go"], _gazelle_imports = [], - embed = [":go_default_library"], + embed = [":importmap"], ) diff --git a/language/go/testdata/lib/BUILD.want b/language/go/testdata/lib/BUILD.want index bb448d306..ad456bab9 100644 --- a/language/go/testdata/lib/BUILD.want +++ b/language/go/testdata/lib/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( - name = "go_default_library", + name = "lib", srcs = [ "asm.h", "asm.s", @@ -19,7 +19,7 @@ go_library( ) go_test( - name = "go_default_test", + name = "lib_test", srcs = [ "lib_external_test.go", "lib_test.go", @@ -28,5 +28,5 @@ go_test( "example.com/repo/lib", "testing", ], - embed = [":go_default_library"], + embed = [":lib"], ) diff --git a/language/go/testdata/lib/internal/deep/BUILD.want b/language/go/testdata/lib/internal/deep/BUILD.want index 604c0a47c..3e4c80217 100644 --- a/language/go/testdata/lib/internal/deep/BUILD.want +++ b/language/go/testdata/lib/internal/deep/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "deep", srcs = ["thought.go"], _gazelle_imports = [], importpath = "example.com/repo/lib/internal/deep", diff --git a/language/go/testdata/lib/internal/go_visibility/BUILD.want b/language/go/testdata/lib/internal/go_visibility/BUILD.want index b3b3242ca..8d6fc1776 100644 --- a/language/go/testdata/lib/internal/go_visibility/BUILD.want +++ b/language/go/testdata/lib/internal/go_visibility/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "go_visibility", srcs = ["thought.go"], _gazelle_imports = [], importpath = "example.com/repo/lib/internal/go_visibility", diff --git a/language/go/testdata/lib/relativeimporter/BUILD.want b/language/go/testdata/lib/relativeimporter/BUILD.want index a0254cbdd..5a8e9df52 100644 --- a/language/go/testdata/lib/relativeimporter/BUILD.want +++ b/language/go/testdata/lib/relativeimporter/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "relativeimporter", srcs = ["importer.go"], _gazelle_imports = [ "../internal/deep", diff --git a/language/go/testdata/main_test_only/BUILD.want b/language/go/testdata/main_test_only/BUILD.want index a6d5d6f67..7dd520bd9 100644 --- a/language/go/testdata/main_test_only/BUILD.want +++ b/language/go/testdata/main_test_only/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test") go_test( - name = "go_default_test", + name = "main_test_only_test", srcs = ["foo_test.go"], _gazelle_imports = [], ) diff --git a/language/go/testdata/naming_convention/import_alias/lib/lib.go b/language/go/testdata/naming_convention/import_alias/lib/lib.go index 9c41e7826..f4a3c179d 100644 --- a/language/go/testdata/naming_convention/import_alias/lib/lib.go +++ b/language/go/testdata/naming_convention/import_alias/lib/lib.go @@ -13,7 +13,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -package lib +// Package libbbbb has a name that doens't match its import path. It shouldn't +// appear in the build file. +package libbbbb // Answer returns the ultimate answer to life, the universe and everything. func Answer() int { diff --git a/language/go/testdata/naming_convention/import_alias/lib/lib_test.go b/language/go/testdata/naming_convention/import_alias/lib/lib_test.go index d88361f18..d0e0f5556 100644 --- a/language/go/testdata/naming_convention/import_alias/lib/lib_test.go +++ b/language/go/testdata/naming_convention/import_alias/lib/lib_test.go @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package lib +package libbbbb import ( "testing" diff --git a/language/go/testdata/platforms/BUILD.want b/language/go/testdata/platforms/BUILD.want index 10265e914..5198b5583 100644 --- a/language/go/testdata/platforms/BUILD.want +++ b/language/go/testdata/platforms/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( - name = "go_default_library", + name = "platforms", srcs = [ "cgo_generic.c", "cgo_generic.go", @@ -52,11 +52,11 @@ go_library( ) go_test( - name = "go_default_test", + name = "platforms_test", srcs = [ "generic_test.go", "suffix_linux_test.go", ], _gazelle_imports = [], - embed = [":go_default_library"], + embed = [":platforms"], ) diff --git a/language/go/testdata/proto_package_mode_extras/BUILD.want b/language/go/testdata/proto_package_mode_extras/BUILD.want index d23b2eaac..b4c2ae1b2 100644 --- a/language/go/testdata/proto_package_mode_extras/BUILD.want +++ b/language/go/testdata/proto_package_mode_extras/BUILD.want @@ -41,15 +41,15 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "proto_package_mode_extras", _gazelle_imports = [], embed = [":foo_go_proto"], importpath = "example.com/repo/proto_package_mode_extras", ) go_test( - name = "go_default_test", + name = "proto_package_mode_extras_test", srcs = ["foo_test.go"], _gazelle_imports = [], - embed = [":go_default_library"], + embed = [":proto_package_mode_extras"], ) diff --git a/language/go/testdata/protos/BUILD.want b/language/go/testdata/protos/BUILD.want index fb798e856..8251f96c7 100644 --- a/language/go/testdata/protos/BUILD.want +++ b/language/go/testdata/protos/BUILD.want @@ -24,7 +24,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "protos", srcs = ["extra.go"], _gazelle_imports = [], embed = [":protos_go_proto"], diff --git a/language/go/testdata/protos_explicit_default/BUILD.want b/language/go/testdata/protos_explicit_default/BUILD.want index bf3bb89c8..fa6560c51 100644 --- a/language/go/testdata/protos_explicit_default/BUILD.want +++ b/language/go/testdata/protos_explicit_default/BUILD.want @@ -25,7 +25,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "protos_explicit_default", srcs = ["extra.go"], _gazelle_imports = [], embed = [":protos_explicit_default_go_proto"], diff --git a/language/go/testdata/protos_gogo/BUILD.want b/language/go/testdata/protos_gogo/BUILD.want index 94ed66b29..f99c368ca 100644 --- a/language/go/testdata/protos_gogo/BUILD.want +++ b/language/go/testdata/protos_gogo/BUILD.want @@ -25,7 +25,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "protos_gogo", srcs = ["extra.go"], _gazelle_imports = [], embed = [":protos_gogo_go_proto"], diff --git a/language/go/testdata/protos_gogo_subdir_reset/BUILD.want b/language/go/testdata/protos_gogo_subdir_reset/BUILD.want index 110b916cd..b3528e5d5 100644 --- a/language/go/testdata/protos_gogo_subdir_reset/BUILD.want +++ b/language/go/testdata/protos_gogo_subdir_reset/BUILD.want @@ -22,7 +22,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "protos_gogo", _gazelle_imports = [], embed = [":protos_gogo_go_proto"], importpath = "example.com/repo/protos_gogo", diff --git a/language/go/testdata/protos_gogo_subdir_reset/sub/BUILD.want b/language/go/testdata/protos_gogo_subdir_reset/sub/BUILD.want index 12c9ff22d..6977c52f2 100644 --- a/language/go/testdata/protos_gogo_subdir_reset/sub/BUILD.want +++ b/language/go/testdata/protos_gogo_subdir_reset/sub/BUILD.want @@ -18,7 +18,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "protos_gogo", _gazelle_imports = [], embed = [":protos_gogo_go_proto"], importpath = "example.com/repo/protos_gogo", diff --git a/language/go/testdata/service/BUILD.want b/language/go/testdata/service/BUILD.want index 28afb1d9b..9c24d4cda 100644 --- a/language/go/testdata/service/BUILD.want +++ b/language/go/testdata/service/BUILD.want @@ -25,7 +25,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "service", srcs = ["extra.go"], _gazelle_imports = [], embed = [":service_go_proto"], diff --git a/language/go/testdata/service_gogo/BUILD.want b/language/go/testdata/service_gogo/BUILD.want index e547b2af5..c77511078 100644 --- a/language/go/testdata/service_gogo/BUILD.want +++ b/language/go/testdata/service_gogo/BUILD.want @@ -25,7 +25,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "service_gogo", srcs = ["extra.go"], _gazelle_imports = [], embed = [":service_gogo_go_proto"], diff --git a/language/go/testdata/service_gogo_subdir_reset/BUILD.want b/language/go/testdata/service_gogo_subdir_reset/BUILD.want index b8db28fdf..16c3a75df 100644 --- a/language/go/testdata/service_gogo_subdir_reset/BUILD.want +++ b/language/go/testdata/service_gogo_subdir_reset/BUILD.want @@ -22,7 +22,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "protos_gogo", _gazelle_imports = [], embed = [":protos_gogo_go_proto"], importpath = "example.com/repo/protos_gogo", diff --git a/language/go/testdata/service_gogo_subdir_reset/sub/BUILD.want b/language/go/testdata/service_gogo_subdir_reset/sub/BUILD.want index 9ee49e09b..1845fec8b 100644 --- a/language/go/testdata/service_gogo_subdir_reset/sub/BUILD.want +++ b/language/go/testdata/service_gogo_subdir_reset/sub/BUILD.want @@ -19,7 +19,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "protos_gogo", _gazelle_imports = [], embed = [":protos_gogo_go_proto"], importpath = "example.com/repo/protos_gogo", diff --git a/language/go/testdata/tests_import_testdata/BUILD.want b/language/go/testdata/tests_import_testdata/BUILD.want index aa6feff65..bf32ce8cc 100644 --- a/language/go/testdata/tests_import_testdata/BUILD.want +++ b/language/go/testdata/tests_import_testdata/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test") go_test( - name = "go_default_test", + name = "tests_import_testdata_test", srcs = [ "external_test.go", "internal_test.go", diff --git a/language/go/testdata/tests_with_testdata/BUILD.want b/language/go/testdata/tests_with_testdata/BUILD.want index 3a0dcd7e3..32b52f589 100644 --- a/language/go/testdata/tests_with_testdata/BUILD.want +++ b/language/go/testdata/tests_with_testdata/BUILD.want @@ -1,7 +1,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test") go_test( - name = "go_default_test", + name = "tests_with_testdata_test", srcs = [ "external_test.go", "internal_test.go", From f590195d66944e36a9fdd0869d3585a357db66a7 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Thu, 6 Aug 2020 18:00:59 -0400 Subject: [PATCH 2/3] fix all tests --- cmd/gazelle/integration_test.go | 69 +++++++++++++++++++-------------- internal/runner_test.go | 2 +- language/go/config.go | 26 +++++++++---- language/go/fix.go | 21 +++++----- language/go/generate.go | 31 +++++++-------- language/go/package.go | 40 ++++++++++++------- language/go/resolve.go | 8 ++-- 7 files changed, 116 insertions(+), 81 deletions(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index 3aa47632d..cee206c2c 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -898,11 +898,11 @@ import ( load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "bar", srcs = ["bar.go"], importpath = "bar", visibility = ["//visibility:public"], - deps = ["//foo:go_default_library"], + deps = ["//foo"], ) `, }}) @@ -1007,7 +1007,7 @@ import _ "example.com/foo" load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "foo", srcs = ["foo.go"], importmap = "example.com/repo/sub/vendor/example.com/foo", importpath = "example.com/foo", @@ -1020,11 +1020,11 @@ go_library( load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "bar", srcs = ["bar.go"], importpath = "example.com/repo/sub/bar", visibility = ["//visibility:public"], - deps = ["//sub/vendor/example.com/foo:go_default_library"], + deps = ["//sub/vendor/example.com/foo"], ) `, }, @@ -1140,11 +1140,11 @@ import _ "example.com/bar" load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "foo", srcs = ["foo.go"], importpath = "example.com/foo", visibility = ["//visibility:public"], - deps = ["@custom_repo//:go_default_library"], + deps = ["@custom_repo//:bar"], ) `, }, @@ -1179,6 +1179,7 @@ import _ "example.com/bar" extDir := filepath.Join(dir, "ext") args := []string{ "-go_prefix=example.com/foo", + "-go_naming_convention=import_alias", "-mode=fix", "-repo_root=" + extDir, "-repo_config=" + filepath.Join(dir, "main", "WORKSPACE"), @@ -1194,11 +1195,17 @@ import _ "example.com/bar" load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "foo", srcs = ["foo.go"], importpath = "example.com/foo", visibility = ["//visibility:public"], - deps = ["@custom_repo//:go_default_library"], + deps = ["@custom_repo//:bar"], +) + +alias( + name = "go_default_library", + actual = ":foo", + visibility = ["//visibility:public"], ) `, }, @@ -2246,7 +2253,7 @@ import _ "example.com/bar" load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "bar", srcs = ["bar.go"], importpath = "example.com/bar", visibility = ["//visibility:public"], @@ -2272,11 +2279,11 @@ go_library( load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "foo", srcs = ["foo.go"], importpath = "example.com/repo/foo", visibility = ["//visibility:public"], - deps = ["//vendor/example.com/bar:go_default_library"], + deps = ["//vendor/example.com/bar"], ) `, }}) @@ -2311,7 +2318,7 @@ import ( # this should be ignored because -index=false go_library( - name = "go_default_library", + name = "baz", srcs = ["baz.go"], importpath = "example.com/dep/baz", visibility = ["//visibility:public"], @@ -2342,11 +2349,11 @@ go_library( Content: `load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "foo", srcs = ["foo.go"], importpath = "example.com/repo/foo", visibility = ["//visibility:public"], - deps = ["//vendor/example.com/dep/baz:go_default_library"], + deps = ["//vendor/example.com/dep/baz"], ) `, }, @@ -2394,13 +2401,13 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") # gazelle:prefix example.com/sub go_library( - name = "go_default_library", + name = "sub", srcs = ["sub.go"], importpath = "example.com/sub", visibility = ["//visibility:public"], deps = [ - "//sub/missing:go_default_library", - "//vendor/example.com/external:go_default_library", + "//sub/missing", + "//vendor/example.com/external", ], ) `, @@ -2564,8 +2571,11 @@ func TestMapKind(t *testing.T) { { Path: "WORKSPACE", }, { - Path: "BUILD.bazel", - Content: "# gazelle:prefix example.com/mapkind", + Path: "BUILD.bazel", + Content: ` +# gazelle:prefix example.com/mapkind +# gazelle:go_naming_convention go_default_library +`, }, { Path: "root_lib.go", Content: `package mapkind`, @@ -2654,6 +2664,7 @@ go_library( load("@io_bazel_rules_go//go:def.bzl", "go_library") # gazelle:prefix example.com/mapkind +# gazelle:go_naming_convention go_default_library go_library( name = "go_default_library", @@ -2820,7 +2831,7 @@ func TestMinimalModuleCompatibilityAliases(t *testing.T) { load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "foo", srcs = ["foo.go"], importpath = "example.com/foo/v2", importpath_aliases = ["example.com/foo"], @@ -2833,7 +2844,7 @@ go_library( load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( - name = "go_default_library", + name = "bar", srcs = ["bar.go"], importpath = "example.com/foo/v2/bar", importpath_aliases = ["example.com/foo/bar"], @@ -2880,7 +2891,7 @@ go_repository( load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( - name = "go_default_library", + name = "version", srcs = ["version.go"], importpath = "example.com/m/internal/version", visibility = [ @@ -2890,9 +2901,9 @@ go_library( ) go_test( - name = "go_default_test", + name = "version_test", srcs = ["version_test.go"], - embed = [":go_default_library"], + embed = [":version"], ) `, }}) @@ -3101,7 +3112,7 @@ go_proto_library( ) go_library( - name = "go_default_library", + name = "foo", embed = [":foo_go_proto"], importpath = "example.com/foo", visibility = ["//visibility:public"], @@ -3159,7 +3170,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") # gazelle:lang go_library( - name = "go_default_library", + name = "bar", srcs = ["bar.go"], importpath = "", visibility = ["//visibility:public"], @@ -3173,7 +3184,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") # gazelle:lang go,proto go_library( - name = "go_default_library", + name = "baz", srcs = ["baz.go"], importpath = "", visibility = ["//visibility:public"], @@ -3241,7 +3252,7 @@ proto_library( ) go_library( - name = "go_default_library", + name = "proto", srcs = ["foo.pb.go"], importpath = "example.com/proto", visibility = ["//visibility:public"], diff --git a/internal/runner_test.go b/internal/runner_test.go index 4daf5e91d..77a7f1ceb 100644 --- a/internal/runner_test.go +++ b/internal/runner_test.go @@ -34,7 +34,7 @@ func TestRunner(t *testing.T) { for _, target := range strings.Split(strings.TrimSpace(string(out)), "\n") { got[target] = true } - want := []string{"//:m", "//:go_default_library"} + want := []string{"//:m", "//:m_lib"} for _, target := range want { if !got[target] { t.Errorf("target missing from query output: %s", target) diff --git a/language/go/config.go b/language/go/config.go index 248146f3f..251fa71e9 100644 --- a/language/go/config.go +++ b/language/go/config.go @@ -633,17 +633,29 @@ var errRulesGoRepoNotFound = errors.New(config.RulesGoRepoName + " external repo // detectNamingConvention attempts to detect the naming convention in use by // reading build files in subdirectories of the repository root directory. -// If no build files are found (for example, in a new project), or if multiple -// naming conventions are found, importNamingConvention is returned. +// +// If detectNamingConvention can't detect the naming convention (for example, +// because no build files are found or multiple naming conventions are found), +// importNamingConvention is returned. func detectNamingConvention(c *config.Config, rootFile *rule.File) namingConvention { + if !c.IndexLibraries { + // Indexing is disabled, which usually means speed is important and I/O + // should be minimized. Let's not open extra files or directories. + return importNamingConvention + } + detectInFile := func(f *rule.File) namingConvention { for _, r := range f.Rules { - if r.Kind() != "go_library" { - continue - } - if name := r.Name(); name == defaultLibName { + // NOTE: map_kind is not supported. c.KindMap will not be accurate in + // subdirectories. + kind := r.Kind() + name := r.Name() + if kind != "alias" && name == defaultLibName { + // Assume any kind of rule with the name "go_default_library" is some + // 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 name == path.Base(r.AttrString("importpath")) { + } else if isGoLibrary(kind) && name == path.Base(r.AttrString("importpath")) { return importNamingConvention } } diff --git a/language/go/fix.go b/language/go/fix.go index fbb1bdc46..c2d9b2391 100644 --- a/language/go/fix.go +++ b/language/go/fix.go @@ -40,18 +40,21 @@ func (_ *goLang) Fix(c *config.Config, f *rule.File) { func migrateNamingConvention(c *config.Config, f *rule.File) { // Determine old and new names for go_library and go_test. nc := getGoConfig(c).goNamingConvention - binName := findBinName(f) importPath := findImportPath(f) if importPath == "" { return } - libName := libNameByConvention(nc, binName, importPath) - testName := testNameByConvention(nc, binName, importPath) + var pkgName string // unknown unless there's a binary + if findBinary(f) { + pkgName = "main" + } + libName := libNameByConvention(nc, importPath, pkgName) + testName := testNameByConvention(nc, importPath) var migrateLibName, migrateTestName string switch nc { case goDefaultLibraryNamingConvention: - migrateLibName = libNameByConvention(importNamingConvention, binName, importPath) - migrateTestName = testNameByConvention(importNamingConvention, binName, importPath) + migrateLibName = libNameByConvention(importNamingConvention, importPath, pkgName) + migrateTestName = testNameByConvention(importNamingConvention, importPath) case importNamingConvention, importAliasNamingConvention: migrateLibName = defaultLibName migrateTestName = defaultTestName @@ -107,14 +110,14 @@ func migrateNamingConvention(c *config.Config, f *rule.File) { } } -// findBinName returns the name of a go_binary rule if one can be found. -func findBinName(f *rule.File) string { +// findBinary returns whether the file has a go_binary rule. +func findBinary(f *rule.File) bool { for _, r := range f.Rules { if r.Kind() == "go_binary" { - return r.Name() + return true } } - return "" + return false } // findImportPath returns the existing import path from the first encountered Go diff --git a/language/go/generate.go b/language/go/generate.go index 34f70d3f3..8c91d19a6 100644 --- a/language/go/generate.go +++ b/language/go/generate.go @@ -255,8 +255,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes _, rs := g.generateProto(pcMode, pkg.proto, pkg.importPath) rules = append(rules, rs...) } - binName := pathtools.RelBaseName(pkg.rel, getGoConfig(g.c).prefix, g.c.RepoRoot) - lib := g.generateLib(pkg, binName, protoEmbed) + lib := g.generateLib(pkg, protoEmbed) var libName string if !lib.IsEmpty(goKinds[lib.Kind()]) { libName = lib.Name() @@ -266,8 +265,8 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes rules = append(rules, r) } rules = append(rules, - g.generateBin(pkg, binName, libName), - g.generateTest(pkg, binName, libName)) + g.generateBin(pkg, libName), + g.generateTest(pkg, libName)) } for _, r := range rules { @@ -446,12 +445,9 @@ func (g *generator) generateProto(mode proto.Mode, target protoTarget, importPat return goProtoName, []*rule.Rule{goProtoLibrary} } -func (g *generator) generateLib(pkg *goPackage, binName, embed string) *rule.Rule { - var bn string - if pkg.isCommand() { - bn = binName - } - name := libNameByConvention(getGoConfig(g.c).goNamingConvention, bn, pkg.importPath) +func (g *generator) generateLib(pkg *goPackage, embed string) *rule.Rule { + gc := getGoConfig(g.c) + name := libNameByConvention(gc.goNamingConvention, pkg.importPath, pkg.name) goLibrary := rule.NewRule("go_library", name) if !pkg.library.sources.hasGo() && embed == "" { return goLibrary // empty @@ -484,8 +480,10 @@ func (g *generator) maybeGenerateAlias(pkg *goPackage, libName string) *rule.Rul return alias } -func (g *generator) generateBin(pkg *goPackage, binName, library string) *rule.Rule { - goBinary := rule.NewRule("go_binary", binName) +func (g *generator) generateBin(pkg *goPackage, library string) *rule.Rule { + gc := getGoConfig(g.c) + name := binName(pkg.rel, gc.prefix, g.c.RepoRoot) + goBinary := rule.NewRule("go_binary", name) if !pkg.isCommand() || pkg.binary.sources.isEmpty() && library == "" { return goBinary // empty } @@ -494,12 +492,9 @@ func (g *generator) generateBin(pkg *goPackage, binName, library string) *rule.R return goBinary } -func (g *generator) generateTest(pkg *goPackage, binName, library string) *rule.Rule { - var bn string - if pkg.isCommand() { - bn = binName - } - name := testNameByConvention(getGoConfig(g.c).goNamingConvention, bn, pkg.importPath) +func (g *generator) generateTest(pkg *goPackage, library string) *rule.Rule { + gc := getGoConfig(g.c) + name := testNameByConvention(gc.goNamingConvention, pkg.importPath) goTest := rule.NewRule("go_test", name) if !pkg.test.sources.hasGo() { return goTest // empty diff --git a/language/go/package.go b/language/go/package.go index 356d973d5..806365d84 100644 --- a/language/go/package.go +++ b/language/go/package.go @@ -25,6 +25,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/config" "github.com/bazelbuild/bazel-gazelle/language/proto" + "github.com/bazelbuild/bazel-gazelle/pathtools" "github.com/bazelbuild/bazel-gazelle/rule" ) @@ -180,29 +181,42 @@ func libNameFromImportPath(dir string) string { return name } -// libNameByConvention returns a suitable lib name based on go_naming_convention -// If go_default_library, "go_default_library" is returned. -// Else if this is a 'main' package, 'foo_lib' is returned, where 'foo' is the name of the go_binary rule. -// Else it is a regular package, the last segment of the importpath is returned. Major version suffixes (eg. "v1") are dropped. -func libNameByConvention(nc namingConvention, binName, imp string) string { +// libNameByConvention returns a suitable name for a go_library using the given +// naming convention, the import path, and the package name. +func libNameByConvention(nc namingConvention, imp string, pkgName string) string { if nc == goDefaultLibraryNamingConvention { return defaultLibName } - if binName != "" { - return binName + "_lib" + name := libNameFromImportPath(imp) + isCommand := pkgName == "main" + if name == "" { + if isCommand { + name = "lib" + } else { + name = pkgName + } + } else if isCommand { + name += "_lib" } - return libNameFromImportPath(imp) + return name } -// testNameByConvention works like libNameByConvention, but always appends '_test'. -func testNameByConvention(nc namingConvention, binName, imp string) string { +// testNameByConvention returns a suitable name for a go_test using the given +// naming convention and the import path. +func testNameByConvention(nc namingConvention, imp string) string { if nc == goDefaultLibraryNamingConvention { return defaultTestName } - if binName != "" { - return binName + "_test" + libName := libNameFromImportPath(imp) + if libName == "" { + libName = "lib" } - return libNameFromImportPath(imp) + "_test" + return libName + "_test" +} + +// binName returns a suitable name for a go_binary. +func binName(rel, prefix, repoRoot string) string { + return pathtools.RelBaseName(rel, prefix, repoRoot) } func InferImportPath(c *config.Config, rel string) string { diff --git a/language/go/resolve.go b/language/go/resolve.go index 23b5c1b9c..801c17f84 100644 --- a/language/go/resolve.go +++ b/language/go/resolve.go @@ -156,7 +156,7 @@ func ResolveGo(c *config.Config, ix *resolve.RuleIndex, rc *repo.RemoteCache, im // current repo if pathtools.HasPrefix(imp, gc.prefix) { pkg := path.Join(gc.prefixRel, pathtools.TrimPrefix(imp, gc.prefix)) - libName := libNameByConvention(gc.goNamingConvention, "", imp) + libName := libNameByConvention(gc.goNamingConvention, imp, "") return label.New("", pkg, libName), nil } } @@ -277,12 +277,12 @@ func resolveExternal(c *config.Config, rc *repo.RemoteCache, imp string) (label. nc = gc.goNamingConvention } - name := libNameByConvention(nc, "", imp) + name := libNameByConvention(nc, imp, "") return label.New(repo, pkg, name), nil } func resolveVendored(gc *goConfig, imp string) (label.Label, error) { - name := libNameByConvention(gc.goNamingConvention, "", imp) + name := libNameByConvention(gc.goNamingConvention, imp, "") return label.New("", path.Join("vendor", imp), name), nil } @@ -312,7 +312,7 @@ func resolveProto(c *config.Config, ix *resolve.RuleIndex, rc *repo.RemoteCache, if from.Pkg == "vendor" || strings.HasPrefix(from.Pkg, "vendor/") { rel = path.Join("vendor", rel) } - libName := libNameByConvention(getGoConfig(c).goNamingConvention, "", imp) + libName := libNameByConvention(getGoConfig(c).goNamingConvention, imp, "") return label.New("", rel, libName), nil } From 48cc13acebf722bde7c60da2554f0e6e71620c53 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Fri, 7 Aug 2020 11:44:47 -0400 Subject: [PATCH 3/3] delete empty go_library rule in directory with go_binary --- cmd/gazelle/integration_test.go | 47 +++++++++++++++++++++++++++++++++ language/go/fix.go | 15 ++++++++--- language/go/generate.go | 17 +++++++++--- language/go/generate_test.go | 2 +- 4 files changed, 72 insertions(+), 9 deletions(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index cee206c2c..13823e709 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -3260,3 +3260,50 @@ go_library( }, }) } + +func TestGoMainLibraryRemoved(t *testing.T) { + files := []testtools.FileSpec{ + { + Path: "WORKSPACE", + }, + { + Path: "BUILD.bazel", + Content: ` +# gazelle:prefix example.com +# gazelle:go_naming_convention import +`, + }, + { + Path: "cmd/foo/BUILD.bazel", + Content: `load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") + +go_library( + name = "foo_lib", + srcs = ["foo.go"], + importpath = "example.com/cmd/foo", + visibility = ["//visibility:private"], +) + +go_binary( + name = "foo", + embed = [":foo_lib"], + visibility = ["//visibility:public"], +) +`, + }, + } + dir, cleanup := testtools.CreateFiles(t, files) + defer cleanup() + + args := []string{"update"} + if err := runGazelle(dir, args); err != nil { + t.Fatal(err) + } + + testtools.CheckFiles(t, dir, []testtools.FileSpec{ + { + Path: "cmd/foo/BUILD.bazel", + Content: "", + }, + }) +} diff --git a/language/go/fix.go b/language/go/fix.go index c2d9b2391..760e80221 100644 --- a/language/go/fix.go +++ b/language/go/fix.go @@ -45,7 +45,7 @@ func migrateNamingConvention(c *config.Config, f *rule.File) { return } var pkgName string // unknown unless there's a binary - if findBinary(f) { + if fileContainsGoBinary(c, f) { pkgName = "main" } libName := libNameByConvention(nc, importPath, pkgName) @@ -110,10 +110,17 @@ func migrateNamingConvention(c *config.Config, f *rule.File) { } } -// findBinary returns whether the file has a go_binary rule. -func findBinary(f *rule.File) bool { +// fileContainsGoBinary returns whether the file has a go_binary rule. +func fileContainsGoBinary(c *config.Config, f *rule.File) bool { + if f == nil { + return false + } for _, r := range f.Rules { - if r.Kind() == "go_binary" { + kind := r.Kind() + if mappedKind, ok := c.KindMap[kind]; ok { + kind = mappedKind.KindName + } + if kind == "go_binary" { return true } } diff --git a/language/go/generate.go b/language/go/generate.go index 8c91d19a6..4de83d6e8 100644 --- a/language/go/generate.go +++ b/language/go/generate.go @@ -127,7 +127,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes // if a go_proto_library rule already exists for this // proto package, treat it as if the proto package // doesn't exist. - pkg = emptyPackage(c, args.Dir, args.Rel) + pkg = emptyPackage(c, args.Dir, args.Rel, args.File) break } pkg = &goPackage{ @@ -139,7 +139,7 @@ func (gl *goLang) GenerateRules(args language.GenerateArgs) language.GenerateRes break } } else { - pkg = emptyPackage(c, args.Dir, args.Rel) + pkg = emptyPackage(c, args.Dir, args.Rel, args.File) } } else { log.Print(err) @@ -373,9 +373,18 @@ func selectPackage(c *config.Config, dir string, packageMap map[string]*goPackag return nil, err } -func emptyPackage(c *config.Config, dir, rel string) *goPackage { +func emptyPackage(c *config.Config, dir, rel string, f *rule.File) *goPackage { + var pkgName string + if fileContainsGoBinary(c, f) { + // If the file contained a go_binary, its library may have a "_lib" suffix. + // Set the package name to "main" so that we generate an empty library rule + // with that name. + pkgName = "main" + } else { + pkgName = defaultPackageName(c, dir) + } pkg := &goPackage{ - name: defaultPackageName(c, dir), + name: pkgName, dir: dir, rel: rel, } diff --git a/language/go/generate_test.go b/language/go/generate_test.go index ab9d10ad8..0581ba318 100644 --- a/language/go/generate_test.go +++ b/language/go/generate_test.go @@ -266,7 +266,7 @@ func prebuiltProtoRules() []*rule.Rule { proto.Package{ Name: "foo", Files: map[string]proto.FileInfo{ - "foo.proto": proto.FileInfo{}, + "foo.proto": {}, }, Imports: map[string]bool{}, Options: map[string]string{},