Miscelleneous changes before adding new proto modes (bazel-contrib#249)

* Language.GenerateRules now accepts an otherEmpty parameter, a list
  of empty rules that may be deleted.
* label.Rel can remove the repo name for labels in the same repo.
* Removed dead config.InferProtoMode.

Related bazel-contrib/rules_go#1548

cmd/gazelle/fix-update.go

@@ -181,7 +181,7 @@ func runFixUpdate(cmd command, args []string) error {
 		// Generate rules.
 		var empty, gen []*rule.Rule
 		for _, l := range languages {
-			lempty, lgen := l.GenerateRules(c, dir, rel, f, subdirs, regularFiles, genFiles, gen)
+			lempty, lgen := l.GenerateRules(c, dir, rel, f, subdirs, regularFiles, genFiles, empty, gen)
 			empty = append(empty, lempty...)
 			gen = append(gen, lgen...)
 		}
@@ -220,7 +220,7 @@ func runFixUpdate(cmd command, args []string) error {
 	rc := repos.NewRemoteCache(uc.repos)
 	for _, v := range visits {
 		for _, r := range v.rules {
-			from := label.New("", v.pkgRel, r.Name())
+			from := label.New(c.RepoName, v.pkgRel, r.Name())
 			kindToResolver[r.Kind()].Resolve(c, ruleIndex, rc, r, from)
 		}
 		merger.MergeFile(v.file, v.empty, v.rules, merger.PostResolve, kinds)

internal/config/BUILD.bazel

@@ -12,7 +12,6 @@ go_library(
     deps = [
         "//internal/rule:go_default_library",
         "//internal/wspace:go_default_library",
-        "//vendor/github.com/bazelbuild/buildtools/build:go_default_library",
     ],
 )
 
@@ -24,8 +23,5 @@ go_test(
         "directives_test.go",
     ],
     embed = [":go_default_library"],
-    deps = [
-        "//internal/rule:go_default_library",
-        "//vendor/github.com/bazelbuild/buildtools/build:go_default_library",
-    ],
+    deps = ["//internal/rule:go_default_library"],
 )

internal/config/directives.go

@@ -17,10 +17,8 @@ package config
 
 import (
 	"log"
-	"path"
 
 	"github.com/bazelbuild/bazel-gazelle/internal/rule"
-	bzl "github.com/bazelbuild/buildtools/build"
 )
 
 // ApplyDirectives applies directives that modify the configuration to a copy of
@@ -73,83 +71,3 @@ func ApplyDirectives(c *Config, directives []rule.Directive, rel string) *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 not change it. 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, proto rule generation is disabled.
-// TODO(jayconrod): this is operating at the wrong level of abstraction, but
-// it can't depend on rule, since rule depends on config. Move to another
-// package after the Language abstraction lands.
-func InferProtoMode(c *Config, rel string, f *bzl.File, directives []rule.Directive) *Config {
-	if c.ProtoMode != DefaultProtoMode || c.ProtoModeExplicit {
-		return c
-	}
-	for _, d := range directives {
-		if d.Key == "proto" {
-			return c
-		}
-	}
-	if c.GoPrefix == WellKnownTypesGoPrefix {
-		// 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 path.Base(rel) == "vendor" {
-		modified := *c
-		modified.ProtoMode = DisableProtoMode
-		return &modified
-	}
-	if f == nil {
-		return c
-	}
-	mode := DefaultProtoMode
-	for _, stmt := range f.Stmt {
-		c, ok := stmt.(*bzl.CallExpr)
-		if !ok {
-			continue
-		}
-		x, ok := c.X.(*bzl.LiteralExpr)
-		if !ok || x.Token != "load" || len(c.List) == 0 {
-			continue
-		}
-		name, ok := c.List[0].(*bzl.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.(*bzl.StringExpr); ok && sym.Value == "go_proto_library" {
-				mode = DisableProtoMode
-				break
-			}
-			kwarg, ok := arg.(*bzl.BinaryExpr)
-			if !ok || kwarg.Op != "=" {
-				continue
-			}
-			if key, ok := kwarg.X.(*bzl.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
-}

internal/config/directives_test.go

@@ -20,7 +20,6 @@ import (
 	"testing"
 
 	"github.com/bazelbuild/bazel-gazelle/internal/rule"
-	bzl "github.com/bazelbuild/buildtools/build"
 )
 
 func TestApplyDirectives(t *testing.T) {
@@ -61,78 +60,3 @@ func TestApplyDirectives(t *testing.T) {
 		})
 	}
 }
-
-func TestInferProtoMode(t *testing.T) {
-	for _, tc := range []struct {
-		desc, content string
-		c             Config
-		rel           string
-		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:    "explicit_no_override",
-			content: `load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library")`,
-			c: Config{
-				ProtoMode:         DefaultProtoMode,
-				ProtoModeExplicit: true,
-			},
-			want: DefaultProtoMode,
-		}, {
-			desc: "vendor",
-			rel:  "vendor",
-			want: DisableProtoMode,
-		}, {
-			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 *bzl.File
-			var directives []rule.Directive
-			if tc.content != "" {
-				var err error
-				f, err = bzl.Parse("BUILD.bazel", []byte(tc.content))
-				if err != nil {
-					t.Fatalf("error parsing build file: %v", err)
-				}
-				directives = rule.ParseDirectives(f)
-			}
-
-			got := InferProtoMode(&tc.c, tc.rel, f, directives)
-			if got.ProtoMode != tc.want {
-				t.Errorf("got proto mode %v ; want %v", got.ProtoMode, tc.want)
-			}
-		})
-	}
-}

internal/label/label.go

@@ -128,10 +128,13 @@ func (l Label) Abs(repo, pkg string) Label {
 }
 
 func (l Label) Rel(repo, pkg string) Label {
-	if !l.Relative && l.Repo == repo && l.Pkg == pkg {
+	if l.Relative || l.Repo != repo {
+		return l
+	}
+	if l.Pkg == pkg {
 		return Label{Name: l.Name, Relative: true}
 	}
-	return l
+	return Label{Pkg: l.Pkg, Name: l.Name}
 }
 
 func (l Label) Equal(other Label) bool {

internal/language/go/generate.go

@@ -30,15 +30,15 @@ import (
 	"github.com/bazelbuild/bazel-gazelle/internal/rule"
 )
 
-func (gl *goLang) GenerateRules(c *config.Config, dir, rel string, f *rule.File, subdirs, regularFiles, genFiles []string, other []*rule.Rule) (empty, gen []*rule.Rule) {
+func (gl *goLang) GenerateRules(c *config.Config, dir, rel string, f *rule.File, subdirs, regularFiles, genFiles []string, otherEmpty, otherGen []*rule.Rule) (empty, gen []*rule.Rule) {
 	// Extract information about proto files. We need this to exclude .pb.go
 	// files and generate go_proto_library rules.
 	pc := proto.GetProtoConfig(c)
 	var protoName string
 	var protoFileInfo map[string]proto.FileInfo
 	if pc.Mode != proto.DisableMode {
 		protoFileInfo = make(map[string]proto.FileInfo)
-		for _, r := range other {
+		for _, r := range otherGen {
 			if r.Kind() != "proto_library" {
 				continue
 			}

internal/language/go/generate_test.go

@@ -47,7 +47,7 @@ func TestGenerateRules(t *testing.T) {
 		t.Run(rel, func(t *testing.T) {
 			var empty, gen []*rule.Rule
 			for _, lang := range langs {
-				e, g := lang.GenerateRules(c, dir, rel, oldFile, subdirs, regularFiles, genFiles, gen)
+				e, g := lang.GenerateRules(c, dir, rel, oldFile, subdirs, regularFiles, genFiles, empty, gen)
 				empty = append(empty, e...)
 				gen = append(gen, g...)
 			}
@@ -88,7 +88,7 @@ func TestGenerateRules(t *testing.T) {
 func TestGenerateRulesEmpty(t *testing.T) {
 	c, _, langs := testConfig()
 	goLang := langs[1].(*goLang)
-	empty, gen := goLang.GenerateRules(c, "./foo", "foo", nil, nil, nil, nil, nil)
+	empty, gen := goLang.GenerateRules(c, "./foo", "foo", nil, nil, nil, nil, nil, nil)
 	if len(gen) > 0 {
 		t.Errorf("got %d generated rules; want 0", len(gen))
 	}
@@ -114,12 +114,12 @@ go_test(name = "go_default_test")
 	}
 }
 
-func TestGeneratorEmptyLegacyProto(t *testing.T) {
+func TestGenerateRulesEmptyLegacyProto(t *testing.T) {
 	c, _, langs := testConfig()
 	goLang := langs[1].(*goLang)
 	pc := proto.GetProtoConfig(c)
 	pc.Mode = proto.LegacyMode
-	empty, _ := goLang.GenerateRules(c, "./foo", "foo", nil, nil, nil, nil, nil)
+	empty, _ := goLang.GenerateRules(c, "./foo", "foo", nil, nil, nil, nil, nil, nil)
 	for _, e := range empty {
 		if kind := e.Kind(); kind == "proto_library" || kind == "go_proto_library" || kind == "go_grpc_library" {
 			t.Errorf("deleted rule %s ; should not delete in legacy proto mode", kind)

internal/language/lang.go

@@ -79,15 +79,15 @@ type Language interface {
 	// subdirs is a list of subdirectory names.
 	// regularFiles is a list of normal files in the directory.
 	// genFiles is a list of generated files, found in outputs of rules.
-	// other is a list of rules generated for languages that were processed
-	// before this language.
+	// otherEmpty and otherGen are lists of empty and generated rules created
+	// by other languages processed before this language.
 	//
 	// empty is a list of empty rules that may be deleted after merge.
 	// gen is a list of generated rules that may be updated or added.
 	//
 	// Any non-fatal errors this function encounters should be logged using
 	// log.Print.
-	GenerateRules(c *config.Config, dir, rel string, f *rule.File, subdirs, regularFiles, genFiles []string, other []*rule.Rule) (empty, gen []*rule.Rule)
+	GenerateRules(c *config.Config, dir, rel string, f *rule.File, subdirs, regularFiles, genFiles []string, otherEmpty, otherGen []*rule.Rule) (empty, gen []*rule.Rule)
 
 	// Fix repairs deprecated usage of language-specific rules in f. This is
 	// called before the file is indexed. Unless c.ShouldFix is true, fixes

internal/language/proto/generate.go

@@ -26,7 +26,7 @@ import (
 	"github.com/bazelbuild/bazel-gazelle/internal/rule"
 )
 
-func (_ *protoLang) GenerateRules(c *config.Config, dir, rel string, f *rule.File, subdirs, regularFiles, genFiles []string, other []*rule.Rule) (empty, gen []*rule.Rule) {
+func (_ *protoLang) GenerateRules(c *config.Config, dir, rel string, f *rule.File, subdirs, regularFiles, genFiles []string, otherEmpty, otherGen []*rule.Rule) (empty, gen []*rule.Rule) {
 	pc := GetProtoConfig(c)
 	if pc.Mode != DefaultMode {
 		// Don't create or delete proto rules in this mode. Any existing rules

internal/language/proto/generate_test.go

@@ -46,7 +46,7 @@ func TestGenerateRules(t *testing.T) {
 			return
 		}
 		t.Run(rel, func(t *testing.T) {
-			empty, gen := lang.GenerateRules(c, dir, rel, oldFile, subdirs, regularFiles, genFiles, nil)
+			empty, gen := lang.GenerateRules(c, dir, rel, oldFile, subdirs, regularFiles, genFiles, nil, nil)
 			if len(empty) > 0 {
 				t.Errorf("got %d empty rules; want 0", len(empty))
 			}
@@ -76,7 +76,7 @@ func TestGenerateRulesEmpty(t *testing.T) {
 	c := config.New()
 	c.Exts[protoName] = &ProtoConfig{}
 
-	empty, gen := lang.GenerateRules(c, "", "foo", nil, nil, nil, nil, nil)
+	empty, gen := lang.GenerateRules(c, "", "foo", nil, nil, nil, nil, nil, nil)
 	if len(gen) > 0 {
 		t.Errorf("got %d generated rules; want 0", len(gen))
 	}
@@ -96,7 +96,7 @@ func TestGenerateFileInfo(t *testing.T) {
 	lang := New()
 	c := testConfig()
 	dir := filepath.FromSlash("testdata/protos")
-	_, gen := lang.GenerateRules(c, dir, "protos", nil, nil, []string{"foo.proto"}, nil, nil)
+	_, gen := lang.GenerateRules(c, dir, "protos", nil, nil, []string{"foo.proto"}, nil, nil, nil)
 	r := gen[0]
 	got := r.PrivateAttr(FileInfoKey).([]FileInfo)
 	want := []FileInfo{{