language/go: implement minimal module compatibility (#564)

* The Go extension now accepts flags -go_repository_mode and
  -go_repository_module_mode.
* The Go extension will emit importpath_aliases attributes for
  libraries with importpath set if -go_repository_mode was set,
  Gazelle is in module mode, the prefix is actually a prefix of
  importpath, the prefix contains a semantic import
  version suffix, and the prefix was set in the root package.
  Such a library can be imported with more than one import path.
* The minimum version of rules_go is now 0.19.0, since support for
  importpath_aliases is needed.
* When initializing RemoteCache, which is used to resolve external
  import paths, if we have a known import path that contains a
  semantic import version suffix, but we don't have a known import
  path for the path without the suffix, we create an aliases.

Also:

* Avoided parsing WORKSPACE and related files multiple times when
  -repo_config is set.

Fixes #477

cmd/gazelle/fix-update.go

@@ -23,6 +23,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"github.com/bazelbuild/bazel-gazelle/config"
@@ -40,12 +41,13 @@ import (
 // update commands. This includes everything in config.Config, but it also
 // includes some additional fields that aren't relevant to other packages.
 type updateConfig struct {
-	dirs        []string
-	emit        emitFunc
-	repos       []repo.Repo
-	walkMode    walk.Mode
-	patchPath   string
-	patchBuffer bytes.Buffer
+	dirs           []string
+	emit           emitFunc
+	repos          []repo.Repo
+	workspaceFiles []*rule.File
+	walkMode       walk.Mode
+	patchPath      string
+	patchBuffer    bytes.Buffer
 }
 
 type emitFunc func(c *config.Config, f *rule.File) error
@@ -122,15 +124,21 @@ func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) erro
 		uc.walkMode = walk.UpdateDirsMode
 	}
 
+	// Load the repo configuration file (WORKSPACE by default) to find out
+	// names and prefixes of other go_repositories. This affects external
+	// dependency resolution for Go.
+	// TODO(jayconrod): this should be moved to language/go.
+	var repoFileMap map[*rule.File][]string
 	if ucr.repoConfigPath == "" {
 		ucr.repoConfigPath = filepath.Join(c.RepoRoot, "WORKSPACE")
 	}
-	if repoConfigFile, err := rule.LoadWorkspaceFile(ucr.repoConfigPath, ""); err != nil {
+	repoConfigFile, err := rule.LoadWorkspaceFile(ucr.repoConfigPath, "")
+	if err != nil {
 		if !os.IsNotExist(err) {
 			return err
 		}
 	} else {
-		uc.repos, _, err = repo.ListRepositories(repoConfigFile)
+		uc.repos, repoFileMap, err = repo.ListRepositories(repoConfigFile)
 		if err != nil {
 			return err
 		}
@@ -150,6 +158,39 @@ func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) erro
 		uc.repos = append(uc.repos, repo)
 	}
 
+	// If the repo configuration file is not WORKSPACE, also load WORKSPACE
+	// so we can apply any necessary fixes.
+	workspacePath := filepath.Join(c.RepoRoot, "WORKSPACE")
+	var workspace *rule.File
+	if ucr.repoConfigPath == workspacePath {
+		workspace = repoConfigFile
+	} else {
+		workspace, err = rule.LoadWorkspaceFile(workspacePath, "")
+		if err != nil && !os.IsNotExist(err) {
+			return err
+		}
+		if workspace != nil {
+			_, repoFileMap, err = repo.ListRepositories(workspace)
+			if err != nil {
+				return err
+			}
+		}
+	}
+	if workspace != nil {
+		c.RepoName = findWorkspaceName(workspace)
+		uc.workspaceFiles = make([]*rule.File, 0, len(repoFileMap))
+		seen := make(map[string]bool)
+		for f := range repoFileMap {
+			if !seen[f.Path] {
+				uc.workspaceFiles = append(uc.workspaceFiles, f)
+				seen[f.Path] = true
+			}
+		}
+		sort.Slice(uc.workspaceFiles, func(i, j int) bool {
+			return uc.workspaceFiles[i].Path < uc.workspaceFiles[j].Path
+		})
+	}
+
 	return nil
 }
 
@@ -382,29 +423,9 @@ func newFixUpdateConfiguration(cmd command, args []string, cexts []config.Config
 		}
 	}
 
-	workspacePath := filepath.Join(c.RepoRoot, "WORKSPACE")
-	if workspace, err := rule.LoadWorkspaceFile(workspacePath, ""); err != nil {
-		if !os.IsNotExist(err) {
-			return nil, err
-		}
-	} else {
-		c.RepoName = findWorkspaceName(workspace)
-		var reposFiles map[*rule.File][]string
-		_, reposFiles, err = repo.ListRepositories(workspace)
-		if err != nil {
-			return nil, err
-		}
-		files := make([]*rule.File, 0, len(reposFiles))
-		seen := make(map[string]bool)
-		for f := range reposFiles {
-			if !seen[f.Path] {
-				files = append(files, f)
-				seen[f.Path] = true
-			}
-		}
-		if err := fixRepoFiles(c, files, loads); err != nil {
-			return nil, err
-		}
+	uc := getUpdateConfig(c)
+	if err := fixRepoFiles(c, uc.workspaceFiles, loads); err != nil {
+		return nil, err
 	}
 
 	return c, nil

cmd/gazelle/integration_test.go

@@ -2643,6 +2643,62 @@ go_binary(
 	})
 }
 
