Gazelle: extract import resolution out of rules package (#1043)

Previously, rules.Generator performed import resolution (using the
resolve package) as it generated rules. With this change, Generator
now produces a "_gazelle_imports" attribute, which is resolved and
replaced with "deps".

This is necessary since we must be able to index generated rules. We
need to generate and index all rules, then resolve imports to deps in
a separate phase.

Related #859

go/tools/gazelle/config/constants.go

@@ -44,4 +44,8 @@ const (
 	// WellKnownTypesGoPrefix is the import path for the Go repository containing
 	// pre-generated code for the Well Known Types.
 	WellKnownTypesGoPrefix = "github.com/golang/protobuf"
+
+	// GazelleImportsKey is an internal attribute that lists imported packages
+	// on generated rules. It is replaced with "deps" during import resolution.
+	GazelleImportsKey = "_gazelle_imports"
 )

go/tools/gazelle/gazelle/main.go

@@ -61,9 +61,15 @@ var commandFromName = map[string]command{
 // visitRecord stores information about about a directory visited with
 // packages.Walk.
 type visitRecord struct {
-	// rel is the slash-separated path to the directory, relative to the
-	// repository root. "" for the repository root itself.
-	rel string
+	// pkgRel is the slash-separated path to the visited directory, relative to
+	// the repository root. "" for the repository root itself.
+	pkgRel string
+
+	// buildRel is the slash-separated path to the directory containing the
+	// relevant build file for the directory being visited, relative to the
+	// repository root. "" for the repository root itself. This may differ
+	// from pkgRel in flat mode.
+	buildRel string
 
 	// rules is a list of generated Go rules.
 	rules []bf.Expr
@@ -75,18 +81,15 @@ type visitRecord struct {
 	oldFile *bf.File
 }
 
-type byRel []visitRecord
-
-var _ sort.Interface = byRel(nil)
+type byPkgRel []visitRecord
 
-func (vs byRel) Len() int           { return len(vs) }
-func (vs byRel) Less(i, j int) bool { return vs[i].rel < vs[j].rel }
-func (vs byRel) Swap(i, j int)      { vs[i], vs[j] = vs[j], vs[i] }
+func (vs byPkgRel) Len() int           { return len(vs) }
+func (vs byPkgRel) Less(i, j int) bool { return vs[i].pkgRel < vs[j].pkgRel }
+func (vs byPkgRel) Swap(i, j int)      { vs[i], vs[j] = vs[j], vs[i] }
 
 func run(c *config.Config, cmd command, emit emitFunc) {
 	shouldFix := c.ShouldFix
 	l := resolve.NewLabeler(c)
-	r := resolve.NewResolver(c, l)
 
 	var visits []visitRecord
 
@@ -118,34 +121,41 @@ func run(c *config.Config, cmd command, emit emitFunc) {
 				} else {
 					buildRel = rel
 				}
-				g := rules.NewGenerator(c, r, l, buildRel, oldFile)
+				g := rules.NewGenerator(c, l, buildRel, oldFile)
 				rules, empty := g.GenerateRules(pkg)
 				visits = append(visits, visitRecord{
-					rel:     rel,
-					rules:   rules,
-					empty:   empty,
-					oldFile: oldFile,
+					pkgRel:   rel,
+					buildRel: buildRel,
+					rules:    rules,
+					empty:    empty,
+					oldFile:  oldFile,
 				})
 			}
 		})
 
 		// TODO: resolve dependencies using the index.
+		resolver := resolve.NewResolver(c, l)
+		for _, v := range visits {
+			for _, r := range v.rules {
+				resolver.ResolveRule(r, v.pkgRel, v.buildRel)
+			}
+		}
 
 		// Merge old files and generated files. Emit merged files.
 		switch c.StructureMode {
 		case config.HierarchicalMode:
 			for _, v := range visits {
 				genFile := &bf.File{
-					Path: filepath.Join(c.RepoRoot, filepath.FromSlash(v.rel), c.DefaultBuildFileName()),
+					Path: filepath.Join(c.RepoRoot, filepath.FromSlash(v.pkgRel), c.DefaultBuildFileName()),
 					Stmt: v.rules,
 				}
 				mergeAndEmit(c, genFile, v.oldFile, v.empty, emit)
 			}
 
 		case config.FlatMode:
-			sort.Stable(byRel(visits))
+			sort.Stable(byPkgRel(visits))
 			var oldFile *bf.File
-			if len(visits) > 0 && visits[0].rel == "" {
+			if len(visits) > 0 && visits[0].pkgRel == "" {
 				oldFile = visits[0].oldFile
 			}
 

go/tools/gazelle/resolve/BUILD.bazel

@@ -12,6 +12,7 @@ go_library(
     ],
     visibility = ["//visibility:public"],
     deps = [
+        "@com_github_bazelbuild_buildtools//build:go_default_library",
         "@io_bazel_rules_go//go/tools/gazelle/config:go_default_library",
         "@org_golang_x_tools//go/vcs:go_default_library",
     ],
@@ -39,6 +40,6 @@ genrule(
     name = "std_package_list",
     srcs = ["@go_sdk//:packages.txt"],
     outs = ["std_package_list.go"],
-    tools = ["//go/tools/gazelle/resolve/internal/gen_std_package_list"],
     cmd = "$(location //go/tools/gazelle/resolve/internal/gen_std_package_list) $< $@",
+    tools = ["//go/tools/gazelle/resolve/internal/gen_std_package_list"],
 )

go/tools/gazelle/resolve/resolve.go

@@ -22,6 +22,7 @@ import (
 	"path"
 	"strings"
 
+	bf "github.com/bazelbuild/buildtools/build"
 	"github.com/bazelbuild/rules_go/go/tools/gazelle/config"
 )
 
@@ -59,10 +60,128 @@ func NewResolver(c *config.Config, l Labeler) *Resolver {
 	}
 }
 
-// ResolveGo resolves an import path from a Go source file to a label.
+// ResolveRule modifies a generated rule e by replacing the import paths in the
+// "_gazelle_imports" attribute with labels in a "deps" attribute. This may
+// may safely called on expressions that aren't Go rules (nothing will happen).
+func (r *Resolver) ResolveRule(e bf.Expr, pkgRel, buildRel string) {
+	call, ok := e.(*bf.CallExpr)
+	if !ok {
+		return
+	}
+	rule := bf.Rule{Call: call}
+
+	var resolve func(imp, pkgRel string) (Label, error)
+	switch rule.Kind() {
+	case "go_library", "go_binary", "go_test":
+		resolve = r.resolveGo
+	case "proto_library":
+		resolve = r.resolveProto
+	case "go_proto_library", "go_grpc_library":
+		resolve = r.resolveGoProto
+	default:
+		return
+	}
+
+	imports := rule.AttrDefn(config.GazelleImportsKey)
+	if imports == nil {
+		return
+	}
+
+	deps := mapExprStrings(imports.Y, func(imp string) string {
+		label, err := resolve(imp, pkgRel)
+		if err != nil {
+			log.Print(err)
+			return ""
+		}
+		label.Relative = label.Repo == "" && label.Pkg == buildRel
+		return label.String()
+	})
+	if deps == nil {
+		rule.DelAttr(config.GazelleImportsKey)
+	} else {
+		imports.X.(*bf.LiteralExpr).Token = "deps"
+		imports.Y = deps
+	}
+}
+
+// mapExprStrings applies a function f to the strings in e and returns a new
+// expression with the results. Scalar strings, lists, dicts, selects, and
+// concatenations are supported.
+func mapExprStrings(e bf.Expr, f func(string) string) bf.Expr {
+	switch expr := e.(type) {
+	case *bf.StringExpr:
+		s := f(expr.Value)
+		if s == "" {
+			return nil
+		}
+		return &bf.StringExpr{Value: s}
+
+	case *bf.ListExpr:
+		var list []bf.Expr
+		for _, elem := range expr.List {
+			elem = mapExprStrings(elem, f)
+			if elem != nil {
+				list = append(list, elem)
+			}
+		}
+		if len(list) == 0 && len(expr.List) > 0 {
+			return nil
+		}
+		return &bf.ListExpr{List: list}
+
+	case *bf.DictExpr:
+		var cases []bf.Expr
+		for _, kv := range expr.List {
+			keyval, ok := kv.(*bf.KeyValueExpr)
+			if !ok {
+				log.Panicf("unexpected expression in generated imports dict: %#v", kv)
+			}
+			value := mapExprStrings(keyval.Value, f)
+			if value != nil {
+				cases = append(cases, &bf.KeyValueExpr{Key: keyval.Key, Value: value})
+			}
+		}
+		if len(cases) == 0 {
+			return nil
+		}
+		return &bf.DictExpr{List: cases}
+
+	case *bf.CallExpr:
+		if x, ok := expr.X.(*bf.LiteralExpr); !ok || x.Token != "select" || len(expr.List) != 1 {
+			log.Panicf("unexpected call expression in generated imports: %#v", e)
+		}
+		arg := mapExprStrings(expr.List[0], f)
+		if arg == nil {
+			return nil
+		}
+		call := *expr
+		call.List[0] = arg
+		return &call
+
+	case *bf.BinaryExpr:
+		x := mapExprStrings(expr.X, f)
+		y := mapExprStrings(expr.Y, f)
+		if x == nil {
+			return y
+		}
+		if y == nil {
+			return x
+		}
+		binop := *expr
+		binop.X = x
+		binop.Y = y
+		return &binop
+
+	default:
+		log.Panicf("unexpected expression in generated imports: %#v", e)
+		return nil
+	}
+}
+
+// resolveGo resolves an import path from a Go source file to a label.
 // pkgRel is the path to the Go package relative to the repository root; it
 // is used to resolve relative imports.
-func (r *Resolver) ResolveGo(imp, pkgRel string) (Label, error) {
+func (r *Resolver) resolveGo(imp, pkgRel string) (Label, error) {
 	if build.IsLocalImport(imp) {
 		cleanRel := path.Clean(path.Join(pkgRel, imp))
 		if build.IsLocalImport(cleanRel) {
@@ -89,9 +208,9 @@ const (
 	descriptorPkg       = "protoc-gen-go/descriptor"
 )
 
-// ResolveProto resolves an import statement in a .proto file to a label
+// resolveProto resolves an import statement in a .proto file to a label
 // for a proto_library rule.
-func (r *Resolver) ResolveProto(imp, pkgRel string) (Label, error) {
+func (r *Resolver) resolveProto(imp, pkgRel string) (Label, error) {
 	if !strings.HasSuffix(imp, ".proto") {
 		return Label{}, fmt.Errorf("can't import non-proto: %q", imp)
 	}
@@ -111,9 +230,9 @@ func (r *Resolver) ResolveProto(imp, pkgRel string) (Label, error) {
 	return r.l.ProtoLabel(rel, name), nil
 }
 
-// ResolveGoProto resolves an import statement in a .proto file to a
+// 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, pkgRel string) (Label, error) {
+func (r *Resolver) resolveGoProto(imp, pkgRel string) (Label, error) {
 	if !strings.HasSuffix(imp, ".proto") {
 		return Label{}, fmt.Errorf("can't import non-proto: %q", imp)
 	}

go/tools/gazelle/resolve/resolve_test.go

@@ -92,13 +92,13 @@ func TestResolveGoLocal(t *testing.T) {
 		c := &config.Config{GoPrefix: "example.com/repo", StructureMode: spec.mode}
 		l := NewLabeler(c)
 		r := NewResolver(c, l)
-		label, err := r.ResolveGo(spec.importpath, spec.pkgRel)
+		label, err := r.resolveGo(spec.importpath, spec.pkgRel)
 		if err != nil {
-			t.Errorf("r.ResolveGo(%q) failed with %v; want success", spec.importpath, err)
+			t.Errorf("r.resolveGo(%q) failed with %v; want success", spec.importpath, err)
 			continue
 		}
 		if got, want := label, spec.want; !reflect.DeepEqual(got, want) {
-			t.Errorf("r.ResolveGo(%q) = %s; want %s", spec.importpath, got, want)
+			t.Errorf("r.resolveGo(%q) = %s; want %s", spec.importpath, got, want)
 		}
 	}
 }
@@ -114,13 +114,13 @@ func TestResolveGoLocalError(t *testing.T) {
 		"example.com/another/sub",
 		"example.com/repo_suffix",
 	} {
-		if l, err := r.ResolveGo(importpath, ""); err == nil {
-			t.Errorf("r.ResolveGo(%q) = %s; want error", importpath, l)
+		if l, err := r.resolveGo(importpath, ""); err == nil {
+			t.Errorf("r.resolveGo(%q) = %s; want error", importpath, l)
 		}
 	}
 
-	if l, err := r.ResolveGo("..", ""); err == nil {
-		t.Errorf("r.ResolveGo(%q) = %s; want error", "..", l)
+	if l, err := r.resolveGo("..", ""); err == nil {
+		t.Errorf("r.resolveGo(%q) = %s; want error", "..", l)
 	}
 }
 
@@ -131,15 +131,15 @@ func TestResolveGoEmptyPrefix(t *testing.T) {
 
 	imp := "foo"
 	want := Label{Pkg: "foo", Name: config.DefaultLibName}
-	if got, err := r.ResolveGo(imp, ""); err != nil {
-		t.Errorf("r.ResolveGo(%q) failed with %v; want success", imp, err)
+	if got, err := r.resolveGo(imp, ""); err != nil {
+		t.Errorf("r.resolveGo(%q) failed with %v; want success", imp, err)
 	} else if !reflect.DeepEqual(got, want) {
-		t.Errorf("r.ResolveGo(%q) = %s; want %s", imp, got, want)
+		t.Errorf("r.resolveGo(%q) = %s; want %s", imp, got, want)
 	}
 
 	imp = "fmt"
-	if _, err := r.ResolveGo(imp, ""); err == nil {
-		t.Errorf("r.ResolveGo(%q) succeeded; want failure")
+	if _, err := r.resolveGo(imp, ""); err == nil {
+		t.Errorf("r.resolveGo(%q) succeeded; want failure")
 	}
 }
 
@@ -221,20 +221,20 @@ func TestResolveProto(t *testing.T) {
 			l := NewLabeler(c)
 			r := NewResolver(c, l)
 
-			got, err := r.ResolveProto(tc.imp, tc.pkgRel)
+			got, err := r.resolveProto(tc.imp, tc.pkgRel)
 			if err != nil {
-				t.Errorf("ResolveProto: got error %v ; want success", err)
+				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)
+				t.Errorf("resolveProto: got %s ; want %s", got, tc.wantProto)
 			}
 
-			got, err = r.ResolveGoProto(tc.imp, tc.pkgRel)
+			got, err = r.resolveGoProto(tc.imp, tc.pkgRel)
 			if err != nil {
-				t.Errorf("ResolveGoProto: go error %v ; want success", err)
+				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)
+				t.Errorf("resolveGoProto: got %s ; want %s", got, tc.wantGoProto)
 			}
 		})
 	}