From b543f9c573d3784abd5ee4dfa540cf9caf96b474 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Tue, 21 Nov 2017 15:33:18 -0500 Subject: [PATCH 1/2] Gazelle: initial implementation of rule index (Unit tests will be in the PR to wire this into Resolver). Related #859 --- go/tools/gazelle/config/constants.go | 25 ++ go/tools/gazelle/resolve/BUILD.bazel | 1 + go/tools/gazelle/resolve/index.go | 358 +++++++++++++++++++++++ go/tools/gazelle/resolve/label.go | 78 +++++ go/tools/gazelle/resolve/label_test.go | 85 ++++++ go/tools/gazelle/resolve/resolve_test.go | 36 --- 6 files changed, 547 insertions(+), 36 deletions(-) create mode 100644 go/tools/gazelle/resolve/index.go create mode 100644 go/tools/gazelle/resolve/label_test.go diff --git a/go/tools/gazelle/config/constants.go b/go/tools/gazelle/config/constants.go index 342c1bc4ce..f3a34b0f25 100644 --- a/go/tools/gazelle/config/constants.go +++ b/go/tools/gazelle/config/constants.go @@ -49,3 +49,28 @@ const ( // on generated rules. It is replaced with "deps" during import resolution. GazelleImportsKey = "_gazelle_imports" ) + +// Language is the name of a programming langauge that Gazelle knows about. +// This is used to specify import paths. +type Language int + +const ( + GoLang Language = iota + ProtoLang +) + +func (l Language) String() string { + switch l { + case GoLang: + return "go" + case ProtoLang: + return "proto" + default: + return "unknown" + } +} + +const ( + PublicVisibility = "//visibility:public" + PrivateVisibility = "//visibility:private" +) diff --git a/go/tools/gazelle/resolve/BUILD.bazel b/go/tools/gazelle/resolve/BUILD.bazel index 5ea78cad8d..fb5e5ea6e4 100644 --- a/go/tools/gazelle/resolve/BUILD.bazel +++ b/go/tools/gazelle/resolve/BUILD.bazel @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ + "index.go", "label.go", "labeler.go", "resolve.go", diff --git a/go/tools/gazelle/resolve/index.go b/go/tools/gazelle/resolve/index.go new file mode 100644 index 0000000000..e1b29636a8 --- /dev/null +++ b/go/tools/gazelle/resolve/index.go @@ -0,0 +1,358 @@ +/* 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 resolve + +import ( + "fmt" + "log" + "path" + "path/filepath" + "strings" + + bf "github.com/bazelbuild/buildtools/build" + "github.com/bazelbuild/rules_go/go/tools/gazelle/config" +) + +// RuleIndex is a table of rules in a workspace, indexed by label and by +// import path. Used by Resolver to map import paths to labels. +type RuleIndex struct { + rules []*ruleRecord + labelMap map[Label]*ruleRecord + importMap map[importSpec][]*ruleRecord +} + +// ruleRecord contains information about a rule relevant to import indexing. +type ruleRecord struct { + call *bf.CallExpr + label Label + lang config.Language + importedAs []importSpec + visibility visibilitySpec + generated bool + replaced bool +} + +// importSpec describes a package to be imported. Language is specified, since +// different languages have different formats for their imports. +type importSpec struct { + lang config.Language + imp string +} + +// visibilitySpec describes the visibility of a rule. Gazelle only attempts +// to address common cases here: we handle "//visibility:public", +// "//visibility:private", and "//path/to/pkg:__subpackages__" (which is +// represented here as a relative path, e.g., "path/to/pkg". +type visibilitySpec []string + +func NewRuleIndex() *RuleIndex { + return &RuleIndex{ + labelMap: make(map[Label]*ruleRecord), + } +} + +// AddRulesFromFile adds existing rules to the index from oldFile +// (which must not be nil). +func (ix *RuleIndex) AddRulesFromFile(c *config.Config, oldFile *bf.File) { + buildRel, err := filepath.Rel(c.RepoRoot, oldFile.Path) + if err != nil { + log.Panicf("file not in repo: %s", oldFile.Path) + } + buildRel = path.Dir(filepath.ToSlash(buildRel)) + defaultVisibility := findDefaultVisibility(oldFile, buildRel) + for _, stmt := range oldFile.Stmt { + if call, ok := stmt.(*bf.CallExpr); ok { + ix.addRule(call, c.GoPrefix, buildRel, defaultVisibility, false) + } + } +} + +// AddGeneratedRules adds newly generated rules to the index. These may +// replace existing rules with the same label. +func (ix *RuleIndex) AddGeneratedRules(c *config.Config, buildRel string, oldFile *bf.File, rules []bf.Expr) { + defaultVisibility := findDefaultVisibility(oldFile, buildRel) + for _, stmt := range rules { + if call, ok := stmt.(*bf.CallExpr); ok { + ix.addRule(call, c.GoPrefix, buildRel, defaultVisibility, true) + } + } +} + +func (ix *RuleIndex) addRule(call *bf.CallExpr, goPrefix, buildRel string, defaultVisibility []string, generated bool) { + rule := bf.Rule{Call: call} + record := &ruleRecord{ + call: call, + label: Label{Pkg: buildRel, Name: rule.Name()}, + generated: generated, + } + + if old, ok := ix.labelMap[record.label]; ok { + if !old.generated && !generated { + log.Printf("multiple rules found with label %s", record.label) + } + if old.generated && generated { + log.Panicf("multiple rules generated with label %s", record.label) + } + if !generated { + // Don't index an existing rule if we already have a generated rule + // of the same name. + return + } + old.replaced = true + } + + kind := rule.Kind() + switch kind { + case "go_library": + record.lang = config.GoLang + record.importedAs = []importSpec{{lang: config.GoLang, imp: getGoImportPath(rule, goPrefix, buildRel)}} + + case "go_proto_library", "go_grpc_library": + record.lang = config.GoLang + // importedAs is set in Finish, since we need to dereference the "proto" + // attribute to find the sources. These rules are not automatically + // importable from Go. + + case "proto_library": + record.lang = config.ProtoLang + for _, s := range findSources(rule, buildRel, ".proto") { + record.importedAs = append(record.importedAs, importSpec{lang: config.ProtoLang, imp: s}) + } + + default: + return + } + + visExpr := rule.Attr("visibility") + if visExpr != nil { + record.visibility = parseVisibility(visExpr, buildRel) + } else { + record.visibility = defaultVisibility + } + + ix.rules = append(ix.rules, record) + ix.labelMap[record.label] = record +} + +// Finish constructs the import index and performs any other necessary indexing +// actions after all rules have been added. This step is necessary because +// some rules that are added may later be replaced (existing rules may be +// replaced by generated rules). Also, for proto rules, we need to be able +// to dereference a label to find the sources. +// +// This function must be called after all AddRulesFromFile and AddGeneratedRules +// calls but before any findRuleByImport calls. +func (ix *RuleIndex) Finish() { + ix.importMap = make(map[importSpec][]*ruleRecord) + oldRules := ix.rules + ix.rules = nil + for _, r := range oldRules { + if r.replaced { + continue + } + rule := bf.Rule{Call: r.call} + kind := rule.Kind() + if kind == "go_proto_library" || kind == "go_grpc_library" { + r.importedAs = findGoProtoSources(ix, r) + } + for _, imp := range r.importedAs { + ix.importMap[imp] = append(ix.importMap[imp], r) + } + } +} + +type ruleNotFoundError struct { + imp string + fromRel string +} + +func (e ruleNotFoundError) Error() string { + return fmt.Sprintf("no rule found for %q visible from %s", e.imp, e.fromRel) +} + +func (ix *RuleIndex) findRuleByLabel(label Label, fromRel string) (*ruleRecord, error) { + label = label.Abs("", fromRel) + r, ok := ix.labelMap[label] + if !ok { + return nil, ruleNotFoundError{label.String(), fromRel} + } + return r, nil +} + +// findRuleByImport attempts to resolve an import string to a rule record. +// imp is the import to resolve (which includes the target language). lang is +// the language of the rule with the dependency (for example, in +// go_proto_library, imp will have ProtoLang and lang will be GoLang). +// fromRel is the slash-separated path to the directory containing the import, +// relative to the repository root. +// +// Any number of rules may provide the same import. If no rules provide +// the import, ruleNotFoundError is returned. If multiple rules provide the +// import, this function will attempt to choose one based on visibility. +// An error is returned if the import is still ambiguous. +// +// Note that a rule may be returned even if visibility restrictions will be +// be violated. Bazel will give a descriptive error message when a build +// is attempted. +func (ix *RuleIndex) findRuleByImport(imp importSpec, lang config.Language, fromRel string) (*ruleRecord, error) { + matches := ix.importMap[imp] + var bestMatches []*ruleRecord + bestMatchesAreVisible := false + for _, m := range matches { + if m.lang != lang { + continue + } + visible := isVisibleFrom(m.visibility, m.label.Pkg, fromRel) + if bestMatchesAreVisible && !visible { + continue + } + if !bestMatchesAreVisible && visible { + bestMatchesAreVisible = true + bestMatches = nil + } + bestMatches = append(bestMatches, m) + } + if len(bestMatches) == 0 { + return nil, ruleNotFoundError{imp.imp, fromRel} + } + if len(bestMatches) >= 2 { + return nil, fmt.Errorf("multiple rules (%s and %s) may be imported with %q", bestMatches[0].label, bestMatches[1].label, imp.imp) + } + return bestMatches[0], nil +} + +func (ix *RuleIndex) findLabelByImport(imp importSpec, lang config.Language, fromRel string) (Label, error) { + r, err := ix.findRuleByImport(imp, lang, fromRel) + if err != nil { + return NoLabel, err + } + return r.label, nil +} + +func getGoImportPath(r bf.Rule, goPrefix, buildRel string) string { + // TODO(#597): account for subdirectory where goPrefix was set, when we + // support multiple prefixes. + imp := r.AttrString("importpath") + if imp != "" { + return imp + } + imp = path.Join(goPrefix, buildRel) + if name := r.Name(); name != config.DefaultLibName { + imp = path.Join(imp, name) + } + return imp +} + +func findGoProtoSources(ix *RuleIndex, r *ruleRecord) []importSpec { + rule := bf.Rule{Call: r.call} + protoExpr, ok := rule.Attr("proto").(*bf.StringExpr) + if !ok { + return nil + } + protoLabel, err := ParseLabel(protoExpr.Value) + if err != nil { + return nil + } + protoRule, err := ix.findRuleByLabel(protoLabel, r.label.Pkg) + if err != nil { + return nil + } + var importedAs []importSpec + for _, source := range findSources(bf.Rule{Call: protoRule.call}, protoRule.label.Pkg, ".proto") { + importedAs = append(importedAs, importSpec{lang: config.ProtoLang, imp: source}) + } + return importedAs +} + +func findSources(r bf.Rule, buildRel, ext string) []string { + srcsExpr := r.Attr("srcs") + srcsList, ok := srcsExpr.(*bf.ListExpr) + if !ok { + return nil + } + var srcs []string + for _, srcExpr := range srcsList.List { + src, ok := srcExpr.(*bf.StringExpr) + if !ok { + continue + } + label, err := ParseLabel(src.Value) + if err != nil || !label.Relative || !strings.HasSuffix(label.Name, ext) { + continue + } + srcs = append(srcs, path.Join(buildRel, label.Name)) + } + return srcs +} + +func findDefaultVisibility(oldFile *bf.File, buildRel string) visibilitySpec { + if oldFile == nil { + return visibilitySpec{config.PrivateVisibility} + } + for _, stmt := range oldFile.Stmt { + call, ok := stmt.(*bf.CallExpr) + if !ok { + continue + } + rule := bf.Rule{Call: call} + if rule.Kind() == "package" { + return parseVisibility(rule.Attr("default_visibility"), buildRel) + } + } + return visibilitySpec{config.PrivateVisibility} +} + +func parseVisibility(visExpr bf.Expr, buildRel string) visibilitySpec { + visList, ok := visExpr.(*bf.ListExpr) + if !ok { + return visibilitySpec{config.PrivateVisibility} + } + var visibility visibilitySpec + for _, elemExpr := range visList.List { + elemStr, ok := elemExpr.(*bf.StringExpr) + if !ok { + continue + } + if elemStr.Value == config.PublicVisibility || elemStr.Value == config.PrivateVisibility { + visibility = append(visibility, elemStr.Value) + continue + } + label, err := ParseLabel(elemStr.Value) + if err != nil { + continue + } + label = label.Abs("", buildRel) + if label.Repo != "" || label.Name != "__subpackages__" { + continue + } + visibility = append(visibility, label.Pkg) + } + return visibility +} + +func isVisibleFrom(visibility visibilitySpec, defRel, useRel string) bool { + for _, vis := range visibility { + switch vis { + case config.PublicVisibility: + return true + case config.PrivateVisibility: + return defRel == useRel + default: + return useRel == vis || strings.HasPrefix(useRel, vis+"/") + } + } + return false +} diff --git a/go/tools/gazelle/resolve/label.go b/go/tools/gazelle/resolve/label.go index 80a0de6317..5aab26ed58 100644 --- a/go/tools/gazelle/resolve/label.go +++ b/go/tools/gazelle/resolve/label.go @@ -18,6 +18,8 @@ package resolve import ( "fmt" "path" + "regexp" + "strings" ) // A Label represents a label of a build target in Bazel. @@ -26,6 +28,75 @@ type Label struct { Relative bool } +// NoLabel is the nil value of Label. It is not a valid label and may be +// returned when an error occurs. +var NoLabel = Label{} + +var ( + labelRepoRegexp = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_]*$`) + labelPkgRegexp = regexp.MustCompile(`^[A-Za-z0-9/.-]*$`) + labelNameRegexp = regexp.MustCompile(`^[A-Za-z0-9_/.+=,@~-]*$`) +) + +// ParseLabel reads a label from a string. +// See https://docs.bazel.build/versions/master/build-ref.html#lexi. +func ParseLabel(s string) (Label, error) { + origStr := s + + relative := true + var repo string + if strings.HasPrefix(s, "@") { + relative = false + endRepo := strings.Index(s, "//") + if endRepo < 0 { + return NoLabel, fmt.Errorf("label parse error: repository does not end with '//': %q", origStr) + } + repo = s[len("@"):endRepo] + if !labelRepoRegexp.MatchString(repo) { + return NoLabel, fmt.Errorf("label parse error: repository has invalid characters: %q", origStr) + } + s = s[endRepo:] + } + + var pkg string + if strings.HasPrefix(s, "//") { + relative = false + endPkg := strings.Index(s, ":") + if endPkg < 0 { + pkg = s[len("//"):] + s = "" + } else { + pkg = s[len("//"):endPkg] + s = s[endPkg:] + } + if !labelPkgRegexp.MatchString(pkg) { + return NoLabel, fmt.Errorf("label parse error: package has invalid characters: %q", origStr) + } + } + + if s == ":" { + return NoLabel, fmt.Errorf("label parse error: empty name: %q", origStr) + } + name := strings.TrimPrefix(s, ":") + if !labelNameRegexp.MatchString(name) { + return NoLabel, fmt.Errorf("label parse error: name has invalid characters: %q", origStr) + } + + if pkg == "" && name == "" { + return NoLabel, fmt.Errorf("label parse error: empty package and name: %q", origStr) + } + if name == "" { + name = pkg + } + + return Label{ + Repo: repo, + Pkg: pkg, + Name: name, + Relative: relative, + }, nil +} + func (l Label) String() string { if l.Relative { return fmt.Sprintf(":%s", l.Name) @@ -41,3 +112,10 @@ func (l Label) String() string { } return fmt.Sprintf("%s//%s:%s", repo, l.Pkg, l.Name) } + +func (l Label) Abs(repo, pkg string) Label { + if !l.Relative { + return l + } + return Label{Repo: repo, Pkg: pkg, Name: l.Name} +} diff --git a/go/tools/gazelle/resolve/label_test.go b/go/tools/gazelle/resolve/label_test.go new file mode 100644 index 0000000000..e7f61fc0ab --- /dev/null +++ b/go/tools/gazelle/resolve/label_test.go @@ -0,0 +1,85 @@ +/* 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 resolve + +import ( + "reflect" + "testing" +) + +func TestLabelString(t *testing.T) { + for _, spec := range []struct { + l Label + want string + }{ + { + l: Label{Name: "foo"}, + want: "//:foo", + }, { + l: Label{Pkg: "foo/bar", Name: "baz"}, + want: "//foo/bar:baz", + }, { + l: Label{Pkg: "foo/bar", Name: "bar"}, + want: "//foo/bar", + }, { + l: Label{Repo: "com_example_repo", Pkg: "foo/bar", Name: "baz"}, + want: "@com_example_repo//foo/bar:baz", + }, { + l: Label{Repo: "com_example_repo", Pkg: "foo/bar", Name: "bar"}, + want: "@com_example_repo//foo/bar", + }, { + l: Label{Relative: true, Name: "foo"}, + want: ":foo", + }, + } { + if got, want := spec.l.String(), spec.want; got != want { + t.Errorf("%#v.String() = %q; want %q", spec.l, got, want) + } + } +} + +func TestParseLabel(t *testing.T) { + for _, tc := range []struct { + str string + want Label + wantErr bool + }{ + {str: "", wantErr: true}, + {str: "@//:", wantErr: true}, + {str: "@//:a", wantErr: true}, + {str: "@a:b", wantErr: true}, + {str: ":a", want: Label{Name: "a", Relative: true}}, + {str: "a", want: Label{Name: "a", Relative: true}}, + {str: "//:a", want: Label{Name: "a", Relative: false}}, + {str: "//a", want: Label{Pkg: "a", Name: "a"}}, + {str: "//a:b", want: Label{Pkg: "a", Name: "b"}}, + {str: "@a//b", want: Label{Repo: "a", Pkg: "b", Name: "b"}}, + {str: "@a//b:c", want: Label{Repo: "a", Pkg: "b", Name: "c"}}, + } { + got, err := ParseLabel(tc.str) + if err != nil && !tc.wantErr { + t.Errorf("for string %q: got error %s ; want success", tc.str, err) + continue + } + if err == nil && tc.wantErr { + t.Errorf("for string %q: got label %s ; want error", tc.str, got) + continue + } + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("for string %q: got %s ; want %s", tc.str, got, tc.want) + } + } +} diff --git a/go/tools/gazelle/resolve/resolve_test.go b/go/tools/gazelle/resolve/resolve_test.go index b9bdbdacfc..b3186d05c1 100644 --- a/go/tools/gazelle/resolve/resolve_test.go +++ b/go/tools/gazelle/resolve/resolve_test.go @@ -22,42 +22,6 @@ import ( "github.com/bazelbuild/rules_go/go/tools/gazelle/config" ) -func TestLabelString(t *testing.T) { - for _, spec := range []struct { - l Label - want string - }{ - { - l: Label{Name: "foo"}, - want: "//:foo", - }, - { - l: Label{Pkg: "foo/bar", Name: "baz"}, - want: "//foo/bar:baz", - }, - { - l: Label{Pkg: "foo/bar", Name: "bar"}, - want: "//foo/bar", - }, - { - l: Label{Repo: "com_example_repo", Pkg: "foo/bar", Name: "baz"}, - want: "@com_example_repo//foo/bar:baz", - }, - { - l: Label{Repo: "com_example_repo", Pkg: "foo/bar", Name: "bar"}, - want: "@com_example_repo//foo/bar", - }, - { - l: Label{Relative: true, Name: "foo"}, - want: ":foo", - }, - } { - if got, want := spec.l.String(), spec.want; got != want { - t.Errorf("%#v.String() = %q; want %q", spec.l, got, want) - } - } -} - func TestResolveGoLocal(t *testing.T) { for _, spec := range []struct { mode config.StructureMode From e3a6ee72ae676688e34f90bf28a88a3309cd40a3 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Wed, 22 Nov 2017 16:01:36 -0500 Subject: [PATCH 2/2] review fixes * The index no longer handles visibility at all. * Vendoring logic is used to disambiguate imports for Go. * Embedded Go libraries won't be importable from Go, but they can still be imported from proto. * We also do a second looking for proto. --- go/tools/gazelle/config/constants.go | 8 +- go/tools/gazelle/resolve/index.go | 302 ++++++++++++++------------- 2 files changed, 160 insertions(+), 150 deletions(-) diff --git a/go/tools/gazelle/config/constants.go b/go/tools/gazelle/config/constants.go index f3a34b0f25..2d03bb1e47 100644 --- a/go/tools/gazelle/config/constants.go +++ b/go/tools/gazelle/config/constants.go @@ -55,7 +55,10 @@ const ( type Language int const ( + // GoLang marks Go targets. GoLang Language = iota + + // ProtoLang marks protocol buffer targets. ProtoLang ) @@ -69,8 +72,3 @@ func (l Language) String() string { return "unknown" } } - -const ( - PublicVisibility = "//visibility:public" - PrivateVisibility = "//visibility:private" -) diff --git a/go/tools/gazelle/resolve/index.go b/go/tools/gazelle/resolve/index.go index e1b29636a8..bd3afad4a4 100644 --- a/go/tools/gazelle/resolve/index.go +++ b/go/tools/gazelle/resolve/index.go @@ -36,13 +36,13 @@ type RuleIndex struct { // ruleRecord contains information about a rule relevant to import indexing. type ruleRecord struct { - call *bf.CallExpr + rule bf.Rule label Label lang config.Language importedAs []importSpec - visibility visibilitySpec generated bool replaced bool + embedded bool } // importSpec describes a package to be imported. Language is specified, since @@ -52,12 +52,6 @@ type importSpec struct { imp string } -// visibilitySpec describes the visibility of a rule. Gazelle only attempts -// to address common cases here: we handle "//visibility:public", -// "//visibility:private", and "//path/to/pkg:__subpackages__" (which is -// represented here as a relative path, e.g., "path/to/pkg". -type visibilitySpec []string - func NewRuleIndex() *RuleIndex { return &RuleIndex{ labelMap: make(map[Label]*ruleRecord), @@ -72,29 +66,31 @@ func (ix *RuleIndex) AddRulesFromFile(c *config.Config, oldFile *bf.File) { log.Panicf("file not in repo: %s", oldFile.Path) } buildRel = path.Dir(filepath.ToSlash(buildRel)) - defaultVisibility := findDefaultVisibility(oldFile, buildRel) + if buildRel == "." || buildRel == "/" { + buildRel = "" + } + for _, stmt := range oldFile.Stmt { if call, ok := stmt.(*bf.CallExpr); ok { - ix.addRule(call, c.GoPrefix, buildRel, defaultVisibility, false) + ix.addRule(call, c.GoPrefix, buildRel, false) } } } // AddGeneratedRules adds newly generated rules to the index. These may // replace existing rules with the same label. -func (ix *RuleIndex) AddGeneratedRules(c *config.Config, buildRel string, oldFile *bf.File, rules []bf.Expr) { - defaultVisibility := findDefaultVisibility(oldFile, buildRel) +func (ix *RuleIndex) AddGeneratedRules(c *config.Config, buildRel string, rules []bf.Expr) { for _, stmt := range rules { if call, ok := stmt.(*bf.CallExpr); ok { - ix.addRule(call, c.GoPrefix, buildRel, defaultVisibility, true) + ix.addRule(call, c.GoPrefix, buildRel, true) } } } -func (ix *RuleIndex) addRule(call *bf.CallExpr, goPrefix, buildRel string, defaultVisibility []string, generated bool) { +func (ix *RuleIndex) addRule(call *bf.CallExpr, goPrefix, buildRel string, generated bool) { rule := bf.Rule{Call: call} record := &ruleRecord{ - call: call, + rule: rule, label: Label{Pkg: buildRel, Name: rule.Name()}, generated: generated, } @@ -115,18 +111,15 @@ func (ix *RuleIndex) addRule(call *bf.CallExpr, goPrefix, buildRel string, defau } kind := rule.Kind() - switch kind { - case "go_library": - record.lang = config.GoLang - record.importedAs = []importSpec{{lang: config.GoLang, imp: getGoImportPath(rule, goPrefix, buildRel)}} - - case "go_proto_library", "go_grpc_library": + switch { + case isGoLibrary(kind): record.lang = config.GoLang - // importedAs is set in Finish, since we need to dereference the "proto" - // attribute to find the sources. These rules are not automatically - // importable from Go. + if imp := rule.AttrString("importpath"); imp != "" { + record.importedAs = []importSpec{{lang: config.GoLang, imp: imp}} + } + // Additional proto imports may be added in Finish. - case "proto_library": + case kind == "proto_library": record.lang = config.ProtoLang for _, s := range findSources(rule, buildRel, ".proto") { record.importedAs = append(record.importedAs, importSpec{lang: config.ProtoLang, imp: s}) @@ -136,39 +129,91 @@ func (ix *RuleIndex) addRule(call *bf.CallExpr, goPrefix, buildRel string, defau return } - visExpr := rule.Attr("visibility") - if visExpr != nil { - record.visibility = parseVisibility(visExpr, buildRel) - } else { - record.visibility = defaultVisibility - } - ix.rules = append(ix.rules, record) ix.labelMap[record.label] = record } // Finish constructs the import index and performs any other necessary indexing // actions after all rules have been added. This step is necessary because -// some rules that are added may later be replaced (existing rules may be -// replaced by generated rules). Also, for proto rules, we need to be able -// to dereference a label to find the sources. +// a rule may be indexed differently based on what rules are added later. // // This function must be called after all AddRulesFromFile and AddGeneratedRules // calls but before any findRuleByImport calls. func (ix *RuleIndex) Finish() { - ix.importMap = make(map[importSpec][]*ruleRecord) + ix.removeReplacedRules() + ix.skipGoEmbds() + ix.buildImportIndex() +} + +// removeReplacedRules removes rules from existing files that were replaced +// by generated rules. +func (ix *RuleIndex) removeReplacedRules() { oldRules := ix.rules ix.rules = nil for _, r := range oldRules { - if r.replaced { + if !r.replaced { + ix.rules = append(ix.rules, r) + } + } +} + +// skipGoEmbeds sets the embedded flag on Go library rules that are imported +// by other Go library rules with the same import path. Note that embedded +// rules may still be imported with non-Go imports. For example, a +// go_proto_library may be imported with either a Go import path or a proto +// path. If the library is embedded, only the proto path will be indexed. +func (ix *RuleIndex) skipGoEmbds() { + for _, r := range ix.rules { + if !isGoLibrary(r.rule.Kind()) { continue } - rule := bf.Rule{Call: r.call} - kind := rule.Kind() - if kind == "go_proto_library" || kind == "go_grpc_library" { - r.importedAs = findGoProtoSources(ix, r) + importpath := r.rule.AttrString("importpath") + + var embedLabels []Label + if embedList, ok := r.rule.Attr("embed").(*bf.ListExpr); ok { + for _, embedElem := range embedList.List { + embedStr, ok := embedElem.(*bf.StringExpr) + if !ok { + continue + } + embedLabel, err := ParseLabel(embedStr.Value) + if err != nil { + continue + } + embedLabels = append(embedLabels, embedLabel) + } + } + if libraryStr, ok := r.rule.Attr("library").(*bf.StringExpr); ok { + if libraryLabel, err := ParseLabel(libraryStr.Value); err == nil { + embedLabels = append(embedLabels, libraryLabel) + } + } + + for _, l := range embedLabels { + embed, ok := ix.findRuleByLabel(l, r.label.Pkg) + if !ok { + continue + } + if embed.rule.AttrString("importpath") != importpath { + continue + } + embed.embedded = true + } + } +} + +// buildImportIndex constructs the map used by findRuleByImport. +func (ix *RuleIndex) buildImportIndex() { + ix.importMap = make(map[importSpec][]*ruleRecord) + for _, r := range ix.rules { + if isGoProtoLibrary(r.rule.Kind()) { + protoImports := findGoProtoSources(ix, r) + r.importedAs = append(r.importedAs, protoImports...) } for _, imp := range r.importedAs { + if imp.lang == config.GoLang && r.embedded { + continue + } ix.importMap[imp] = append(ix.importMap[imp], r) } } @@ -180,16 +225,13 @@ type ruleNotFoundError struct { } func (e ruleNotFoundError) Error() string { - return fmt.Sprintf("no rule found for %q visible from %s", e.imp, e.fromRel) + return fmt.Sprintf("no rule found for import %q, needed in package %q", e.imp, e.fromRel) } -func (ix *RuleIndex) findRuleByLabel(label Label, fromRel string) (*ruleRecord, error) { +func (ix *RuleIndex) findRuleByLabel(label Label, fromRel string) (*ruleRecord, bool) { label = label.Abs("", fromRel) r, ok := ix.labelMap[label] - if !ok { - return nil, ruleNotFoundError{label.String(), fromRel} - } - return r, nil + return r, ok } // findRuleByImport attempts to resolve an import string to a rule record. @@ -201,37 +243,77 @@ func (ix *RuleIndex) findRuleByLabel(label Label, fromRel string) (*ruleRecord, // // Any number of rules may provide the same import. If no rules provide // the import, ruleNotFoundError is returned. If multiple rules provide the -// import, this function will attempt to choose one based on visibility. -// An error is returned if the import is still ambiguous. -// -// Note that a rule may be returned even if visibility restrictions will be -// be violated. Bazel will give a descriptive error message when a build -// is attempted. +// import, this function will attempt to choose one based on Go vendoring logic. +// In ambiguous cases, an error is returned. func (ix *RuleIndex) findRuleByImport(imp importSpec, lang config.Language, fromRel string) (*ruleRecord, error) { matches := ix.importMap[imp] - var bestMatches []*ruleRecord - bestMatchesAreVisible := false + var bestMatch *ruleRecord + var bestMatchIsVendored bool + var bestMatchVendorRoot string + var matchError error for _, m := range matches { if m.lang != lang { continue } - visible := isVisibleFrom(m.visibility, m.label.Pkg, fromRel) - if bestMatchesAreVisible && !visible { - continue - } - if !bestMatchesAreVisible && visible { - bestMatchesAreVisible = true - bestMatches = nil + + switch imp.lang { + case config.GoLang: + // Apply vendoring logic for Go libraries. A library in a vendor directory + // is only visible in the parent tree. Vendored libraries supercede + // non-vendored libraries, and libraries closer to fromRel supercede + // those further up the tree. + isVendored := false + vendorRoot := "" + if m.label.Repo == "" { + parts := strings.Split(m.label.Pkg, "/") + for i, part := range parts { + if part == "vendor" { + isVendored = true + vendorRoot = strings.Join(parts[:i], "/") + break + } + } + } + if isVendored && fromRel != vendorRoot && !strings.HasPrefix(fromRel, vendorRoot+"/") { + // vendor directory not visible + continue + } + if bestMatch == nil || isVendored && (!bestMatchIsVendored || len(vendorRoot) > len(bestMatchVendorRoot)) { + // Current match is better + bestMatch = m + bestMatchIsVendored = isVendored + bestMatchVendorRoot = vendorRoot + matchError = nil + } else if !isVendored && (bestMatchIsVendored || len(vendorRoot) < len(bestMatchVendorRoot)) { + // Current match is worse + } else { + // Match is ambiguous + matchError = fmt.Errorf("multiple rules (%s and %s) may be imported with %q", bestMatch.label, m.label, imp.imp) + } + + default: + if bestMatch == nil { + bestMatch = m + } else { + matchError = fmt.Errorf("multiple rules (%s and %s) may be imported with %q", bestMatch.label, m.label, imp.imp) + } } - bestMatches = append(bestMatches, m) } - if len(bestMatches) == 0 { + if matchError != nil { + return nil, matchError + } + if bestMatch == nil { return nil, ruleNotFoundError{imp.imp, fromRel} } - if len(bestMatches) >= 2 { - return nil, fmt.Errorf("multiple rules (%s and %s) may be imported with %q", bestMatches[0].label, bestMatches[1].label, imp.imp) + + if imp.lang == config.ProtoLang && lang == config.GoLang { + importpath := bestMatch.rule.AttrString("importpath") + if betterMatch, err := ix.findRuleByImport(importSpec{config.GoLang, importpath}, config.GoLang, fromRel); err != nil { + return betterMatch, nil + } } - return bestMatches[0], nil + + return bestMatch, nil } func (ix *RuleIndex) findLabelByImport(imp importSpec, lang config.Language, fromRel string) (Label, error) { @@ -242,36 +324,17 @@ func (ix *RuleIndex) findLabelByImport(imp importSpec, lang config.Language, fro return r.label, nil } -func getGoImportPath(r bf.Rule, goPrefix, buildRel string) string { - // TODO(#597): account for subdirectory where goPrefix was set, when we - // support multiple prefixes. - imp := r.AttrString("importpath") - if imp != "" { - return imp - } - imp = path.Join(goPrefix, buildRel) - if name := r.Name(); name != config.DefaultLibName { - imp = path.Join(imp, name) - } - return imp -} - func findGoProtoSources(ix *RuleIndex, r *ruleRecord) []importSpec { - rule := bf.Rule{Call: r.call} - protoExpr, ok := rule.Attr("proto").(*bf.StringExpr) - if !ok { - return nil - } - protoLabel, err := ParseLabel(protoExpr.Value) + protoLabel, err := ParseLabel(r.rule.AttrString("proto")) if err != nil { return nil } - protoRule, err := ix.findRuleByLabel(protoLabel, r.label.Pkg) - if err != nil { + proto, ok := ix.findRuleByLabel(protoLabel, r.label.Pkg) + if !ok { return nil } var importedAs []importSpec - for _, source := range findSources(bf.Rule{Call: protoRule.call}, protoRule.label.Pkg, ".proto") { + for _, source := range findSources(proto.rule, proto.label.Pkg, ".proto") { importedAs = append(importedAs, importSpec{lang: config.ProtoLang, imp: source}) } return importedAs @@ -298,61 +361,10 @@ func findSources(r bf.Rule, buildRel, ext string) []string { return srcs } -func findDefaultVisibility(oldFile *bf.File, buildRel string) visibilitySpec { - if oldFile == nil { - return visibilitySpec{config.PrivateVisibility} - } - for _, stmt := range oldFile.Stmt { - call, ok := stmt.(*bf.CallExpr) - if !ok { - continue - } - rule := bf.Rule{Call: call} - if rule.Kind() == "package" { - return parseVisibility(rule.Attr("default_visibility"), buildRel) - } - } - return visibilitySpec{config.PrivateVisibility} -} - -func parseVisibility(visExpr bf.Expr, buildRel string) visibilitySpec { - visList, ok := visExpr.(*bf.ListExpr) - if !ok { - return visibilitySpec{config.PrivateVisibility} - } - var visibility visibilitySpec - for _, elemExpr := range visList.List { - elemStr, ok := elemExpr.(*bf.StringExpr) - if !ok { - continue - } - if elemStr.Value == config.PublicVisibility || elemStr.Value == config.PrivateVisibility { - visibility = append(visibility, elemStr.Value) - continue - } - label, err := ParseLabel(elemStr.Value) - if err != nil { - continue - } - label = label.Abs("", buildRel) - if label.Repo != "" || label.Name != "__subpackages__" { - continue - } - visibility = append(visibility, label.Pkg) - } - return visibility +func isGoLibrary(kind string) bool { + return kind == "go_library" || isGoProtoLibrary(kind) } -func isVisibleFrom(visibility visibilitySpec, defRel, useRel string) bool { - for _, vis := range visibility { - switch vis { - case config.PublicVisibility: - return true - case config.PrivateVisibility: - return defRel == useRel - default: - return useRel == vis || strings.HasPrefix(useRel, vis+"/") - } - } - return false +func isGoProtoLibrary(kind string) bool { + return kind == "go_proto_library" || kind == "go_grpc_library" }