Skip to content

Commit

Permalink
Go naming convention (#808)
Browse files Browse the repository at this point in the history
Add directive 'go_naming_convention' to control the name of
generated Go rules.

go_default_library: Legacy behaviour.
import: Name targets after their import path.
import_alias: Same as import, but generate alias targets to
  maintain backwards compatibility with go_default_library.

We also add a `build_naming_convention` attribute to
`go_repository` rules, allowing per-external control over
the naming convention.

gazelle fix is augmented to migrate between the different
styles, both forwards and backwards.

For #5
  • Loading branch information
tomlu authored Jul 15, 2020
1 parent 4d8b5cf commit 3dc0f77
Show file tree
Hide file tree
Showing 37 changed files with 1,199 additions and 28 deletions.
23 changes: 23 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,11 @@ The following flags are accepted:
| See `Predefined plugins`_ for available options; commonly used options include |
| ``@io_bazel_rules_go//proto:gofast_grpc`` and ``@io_bazel_rules_go//proto:gogofaster_grpc``. |
+--------------------------------------------------------------+----------------------------------------+
| :flag:`-go_naming_convention` | |
+--------------------------------------------------------------+----------------------------------------+
| Controls the names of generated Go targets. Equivalent to the |
| ``# gazelle:go_naming_convention`` directive. |
+--------------------------------------------------------------+----------------------------------------+
| :flag:`-go_prefix example.com/repo` | |
+--------------------------------------------------------------+----------------------------------------+
| A prefix of import paths for libraries in the repository that corresponds to |
Expand Down Expand Up @@ -575,6 +580,24 @@ The following directives are recognized:
| ``@io_bazel_rules_go//proto:gofast_grpc`` and |
| ``@io_bazel_rules_go//proto:gogofaster_grpc``. |
+---------------------------------------------------+----------------------------------------+
| :direc:`# gazelle:go_naming_convention` | n/a |
+---------------------------------------------------+----------------------------------------+
| Controls the names of generated Go targets. By default, library targets are named |
| ``go_default_library`` and test targets ``go_default_test``. |
| |
| Valid values are: |
| |
| * ``go_default_library``: Library targets are named ``go_default_library``, test targets |
| are named ``go_default_test``. |
| * ``import``: Library and test targets are named after the last segment of their import |
| path. |
| For example, ``example.repo/foo`` is named ``foo``, and the test target is ``foo_test``. |
| Major version suffixes like ``/v2`` are dropped. |
| For a main package with a binary ``foobin``, the names are instead ``foobin_lib`` and |
| ``foobin_test``. |
| * ``import_alias``: Same as ``import``, but an ``alias`` target is generated named |
| ``go_default_library`` to ensure backwards compatibility. |
+---------------------------------------------------+----------------------------------------+
| :direc:`# gazelle:go_proto_compilers` | ``@io_bazel_rules_go//proto:go_proto`` |
+---------------------------------------------------+----------------------------------------+
| The protocol buffers compiler(s) to use for building go bindings. |
Expand Down
4 changes: 3 additions & 1 deletion cmd/gazelle/fix-update.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,9 @@ func runFixUpdate(cmd command, args []string) (err error) {
for _, v := range visits {
for i, r := range v.rules {
from := label.New(c.RepoName, v.pkgRel, r.Name())
mrslv.Resolver(r, v.pkgRel).Resolve(v.c, ruleIndex, rc, r, v.imports[i], from)
if rslv := mrslv.Resolver(r, v.pkgRel); rslv != nil {
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))
Expand Down
10 changes: 10 additions & 0 deletions internal/go_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ def _go_repository_impl(ctx):
cmd.extend(["-external", ctx.attr.build_external])
if ctx.attr.build_file_proto_mode:
cmd.extend(["-proto", ctx.attr.build_file_proto_mode])
if ctx.attr.build_naming_convention:
cmd.extend(["-go_naming_convention", ctx.attr.build_naming_convention])
cmd.extend(ctx.attr.build_extra_args)
cmd.append(ctx.path(""))
result = env_execute(ctx, cmd, environment = env, timeout = _GO_REPOSITORY_TIMEOUT)
Expand Down Expand Up @@ -237,6 +239,14 @@ go_repository = repository_rule(
"off",
],
),
"build_naming_convention": attr.string(
values = [
"go_default_library",
"import",
"import_alias",
],
default = "import_alias",
),
"build_tags": attr.string_list(),
"build_file_proto_mode": attr.string(
values = [
Expand Down
1 change: 1 addition & 0 deletions internal/go_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ go_repository(
commit = "30136e27e2ac8d167177e8a583aa4c3fea5be833",
patches = ["@bazel_gazelle//internal:repository_rules_test_errors.patch"],
patch_args = ["-p1"],
build_naming_convention = "go_default_library",
)
go_repository(
Expand Down
95 changes: 94 additions & 1 deletion language/go/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ type goConfig struct {
// goGenerateProto indicates whether to generate go_proto_library
goGenerateProto bool

// goNamingConvention controls the name of generated targets
goNamingConvention namingConvention

// goProtoCompilers is the protocol buffers compiler(s) to use for go code.
goProtoCompilers []string

Expand Down Expand Up @@ -104,6 +107,10 @@ type goConfig struct {
// resolved differently (also depending on goRepositoryMode).
moduleMode bool

// map between external repo names and their `build_naming_convention`
// attribute.
repoNamingConvention map[string]namingConvention

// submodules is a list of modules which have the current module's path
// as a prefix of their own path. This affects visibility attributes
// in internal packages.
Expand Down Expand Up @@ -236,6 +243,69 @@ func (f tagsFlag) String() string {
return ""
}

type namingConventionFlag struct {
nc *namingConvention
}

func (f namingConventionFlag) Set(value string) error {
if nc, err := namingConventionFromString(value); err != nil {
return err
} else {
*f.nc = nc
return nil
}
}

func (f *namingConventionFlag) String() string {
if f == nil || f.nc == nil {
return "naming_convention"
}
return f.nc.String()
}

// namingConvention determines how go targets are named.
type namingConvention int

const (
// 'go_default_library' and 'go_default_test'
goDefaultLibraryNamingConvention = iota

// For an import path that ends with foo, the go_library rules target is
// named 'foo', the go_test is named 'foo_test'.
// For a main package, the go_binary takes the 'foo' name, the library
// is named 'foo_lib', and the go_test is named 'foo_test'.
importNamingConvention

// Same as importNamingConvention, but generate alias rules for libraries that have
// the legacy 'go_default_library' name.
importAliasNamingConvention
)

func (nc namingConvention) String() string {
switch nc {
case goDefaultLibraryNamingConvention:
return "go_default_library"
case importNamingConvention:
return "import"
case importAliasNamingConvention:
return "import_alias"
}
return ""
}

func namingConventionFromString(s string) (namingConvention, error) {
switch s {
case "go_default_library":
return goDefaultLibraryNamingConvention, nil
case "import":
return importNamingConvention, nil
case "import_alias":
return importAliasNamingConvention, nil
default:
return goDefaultLibraryNamingConvention, fmt.Errorf("unknown naming convention %q", s)
}
}

type moduleRepo struct {
repoName, modulePath string
}
Expand All @@ -249,6 +319,7 @@ func (*goLang) KnownDirectives() []string {
"build_tags",
"go_generate_proto",
"go_grpc_compilers",
"go_naming_convention",
"go_proto_compilers",
"go_visibility",
"importmap_prefix",
Expand Down Expand Up @@ -290,6 +361,10 @@ func (*goLang) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) {
"go_repository_module_mode",
false,
"set when gazelle is invoked by go_repository in module mode")
fs.Var(
&namingConventionFlag{&gc.goNamingConvention},
"go_naming_convention",
"controls generated library names. One of (go_default_library, import, import_alias)")

case "update-repos":
fs.Var(&gzflag.AllowedStringFlag{Value: &gc.buildExternalAttr, Allowed: validBuildExternalAttr},
Expand Down Expand Up @@ -372,6 +447,19 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.`
log.Printf("Found RULES_GO_VERSION %s. Minimum compatible version is %s.\n%s", gc.rulesGoVersion, minimumRulesGoVersion, message)
}
}
repoNamingConvention := map[string]namingConvention{}
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
} else if nc, err := namingConventionFromString(attr); err != nil {
log.Printf("in go_repository named %q: %v", repo.Name(), err)
} else {
repoNamingConvention[repo.Name()] = nc
}
}
}
gc.repoNamingConvention = repoNamingConvention
}

if !gc.moduleMode {
Expand Down Expand Up @@ -408,12 +496,17 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.`
gc.preprocessTags()
gc.setBuildTags(d.Value)
case "go_generate_proto":

if goGenerateProto, err := strconv.ParseBool(d.Value); err == nil {
gc.goGenerateProto = goGenerateProto
} else {
log.Printf("parsing go_generate_proto: %v", err)
}
case "go_naming_convention":
if nc, err := namingConventionFromString(d.Value); err == nil {
gc.goNamingConvention = nc
} else {
log.Print(err)
}
case "go_grpc_compilers":
// Special syntax (empty value) to reset directive.
if d.Value == "" {
Expand Down
4 changes: 4 additions & 0 deletions language/go/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func TestCommandLine(t *testing.T) {
t,
"-build_tags=foo,bar",
"-go_prefix=example.com/repo",
"-go_naming_convention=import_alias",
"-external=vendored",
"-repo_root=.")
gc := getGoConfig(c)
Expand All @@ -78,6 +79,9 @@ func TestCommandLine(t *testing.T) {
if gc.depMode != vendorMode {
t.Errorf("got dep mode %v; want %v", gc.depMode, vendorMode)
}
if gc.goNamingConvention != importAliasNamingConvention {
t.Errorf("got naming convention %v; want %v", gc.goNamingConvention, importAliasNamingConvention)
}
}

func TestDirectives(t *testing.T) {
Expand Down
82 changes: 82 additions & 0 deletions language/go/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,92 @@ 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 renames rules according to go_naming_convention directives.
func migrateNamingConvention(c *config.Config, f *rule.File) {
nc := getGoConfig(c).goNamingConvention

binName := binName(f)
importPath := importPath(f)
if importPath == "" {
return
}
libName := libNameByConvention(nc, binName, importPath)
testName := testNameByConvention(nc, binName, importPath)
var migrateLibName, migrateTestName string
switch nc {
case goDefaultLibraryNamingConvention:
migrateLibName = libNameByConvention(importNamingConvention, binName, importPath)
migrateTestName = testNameByConvention(importNamingConvention, binName, importPath)
case importNamingConvention, importAliasNamingConvention:
migrateLibName = defaultLibName
migrateTestName = defaultTestName
default:
return
}

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)
case "go_library":
if r.Name() == migrateLibName {
r.SetName(libName)
}
case "go_test":
if r.Name() == migrateTestName {
r.SetName(testName)
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 {
for _, r := range f.Rules {
if r.Kind() == "go_binary" {
return r.Name()
}
}
return ""
}

// import path returns the existing import path from the first encountered Go rule with the attribute set.
func importPath(f *rule.File) string {
for _, r := range f.Rules {
switch r.Kind() {
case "go_binary", "go_library", "go_test":
if ip, ok := r.Attr("importpath").(*bzl.StringExpr); ok && ip.Value != "" {
return ip.Value
}
}
}
return ""
}

func replaceInStrListAttr(r *rule.Rule, attr, old, new string) {
l := r.AttrStrings(attr)
var shouldAdd = true
var items []string
for _, v := range l {
if v != old {
items = append(items, v)
}
shouldAdd = shouldAdd && v != new
}
if shouldAdd {
items = append(items, new)
}
r.SetAttr(attr, items)
}

// migrateLibraryEmbed converts "library" attributes to "embed" attributes,
// preserving comments. This only applies to Go rules, and only if there is
// no keep comment on "library" and no existing "embed" attribute.
Expand Down
Loading

0 comments on commit 3dc0f77

Please sign in to comment.