+// TestMinimalModuleCompatibilityAliases checks that importpath_aliases
+// are emitted for go_libraries when needed. This can't easily be checked
+// in language/go because the generator tests don't support running at
+// the repository root or with additional flags, both of which are required.
+func TestMinimalModuleCompatibilityAliases(t *testing.T) {
+	files := []testtools.FileSpec{
+		{
+			Path:    "go.mod",
+			Content: "module example.com/foo/v2",
+		}, {
+			Path:    "foo.go",
+			Content: "package foo",
+		}, {
+			Path:    "bar/bar.go",
+			Content: "package bar",
+		},
+	}
+	dir, cleanup := testtools.CreateFiles(t, files)
+	defer cleanup()
+
+	args := []string{"update", "-repo_root", dir, "-go_prefix", "example.com/foo/v2", "-go_repository_mode", "-go_repository_module_mode"}
+	if err := runGazelle(dir, args); err != nil {
+		t.Fatal(err)
+	}
+
+	testtools.CheckFiles(t, dir, []testtools.FileSpec{
+		{
+			Path: "BUILD.bazel",
+			Content: `
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["foo.go"],
+    importpath = "example.com/foo/v2",
+    importpath_aliases = ["example.com/foo"],
+    visibility = ["//visibility:public"],
+)
+`,
+		}, {
+			Path: "bar/BUILD.bazel",
+			Content: `
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["bar.go"],
+    importpath = "example.com/foo/v2/bar",
+    importpath_aliases = ["example.com/foo/bar"],
+    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)

cmd/gazelle/version.go

@@ -26,7 +26,7 @@ import (
 	"github.com/bazelbuild/bazel-gazelle/repo"
 )
 
-var minimumRulesGoVersion = version.Version{0, 13, 0}
+var minimumRulesGoVersion = version.Version{0, 19, 0}
 
 // checkRulesGoVersion checks whether a compatible version of rules_go is
 // being used in the workspace. A message will be logged if an incompatible

internal/go_repository.bzl

@@ -132,6 +132,7 @@ def _go_repository_impl(ctx):
         gazelle = ctx.path(Label(_gazelle))
         cmd = [
             gazelle,
+            "-go_repository_mode",
             "-go_prefix",
             ctx.attr.importpath,
             "-mode",
@@ -142,7 +143,7 @@ def _go_repository_impl(ctx):
         if config_path:
             cmd.extend(["-repo_config", str(config_path)])
         if ctx.attr.version:
-            cmd.append("-go_experimental_module_mode")
+            cmd.append("-go_repository_module_mode")
         if ctx.attr.build_file_name:
             cmd.extend(["-build_file_name", ctx.attr.build_file_name])
         if ctx.attr.build_tags:

language/go/config.go

@@ -74,8 +74,14 @@ type goConfig struct {
 	// goGrpcCompilersSet indicates whether goGrpcCompiler was set explicitly.
 	goGrpcCompilersSet bool
 
-	// moduleMode is true if external dependencies should be resolved as modules.
-	// TODO(jayconrod): this should be the only mode in the future.
+	// goRepositoryMode is true if Gazelle was invoked by a go_repository rule.
+	// In this mode, we won't go out to the network to resolve external deps.
+	goRepositoryMode bool
+
+	// moduleMode is true if the current directory is intended to be built
+	// as part of a module. Minimal module compatibility won't be supported
+	// if this is true in the root directory. External dependencies may be
+	// resolved differently (also depending on goRepositoryMode).
 	moduleMode bool
 }
 
@@ -228,16 +234,16 @@ func (_ *goLang) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) {
 			&gzflag.MultiFlag{Values: &gc.goGrpcCompilers, IsSet: &gc.goGrpcCompilersSet},
 			"go_grpc_compiler",
 			"go_proto_library compiler to use for gRPC (may be repeated)")
-		// TODO(jayconrod): remove this flag when gazelle within go_repository
-		// automatically has a list of repositories from the main WORKSPACE.
-		// Enabling module mode without this will make go_repository slower,
-		// since we'd be going out to the network much more often.
+		fs.BoolVar(
+			&gc.goRepositoryMode,
+			"go_repository_mode",
+			false,
+			"set when gazelle is invoked by go_repository")
 		fs.BoolVar(
 			&gc.moduleMode,
-			"go_experimental_module_mode",
+			"go_repository_module_mode",
 			false,
-			"when enabled, external Go imports will be resolved as modules",
-		)
+			"set when gazelle is invoked by go_repository in module mode")
 	}
 	c.Exts[goName] = gc
 }

language/go/generate.go

@@ -495,11 +495,22 @@ func (g *generator) setCommonAttrs(r *rule.Rule, pkgRel, visibility string, targ
 }
 
 func (g *generator) setImportAttrs(r *rule.Rule, importPath string) {
+	gc := getGoConfig(g.c)
 	r.SetAttr("importpath", importPath)
-	goConf := getGoConfig(g.c)
-	if goConf.importMapPrefix != "" {
-		fromPrefixRel := pathtools.TrimPrefix(g.rel, goConf.importMapPrefixRel)
-		importMap := path.Join(goConf.importMapPrefix, fromPrefixRel)
+
+	// Set importpath_aliases if we need minimal module compatibility.
+	// If a package is part of a module with a v2+ semantic import version
+	// suffix, packages that are not part of modules may import it without
+	// the suffix.
+	if gc.goRepositoryMode && gc.moduleMode && pathtools.HasPrefix(importPath, gc.prefix) && gc.prefixRel == "" {
+		if mmcImportPath := pathWithoutSemver(importPath); mmcImportPath != "" {
+			r.SetAttr("importpath_aliases", []string{mmcImportPath})
+		}
+	}
+
+	if gc.importMapPrefix != "" {
+		fromPrefixRel := pathtools.TrimPrefix(g.rel, gc.importMapPrefixRel)
+		importMap := path.Join(gc.importMapPrefix, fromPrefixRel)
 		if importMap != importPath {
 			r.SetAttr("importmap", importMap)
 		}

language/go/package.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"log"
 	"path"
+	"regexp"
 	"sort"
 	"strings"
 
@@ -486,3 +487,21 @@ func (si *platformStringInfo) convertToPlatforms() {
 		si.archs = nil
 	}
 }
+
+var semverRex = regexp.MustCompile(`^.*?(/v\d+)(?:/.*)?$`)
+
+// pathWithoutSemver removes a semantic version suffix from path.
+// For example, if path is "example.com/foo/v2/bar", pathWithoutSemver
+// will return "example.com/foo/bar". If there is no semantic version suffix,
+// "" will be returned.
+func pathWithoutSemver(path string) string {
+	m := semverRex.FindStringSubmatchIndex(path)
+	if m == nil {
+		return ""
+	}
+	v := path[m[2]+2 : m[3]]
+	if v[0] == '0' || v == "1" {
+		return ""
+	}
+	return path[:m[2]] + path[m[3]:]
+}

language/go/resolve.go

@@ -256,7 +256,7 @@ func resolveExternal(moduleMode bool, rc *repo.RemoteCache, imp string) (label.L
 	// and not the common case, especially when known repositories aren't
 	// listed in WORKSPACE (which is currently the case within go_repository).
 	if !moduleMode {
-		moduleMode = modMajorRex.FindStringIndex(imp) != nil
+		moduleMode = pathWithoutSemver(imp) != ""
 	}
 
 	var prefix, repo string
@@ -271,8 +271,12 @@ func resolveExternal(moduleMode bool, rc *repo.RemoteCache, imp string) (label.L
 	}
 
 	var pkg string
-	if imp != prefix {
+	if pathtools.HasPrefix(imp, prefix) {
 		pkg = pathtools.TrimPrefix(imp, prefix)
+	} else if impWithoutSemver := pathWithoutSemver(imp); pathtools.HasPrefix(impWithoutSemver, prefix) {
+		// We may have used minimal module compatibility to resolve a path
+		// without a semantic import version suffix to a repository that has one.
+		pkg = pathtools.TrimPrefix(impWithoutSemver, prefix)
 	}
 
 	return label.New(repo, pkg, defaultLibName), nil

language/go/resolve_test.go

@@ -1117,6 +1117,29 @@ func TestResolveExternal(t *testing.T) {
 			}},
 			moduleMode: true,
 			want:       "@custom_repo//v2/foo:go_default_library",
+		}, {
+			desc:       "min_module_compat",
+			importpath: "example.com/foo",
+			repos: []repo.Repo{{
+				Name:     "com_example_foo_v2",
+				GoPrefix: "example.com/foo/v2",
+			}},
+			moduleMode: true,
+			want:       "@com_example_foo_v2//:go_default_library",
+		}, {
+			desc:       "min_module_compat_both",
+			importpath: "example.com/foo",
+			repos: []repo.Repo{
+				{
+					Name:     "com_example_foo",
+					GoPrefix: "example.com/foo",
+				}, {
+					Name:     "com_example_foo_v2",
+					GoPrefix: "example.com/foo/v2",
+				},
+			},
+			moduleMode: true,
+			want:       "@com_example_foo//:go_default_library",
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {