From 4c930b44494ae34e8734241f5b7628673d5b351b Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Tue, 12 Sep 2017 16:07:00 -0400 Subject: [PATCH] Gazelle: support for proto rules Gazelle now generates proto_library, go_proto_library, and go_grpc_library rules. proto_library will contain .proto files in a directory. go_{proto,grpc}_library are buildable libraries, linked with the proto_library. A go_library rules embeds the go_{proto,grpc}_library and includes an extra .go sources (but .pb.go files are excluded). The new proto rule generation is turned on by default in packages that don't have existing proto rules. If there are existing rules, proto rule generation will either follow previous behavior (if go_proto_library.bzl is loaded) or will be disabled (if go_proto_library is loaded from some other repo). The proto mode can be selected explicitly with the -proto command line flag or with the '# gazelle:proto' directive. Legacy rules can be migrated with 'gazelle fix'. Limitations: * Gazelle still only allows one package per directory. Gazelle infers package name and import path from .proto files. These must match other files in the directory. * Import resolution is fairly crude and is just a guess, based on the import string. * There's no way to import protos from other repositories, except for Well Known Types. Fixes #808 --- go/tools/gazelle/config/config.go | 37 ++ go/tools/gazelle/config/constants.go | 10 + go/tools/gazelle/config/directives.go | 80 ++++ go/tools/gazelle/config/directives_test.go | 62 +++ go/tools/gazelle/gazelle/integration_test.go | 246 +++++++++++ go/tools/gazelle/gazelle/main.go | 40 +- go/tools/gazelle/merger/BUILD.bazel | 5 +- go/tools/gazelle/merger/fix.go | 71 +++- go/tools/gazelle/merger/fix_test.go | 48 ++- go/tools/gazelle/merger/merger.go | 22 +- go/tools/gazelle/packages/BUILD.bazel | 4 + go/tools/gazelle/packages/fileinfo.go | 258 +----------- go/tools/gazelle/packages/fileinfo_go.go | 273 ++++++++++++ go/tools/gazelle/packages/fileinfo_go_test.go | 398 ++++++++++++++++++ go/tools/gazelle/packages/fileinfo_proto.go | 133 ++++++ .../gazelle/packages/fileinfo_proto_test.go | 147 +++++++ go/tools/gazelle/packages/fileinfo_test.go | 374 ---------------- go/tools/gazelle/packages/package.go | 53 ++- go/tools/gazelle/packages/package_test.go | 4 +- go/tools/gazelle/packages/walk.go | 85 ++-- go/tools/gazelle/packages/walk_test.go | 182 +++++++- go/tools/gazelle/resolve/labeler.go | 18 + go/tools/gazelle/resolve/labeler_test.go | 52 ++- go/tools/gazelle/resolve/resolve.go | 64 +++ go/tools/gazelle/resolve/resolve_test.go | 60 +++ go/tools/gazelle/rules/generator.go | 129 ++++-- go/tools/gazelle/rules/generator_test.go | 61 +-- .../gazelle/testdata/repo/protos/BUILD.want | 31 ++ .../gazelle/testdata/repo/protos/extra.go | 1 + .../gazelle/testdata/repo/protos/foo.proto | 6 + .../testdata/repo/protos/sub/sub.proto | 1 + .../gazelle/testdata/repo/service/BUILD.want | 31 ++ .../gazelle/testdata/repo/service/extra.go | 1 + .../testdata/repo/service/service.proto | 8 + .../testdata/repo/service/sub/sub.proto | 1 + 35 files changed, 2222 insertions(+), 774 deletions(-) create mode 100644 go/tools/gazelle/packages/fileinfo_go.go create mode 100644 go/tools/gazelle/packages/fileinfo_go_test.go create mode 100644 go/tools/gazelle/packages/fileinfo_proto.go create mode 100644 go/tools/gazelle/packages/fileinfo_proto_test.go create mode 100644 go/tools/gazelle/testdata/repo/protos/BUILD.want create mode 100644 go/tools/gazelle/testdata/repo/protos/extra.go create mode 100644 go/tools/gazelle/testdata/repo/protos/foo.proto create mode 100644 go/tools/gazelle/testdata/repo/protos/sub/sub.proto create mode 100644 go/tools/gazelle/testdata/repo/service/BUILD.want create mode 100644 go/tools/gazelle/testdata/repo/service/extra.go create mode 100644 go/tools/gazelle/testdata/repo/service/service.proto create mode 100644 go/tools/gazelle/testdata/repo/service/sub/sub.proto diff --git a/go/tools/gazelle/config/config.go b/go/tools/gazelle/config/config.go index 78c24cf4d8..80fc0160a2 100644 --- a/go/tools/gazelle/config/config.go +++ b/go/tools/gazelle/config/config.go @@ -46,6 +46,10 @@ type Config struct { // This is used to map imports to labels within the repository. GoPrefix string + // ShouldFix determines whether Gazelle attempts to remove and replace + // usage of deprecated rules. + ShouldFix bool + // DepMode determines how imports outside of GoPrefix are resolved. DepMode DependencyMode @@ -54,6 +58,9 @@ type Config struct { // StructureMode determines how build files are organized within a project. StructureMode StructureMode + + // ProtoMode determines how rules are generated for protos. + ProtoMode ProtoMode } var DefaultValidBuildFileNames = []string{"BUILD.bazel", "BUILD"} @@ -166,3 +173,33 @@ const ( // new_http_archive. FlatMode ) + +// ProtoMode determines how proto rules are generated. +type ProtoMode int + +const ( + // In DefaultProtoMode, Gazelle generates proto_library and new + // go_proto_library or grpc_proto_library rules. + DefaultProtoMode ProtoMode = iota + + // In DisableProtoMode, Gazelle will ignore .proto files. .pb.go files are + // treated as normal sources. + DisableProtoMode + + // In LegacyProtoMode, Gazelle generates filegroups for .proto files if + // .pb.go files are present in the same directory. + LegacyProtoMode +) + +func ProtoModeFromString(s string) (ProtoMode, error) { + switch s { + case "default": + return DefaultProtoMode, nil + case "disable": + return DisableProtoMode, nil + case "legacy": + return LegacyProtoMode, nil + default: + return 0, fmt.Errorf("unrecognized proto mode: %q", s) + } +} diff --git a/go/tools/gazelle/config/constants.go b/go/tools/gazelle/config/constants.go index 49690a6298..461b0c06d8 100644 --- a/go/tools/gazelle/config/constants.go +++ b/go/tools/gazelle/config/constants.go @@ -34,4 +34,14 @@ const ( DefaultProtosName = "go_default_library_protos" // DefaultCgoLibName is the name of the default cgo_library rule in a Go package directory. DefaultCgoLibName = "cgo_default_library" + + // WellKnownTypesProtoRepo is the repository containing proto_library rules + // for the Well Known Types. + WellKnownTypesProtoRepo = "com_google_protobuf" + // WellKnownTypesGoProtoRepo is the repository containing go_library rules + // for the Well Known Types. + WellKnownTypesGoProtoRepo = "com_github_golang_protobuf" + // WellKnownTypesGoPrefix is the import path for the Go repository containing + // pre-generated code for the Well Known Types. + WellKnownTypesGoPrefix = "github.com/golang/protobuf" ) diff --git a/go/tools/gazelle/config/directives.go b/go/tools/gazelle/config/directives.go index 6f0512568e..02e8b60f98 100644 --- a/go/tools/gazelle/config/directives.go +++ b/go/tools/gazelle/config/directives.go @@ -41,6 +41,7 @@ var knownTopLevelDirectives = map[string]bool{ "build_tags": true, "exclude": true, "ignore": true, + "proto": true, } // TODO(jayconrod): annotation directives will apply to an individual rule. @@ -107,6 +108,14 @@ func ApplyDirectives(c *Config, directives []Directive) *Config { case "build_file_name": modified.ValidBuildFileNames = strings.Split(d.Value, ",") didModify = true + case "proto": + protoMode, err := ProtoModeFromString(d.Value) + if err != nil { + log.Print(err) + continue + } + modified.ProtoMode = protoMode + didModify = true } } if !didModify { @@ -114,3 +123,74 @@ func ApplyDirectives(c *Config, directives []Directive) *Config { } return &modified } + +// InferProtoMode sets Config.ProtoMode, based on the contents of f. If the +// proto mode is already set to something other than the default, or if the mode +// is set explicitly in directives, this function does nothing. If the legacy +// go_proto_library.bzl is loaded, or if this is the Well Known Types +// repository, legacy mode is used. If go_proto_library is loaded from another +// file or if this is the Well Known Types repository, protos are disabled. +func InferProtoMode(c *Config, f *bf.File, directives []Directive) *Config { + if c.ProtoMode != DefaultProtoMode { + return c + } + for _, d := range directives { + if d.Key == "proto" { + return c + } + } + if c.GoPrefix == WellKnownTypesGoPrefix && c.ProtoMode != DisableProtoMode { + // Use legacy mode in this repo. We don't need proto_library or + // go_proto_library, since we get that from @com_google_protobuf. + // Legacy rules still refer to .proto files in here, which need are + // exposed by filegroup. go_library rules from .pb.go files will be + // generated, which are depended upon by the new rules. + modified := *c + modified.ProtoMode = LegacyProtoMode + return &modified + } + if f == nil { + return c + } + mode := DefaultProtoMode + for _, stmt := range f.Stmt { + c, ok := stmt.(*bf.CallExpr) + if !ok { + continue + } + if x, ok := c.X.(*bf.LiteralExpr); !ok || x.Token != "load" || len(c.List) == 0 { + continue + } + name, ok := c.List[0].(*bf.StringExpr) + if !ok { + continue + } + if name.Value == "@io_bazel_rules_go//proto:def.bzl" { + break + } + if name.Value == "@io_bazel_rules_go//proto:go_proto_library.bzl" { + mode = LegacyProtoMode + break + } + for _, arg := range c.List[1:] { + if sym, ok := arg.(*bf.StringExpr); ok && sym.Value == "go_proto_library" { + mode = DisableProtoMode + break + } + kwarg, ok := arg.(*bf.BinaryExpr) + if !ok || kwarg.Op != "=" { + continue + } + if key, ok := kwarg.X.(*bf.LiteralExpr); ok && key.Token == "go_proto_library" { + mode = DisableProtoMode + break + } + } + } + if mode == DefaultProtoMode || c.ProtoMode == mode || c.ShouldFix && mode == LegacyProtoMode { + return c + } + modified := *c + modified.ProtoMode = mode + return &modified +} diff --git a/go/tools/gazelle/config/directives_test.go b/go/tools/gazelle/config/directives_test.go index 7876aa8c30..511f2f2989 100644 --- a/go/tools/gazelle/config/directives_test.go +++ b/go/tools/gazelle/config/directives_test.go @@ -91,3 +91,65 @@ func TestApplyDirectives(t *testing.T) { }) } } + +func TestInferProtoMode(t *testing.T) { + for _, tc := range []struct { + desc, content string + c Config + want ProtoMode + }{ + { + desc: "default", + }, { + desc: "previous", + c: Config{ProtoMode: LegacyProtoMode}, + want: LegacyProtoMode, + }, { + desc: "explicit", + content: `# gazelle:proto default + +load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library") +`, + want: DefaultProtoMode, + }, { + desc: "legacy", + content: `load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library")`, + want: LegacyProtoMode, + }, { + desc: "disable", + content: `load("@com_example_repo//proto:go_proto_library.bzl", go_proto_library = "x")`, + want: DisableProtoMode, + }, { + desc: "fix legacy", + content: `load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library")`, + c: Config{ShouldFix: true}, + }, { + desc: "fix disabled", + content: `load("@com_example_repo//proto:go_proto_library.bzl", go_proto_library = "x")`, + c: Config{ShouldFix: true}, + want: DisableProtoMode, + }, { + desc: "well known types", + c: Config{GoPrefix: "github.com/golang/protobuf"}, + want: LegacyProtoMode, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + var f *bf.File + var directives []Directive + if tc.content != "" { + var err error + f, err = bf.Parse("BUILD.bazel", []byte(tc.content)) + if err != nil { + t.Fatalf("error parsing build file: %v", err) + } + directives = ParseDirectives(f) + } + + got := InferProtoMode(&tc.c, f, directives) + if got.ProtoMode != tc.want { + t.Errorf("got proto mode %v ; want %v", got.ProtoMode, tc.want) + } + }) + } +} diff --git a/go/tools/gazelle/gazelle/integration_test.go b/go/tools/gazelle/gazelle/integration_test.go index f42e6de90e..81631ea64c 100644 --- a/go/tools/gazelle/gazelle/integration_test.go +++ b/go/tools/gazelle/gazelle/integration_test.go @@ -745,6 +745,252 @@ go_library( }) } +func TestMigrateProtoRules(t *testing.T) { + files := []fileSpec{ + {path: "WORKSPACE"}, + { + path: config.DefaultValidBuildFileNames[0], + content: `load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library") + +filegroup( + name = "go_default_library_protos", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], +) + +go_proto_library( + name = "go_default_library", + srcs = [":go_default_library_protos"], +) +`, + }, { + path: "foo.proto", + content: `syntax = "proto3"; + +option go_package = "example.com/repo"; +`, + }, { + path: "foo.pb.go", + content: `package repo`, + }, + } + + for _, tc := range []struct { + args []string + want string + }{ + { + args: []string{"update", "-go_prefix", "example.com/repo"}, + want: `load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library") + +filegroup( + name = "go_default_library_protos", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], +) + +go_proto_library( + name = "go_default_library", + srcs = [":go_default_library_protos"], +) + +go_library( + name = "go_default_library", + srcs = ["foo.pb.go"], + importpath = "example.com/repo", + visibility = ["//visibility:public"], +) +`, + }, { + args: []string{"fix", "-go_prefix", "example.com/repo"}, + want: `load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library") + +proto_library( + name = "repo_proto", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], +) + +go_proto_library( + name = "repo_go_proto", + importpath = "example.com/repo", + proto = ":repo_proto", + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + importpath = "example.com/repo", + library = ":repo_go_proto", + visibility = ["//visibility:public"], +) +`, + }, + } { + t.Run(tc.args[0], func(t *testing.T) { + dir, err := createFiles(files) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + if err := runGazelle(dir, tc.args); err != nil { + t.Fatal(err) + } + + checkFiles(t, dir, []fileSpec{{ + path: config.DefaultValidBuildFileNames[0], + content: tc.want, + }}) + }) + } +} + +func TestRemoveProtoDeletesRules(t *testing.T) { + files := []fileSpec{ + {path: "WORKSPACE"}, + { + path: config.DefaultValidBuildFileNames[0], + content: `load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library") + +filegroup( + name = "go_default_library_protos", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], +) + +proto_library( + name = "repo_proto", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], +) + +go_proto_library( + name = "repo_go_proto", + importpath = "example.com/repo", + proto = ":repo_proto", + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + srcs = ["extra.go"], + importpath = "example.com/repo", + library = ":repo_go_proto", + visibility = ["//visibility:public"], +) +`, + }, { + path: "extra.go", + content: `package repo`, + }, + } + dir, err := createFiles(files) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + args := []string{"fix", "-go_prefix", "example.com/repo"} + if err := runGazelle(dir, args); err != nil { + t.Fatal(err) + } + + checkFiles(t, dir, []fileSpec{{ + path: config.DefaultValidBuildFileNames[0], + content: `load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["extra.go"], + importpath = "example.com/repo", + visibility = ["//visibility:public"], +) +`, + }}) +} + +func TestAddServiceConvertsToGrpc(t *testing.T) { + files := []fileSpec{ + {path: "WORKSPACE"}, + { + path: config.DefaultValidBuildFileNames[0], + content: `load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library") + +proto_library( + name = "repo_proto", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], +) + +go_proto_library( + name = "repo_go_proto", + importpath = "example.com/repo", + proto = ":repo_proto", + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + importpath = "example.com/repo", + library = ":repo_go_proto", + visibility = ["//visibility:public"], +) +`, + }, { + path: "foo.proto", + content: `syntax = "proto3"; + +option go_package = "example.com/repo"; + +service {} +`, + }, + } + + dir, err := createFiles(files) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + args := []string{"-go_prefix", "example.com/repo"} + if err := runGazelle(dir, args); err != nil { + t.Fatal(err) + } + + checkFiles(t, dir, []fileSpec{{ + path: config.DefaultValidBuildFileNames[0], + content: `load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library") + +proto_library( + name = "repo_proto", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], +) + +go_library( + name = "go_default_library", + importpath = "example.com/repo", + library = ":repo_go_proto", + visibility = ["//visibility:public"], +) + +go_grpc_library( + name = "repo_go_proto", + importpath = "example.com/repo", + proto = ":repo_proto", + visibility = ["//visibility:public"], +) +`, + }}) +} + // TODO(jayconrod): more tests // run in fix mode in testdata directories to create new files // run in diff mode in testdata directories to update existing files (no change) diff --git a/go/tools/gazelle/gazelle/main.go b/go/tools/gazelle/gazelle/main.go index 12d89e4bde..6e62c67b00 100644 --- a/go/tools/gazelle/gazelle/main.go +++ b/go/tools/gazelle/gazelle/main.go @@ -77,22 +77,20 @@ type visitor interface { } type visitorBase struct { - c *config.Config - r *resolve.Resolver - l resolve.Labeler - shouldFix bool - emit emitFunc + c *config.Config + r *resolve.Resolver + l resolve.Labeler + emit emitFunc } func newVisitor(c *config.Config, cmd command, emit emitFunc) visitor { l := resolve.NewLabeler(c) r := resolve.NewResolver(c, l) base := visitorBase{ - c: c, - r: r, - l: l, - shouldFix: cmd == fixCmd, - emit: emit, + c: c, + r: r, + l: l, + emit: emit, } if c.StructureMode == config.HierarchicalMode { v := &hierarchicalVisitor{visitorBase: base} @@ -124,7 +122,7 @@ func (v *hierarchicalVisitor) visit(c *config.Config, pkg *packages.Package, old Path: filepath.Join(pkg.Dir, c.DefaultBuildFileName()), Stmt: rules, } - v.mergeAndEmit(genFile, oldFile, empty) + v.mergeAndEmit(c, genFile, oldFile, empty) } func (v *hierarchicalVisitor) finish() { @@ -191,14 +189,14 @@ func (v *flatVisitor) finish() { genFile.Stmt = append(genFile.Stmt, rs...) } - v.mergeAndEmit(genFile, v.oldRootFile, v.empty) + v.mergeAndEmit(v.c, genFile, v.oldRootFile, v.empty) } // mergeAndEmit merges "genFile" with "oldFile". "oldFile" may be nil if -// no file exists. If v.shouldFix is true, deprecated usage of old rules in +// no file exists. If v.c.ShouldFix is true, deprecated usage of old rules in // "oldFile" will be fixed. The resulting merged file will be emitted using // the "v.emit" function. -func (v *visitorBase) mergeAndEmit(genFile, oldFile *bf.File, empty []bf.Expr) { +func (v *visitorBase) mergeAndEmit(c *config.Config, genFile, oldFile *bf.File, empty []bf.Expr) { if oldFile == nil { // No existing file, so no merge required. rules.SortLabels(genFile) @@ -211,10 +209,10 @@ func (v *visitorBase) mergeAndEmit(genFile, oldFile *bf.File, empty []bf.Expr) { } // Existing file. Fix it or see if it needs fixing before merging. - if v.shouldFix { - oldFile = merger.FixFile(oldFile) + if c.ShouldFix { + oldFile = merger.FixFile(c, oldFile) } else { - fixedFile := merger.FixFile(oldFile) + fixedFile := merger.FixFile(c, oldFile) if fixedFile != oldFile { log.Printf("%s: warning: file contains rules whose structure is out of date. Consider running 'gazelle fix'.", oldFile.Path) } @@ -309,6 +307,7 @@ func newConfiguration(args []string) (*config.Config, command, emitFunc, error) fs.Var(&knownImports, "known_import", "import path for which external resolution is skipped (can specify multiple times)") mode := fs.String("mode", "fix", "print: prints all of the updated BUILD files\n\tfix: rewrites all of the BUILD files in place\n\tdiff: computes the rewrite but then just does a diff") flat := fs.Bool("experimental_flat", false, "whether gazelle should generate a single, combined BUILD file.\nThis mode is experimental and may not work yet.") + proto := fs.String("proto", "default", "default: generates new proto rules\n\tdisable: does not touch proto rules\n\tlegacy (deprecated): generates old proto rules") if err := fs.Parse(args); err != nil { if err == flag.ErrHelp { usage(fs) @@ -373,6 +372,8 @@ func newConfiguration(args []string) (*config.Config, command, emitFunc, error) } } + c.ShouldFix = cmd == fixCmd + c.DepMode, err = config.DependencyModeFromString(*external) if err != nil { return nil, cmd, nil, err @@ -384,6 +385,11 @@ func newConfiguration(args []string) (*config.Config, command, emitFunc, error) c.StructureMode = config.HierarchicalMode } + c.ProtoMode, err = config.ProtoModeFromString(*proto) + if err != nil { + return nil, cmd, nil, err + } + emit, ok := modeFromName[*mode] if !ok { return nil, cmd, nil, fmt.Errorf("unrecognized emit mode: %q", *mode) diff --git a/go/tools/gazelle/merger/BUILD.bazel b/go/tools/gazelle/merger/BUILD.bazel index 64e9ed7653..c64b353e1e 100644 --- a/go/tools/gazelle/merger/BUILD.bazel +++ b/go/tools/gazelle/merger/BUILD.bazel @@ -21,5 +21,8 @@ go_test( "merger_test.go", ], library = ":go_default_library", - deps = ["@com_github_bazelbuild_buildtools//build:go_default_library"], + deps = [ + "@com_github_bazelbuild_buildtools//build:go_default_library", + "@io_bazel_rules_go//go/tools/gazelle/config:go_default_library", + ], ) diff --git a/go/tools/gazelle/merger/fix.go b/go/tools/gazelle/merger/fix.go index e75f3e6d59..28802b2417 100644 --- a/go/tools/gazelle/merger/fix.go +++ b/go/tools/gazelle/merger/fix.go @@ -33,8 +33,9 @@ import ( // // FixLoads should be called after this, since it will fix load // statements that may be broken by transformations applied by this function. -func FixFile(oldFile *bf.File) *bf.File { - return squashCgoLibrary(oldFile) +func FixFile(c *config.Config, oldFile *bf.File) *bf.File { + fixedFile := squashCgoLibrary(oldFile) + return removeLegacyProto(c, fixedFile) } // squashCgoLibrary removes cgo_library rules with the default name and @@ -248,6 +249,72 @@ func squashDict(x, y *bf.DictExpr) (*bf.DictExpr, error) { return &squashed, nil } +// removeLegacyProto removes uses of the old proto rules. It deletes loads +// from go_proto_library.bzl. It deletes proto filegroups. It removes +// go_proto_library attributes which are no longer recognized. New rules +// are generated in place of the deleted rules, but attributes and comments +// are not migrated. +func removeLegacyProto(c *config.Config, oldFile *bf.File) *bf.File { + // Don't fix if the proto mode was set to something other than the default. + if c.ProtoMode != config.DefaultProtoMode { + return oldFile + } + + // Scan for definitions to delete. + var deletedIndices []int + var protoIndices []int + shouldDeleteProtos := false + for i, stmt := range oldFile.Stmt { + c, ok := stmt.(*bf.CallExpr) + if !ok { + continue + } + x, ok := c.X.(*bf.LiteralExpr) + if !ok { + continue + } + + if x.Token == "load" && len(c.List) > 0 { + if name, ok := c.List[0].(*bf.StringExpr); ok && name.Value == "@io_bazel_rules_go//proto:go_proto_library.bzl" { + deletedIndices = append(deletedIndices, i) + shouldDeleteProtos = true + } + continue + } + if x.Token == "filegroup" { + r := bf.Rule{Call: c} + if r.Name() == config.DefaultProtosName { + deletedIndices = append(deletedIndices, i) + } + continue + } + if x.Token == "go_proto_library" { + protoIndices = append(protoIndices, i) + } + } + if len(deletedIndices) == 0 { + return oldFile + } + + // Rebuild the file without deleted statements. Only delete go_proto_library + // rules if we deleted a load. + if shouldDeleteProtos { + deletedIndices = append(deletedIndices, protoIndices...) + sort.Ints(deletedIndices) + } + fixedFile := *oldFile + fixedFile.Stmt = make([]bf.Expr, 0, len(oldFile.Stmt)-len(deletedIndices)) + di := 0 + for i, stmt := range oldFile.Stmt { + if di < len(deletedIndices) && i == deletedIndices[di] { + di++ + continue + } + fixedFile.Stmt = append(fixedFile.Stmt, stmt) + } + return &fixedFile +} + // FixLoads removes loads of unused go rules and adds loads of newly used rules. // This should be called after FixFile and MergeWithExisting, since symbols // may be introduced that aren't loaded. diff --git a/go/tools/gazelle/merger/fix_test.go b/go/tools/gazelle/merger/fix_test.go index 57387a43bf..5bcc25c35d 100644 --- a/go/tools/gazelle/merger/fix_test.go +++ b/go/tools/gazelle/merger/fix_test.go @@ -19,6 +19,7 @@ import ( "testing" bf "github.com/bazelbuild/buildtools/build" + "github.com/bazelbuild/rules_go/go/tools/gazelle/config" ) type fixTestCase struct { @@ -174,11 +175,56 @@ go_library( ) # after go_library # after cgo_library +`, + }, + // fixLegacyProto tests + { + desc: "current proto preserved", + old: `load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library") + +go_proto_library( + name = "foo_go_proto", + proto = ":foo_proto", +) +`, + want: `load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library") + +go_proto_library( + name = "foo_go_proto", + proto = ":foo_proto", +) +`, + }, + { + desc: "load and proto removed", + old: `load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library") + +go_proto_library( + name = "go_default_library_protos", + srcs = ["foo.proto"], + visibility = ["//visibility:private"], +) +`, + want: "", + }, + { + desc: "proto filegroup removed", + old: `filegroup( + name = "go_default_library_protos", + srcs = ["foo.proto"], +) + +go_proto_library(name = "foo_proto") +`, + want: `go_proto_library(name = "foo_proto") `, }, } { t.Run(tc.desc, func(t *testing.T) { - testFix(t, tc, FixFile) + fix := func(f *bf.File) *bf.File { + return FixFile(&config.Config{}, f) + } + testFix(t, tc, fix) }) } } diff --git a/go/tools/gazelle/merger/merger.go b/go/tools/gazelle/merger/merger.go index 188c285f75..f8046af2f7 100644 --- a/go/tools/gazelle/merger/merger.go +++ b/go/tools/gazelle/merger/merger.go @@ -36,6 +36,7 @@ var ( "deps": true, "importpath": true, "library": true, + "proto": true, "srcs": true, } ) @@ -502,15 +503,20 @@ func name(c *bf.CallExpr) string { } func isEmpty(c *bf.CallExpr) bool { - if len(c.List) != 1 { - return false - } - arg, ok := c.List[0].(*bf.BinaryExpr) - if !ok { - return false + for _, arg := range c.List { + kwarg, ok := arg.(*bf.BinaryExpr) + if !ok || kwarg.Op != "=" { + return false + } + key, ok := kwarg.X.(*bf.LiteralExpr) + if !ok { + return false + } + if key.Token != "name" && key.Token != "visibility" { + return false + } } - x, ok := arg.X.(*bf.LiteralExpr) - return ok && x.Token == "name" + return true } func isScalar(e bf.Expr) bool { diff --git a/go/tools/gazelle/packages/BUILD.bazel b/go/tools/gazelle/packages/BUILD.bazel index 80f93b1751..77b4e4388b 100644 --- a/go/tools/gazelle/packages/BUILD.bazel +++ b/go/tools/gazelle/packages/BUILD.bazel @@ -5,6 +5,8 @@ go_library( srcs = [ "doc.go", "fileinfo.go", + "fileinfo_go.go", + "fileinfo_proto.go", "package.go", "walk.go", ], @@ -20,6 +22,8 @@ go_test( size = "small", srcs = [ "fileinfo_test.go", + "fileinfo_go_test.go", + "fileinfo_proto_test.go", "package_test.go", ], library = ":go_default_library", diff --git a/go/tools/gazelle/packages/fileinfo.go b/go/tools/gazelle/packages/fileinfo.go index 71da46ec37..c1d0101b59 100644 --- a/go/tools/gazelle/packages/fileinfo.go +++ b/go/tools/gazelle/packages/fileinfo.go @@ -17,22 +17,11 @@ package packages import ( "bufio" - "bytes" - "errors" - "fmt" - "go/ast" - "go/parser" - "go/token" "log" "os" "path" "path/filepath" - "strconv" "strings" - "unicode" - "unicode/utf8" - - "github.com/bazelbuild/rules_go/go/tools/gazelle/config" ) // fileInfo holds information used to decide how to build a file. This @@ -45,6 +34,10 @@ type fileInfo struct { // "_test" suffix if it was present. It is empty for non-Go files. packageName string + // importPath is the canonical import path for the package this file + // belongs to. Will be empty file files that don't specify this. + importPath string + // category is the type of file, based on extension. category extCategory @@ -74,6 +67,9 @@ type fileInfo struct { // copts and clinkopts contain flags that are part of CFLAGS, CPPFLAGS, // CXXFLAGS, and LDFLAGS directives in cgo comments. copts, clinkopts []taggedOpts + + // hasServices indicates whether a .proto file has service definitions. + hasServices bool } // taggedOpts a list of compile or link options which should only be applied @@ -177,246 +173,6 @@ func fileNameInfo(dir, rel, name string) fileInfo { } } -// goFileInfo returns information about a .go file. It will parse part of the -// file to determine the package name, imports, and build constraints. -// If the file can't be read, an error will be logged, and partial information -// will be returned. -// This function is intended to match go/build.Context.Import. -func goFileInfo(c *config.Config, dir, rel, name string) fileInfo { - info := fileNameInfo(dir, rel, name) - fset := token.NewFileSet() - pf, err := parser.ParseFile(fset, info.path, nil, parser.ImportsOnly|parser.ParseComments) - if err != nil { - log.Printf("%s: error reading go file: %v", info.path, err) - return info - } - - info.packageName = pf.Name.Name - if info.isTest && strings.HasSuffix(info.packageName, "_test") { - info.isXTest = true - info.packageName = info.packageName[:len(info.packageName)-len("_test")] - } - - for _, decl := range pf.Decls { - d, ok := decl.(*ast.GenDecl) - if !ok { - continue - } - for _, dspec := range d.Specs { - spec, ok := dspec.(*ast.ImportSpec) - if !ok { - continue - } - quoted := spec.Path.Value - path, err := strconv.Unquote(quoted) - if err != nil { - log.Printf("%s: error reading go file: %v", info.path, err) - continue - } - - if path == "C" { - if info.isTest { - log.Printf("%s: warning: use of cgo in test not supported", info.path) - } - info.isCgo = true - cg := spec.Doc - if cg == nil && len(d.Specs) == 1 { - cg = d.Doc - } - if cg != nil { - if err := saveCgo(&info, cg); err != nil { - log.Printf("%s: error reading go file: %v", info.path, err) - } - } - } else if !isStandard(c.GoPrefix, path) { - info.imports = append(info.imports, path) - } - } - } - - tags, err := readTags(info.path) - if err != nil { - log.Printf("%s: error reading go file: %v", info.path, err) - return info - } - info.tags = tags - - return info -} - -// saveCgo extracts CFLAGS, CPPFLAGS, CXXFLAGS, and LDFLAGS directives -// from a comment above a "C" import. This is intended to match logic in -// go/build.Context.saveCgo. -func saveCgo(info *fileInfo, cg *ast.CommentGroup) error { - text := cg.Text() - for _, line := range strings.Split(text, "\n") { - orig := line - - // Line is - // #cgo [GOOS/GOARCH...] LDFLAGS: stuff - // - line = strings.TrimSpace(line) - if len(line) < 5 || line[:4] != "#cgo" || (line[4] != ' ' && line[4] != '\t') { - continue - } - - // Split at colon. - line = strings.TrimSpace(line[4:]) - i := strings.Index(line, ":") - if i < 0 { - return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) - } - line, optstr := strings.TrimSpace(line[:i]), strings.TrimSpace(line[i+1:]) - - // Parse tags and verb. - f := strings.Fields(line) - if len(f) < 1 { - return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) - } - verb := f[len(f)-1] - tags := strings.Join(f[:len(f)-1], " ") - - // Parse options. - opts, err := splitQuoted(optstr) - if err != nil { - return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) - } - var ok bool - for i, opt := range opts { - if opt, ok = expandSrcDir(opt, info.rel); !ok { - return fmt.Errorf("%s: malformed #cgo argument: %s", info.path, orig) - } - opts[i] = opt - } - - // Add tags to appropriate list. - switch verb { - case "CFLAGS", "CPPFLAGS", "CXXFLAGS": - info.copts = append(info.copts, taggedOpts{tags, opts}) - case "LDFLAGS": - info.clinkopts = append(info.clinkopts, taggedOpts{tags, opts}) - case "pkg-config": - return fmt.Errorf("%s: pkg-config not supported: %s", info.path, orig) - default: - return fmt.Errorf("%s: invalid #cgo verb: %s", info.path, orig) - } - } - return nil -} - -// splitQuoted splits the string s around each instance of one or more consecutive -// white space characters while taking into account quotes and escaping, and -// returns an array of substrings of s or an empty list if s contains only white space. -// Single quotes and double quotes are recognized to prevent splitting within the -// quoted region, and are removed from the resulting substrings. If a quote in s -// isn't closed err will be set and r will have the unclosed argument as the -// last element. The backslash is used for escaping. -// -// For example, the following string: -// -// a b:"c d" 'e''f' "g\"" -// -// Would be parsed as: -// -// []string{"a", "b:c d", "ef", `g"`} -// -// Copied from go/build.splitQuoted -func splitQuoted(s string) (r []string, err error) { - var args []string - arg := make([]rune, len(s)) - escaped := false - quoted := false - quote := '\x00' - i := 0 - for _, rune := range s { - switch { - case escaped: - escaped = false - case rune == '\\': - escaped = true - continue - case quote != '\x00': - if rune == quote { - quote = '\x00' - continue - } - case rune == '"' || rune == '\'': - quoted = true - quote = rune - continue - case unicode.IsSpace(rune): - if quoted || i > 0 { - quoted = false - args = append(args, string(arg[:i])) - i = 0 - } - continue - } - arg[i] = rune - i++ - } - if quoted || i > 0 { - args = append(args, string(arg[:i])) - } - if quote != 0 { - err = errors.New("unclosed quote") - } else if escaped { - err = errors.New("unfinished escaping") - } - return args, err -} - -// expandSrcDir expands any occurrence of ${SRCDIR}, making sure -// the result is safe for the shell. -// -// Copied from go/build.expandSrcDir -func expandSrcDir(str string, srcdir string) (string, bool) { - // "\" delimited paths cause safeCgoName to fail - // so convert native paths with a different delimiter - // to "/" before starting (eg: on windows). - srcdir = filepath.ToSlash(srcdir) - - // Spaces are tolerated in ${SRCDIR}, but not anywhere else. - chunks := strings.Split(str, "${SRCDIR}") - if len(chunks) < 2 { - return str, safeCgoName(str, false) - } - ok := true - for _, chunk := range chunks { - ok = ok && (chunk == "" || safeCgoName(chunk, false)) - } - ok = ok && (srcdir == "" || safeCgoName(srcdir, true)) - res := strings.Join(chunks, srcdir) - return res, ok && res != "" -} - -// NOTE: $ is not safe for the shell, but it is allowed here because of linker options like -Wl,$ORIGIN. -// We never pass these arguments to a shell (just to programs we construct argv for), so this should be okay. -// See golang.org/issue/6038. -// The @ is for OS X. See golang.org/issue/13720. -// The % is for Jenkins. See golang.org/issue/16959. -const safeString = "+-.,/0123456789=ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz:$@%" -const safeSpaces = " " - -var safeBytes = []byte(safeSpaces + safeString) - -// Copied from go/build.safeCgoName -func safeCgoName(s string, spaces bool) bool { - if s == "" { - return false - } - safe := safeBytes - if !spaces { - safe = safe[len(safeSpaces):] - } - for i := 0; i < len(s); i++ { - if c := s[i]; c < utf8.RuneSelf && bytes.IndexByte(safe, c) < 0 { - return false - } - } - return true -} - // JoinOptions combines shell options grouped by a special separator token // and returns a string for each group. The group strings will be escaped // such that the original options can be recovered after Bourne shell diff --git a/go/tools/gazelle/packages/fileinfo_go.go b/go/tools/gazelle/packages/fileinfo_go.go new file mode 100644 index 0000000000..96f54f074f --- /dev/null +++ b/go/tools/gazelle/packages/fileinfo_go.go @@ -0,0 +1,273 @@ +/* Copyright 2017 The Bazel Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package packages + +import ( + "bytes" + "errors" + "fmt" + "go/ast" + "go/parser" + "go/token" + "log" + "path/filepath" + "strconv" + "strings" + "unicode" + "unicode/utf8" + + "github.com/bazelbuild/rules_go/go/tools/gazelle/config" +) + +// goFileInfo returns information about a .go file. It will parse part of the +// file to determine the package name, imports, and build constraints. +// If the file can't be read, an error will be logged, and partial information +// will be returned. +// This function is intended to match go/build.Context.Import. +func goFileInfo(c *config.Config, dir, rel, name string) fileInfo { + info := fileNameInfo(dir, rel, name) + fset := token.NewFileSet() + pf, err := parser.ParseFile(fset, info.path, nil, parser.ImportsOnly|parser.ParseComments) + if err != nil { + log.Printf("%s: error reading go file: %v", info.path, err) + return info + } + + info.packageName = pf.Name.Name + if info.isTest && strings.HasSuffix(info.packageName, "_test") { + info.isXTest = true + info.packageName = info.packageName[:len(info.packageName)-len("_test")] + } + + for _, decl := range pf.Decls { + d, ok := decl.(*ast.GenDecl) + if !ok { + continue + } + for _, dspec := range d.Specs { + spec, ok := dspec.(*ast.ImportSpec) + if !ok { + continue + } + quoted := spec.Path.Value + path, err := strconv.Unquote(quoted) + if err != nil { + log.Printf("%s: error reading go file: %v", info.path, err) + continue + } + + if path == "C" { + if info.isTest { + log.Printf("%s: warning: use of cgo in test not supported", info.path) + } + info.isCgo = true + cg := spec.Doc + if cg == nil && len(d.Specs) == 1 { + cg = d.Doc + } + if cg != nil { + if err := saveCgo(&info, cg); err != nil { + log.Printf("%s: error reading go file: %v", info.path, err) + } + } + } else if !isStandard(c.GoPrefix, path) { + info.imports = append(info.imports, path) + } + } + } + + tags, err := readTags(info.path) + if err != nil { + log.Printf("%s: error reading go file: %v", info.path, err) + return info + } + info.tags = tags + + return info +} + +// saveCgo extracts CFLAGS, CPPFLAGS, CXXFLAGS, and LDFLAGS directives +// from a comment above a "C" import. This is intended to match logic in +// go/build.Context.saveCgo. +func saveCgo(info *fileInfo, cg *ast.CommentGroup) error { + text := cg.Text() + for _, line := range strings.Split(text, "\n") { + orig := line + + // Line is + // #cgo [GOOS/GOARCH...] LDFLAGS: stuff + // + line = strings.TrimSpace(line) + if len(line) < 5 || line[:4] != "#cgo" || (line[4] != ' ' && line[4] != '\t') { + continue + } + + // Split at colon. + line = strings.TrimSpace(line[4:]) + i := strings.Index(line, ":") + if i < 0 { + return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) + } + line, optstr := strings.TrimSpace(line[:i]), strings.TrimSpace(line[i+1:]) + + // Parse tags and verb. + f := strings.Fields(line) + if len(f) < 1 { + return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) + } + verb := f[len(f)-1] + tags := strings.Join(f[:len(f)-1], " ") + + // Parse options. + opts, err := splitQuoted(optstr) + if err != nil { + return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) + } + var ok bool + for i, opt := range opts { + if opt, ok = expandSrcDir(opt, info.rel); !ok { + return fmt.Errorf("%s: malformed #cgo argument: %s", info.path, orig) + } + opts[i] = opt + } + + // Add tags to appropriate list. + switch verb { + case "CFLAGS", "CPPFLAGS", "CXXFLAGS": + info.copts = append(info.copts, taggedOpts{tags, opts}) + case "LDFLAGS": + info.clinkopts = append(info.clinkopts, taggedOpts{tags, opts}) + case "pkg-config": + return fmt.Errorf("%s: pkg-config not supported: %s", info.path, orig) + default: + return fmt.Errorf("%s: invalid #cgo verb: %s", info.path, orig) + } + } + return nil +} + +// splitQuoted splits the string s around each instance of one or more consecutive +// white space characters while taking into account quotes and escaping, and +// returns an array of substrings of s or an empty list if s contains only white space. +// Single quotes and double quotes are recognized to prevent splitting within the +// quoted region, and are removed from the resulting substrings. If a quote in s +// isn't closed err will be set and r will have the unclosed argument as the +// last element. The backslash is used for escaping. +// +// For example, the following string: +// +// a b:"c d" 'e''f' "g\"" +// +// Would be parsed as: +// +// []string{"a", "b:c d", "ef", `g"`} +// +// Copied from go/build.splitQuoted +func splitQuoted(s string) (r []string, err error) { + var args []string + arg := make([]rune, len(s)) + escaped := false + quoted := false + quote := '\x00' + i := 0 + for _, rune := range s { + switch { + case escaped: + escaped = false + case rune == '\\': + escaped = true + continue + case quote != '\x00': + if rune == quote { + quote = '\x00' + continue + } + case rune == '"' || rune == '\'': + quoted = true + quote = rune + continue + case unicode.IsSpace(rune): + if quoted || i > 0 { + quoted = false + args = append(args, string(arg[:i])) + i = 0 + } + continue + } + arg[i] = rune + i++ + } + if quoted || i > 0 { + args = append(args, string(arg[:i])) + } + if quote != 0 { + err = errors.New("unclosed quote") + } else if escaped { + err = errors.New("unfinished escaping") + } + return args, err +} + +// expandSrcDir expands any occurrence of ${SRCDIR}, making sure +// the result is safe for the shell. +// +// Copied from go/build.expandSrcDir +func expandSrcDir(str string, srcdir string) (string, bool) { + // "\" delimited paths cause safeCgoName to fail + // so convert native paths with a different delimiter + // to "/" before starting (eg: on windows). + srcdir = filepath.ToSlash(srcdir) + + // Spaces are tolerated in ${SRCDIR}, but not anywhere else. + chunks := strings.Split(str, "${SRCDIR}") + if len(chunks) < 2 { + return str, safeCgoName(str, false) + } + ok := true + for _, chunk := range chunks { + ok = ok && (chunk == "" || safeCgoName(chunk, false)) + } + ok = ok && (srcdir == "" || safeCgoName(srcdir, true)) + res := strings.Join(chunks, srcdir) + return res, ok && res != "" +} + +// NOTE: $ is not safe for the shell, but it is allowed here because of linker options like -Wl,$ORIGIN. +// We never pass these arguments to a shell (just to programs we construct argv for), so this should be okay. +// See golang.org/issue/6038. +// The @ is for OS X. See golang.org/issue/13720. +// The % is for Jenkins. See golang.org/issue/16959. +const safeString = "+-.,/0123456789=ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz:$@%" +const safeSpaces = " " + +var safeBytes = []byte(safeSpaces + safeString) + +// Copied from go/build.safeCgoName +func safeCgoName(s string, spaces bool) bool { + if s == "" { + return false + } + safe := safeBytes + if !spaces { + safe = safe[len(safeSpaces):] + } + for i := 0; i < len(s); i++ { + if c := s[i]; c < utf8.RuneSelf && bytes.IndexByte(safe, c) < 0 { + return false + } + } + return true +} diff --git a/go/tools/gazelle/packages/fileinfo_go_test.go b/go/tools/gazelle/packages/fileinfo_go_test.go new file mode 100644 index 0000000000..bb4ea6646b --- /dev/null +++ b/go/tools/gazelle/packages/fileinfo_go_test.go @@ -0,0 +1,398 @@ +/* Copyright 2017 The Bazel Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package packages + +import ( + "io/ioutil" + "os" + "path/filepath" + "reflect" + "testing" + + "github.com/bazelbuild/rules_go/go/tools/gazelle/config" +) + +func TestGoFileInfo(t *testing.T) { + c := &config.Config{} + dir := "." + rel := "" + for _, tc := range []struct { + desc, name, source string + want fileInfo + }{ + { + "empty file", + "foo.go", + "package foo\n", + fileInfo{ + packageName: "foo", + }, + }, + { + "xtest file", + "foo_test.go", + "package foo_test\n", + fileInfo{ + packageName: "foo", + isTest: true, + isXTest: true, + }, + }, + { + "xtest suffix on non-test", + "foo_xtest.go", + "package foo_test\n", + fileInfo{ + packageName: "foo_test", + isTest: false, + isXTest: false, + }, + }, + { + "single import", + "foo.go", + `package foo + +import "github.com/foo/bar" +`, + fileInfo{ + packageName: "foo", + imports: []string{"github.com/foo/bar"}, + }, + }, + { + "multiple imports", + "foo.go", + `package foo + +import ( + "github.com/foo/bar" + x "github.com/local/project/y" +) +`, + fileInfo{ + packageName: "foo", + imports: []string{"github.com/foo/bar", "github.com/local/project/y"}, + }, + }, + { + "standard imports not included", + "foo.go", + `package foo + +import "fmt" +`, + fileInfo{ + packageName: "foo", + }, + }, + { + "cgo", + "foo.go", + `package foo + +import "C" +`, + fileInfo{ + packageName: "foo", + isCgo: true, + }, + }, + { + "build tags", + "foo.go", + `// +build linux darwin + +// +build !ignore + +package foo +`, + fileInfo{ + packageName: "foo", + tags: []string{"linux darwin", "!ignore"}, + }, + }, + { + "build tags without blank line", + "route.go", + `// Copyright 2017 + +// +build darwin dragonfly freebsd netbsd openbsd + +// Package route provides basic functions for the manipulation of +// packet routing facilities on BSD variants. +package route +`, + fileInfo{ + packageName: "route", + tags: []string{"darwin dragonfly freebsd netbsd openbsd"}, + }, + }, + } { + if err := ioutil.WriteFile(tc.name, []byte(tc.source), 0600); err != nil { + t.Fatal(err) + } + defer os.Remove(tc.name) + + got := goFileInfo(c, dir, rel, tc.name) + + // Clear fields we don't care about for testing. + got = fileInfo{ + packageName: got.packageName, + isTest: got.isTest, + isXTest: got.isXTest, + imports: got.imports, + isCgo: got.isCgo, + tags: got.tags, + } + + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("case %q: got %#v; want %#v", tc.desc, got, tc.want) + } + } +} + +func TestGoFileInfoFailure(t *testing.T) { + dir := "." + name := "foo_linux_amd64.go" + if err := ioutil.WriteFile(name, []byte("pakcage foo"), 0600); err != nil { + t.Fatal(err) + } + defer os.Remove(name) + + c := &config.Config{} + got := goFileInfo(c, dir, "", name) + want := fileInfo{ + path: filepath.Join(dir, name), + name: name, + ext: ".go", + category: goExt, + goos: "linux", + goarch: "amd64", + } + if !reflect.DeepEqual(got, want) { + t.Errorf("got %#v ; want %#v", got, want) + } +} + +func TestCgo(t *testing.T) { + c := &config.Config{} + dir := "." + rel := "" + for _, tc := range []struct { + desc, source string + want fileInfo + }{ + { + "not cgo", + "package foo\n", + fileInfo{isCgo: false}, + }, + { + "empty cgo", + `package foo + +import "C" +`, + fileInfo{isCgo: true}, + }, + { + "simple flags", + `package foo + +/* +#cgo CFLAGS: -O0 + #cgo CPPFLAGS: -O1 +#cgo CXXFLAGS: -O2 +#cgo LDFLAGS: -O3 -O4 +*/ +import "C" +`, + fileInfo{ + isCgo: true, + copts: []taggedOpts{ + {opts: []string{"-O0"}}, + {opts: []string{"-O1"}}, + {opts: []string{"-O2"}}, + }, + clinkopts: []taggedOpts{ + {opts: []string{"-O3", "-O4"}}, + }, + }, + }, + { + "cflags with conditions", + `package foo + +/* +#cgo foo bar,!baz CFLAGS: -O0 +*/ +import "C" +`, + fileInfo{ + isCgo: true, + copts: []taggedOpts{ + {tags: "foo bar,!baz", opts: []string{"-O0"}}, + }, + }, + }, + { + "slashslash comments", + `package foo + +// #cgo CFLAGS: -O0 +// #cgo CFLAGS: -O1 +import "C" +`, + fileInfo{ + isCgo: true, + copts: []taggedOpts{ + {opts: []string{"-O0"}}, + {opts: []string{"-O1"}}, + }, + }, + }, + { + "comment above single import group", + `package foo + +/* +#cgo CFLAGS: -O0 +*/ +import ("C") +`, + fileInfo{ + isCgo: true, + copts: []taggedOpts{ + {opts: []string{"-O0"}}, + }, + }, + }, + } { + path := "TestCgo.go" + if err := ioutil.WriteFile(path, []byte(tc.source), 0600); err != nil { + t.Fatal(err) + } + defer os.Remove(path) + + got := goFileInfo(c, dir, rel, path) + + // Clear fields we don't care about for testing. + got = fileInfo{isCgo: got.isCgo, copts: got.copts, clinkopts: got.clinkopts} + + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("case %q: got %#v; want %#v", tc.desc, got, tc.want) + } + } +} + +// Copied from go/build build_test.go +var ( + expandSrcDirPath = filepath.Join(string(filepath.Separator)+"projects", "src", "add") +) + +// Copied from go/build build_test.go +var expandSrcDirTests = []struct { + input, expected string +}{ + {"-L ${SRCDIR}/libs -ladd", "-L /projects/src/add/libs -ladd"}, + {"${SRCDIR}/add_linux_386.a -pthread -lstdc++", "/projects/src/add/add_linux_386.a -pthread -lstdc++"}, + {"Nothing to expand here!", "Nothing to expand here!"}, + {"$", "$"}, + {"$$", "$$"}, + {"${", "${"}, + {"$}", "$}"}, + {"$FOO ${BAR}", "$FOO ${BAR}"}, + {"Find me the $SRCDIRECTORY.", "Find me the $SRCDIRECTORY."}, + {"$SRCDIR is missing braces", "$SRCDIR is missing braces"}, +} + +// Copied from go/build build_test.go +func TestExpandSrcDir(t *testing.T) { + for _, test := range expandSrcDirTests { + output, _ := expandSrcDir(test.input, expandSrcDirPath) + if output != test.expected { + t.Errorf("%q expands to %q with SRCDIR=%q when %q is expected", test.input, output, expandSrcDirPath, test.expected) + } else { + t.Logf("%q expands to %q with SRCDIR=%q", test.input, output, expandSrcDirPath) + } + } +} + +func TestExpandSrcDirRepoRelative(t *testing.T) { + repo, err := ioutil.TempDir(os.Getenv("TEST_TEMPDIR"), "repo") + if err != nil { + t.Fatal(err) + } + sub := filepath.Join(repo, "sub") + if err := os.Mkdir(sub, 0755); err != nil { + t.Fatal(err) + } + goFile := filepath.Join(sub, "sub.go") + content := []byte(`package sub + +/* +#cgo CFLAGS: -I${SRCDIR}/.. +*/ +import "C" +`) + if err := ioutil.WriteFile(goFile, content, 0644); err != nil { + t.Fatal(err) + } + c := &config.Config{RepoRoot: repo} + got := buildPackage(c, sub, []string{"sub.go"}, nil, nil, false) + want := &Package{ + Name: "sub", + Dir: sub, + Rel: "sub", + Library: GoTarget{ + Sources: PlatformStrings{ + Generic: []string{"sub.go"}, + }, + COpts: PlatformStrings{ + Generic: []string{"-Isub/..", optSeparator}, + }, + Cgo: true, + }, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("got %#v ; want %#v", got, want) + } +} + +// Copied from go/build build_test.go +func TestShellSafety(t *testing.T) { + tests := []struct { + input, srcdir, expected string + result bool + }{ + {"-I${SRCDIR}/../include", "/projects/src/issue 11868", "-I/projects/src/issue 11868/../include", true}, + {"-I${SRCDIR}", "wtf$@%", "-Iwtf$@%", true}, + {"-X${SRCDIR}/1,${SRCDIR}/2", "/projects/src/issue 11868", "-X/projects/src/issue 11868/1,/projects/src/issue 11868/2", true}, + {"-I/tmp -I/tmp", "/tmp2", "-I/tmp -I/tmp", false}, + {"-I/tmp", "/tmp/[0]", "-I/tmp", true}, + {"-I${SRCDIR}/dir", "/tmp/[0]", "-I/tmp/[0]/dir", false}, + } + for _, test := range tests { + output, ok := expandSrcDir(test.input, test.srcdir) + if ok != test.result { + t.Errorf("Expected %t while %q expands to %q with SRCDIR=%q; got %t", test.result, test.input, output, test.srcdir, ok) + } + if output != test.expected { + t.Errorf("Expected %q while %q expands with SRCDIR=%q; got %q", test.expected, test.input, test.srcdir, output) + } + } +} diff --git a/go/tools/gazelle/packages/fileinfo_proto.go b/go/tools/gazelle/packages/fileinfo_proto.go new file mode 100644 index 0000000000..4e990f01d8 --- /dev/null +++ b/go/tools/gazelle/packages/fileinfo_proto.go @@ -0,0 +1,133 @@ +/* Copyright 2017 The Bazel Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package packages + +import ( + "bytes" + "io/ioutil" + "log" + "path" + "regexp" + "sort" + "strconv" + "strings" + + "github.com/bazelbuild/rules_go/go/tools/gazelle/config" +) + +var protoRe = buildProtoRegexp() + +const ( + importSubexpIndex = 1 + packageSubexpIndex = 2 + goPackageSubexpIndex = 3 + serviceSubexpIndex = 4 +) + +func protoFileInfo(c *config.Config, dir, rel, name string) fileInfo { + info := fileNameInfo(dir, rel, name) + content, err := ioutil.ReadFile(info.path) + if err != nil { + log.Printf("%s: error reading proto file: %v", info.path, err) + return info + } + + for _, match := range protoRe.FindAllSubmatch(content, -1) { + switch { + case match[importSubexpIndex] != nil: + imp := unquoteProtoString(match[importSubexpIndex]) + info.imports = append(info.imports, imp) + + case match[packageSubexpIndex] != nil: + pkg := string(match[packageSubexpIndex]) + if info.packageName == "" { + info.packageName = strings.Replace(pkg, ".", "_", -1) + } + + case match[goPackageSubexpIndex] != nil: + gopkg := unquoteProtoString(match[goPackageSubexpIndex]) + if i := strings.LastIndexByte(gopkg, ';'); i != -1 { + info.importPath = gopkg[:i] + info.packageName = gopkg[i+1:] + } else { + info.importPath = gopkg + info.packageName = path.Base(gopkg) + } + + case match[serviceSubexpIndex] != nil: + info.hasServices = true + + default: + // Comment matched. Nothing to extract. + } + } + sort.Strings(info.imports) + + if info.packageName == "" { + stem := strings.TrimSuffix(name, ".proto") + fs := strings.FieldsFunc(stem, func(r rune) bool { + return !('A' <= r && r <= 'Z' || + 'a' <= r && r <= 'z' || + '0' <= r && r <= '9' || + r == '_') + }) + info.packageName = strings.Join(fs, "_") + } + + return info +} + +func buildProtoRegexp() *regexp.Regexp { + hexEscape := `\\[xX][0-9a-fA-f]{2}` + octEscape := `\\[0-7]{3}` + charEscape := `\\[abfnrtv'"\\]` + charValue := strings.Join([]string{hexEscape, octEscape, charEscape, "[^\x00\\'\\\"\\\\]"}, "|") + strLit := `'(?:` + charValue + `|")*'|"(?:` + charValue + `|')*"` + ident := `[A-Za-z][A-Za-z0-9_]*` + fullIdent := ident + `(?:\.` + ident + `)*` + importStmt := `\bimport\s*(?:public|weak)?\s*(?P` + strLit + `)\s*;` + packageStmt := `\bpackage\s*(?P` + fullIdent + `)\s*;` + goPackageStmt := `\boption\s*go_package\s*=\s*(?P` + strLit + `)\s*;` + serviceStmt := `(?Pservice)` + comment := `//[^\n]*` + protoReSrc := strings.Join([]string{importStmt, packageStmt, goPackageStmt, serviceStmt, comment}, "|") + return regexp.MustCompile(protoReSrc) +} + +func unquoteProtoString(q []byte) string { + // Adjust quotes so that Unquote is happy. We need a double quoted string + // without unescaped double quote characters inside. + noQuotes := bytes.Split(q[1:len(q)-1], []byte{'"'}) + if len(noQuotes) != 1 { + for i := 0; i < len(noQuotes)-1; i++ { + if len(noQuotes[i]) == 0 || noQuotes[i][len(noQuotes[i])-1] != '\\' { + noQuotes[i] = append(noQuotes[i], '\\') + } + } + q = append([]byte{'"'}, bytes.Join(noQuotes, []byte{'"'})...) + q = append(q, '"') + } + if q[0] == '\'' { + q[0] = '"' + q[len(q)-1] = '"' + } + + s, err := strconv.Unquote(string(q)) + if err != nil { + log.Panicf("unquoting string literal %s from proto: %v", q, err) + } + return s +} diff --git a/go/tools/gazelle/packages/fileinfo_proto_test.go b/go/tools/gazelle/packages/fileinfo_proto_test.go new file mode 100644 index 0000000000..f898ed8d46 --- /dev/null +++ b/go/tools/gazelle/packages/fileinfo_proto_test.go @@ -0,0 +1,147 @@ +/* Copyright 2017 The Bazel Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package packages + +import ( + "io/ioutil" + "os" + "reflect" + "testing" + + "github.com/bazelbuild/rules_go/go/tools/gazelle/config" +) + +func TestProtoRegexpGroupNames(t *testing.T) { + names := protoRe.SubexpNames() + nameMap := map[string]int{ + "import": importSubexpIndex, + "package": packageSubexpIndex, + "go_package": goPackageSubexpIndex, + "service": serviceSubexpIndex, + } + for name, index := range nameMap { + if names[index] != name { + t.Errorf("proto regexp subexp %d is %s ; want %s", index, names[index], name) + } + } + if len(names)-1 != len(nameMap) { + t.Errorf("proto regexp has %d groups ; want %d", len(names), len(nameMap)) + } +} + +func TestProtoFileInfo(t *testing.T) { + c := &config.Config{} + dir := "." + rel := "" + for _, tc := range []struct { + desc, name, proto string + want fileInfo + }{ + { + desc: "empty", + name: "empty^file.proto", + proto: "", + want: fileInfo{ + packageName: "empty_file", + }, + }, { + desc: "simple package", + name: "package.proto", + proto: "package foo;", + want: fileInfo{ + packageName: "foo", + }, + }, { + desc: "full package", + name: "full.proto", + proto: "package foo.bar.baz;", + want: fileInfo{ + packageName: "foo_bar_baz", + }, + }, { + desc: "import simple", + name: "imp.proto", + proto: `import 'single.proto'; +import "double.proto";`, + want: fileInfo{ + packageName: "imp", + imports: []string{"double.proto", "single.proto"}, + }, + }, { + desc: "import quote", + name: "quote.proto", + proto: `import '""\".proto"'; +import "'.proto";`, + want: fileInfo{ + packageName: "quote", + imports: []string{"\"\"\".proto\"", "'.proto"}, + }, + }, { + desc: "import escape", + name: "escape.proto", + proto: `import '\n\012\x0a.proto';`, + want: fileInfo{ + packageName: "escape", + imports: []string{"\n\n\n.proto"}, + }, + }, { + desc: "import two", + name: "two.proto", + proto: `import "first.proto"; +import "second.proto";`, + want: fileInfo{ + packageName: "two", + imports: []string{"first.proto", "second.proto"}, + }, + }, { + desc: "go_package", + name: "gopkg.proto", + proto: `option go_package = "github.com/example/project;projectpb";`, + want: fileInfo{ + packageName: "projectpb", + importPath: "github.com/example/project", + }, + }, { + desc: "service", + name: "service.proto", + proto: `service ChatService {}`, + want: fileInfo{ + packageName: "service", + hasServices: true, + }, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + if err := ioutil.WriteFile(tc.name, []byte(tc.proto), 0600); err != nil { + t.Fatal(err) + } + defer os.Remove(tc.name) + + got := protoFileInfo(c, dir, rel, tc.name) + + // Clear fields we don't care about for testing. + got = fileInfo{ + packageName: got.packageName, + imports: got.imports, + importPath: got.importPath, + hasServices: got.hasServices, + } + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("got %#v; want %#v", got, tc.want) + } + }) + } +} diff --git a/go/tools/gazelle/packages/fileinfo_test.go b/go/tools/gazelle/packages/fileinfo_test.go index a54f892f4a..92f2550738 100644 --- a/go/tools/gazelle/packages/fileinfo_test.go +++ b/go/tools/gazelle/packages/fileinfo_test.go @@ -22,172 +22,7 @@ import ( "reflect" "strings" "testing" - - "github.com/bazelbuild/rules_go/go/tools/gazelle/config" -) - -func TestGoFileInfo(t *testing.T) { - c := &config.Config{} - dir := "." - rel := "" - for _, tc := range []struct { - desc, name, source string - want fileInfo - }{ - { - "empty file", - "foo.go", - "package foo\n", - fileInfo{ - packageName: "foo", - }, - }, - { - "xtest file", - "foo_test.go", - "package foo_test\n", - fileInfo{ - packageName: "foo", - isTest: true, - isXTest: true, - }, - }, - { - "xtest suffix on non-test", - "foo_xtest.go", - "package foo_test\n", - fileInfo{ - packageName: "foo_test", - isTest: false, - isXTest: false, - }, - }, - { - "single import", - "foo.go", - `package foo - -import "github.com/foo/bar" -`, - fileInfo{ - packageName: "foo", - imports: []string{"github.com/foo/bar"}, - }, - }, - { - "multiple imports", - "foo.go", - `package foo - -import ( - "github.com/foo/bar" - x "github.com/local/project/y" ) -`, - fileInfo{ - packageName: "foo", - imports: []string{"github.com/foo/bar", "github.com/local/project/y"}, - }, - }, - { - "standard imports not included", - "foo.go", - `package foo - -import "fmt" -`, - fileInfo{ - packageName: "foo", - }, - }, - { - "cgo", - "foo.go", - `package foo - -import "C" -`, - fileInfo{ - packageName: "foo", - isCgo: true, - }, - }, - { - "build tags", - "foo.go", - `// +build linux darwin - -// +build !ignore - -package foo -`, - fileInfo{ - packageName: "foo", - tags: []string{"linux darwin", "!ignore"}, - }, - }, - { - "build tags without blank line", - "route.go", - `// Copyright 2017 - -// +build darwin dragonfly freebsd netbsd openbsd - -// Package route provides basic functions for the manipulation of -// packet routing facilities on BSD variants. -package route -`, - fileInfo{ - packageName: "route", - tags: []string{"darwin dragonfly freebsd netbsd openbsd"}, - }, - }, - } { - if err := ioutil.WriteFile(tc.name, []byte(tc.source), 0600); err != nil { - t.Fatal(err) - } - defer os.Remove(tc.name) - - got := goFileInfo(c, dir, rel, tc.name) - - // Clear fields we don't care about for testing. - got = fileInfo{ - packageName: got.packageName, - isTest: got.isTest, - isXTest: got.isXTest, - imports: got.imports, - isCgo: got.isCgo, - tags: got.tags, - } - - if !reflect.DeepEqual(got, tc.want) { - t.Errorf("case %q: got %#v; want %#v", tc.desc, got, tc.want) - } - } -} - -func TestGoFileInfoFailure(t *testing.T) { - dir := "." - name := "foo_linux_amd64.go" - if err := ioutil.WriteFile(name, []byte("pakcage foo"), 0600); err != nil { - t.Fatal(err) - } - defer os.Remove(name) - - c := &config.Config{} - got := goFileInfo(c, dir, "", name) - want := fileInfo{ - path: filepath.Join(dir, name), - name: name, - ext: ".go", - category: goExt, - goos: "linux", - goarch: "amd64", - } - if !reflect.DeepEqual(got, want) { - t.Errorf("got %#v ; want %#v", got, want) - } -} func TestOtherFileInfo(t *testing.T) { dir := "." @@ -442,215 +277,6 @@ func TestFileNameInfo(t *testing.T) { } } -func TestCgo(t *testing.T) { - c := &config.Config{} - dir := "." - rel := "" - for _, tc := range []struct { - desc, source string - want fileInfo - }{ - { - "not cgo", - "package foo\n", - fileInfo{isCgo: false}, - }, - { - "empty cgo", - `package foo - -import "C" -`, - fileInfo{isCgo: true}, - }, - { - "simple flags", - `package foo - -/* -#cgo CFLAGS: -O0 - #cgo CPPFLAGS: -O1 -#cgo CXXFLAGS: -O2 -#cgo LDFLAGS: -O3 -O4 -*/ -import "C" -`, - fileInfo{ - isCgo: true, - copts: []taggedOpts{ - {opts: []string{"-O0"}}, - {opts: []string{"-O1"}}, - {opts: []string{"-O2"}}, - }, - clinkopts: []taggedOpts{ - {opts: []string{"-O3", "-O4"}}, - }, - }, - }, - { - "cflags with conditions", - `package foo - -/* -#cgo foo bar,!baz CFLAGS: -O0 -*/ -import "C" -`, - fileInfo{ - isCgo: true, - copts: []taggedOpts{ - {tags: "foo bar,!baz", opts: []string{"-O0"}}, - }, - }, - }, - { - "slashslash comments", - `package foo - -// #cgo CFLAGS: -O0 -// #cgo CFLAGS: -O1 -import "C" -`, - fileInfo{ - isCgo: true, - copts: []taggedOpts{ - {opts: []string{"-O0"}}, - {opts: []string{"-O1"}}, - }, - }, - }, - { - "comment above single import group", - `package foo - -/* -#cgo CFLAGS: -O0 -*/ -import ("C") -`, - fileInfo{ - isCgo: true, - copts: []taggedOpts{ - {opts: []string{"-O0"}}, - }, - }, - }, - } { - path := "TestCgo.go" - if err := ioutil.WriteFile(path, []byte(tc.source), 0600); err != nil { - t.Fatal(err) - } - defer os.Remove(path) - - got := goFileInfo(c, dir, rel, path) - - // Clear fields we don't care about for testing. - got = fileInfo{isCgo: got.isCgo, copts: got.copts, clinkopts: got.clinkopts} - - if !reflect.DeepEqual(got, tc.want) { - t.Errorf("case %q: got %#v; want %#v", tc.desc, got, tc.want) - } - } -} - -// Copied from go/build build_test.go -var ( - expandSrcDirPath = filepath.Join(string(filepath.Separator)+"projects", "src", "add") -) - -// Copied from go/build build_test.go -var expandSrcDirTests = []struct { - input, expected string -}{ - {"-L ${SRCDIR}/libs -ladd", "-L /projects/src/add/libs -ladd"}, - {"${SRCDIR}/add_linux_386.a -pthread -lstdc++", "/projects/src/add/add_linux_386.a -pthread -lstdc++"}, - {"Nothing to expand here!", "Nothing to expand here!"}, - {"$", "$"}, - {"$$", "$$"}, - {"${", "${"}, - {"$}", "$}"}, - {"$FOO ${BAR}", "$FOO ${BAR}"}, - {"Find me the $SRCDIRECTORY.", "Find me the $SRCDIRECTORY."}, - {"$SRCDIR is missing braces", "$SRCDIR is missing braces"}, -} - -// Copied from go/build build_test.go -func TestExpandSrcDir(t *testing.T) { - for _, test := range expandSrcDirTests { - output, _ := expandSrcDir(test.input, expandSrcDirPath) - if output != test.expected { - t.Errorf("%q expands to %q with SRCDIR=%q when %q is expected", test.input, output, expandSrcDirPath, test.expected) - } else { - t.Logf("%q expands to %q with SRCDIR=%q", test.input, output, expandSrcDirPath) - } - } -} - -func TestExpandSrcDirRepoRelative(t *testing.T) { - repo, err := ioutil.TempDir(os.Getenv("TEST_TEMPDIR"), "repo") - if err != nil { - t.Fatal(err) - } - sub := filepath.Join(repo, "sub") - if err := os.Mkdir(sub, 0755); err != nil { - t.Fatal(err) - } - goFile := filepath.Join(sub, "sub.go") - content := []byte(`package sub - -/* -#cgo CFLAGS: -I${SRCDIR}/.. -*/ -import "C" -`) - if err := ioutil.WriteFile(goFile, content, 0644); err != nil { - t.Fatal(err) - } - c := &config.Config{RepoRoot: repo} - got := buildPackage(c, sub, []string{"sub.go"}, nil, nil, false) - want := &Package{ - Name: "sub", - Dir: sub, - Rel: "sub", - Library: Target{ - Sources: PlatformStrings{ - Generic: []string{"sub.go"}, - }, - COpts: PlatformStrings{ - Generic: []string{"-Isub/..", optSeparator}, - }, - Cgo: true, - }, - } - if !reflect.DeepEqual(got, want) { - t.Errorf("got %#v ; want %#v", got, want) - } -} - -// Copied from go/build build_test.go -func TestShellSafety(t *testing.T) { - tests := []struct { - input, srcdir, expected string - result bool - }{ - {"-I${SRCDIR}/../include", "/projects/src/issue 11868", "-I/projects/src/issue 11868/../include", true}, - {"-I${SRCDIR}", "wtf$@%", "-Iwtf$@%", true}, - {"-X${SRCDIR}/1,${SRCDIR}/2", "/projects/src/issue 11868", "-X/projects/src/issue 11868/1,/projects/src/issue 11868/2", true}, - {"-I/tmp -I/tmp", "/tmp2", "-I/tmp -I/tmp", false}, - {"-I/tmp", "/tmp/[0]", "-I/tmp", true}, - {"-I${SRCDIR}/dir", "/tmp/[0]", "-I/tmp/[0]/dir", false}, - } - for _, test := range tests { - output, ok := expandSrcDir(test.input, test.srcdir) - if ok != test.result { - t.Errorf("Expected %t while %q expands to %q with SRCDIR=%q; got %t", test.result, test.input, output, test.srcdir, ok) - } - if output != test.expected { - t.Errorf("Expected %q while %q expands with SRCDIR=%q; got %q", test.expected, test.input, test.srcdir, output) - } - } -} - func TestJoinOptions(t *testing.T) { for _, tc := range []struct { opts, want []string diff --git a/go/tools/gazelle/packages/package.go b/go/tools/gazelle/packages/package.go index 746b2a0dcd..5d7e367786 100644 --- a/go/tools/gazelle/packages/package.go +++ b/go/tools/gazelle/packages/package.go @@ -40,20 +40,29 @@ type Package struct { // Components in Rel are separated with slashes. Rel string - Library, Binary, Test, XTest Target + Library, Binary, Test, XTest GoTarget + Proto ProtoTarget - Protos []string - HasPbGo bool HasTestdata bool } -// Target contains metadata about a buildable Go target in a package. -type Target struct { +// GoTarget contains metadata about a buildable Go target in a package. +type GoTarget struct { Sources, Imports PlatformStrings COpts, CLinkOpts PlatformStrings Cgo bool } +// ProtoTarget contains metadata about proto files in a package. +type ProtoTarget struct { + Sources, Imports PlatformStrings + HasServices bool + + // HasPbGo indicates whether unexcluded .pb.go files are present in the + // same package. They will not be in this target's sources. + HasPbGo bool +} + // PlatformStrings contains a set of strings associated with a buildable // Go target in a package. This is used to store source file names, // import paths, and flags. @@ -71,11 +80,12 @@ func (p *Package) IsCommand() bool { return p.Name == "main" } -// HasGo returns true if at least one target in the package contains a -// .go source file. If a package does not contain Go code, Gazelle will -// not generate rules for it. -func (p *Package) HasGo() bool { - return p.Library.HasGo() || p.Binary.HasGo() || p.Test.HasGo() || p.XTest.HasGo() +// isBuildable returns true if anything in the package is buildable. +// This is true if the package has Go code that satisfies build constraints +// on any platform or has proto files not in legacy mode. +func (p *Package) isBuildable(c *config.Config) bool { + return p.Library.HasGo() || p.Binary.HasGo() || p.Test.HasGo() || p.XTest.HasGo() || + p.Proto.HasProto() && c.ProtoMode == config.DefaultProtoMode } // ImportPath returns the inferred Go import path for this package. This @@ -113,14 +123,18 @@ func (p *Package) firstGoFile() string { return p.XTest.firstGoFile() } -func (t *Target) HasGo() bool { +func (t *GoTarget) HasGo() bool { return t.Sources.HasGo() } -func (t *Target) firstGoFile() string { +func (t *GoTarget) firstGoFile() string { return t.Sources.firstGoFile() } +func (t *ProtoTarget) HasProto() bool { + return !t.Sources.IsEmpty() +} + func (ts *PlatformStrings) HasGo() bool { return ts.firstGoFile() != "" } @@ -165,7 +179,8 @@ func (ts *PlatformStrings) firstGoFile() string { func (p *Package) addFile(c *config.Config, info fileInfo, cgo bool) error { switch { case info.category == ignoredExt || info.category == unsupportedExt || - !cgo && (info.category == cExt || info.category == csExt): + !cgo && (info.category == cExt || info.category == csExt) || + c.ProtoMode == config.DisableProtoMode && info.category == protoExt: return nil case info.isXTest: if info.isCgo { @@ -178,18 +193,18 @@ func (p *Package) addFile(c *config.Config, info fileInfo, cgo bool) error { } p.Test.addFile(c, info) case info.category == protoExt: - p.Protos = append(p.Protos, info.name) + p.Proto.addFile(c, info) default: p.Library.addFile(c, info) } if strings.HasSuffix(info.name, ".pb.go") { - p.HasPbGo = true + p.Proto.HasPbGo = true } return nil } -func (t *Target) addFile(c *config.Config, info fileInfo) { +func (t *GoTarget) addFile(c *config.Config, info fileInfo) { if info.isCgo { t.Cgo = true } @@ -211,6 +226,12 @@ func (t *Target) addFile(c *config.Config, info fileInfo) { } } +func (t *ProtoTarget) addFile(c *config.Config, info fileInfo) { + t.Sources.addGenericStrings(info.name) + t.Imports.addGenericStrings(info.imports...) + t.HasServices = t.HasServices || info.hasServices +} + func (ps *PlatformStrings) addGenericStrings(ss ...string) { ps.Generic = append(ps.Generic, ss...) } diff --git a/go/tools/gazelle/packages/package_test.go b/go/tools/gazelle/packages/package_test.go index 9da6931349..6fa3eeb848 100644 --- a/go/tools/gazelle/packages/package_test.go +++ b/go/tools/gazelle/packages/package_test.go @@ -49,7 +49,7 @@ func TestImportPath(t *testing.T) { pkg := Package{ Name: path.Base(tc.rel), Rel: tc.rel, - Library: Target{ + Library: GoTarget{ Sources: PlatformStrings{ Generic: []string{"a.go"}, }, @@ -76,7 +76,7 @@ func TestImportPathCmd(t *testing.T) { pkg := Package{ Name: "main", Rel: "foo/bar", - Library: Target{ + Library: GoTarget{ Sources: PlatformStrings{ Generic: []string{"main.go"}, }, diff --git a/go/tools/gazelle/packages/walk.go b/go/tools/gazelle/packages/walk.go index f58b38d7dd..22a5fb5493 100644 --- a/go/tools/gazelle/packages/walk.go +++ b/go/tools/gazelle/packages/walk.go @@ -83,14 +83,17 @@ func Walk(c *config.Config, dir string, f WalkFunc) { } // Process directives in the build file. - excluded := make(map[string]bool) + var directives []config.Directive if oldFile != nil { - directives := config.ParseDirectives(oldFile) + directives = config.ParseDirectives(oldFile) c = config.ApplyDirectives(c, directives) - for _, d := range directives { - if d.Key == "exclude" { - excluded[d.Value] = true - } + } + c = config.InferProtoMode(c, oldFile, directives) + + excluded := make(map[string]bool) + for _, d := range directives { + if d.Key == "exclude" { + excluded[d.Value] = true } } @@ -100,21 +103,25 @@ func Walk(c *config.Config, dir string, f WalkFunc) { log.Print(err) return false } + if c.ProtoMode == config.DefaultProtoMode { + excludePbGoFiles(files, excluded) + } - var goFiles, otherFiles, subdirs []string + var pkgFiles, otherFiles, subdirs []string for _, f := range files { base := f.Name() switch { case base == "" || base[0] == '.' || base[0] == '_' || - excluded != nil && excluded[base] || + excluded[base] || base == "vendor" && f.IsDir() && c.DepMode != config.VendorMode: continue case f.IsDir(): subdirs = append(subdirs, base) - case strings.HasSuffix(base, ".go"): - goFiles = append(goFiles, base) + case strings.HasSuffix(base, ".go") || + (c.ProtoMode != config.DisableProtoMode && strings.HasSuffix(base, ".proto")): + pkgFiles = append(pkgFiles, base) default: otherFiles = append(otherFiles, base) @@ -142,7 +149,7 @@ func Walk(c *config.Config, dir string, f WalkFunc) { if oldFile != nil { genFiles = findGenFiles(oldFile, excluded) } - pkg := buildPackage(c, path, goFiles, otherFiles, genFiles, hasTestdata) + pkg := buildPackage(c, path, pkgFiles, otherFiles, genFiles, hasTestdata) if pkg != nil { f(c, pkg, oldFile) hasPackage = true @@ -161,7 +168,7 @@ func Walk(c *config.Config, dir string, f WalkFunc) { // name matches the directory base name will be returned. If there is no such // package or if an error occurs, an error will be logged, and nil will be // returned. -func buildPackage(c *config.Config, dir string, goFiles, otherFiles, genFiles []string, hasTestdata bool) *Package { +func buildPackage(c *config.Config, dir string, pkgFiles, otherFiles, genFiles []string, hasTestdata bool) *Package { rel, err := filepath.Rel(c.RepoRoot, dir) if err != nil { log.Print(err) @@ -172,14 +179,22 @@ func buildPackage(c *config.Config, dir string, goFiles, otherFiles, genFiles [] rel = "" } - // Process the .go files first. + // Process .go and .proto files first, since these determine the package name. packageMap := make(map[string]*Package) cgo := false - var goFilesWithUnknownPackage []fileInfo - for _, goFile := range goFiles { - info := goFileInfo(c, dir, rel, goFile) + var pkgFilesWithUnknownPackage []fileInfo + for _, f := range pkgFiles { + var info fileInfo + switch path.Ext(f) { + case ".go": + info = goFileInfo(c, dir, rel, f) + case ".proto": + info = protoFileInfo(c, dir, rel, f) + default: + log.Panicf("file cannot determine package name: %s", f) + } if info.packageName == "" { - goFilesWithUnknownPackage = append(goFilesWithUnknownPackage, info) + pkgFilesWithUnknownPackage = append(pkgFilesWithUnknownPackage, info) continue } if info.packageName == "documentation" { @@ -212,11 +227,11 @@ func buildPackage(c *config.Config, dir string, goFiles, otherFiles, genFiles [] return nil } - // Add .go files with unknown packages. This happens when there are parse + // Add files with unknown packages. This happens when there are parse // or I/O errors. We should keep the file in the srcs list and let the // compiler deal with the error. - for _, goFile := range goFilesWithUnknownPackage { - pkg.addFile(c, goFile, cgo) + for _, info := range pkgFilesWithUnknownPackage { + pkg.addFile(c, info, cgo) } // Process the other static files. @@ -232,7 +247,7 @@ func buildPackage(c *config.Config, dir string, goFiles, otherFiles, genFiles [] // as static files. Bazel will use the generated files, but we will look at // the content of static files, assuming they will be the same. staticFiles := make(map[string]bool) - for _, f := range goFiles { + for _, f := range pkgFiles { staticFiles[f] = true } for _, f := range otherFiles { @@ -253,29 +268,29 @@ func buildPackage(c *config.Config, dir string, goFiles, otherFiles, genFiles [] } func selectPackage(c *config.Config, dir string, packageMap map[string]*Package) (*Package, error) { - packagesWithGo := make(map[string]*Package) + buildablePackages := make(map[string]*Package) for name, pkg := range packageMap { - if pkg.HasGo() { - packagesWithGo[name] = pkg + if pkg.isBuildable(c) { + buildablePackages[name] = pkg } } - if len(packagesWithGo) == 0 { + if len(buildablePackages) == 0 { return nil, &build.NoGoError{Dir: dir} } - if len(packagesWithGo) == 1 { - for _, pkg := range packagesWithGo { + if len(buildablePackages) == 1 { + for _, pkg := range buildablePackages { return pkg, nil } } - if pkg, ok := packagesWithGo[defaultPackageName(c, dir)]; ok { + if pkg, ok := buildablePackages[defaultPackageName(c, dir)]; ok { return pkg, nil } err := &build.MultiplePackageError{Dir: dir} - for name, pkg := range packagesWithGo { + for name, pkg := range buildablePackages { // Add the first file for each package for the error message. // Error() method expects these lists to be the same length. File // lists must be non-empty. These lists are only created by @@ -323,3 +338,15 @@ func findGenFiles(f *bf.File, excluded map[string]bool) []string { } return genFiles } + +func excludePbGoFiles(files []os.FileInfo, excluded map[string]bool) { + for _, f := range files { + name := f.Name() + if excluded[name] { + continue + } + if strings.HasSuffix(name, ".proto") { + excluded[name[:len(name)-len(".proto")]+".pb.go"] = true + } + } +} diff --git a/go/tools/gazelle/packages/walk_test.go b/go/tools/gazelle/packages/walk_test.go index 4e80de04e5..2866ca9416 100644 --- a/go/tools/gazelle/packages/walk_test.go +++ b/go/tools/gazelle/packages/walk_test.go @@ -118,7 +118,7 @@ func TestWalkSimple(t *testing.T) { want := []*packages.Package{ { Name: "lib", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"lib.go"}, }, @@ -138,7 +138,7 @@ func TestWalkNested(t *testing.T) { { Name: "a", Rel: "a", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"foo.go"}, }, @@ -147,7 +147,7 @@ func TestWalkNested(t *testing.T) { { Name: "c", Rel: "b/c", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"bar.go"}, }, @@ -156,7 +156,7 @@ func TestWalkNested(t *testing.T) { { Name: "main", Rel: "b/d", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"baz.go"}, }, @@ -166,6 +166,24 @@ func TestWalkNested(t *testing.T) { checkFiles(t, files, "", want) } +func TestProtoOnly(t *testing.T) { + files := []fileSpec{ + {path: "a/a.proto"}, + } + want := []*packages.Package{ + { + Name: "a", + Rel: "a", + Proto: packages.ProtoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"a.proto"}, + }, + }, + }, + } + checkFiles(t, files, "", want) +} + func TestMultiplePackagesWithDefault(t *testing.T) { files := []fileSpec{ {path: "a/a.go", content: "package a"}, @@ -175,7 +193,7 @@ func TestMultiplePackagesWithDefault(t *testing.T) { { Name: "a", Rel: "a", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -202,6 +220,27 @@ func TestMultiplePackagesWithoutDefault(t *testing.T) { } } +func TestMultiplePackagesWithProtoDefault(t *testing.T) { + files := []fileSpec{ + {path: "a/a.proto", content: `syntax = "proto2"; +package a; +`}, + {path: "a/b.go", content: "package b"}, + } + want := []*packages.Package{ + { + Name: "a", + Rel: "a", + Proto: packages.ProtoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"a.proto"}, + }, + }, + }, + } + checkFiles(t, files, "", want) +} + func TestRootWithPrefix(t *testing.T) { files := []fileSpec{ {path: "a.go", content: "package a"}, @@ -210,7 +249,7 @@ func TestRootWithPrefix(t *testing.T) { want := []*packages.Package{ { Name: "a", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -254,7 +293,7 @@ func TestTestdata(t *testing.T) { { Name: "raw", Rel: "raw", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -264,7 +303,7 @@ func TestTestdata(t *testing.T) { { Name: "with_build", Rel: "with_build", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -274,7 +313,7 @@ func TestTestdata(t *testing.T) { { Name: "with_build_bazel", Rel: "with_build_bazel", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -284,7 +323,7 @@ func TestTestdata(t *testing.T) { { Name: "with_build_nested", Rel: "with_build_nested", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -294,7 +333,7 @@ func TestTestdata(t *testing.T) { { Name: "testdata", Rel: "with_go/testdata", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -303,7 +342,7 @@ func TestTestdata(t *testing.T) { { Name: "with_go", Rel: "with_go", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"a.go"}, }, @@ -342,7 +381,7 @@ import "github.com/jr_hacker/stuff" { Name: "foo", Rel: "gen", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"foo.go", "bar.go", "y.s", "baz.go"}, }, @@ -385,7 +424,7 @@ import "github.com/jr_hacker/stuff" { Name: "foo", Rel: "gen", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"foo.go", "bar.go", "x.c", "y.s", "z.S", "baz.go"}, }, @@ -436,7 +475,7 @@ genrule( { Name: "exclude", Rel: "exclude", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"real.go"}, }, @@ -446,6 +485,113 @@ genrule( checkFiles(t, files, "", want) } +func TestExcludedPbGo(t *testing.T) { + files := []fileSpec{ + { + path: "exclude/BUILD", + content: ` +# gazelle:exclude a.proto +`, + }, + { + path: "exclude/a.proto", + content: `syntax = "proto2"; +package exclude;`, + }, + { + path: "exclude/a.pb.go", + content: `package exclude`, + }, + { + path: "exclude/b.proto", + content: `syntax = "proto2"; +package exclude; +`, + }, + { + path: "exclude/b.pb.go", + content: `package exclude`, + }, + } + want := []*packages.Package{ + { + Name: "exclude", + Rel: "exclude", + Library: packages.GoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"a.pb.go"}, + }, + }, + Proto: packages.ProtoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"b.proto"}, + }, + HasPbGo: true, + }, + }, + } + checkFiles(t, files, "", want) +} + +func TestLegacyProtos(t *testing.T) { + files := []fileSpec{ + { + path: "BUILD", + content: `# gazelle:proto legacy`, + }, { + path: "have_pbgo/a.proto", + content: `syntax = "proto2"; +package have_pbgo;`, + }, { + path: "have_pbgo/a.pb.go", + content: `package have_pbgo`, + }, { + path: "no_pbgo/b.proto", + content: `syntax = "proto2"; +package no_pbgo;`, + }, { + path: "no_pbgo/other.go", + content: `package no_pbgo`, + }, { + path: "proto_only/c.proto", + content: `syntax = "proto2"; +package proto_only;`, + }, + } + want := []*packages.Package{ + { + Name: "have_pbgo", + Rel: "have_pbgo", + Library: packages.GoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"a.pb.go"}, + }, + }, + Proto: packages.ProtoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"a.proto"}, + }, + HasPbGo: true, + }, + }, { + Name: "no_pbgo", + Rel: "no_pbgo", + Library: packages.GoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"other.go"}, + }, + }, + Proto: packages.ProtoTarget{ + Sources: packages.PlatformStrings{ + Generic: []string{"b.proto"}, + }, + HasPbGo: false, + }, + }, + } + checkFiles(t, files, "", want) +} + func TestVendor(t *testing.T) { files := []fileSpec{ {path: "vendor/foo/foo.go", content: "package foo"}, @@ -469,7 +615,7 @@ func TestVendor(t *testing.T) { { Name: "foo", Rel: "vendor/foo", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"foo.go"}, }, @@ -478,7 +624,7 @@ func TestVendor(t *testing.T) { { Name: "bar", Rel: "x/vendor/bar", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"bar.go"}, }, @@ -540,7 +686,7 @@ func TestMalformedGoFile(t *testing.T) { want := []*packages.Package{ { Name: "foo", - Library: packages.Target{ + Library: packages.GoTarget{ Sources: packages.PlatformStrings{ Generic: []string{"b.go", "a.go"}, }, diff --git a/go/tools/gazelle/resolve/labeler.go b/go/tools/gazelle/resolve/labeler.go index 0b9ed80a42..ff55b1db03 100644 --- a/go/tools/gazelle/resolve/labeler.go +++ b/go/tools/gazelle/resolve/labeler.go @@ -28,6 +28,8 @@ type Labeler interface { LibraryLabel(rel string) Label TestLabel(rel string, isXTest bool) Label BinaryLabel(rel string) Label + ProtoLabel(rel, name string) Label + GoProtoLabel(rel, name string) Label } func NewLabeler(c *config.Config) Labeler { @@ -60,6 +62,14 @@ func (l *hierarchicalLabeler) BinaryLabel(rel string) Label { return Label{Pkg: rel, Name: name} } +func (l *hierarchicalLabeler) ProtoLabel(rel, name string) Label { + return Label{Pkg: rel, Name: name + "_proto"} +} + +func (l *hierarchicalLabeler) GoProtoLabel(rel, name string) Label { + return Label{Pkg: rel, Name: name + "_go_proto"} +} + type flatLabeler struct { c *config.Config } @@ -92,6 +102,14 @@ func (l *flatLabeler) BinaryLabel(rel string) Label { return Label{Name: rel + suffix} } +func (l *flatLabeler) ProtoLabel(rel, name string) Label { + return Label{Name: path.Join(rel, name) + "_proto"} +} + +func (l *flatLabeler) GoProtoLabel(rel, name string) Label { + return Label{Name: path.Join(rel, name) + "_go_proto"} +} + func relBaseName(c *config.Config, rel string) string { base := path.Base(rel) if base == "." || base == "/" { diff --git a/go/tools/gazelle/resolve/labeler_test.go b/go/tools/gazelle/resolve/labeler_test.go index 2a4f44c606..4f7f5a4619 100644 --- a/go/tools/gazelle/resolve/labeler_test.go +++ b/go/tools/gazelle/resolve/labeler_test.go @@ -21,7 +21,7 @@ import ( "github.com/bazelbuild/rules_go/go/tools/gazelle/config" ) -func TestLabeler(t *testing.T) { +func TestLabelerGo(t *testing.T) { for _, tc := range []struct { name, rel string mode config.StructureMode @@ -88,3 +88,53 @@ func TestLabeler(t *testing.T) { }) } } + +func TestLabelerProto(t *testing.T) { + for _, tc := range []struct { + desc, rel, name string + mode config.StructureMode + wantProto, wantGoProto string + }{ + { + desc: "root_hierarchical", + rel: "", + name: "foo", + mode: config.HierarchicalMode, + wantProto: "//:foo_proto", + wantGoProto: "//:foo_go_proto", + }, { + desc: "sub_hierarchical", + rel: "sub", + name: "foo", + mode: config.HierarchicalMode, + wantProto: "//sub:foo_proto", + wantGoProto: "//sub:foo_go_proto", + }, { + desc: "root_flat", + rel: "", + name: "foo", + mode: config.FlatMode, + wantProto: "//:foo_proto", + wantGoProto: "//:foo_go_proto", + }, { + desc: "sub_flat", + rel: "sub", + name: "foo", + mode: config.FlatMode, + wantProto: "//:sub/foo_proto", + wantGoProto: "//:sub/foo_go_proto", + }, + } { + t.Run(tc.desc, func(t *testing.T) { + c := &config.Config{StructureMode: tc.mode} + l := NewLabeler(c) + + if got := l.ProtoLabel(tc.rel, tc.name).String(); got != tc.wantProto { + t.Errorf("for proto_library in %s: got %q ; want %q", tc.rel, got, tc.wantProto) + } + if got := l.GoProtoLabel(tc.rel, tc.name).String(); got != tc.wantGoProto { + t.Errorf("for go_proto_library in %s: got %q ; want %q", tc.rel, got, tc.wantGoProto) + } + }) + } +} diff --git a/go/tools/gazelle/resolve/resolve.go b/go/tools/gazelle/resolve/resolve.go index 8ca9c663f4..5ae044699f 100644 --- a/go/tools/gazelle/resolve/resolve.go +++ b/go/tools/gazelle/resolve/resolve.go @@ -79,3 +79,67 @@ func (r *Resolver) ResolveGo(imp, pkgRel string) (Label, error) { } return r.l.LibraryLabel(strings.TrimPrefix(imp, r.c.GoPrefix+"/")), nil } + +const ( + wellKnownPrefix = "google/protobuf/" + wellKnownGoProtoPkg = "ptypes" +) + +// ResolveProto resolves an import statement in a .proto file to a label +// for a proto_library rule. +func (r *Resolver) ResolveProto(imp string) (Label, error) { + if !strings.HasSuffix(imp, ".proto") { + return Label{}, fmt.Errorf("can't import non-proto: %q", imp) + } + imp = imp[:len(imp)-len(".proto")] + + if isWellKnown(imp) { + // Well Known Type + name := path.Base(imp) + "_proto" + return Label{Repo: config.WellKnownTypesProtoRepo, Name: name}, nil + } + + // Temporary hack: guess the label based on the proto file name. We assume + // all proto files in a directory belong to the same package, and the + // package name matches the directory base name. + // TODO(#859): use dependency table to resolve once it exists. + rel := path.Dir(imp) + if rel == "." { + rel = "" + } + name := relBaseName(r.c, rel) + return r.l.ProtoLabel(rel, name), nil +} + +// ResolveGoProto resolves an import statement in a .proto file to a +// label for a go_library rule that embeds the corresponding go_proto_library. +func (r *Resolver) ResolveGoProto(imp string) (Label, error) { + if !strings.HasSuffix(imp, ".proto") { + return Label{}, fmt.Errorf("can't import non-proto: %q", imp) + } + imp = imp[:len(imp)-len(".proto")] + + if isWellKnown(imp) { + // Well Known Type + pkg := path.Join(wellKnownGoProtoPkg, path.Base(imp)) + label := r.l.LibraryLabel(pkg) + if r.c.GoPrefix != config.WellKnownTypesGoPrefix { + label.Repo = config.WellKnownTypesGoProtoRepo + } + return label, nil + } + + // Temporary hack: guess the label based on the proto file name. We assume + // all proto files in a directory belong to the same package, and the + // package name matches the directory base name. + // TODO(#859): use dependency table to resolve once it exists. + rel := path.Dir(imp) + if rel == "." { + rel = "" + } + return r.l.LibraryLabel(rel), nil +} + +func isWellKnown(imp string) bool { + return strings.HasPrefix(imp, wellKnownPrefix) && strings.TrimPrefix(imp, wellKnownPrefix) == path.Base(imp) +} diff --git a/go/tools/gazelle/resolve/resolve_test.go b/go/tools/gazelle/resolve/resolve_test.go index 4c8495a5cb..360d34f92f 100644 --- a/go/tools/gazelle/resolve/resolve_test.go +++ b/go/tools/gazelle/resolve/resolve_test.go @@ -122,3 +122,63 @@ func TestResolveGoLocalError(t *testing.T) { t.Errorf("r.ResolveGo(%q) = %s; want error", "..", l) } } + +func TestResolveProto(t *testing.T) { + prefix := "example.com/repo" + for _, tc := range []struct { + desc, imp string + mode config.StructureMode + wantProto, wantGoProto Label + }{ + { + desc: "root", + imp: "foo.proto", + wantProto: Label{Name: "repo_proto"}, + wantGoProto: Label{Name: config.DefaultLibName}, + }, { + desc: "sub", + imp: "foo/bar/bar.proto", + wantProto: Label{Pkg: "foo/bar", Name: "bar_proto"}, + wantGoProto: Label{Pkg: "foo/bar", Name: config.DefaultLibName}, + }, { + desc: "flat sub", + mode: config.FlatMode, + imp: "foo/bar/bar.proto", + wantProto: Label{Name: "foo/bar/bar_proto"}, + wantGoProto: Label{Name: "foo/bar"}, + }, { + desc: "well known", + imp: "google/protobuf/any.proto", + wantProto: Label{Repo: "com_google_protobuf", Name: "any_proto"}, + wantGoProto: Label{Repo: "com_github_golang_protobuf", Pkg: "ptypes/any", Name: config.DefaultLibName}, + }, { + desc: "well known flat", + mode: config.FlatMode, + imp: "google/protobuf/any.proto", + wantProto: Label{Repo: "com_google_protobuf", Name: "any_proto"}, + wantGoProto: Label{Repo: "com_github_golang_protobuf", Name: "ptypes/any"}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + c := &config.Config{GoPrefix: prefix, StructureMode: tc.mode} + l := NewLabeler(c) + r := NewResolver(c, l) + + got, err := r.ResolveProto(tc.imp) + if err != nil { + t.Errorf("ResolveProto: got error %v ; want success", err) + } + if !reflect.DeepEqual(got, tc.wantProto) { + t.Errorf("ResolveProto: got %s ; want %s", got, tc.wantProto) + } + + got, err = r.ResolveGoProto(tc.imp) + if err != nil { + t.Errorf("ResolveGoProto: go error %v ; want success", err) + } + if !reflect.DeepEqual(got, tc.wantGoProto) { + t.Errorf("ResolveGoProto: got %s ; want %s", got, tc.wantGoProto) + } + }) + } +} diff --git a/go/tools/gazelle/rules/generator.go b/go/tools/gazelle/rules/generator.go index 1779d77785..f8f3deda53 100644 --- a/go/tools/gazelle/rules/generator.go +++ b/go/tools/gazelle/rules/generator.go @@ -50,12 +50,15 @@ type Generator struct { func (g *Generator) GenerateRules(pkg *packages.Package) (rules []bf.Expr, empty []bf.Expr) { var rs []bf.Expr - library, r := g.generateLib(pkg) + protoLibName, protoRules := g.generateProto(pkg) + rs = append(rs, protoRules...) + + libName, libRule := g.generateLib(pkg, protoLibName) + rs = append(rs, libRule) + rs = append(rs, - r, - g.generateBin(pkg, library), - g.filegroup(pkg), - g.generateTest(pkg, library, false), + g.generateBin(pkg, libName), + g.generateTest(pkg, libName, false), g.generateTest(pkg, "", true)) for _, r := range rs { @@ -68,6 +71,80 @@ func (g *Generator) GenerateRules(pkg *packages.Package) (rules []bf.Expr, empty return rules, empty } +func (g *Generator) generateProto(pkg *packages.Package) (string, []bf.Expr) { + if g.c.ProtoMode == config.DisableProtoMode { + // Don't create or delete proto rules in this mode. Any existing rules + // are likely hand-written. + return "", nil + } + + filegroupName := config.DefaultProtosName + protoName := g.l.ProtoLabel(pkg.Rel, pkg.Name).Name + goProtoName := g.l.GoProtoLabel(pkg.Rel, pkg.Name).Name + + if g.c.ProtoMode == config.LegacyProtoMode { + if !pkg.Proto.HasProto() { + return "", []bf.Expr{emptyRule("filegroup", filegroupName)} + } + return "", []bf.Expr{ + newRule("filegroup", []keyvalue{ + {key: "name", value: filegroupName}, + {key: "srcs", value: g.sources(pkg.Proto.Sources, pkg.Rel)}, + {key: "visibility", value: []string{"//visibility:public"}}, + }), + } + } + + if !pkg.Proto.HasProto() { + return "", []bf.Expr{ + emptyRule("filegroup", filegroupName), + emptyRule("proto_library", protoName), + emptyRule("go_proto_library", goProtoName), + emptyRule("go_grpc_library", goProtoName), + } + } + + var rules []bf.Expr + visibility := []string{checkInternalVisibility(pkg.Rel, "//visibility:public")} + protoAttrs := []keyvalue{ + {"name", protoName}, + {"srcs", g.sources(pkg.Proto.Sources, pkg.Rel)}, + {"visibility", visibility}, + } + if !pkg.Proto.Imports.IsEmpty() { + protoAttrs = append(protoAttrs, + keyvalue{"deps", g.dependencies(pkg.Proto.Imports, g.r.ResolveProto)}) + } + rules = append(rules, newRule("proto_library", protoAttrs)) + + goProtoAttrs := []keyvalue{ + {"name", goProtoName}, + {"proto", ":" + protoName}, + {"importpath", pkg.ImportPath(g.c.GoPrefix)}, + {"visibility", visibility}, + } + if !pkg.Proto.Imports.IsEmpty() { + goProtoAttrs = append(goProtoAttrs, + keyvalue{"deps", g.dependencies(pkg.Proto.Imports, g.r.ResolveGoProto)}) + } + + // If a developer adds or removes services from existing protos, this + // will create a new rule and delete the old one, along with any custom + // attributes (assuming no keep comments). We can't currently merge + // rules unless both kind and name match. + if pkg.Proto.HasServices { + rules = append(rules, + newRule("go_grpc_library", goProtoAttrs), + emptyRule("go_proto_library", goProtoName)) + } else { + rules = append(rules, + newRule("go_proto_library", goProtoAttrs), + emptyRule("go_grpc_library", goProtoName)) + } + + return goProtoName, rules +} + func (g *Generator) generateBin(pkg *packages.Package, library string) bf.Expr { name := g.l.BinaryLabel(pkg.Rel).Name if !pkg.IsCommand() || pkg.Binary.Sources.IsEmpty() && library == "" { @@ -84,9 +161,9 @@ func (g *Generator) generateBin(pkg *packages.Package, library string) bf.Expr { return newRule("go_binary", attrs) } -func (g *Generator) generateLib(pkg *packages.Package) (string, *bf.CallExpr) { +func (g *Generator) generateLib(pkg *packages.Package, goProtoName string) (string, *bf.CallExpr) { name := g.l.LibraryLabel(pkg.Rel).Name - if !pkg.Library.HasGo() { + if !pkg.Library.HasGo() && goProtoName == "" { return "", emptyRule("go_library", name) } var visibility string @@ -99,6 +176,9 @@ func (g *Generator) generateLib(pkg *packages.Package) (string, *bf.CallExpr) { attrs := g.commonAttrs(pkg.Rel, name, visibility, pkg.Library) attrs = append(attrs, keyvalue{"importpath", pkg.ImportPath(g.c.GoPrefix)}) + if goProtoName != "" { + attrs = append(attrs, keyvalue{"library", ":" + goProtoName}) + } rule := newRule("go_library", attrs) return name, rule @@ -132,21 +212,6 @@ func checkInternalVisibility(rel, visibility string) string { return visibility } -// filegroup is a small hack for directories with pre-generated .pb.go files -// and also source .proto files. This creates a filegroup for the .proto in -// addition to the usual go_library for the .pb.go files. -func (g *Generator) filegroup(pkg *packages.Package) bf.Expr { - name := config.DefaultProtosName - if !pkg.HasPbGo || len(pkg.Protos) == 0 { - return emptyRule("filegroup", name) - } - return newRule("filegroup", []keyvalue{ - {key: "name", value: config.DefaultProtosName}, - {key: "srcs", value: pkg.Protos}, - {key: "visibility", value: []string{"//visibility:public"}}, - }) -} - func (g *Generator) generateTest(pkg *packages.Package, library string, isXTest bool) bf.Expr { name := g.l.TestLabel(pkg.Rel, isXTest).Name target := pkg.Test @@ -175,7 +240,7 @@ func (g *Generator) generateTest(pkg *packages.Package, library string, isXTest return newRule("go_test", attrs) } -func (g *Generator) commonAttrs(pkgRel, name, visibility string, target packages.Target) []keyvalue { +func (g *Generator) commonAttrs(pkgRel, name, visibility string, target packages.GoTarget) []keyvalue { attrs := []keyvalue{{"name", name}} if !target.Sources.IsEmpty() { attrs = append(attrs, keyvalue{"srcs", g.sources(target.Sources, pkgRel)}) @@ -193,8 +258,10 @@ func (g *Generator) commonAttrs(pkgRel, name, visibility string, target packages attrs = append(attrs, keyvalue{"visibility", []string{visibility}}) } if !target.Imports.IsEmpty() { - deps := g.dependencies(target.Imports, pkgRel) - attrs = append(attrs, keyvalue{"deps", deps}) + resolveFunc := func(imp string) (resolve.Label, error) { + return g.r.ResolveGo(imp, pkgRel) + } + attrs = append(attrs, keyvalue{"deps", g.dependencies(target.Imports, resolveFunc)}) } return attrs } @@ -230,18 +297,20 @@ func (g *Generator) buildPkgRel(pkgRel string) string { return rel } +type resolveFunc func(imp string) (resolve.Label, error) + // dependencies converts import paths in "imports" into Bazel labels. -func (g *Generator) dependencies(imports packages.PlatformStrings, pkgRel string) packages.PlatformStrings { - resolve := func(imp string) (string, error) { - label, err := g.r.ResolveGo(imp, pkgRel) +func (g *Generator) dependencies(imports packages.PlatformStrings, resolve resolveFunc) packages.PlatformStrings { + resolveToString := func(imp string) (string, error) { + label, err := resolve(imp) if err != nil { - return "", fmt.Errorf("in dir %q, could not resolve import path %q: %v", pkgRel, imp, err) + return "", err } label.Relative = label.Repo == "" && label.Pkg == g.buildRel return label.String(), nil } - deps, errors := imports.Map(resolve) + deps, errors := imports.Map(resolveToString) for _, err := range errors { log.Print(err) } diff --git a/go/tools/gazelle/rules/generator_test.go b/go/tools/gazelle/rules/generator_test.go index 6a397143f3..dca3ab7d60 100644 --- a/go/tools/gazelle/rules/generator_test.go +++ b/go/tools/gazelle/rules/generator_test.go @@ -109,35 +109,48 @@ func TestGeneratorEmpty(t *testing.T) { r := resolve.NewResolver(c, l) g := rules.NewGenerator(c, r, l, "", nil) - for _, tc := range []struct { - name string - pkg packages.Package - want string - }{ - { - name: "nothing", - want: `go_library(name = "go_default_library") + pkg := packages.Package{Name: "foo"} + want := `filegroup(name = "go_default_library_protos") -go_binary(name = "repo") +proto_library(name = "foo_proto") + +go_proto_library(name = "foo_go_proto") + +go_grpc_library(name = "foo_go_proto") + +go_library(name = "go_default_library") -filegroup(name = "go_default_library_protos") +go_binary(name = "repo") go_test(name = "go_default_test") go_test(name = "go_default_xtest") -`, - }, - } { - t.Run(tc.name, func(t *testing.T) { - _, empty := g.GenerateRules(&tc.pkg) - emptyStmt := make([]bf.Expr, len(empty)) - for i, s := range empty { - emptyStmt[i] = s - } - got := string(bf.Format(&bf.File{Stmt: emptyStmt})) - if got != tc.want { - t.Errorf("got '%s' ;\nwant %s", got, tc.want) - } - }) +` + _, empty := g.GenerateRules(&pkg) + emptyStmt := make([]bf.Expr, len(empty)) + for i, s := range empty { + emptyStmt[i] = s + } + got := string(bf.Format(&bf.File{Stmt: emptyStmt})) + if got != want { + t.Errorf("got '%s' ;\nwant %s", got, want) + } +} + +func TestGeneratorEmptyLegacyProto(t *testing.T) { + c := testConfig("", "example.com/repo") + c.ProtoMode = config.LegacyProtoMode + l := resolve.NewLabeler(c) + r := resolve.NewResolver(c, l) + g := rules.NewGenerator(c, r, l, "", nil) + + pkg := packages.Package{Name: "foo"} + _, empty := g.GenerateRules(&pkg) + for _, e := range empty { + rule := bf.Rule{Call: e.(*bf.CallExpr)} + kind := rule.Kind() + if kind == "proto_library" || kind == "go_proto_library" || kind == "go_grpc_library" { + t.Errorf("deleted rule %s ; should not delete in legacy proto mode", kind) + } } } diff --git a/go/tools/gazelle/testdata/repo/protos/BUILD.want b/go/tools/gazelle/testdata/repo/protos/BUILD.want new file mode 100644 index 0000000000..492ba5a8ef --- /dev/null +++ b/go/tools/gazelle/testdata/repo/protos/BUILD.want @@ -0,0 +1,31 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library") + +proto_library( + name = "protos_proto", + srcs = ["foo.proto"], + visibility = ["//visibility:public"], + deps = [ + "//protos/sub:sub_proto", + "@com_google_protobuf//:any_proto", + ], +) + +go_proto_library( + name = "protos_go_proto", + importpath = "example.com/repo/protos", + proto = ":protos_proto", + visibility = ["//visibility:public"], + deps = [ + "//protos/sub:go_default_library", + "@com_github_golang_protobuf//ptypes/any:go_default_library", + ], +) + +go_library( + name = "go_default_library", + srcs = ["extra.go"], + importpath = "example.com/repo/protos", + library = ":protos_go_proto", + visibility = ["//visibility:public"], +) diff --git a/go/tools/gazelle/testdata/repo/protos/extra.go b/go/tools/gazelle/testdata/repo/protos/extra.go new file mode 100644 index 0000000000..cb5a989009 --- /dev/null +++ b/go/tools/gazelle/testdata/repo/protos/extra.go @@ -0,0 +1 @@ +package protos diff --git a/go/tools/gazelle/testdata/repo/protos/foo.proto b/go/tools/gazelle/testdata/repo/protos/foo.proto new file mode 100644 index 0000000000..445b04d7a6 --- /dev/null +++ b/go/tools/gazelle/testdata/repo/protos/foo.proto @@ -0,0 +1,6 @@ +syntax = "proto2"; + +option go_package = "example.com/repo/protos"; + +import "google/protobuf/any.proto"; +import "protos/sub/sub.proto"; diff --git a/go/tools/gazelle/testdata/repo/protos/sub/sub.proto b/go/tools/gazelle/testdata/repo/protos/sub/sub.proto new file mode 100644 index 0000000000..fddb20b36c --- /dev/null +++ b/go/tools/gazelle/testdata/repo/protos/sub/sub.proto @@ -0,0 +1 @@ +syntax = "proto2"; diff --git a/go/tools/gazelle/testdata/repo/service/BUILD.want b/go/tools/gazelle/testdata/repo/service/BUILD.want new file mode 100644 index 0000000000..a5922f079c --- /dev/null +++ b/go/tools/gazelle/testdata/repo/service/BUILD.want @@ -0,0 +1,31 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library") + +proto_library( + name = "service_proto", + srcs = ["service.proto"], + visibility = ["//visibility:public"], + deps = [ + "//service/sub:sub_proto", + "@com_google_protobuf//:any_proto", + ], +) + +go_grpc_library( + name = "service_go_proto", + importpath = "example.com/repo/service", + proto = ":service_proto", + visibility = ["//visibility:public"], + deps = [ + "//service/sub:go_default_library", + "@com_github_golang_protobuf//ptypes/any:go_default_library", + ], +) + +go_library( + name = "go_default_library", + srcs = ["extra.go"], + importpath = "example.com/repo/service", + library = ":service_go_proto", + visibility = ["//visibility:public"], +) diff --git a/go/tools/gazelle/testdata/repo/service/extra.go b/go/tools/gazelle/testdata/repo/service/extra.go new file mode 100644 index 0000000000..6d43c3366c --- /dev/null +++ b/go/tools/gazelle/testdata/repo/service/extra.go @@ -0,0 +1 @@ +package service diff --git a/go/tools/gazelle/testdata/repo/service/service.proto b/go/tools/gazelle/testdata/repo/service/service.proto new file mode 100644 index 0000000000..ff38774727 --- /dev/null +++ b/go/tools/gazelle/testdata/repo/service/service.proto @@ -0,0 +1,8 @@ +syntax = "proto2"; + +option go_package = "example.com/repo/service"; + +import "google/protobuf/any.proto"; +import "service/sub/sub.proto"; + +service {} diff --git a/go/tools/gazelle/testdata/repo/service/sub/sub.proto b/go/tools/gazelle/testdata/repo/service/sub/sub.proto new file mode 100644 index 0000000000..fddb20b36c --- /dev/null +++ b/go/tools/gazelle/testdata/repo/service/sub/sub.proto @@ -0,0 +1 @@ +syntax = "proto2";