Skip to content

Commit

Permalink
gopls/internal/cache: treat local replaces as workspace modules
Browse files Browse the repository at this point in the history
Update the view selection algorithm to consider replace directives,
treating locally replaced modules in a go.mod file as workspace modules.
This causes gopls to register watch patterns for these local replaces.

Since this is a fundamental change in behavior, add a hidden off switch
to revert to the old behavior: an "includeReplaceInWorkspace" setting.

Fixes golang/go#64762
Fixes golang/go#64888

Change-Id: I0fea97a05299acd877a220982d51ba9b4d4070e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/562680
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
findleyr committed Feb 9, 2024
1 parent a5af84e commit 9619683
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 13 deletions.
85 changes: 85 additions & 0 deletions gopls/internal/cache/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,91 @@ func TestZeroConfigAlgorithm(t *testing.T) {
[]string{"a/a.go", "b/b.go", "b/c/c.go"},
[]viewSummary{{GoWorkView, ".", nil}, {GoModView, "b/c", []string{"GOWORK=off"}}},
},
{
"go.mod with nested replace",
map[string]string{
"go.mod": "module golang.org/a\n require golang.org/b v1.2.3\nreplace example.com/b => ./b",
"a.go": "package a",
"b/go.mod": "module golang.org/b\ngo 1.18\n",
"b/b.go": "package b",
},
[]folderSummary{{dir: "."}},
[]string{"a/a.go", "b/b.go"},
[]viewSummary{{GoModView, ".", nil}},
},
{
"go.mod with parent replace, parent folder",
map[string]string{
"go.mod": "module golang.org/a",
"a.go": "package a",
"b/go.mod": "module golang.org/b\ngo 1.18\nrequire golang.org/a v1.2.3\nreplace golang.org/a => ../",
"b/b.go": "package b",
},
[]folderSummary{{dir: "."}},
[]string{"a/a.go", "b/b.go"},
[]viewSummary{{GoModView, ".", nil}, {GoModView, "b", nil}},
},
{
"go.mod with multiple replace",
map[string]string{
"go.mod": `
module golang.org/root
require (
golang.org/a v1.2.3
golang.org/b v1.2.3
golang.org/c v1.2.3
)
replace (
golang.org/b => ./b
golang.org/c => ./c
// Note: d is not replaced
)
`,
"a.go": "package a",
"b/go.mod": "module golang.org/b\ngo 1.18",
"b/b.go": "package b",
"c/go.mod": "module golang.org/c\ngo 1.18",
"c/c.go": "package c",
"d/go.mod": "module golang.org/d\ngo 1.18",
"d/d.go": "package d",
},
[]folderSummary{{dir: "."}},
[]string{"b/b.go", "c/c.go", "d/d.go"},
[]viewSummary{{GoModView, ".", nil}, {GoModView, "d", nil}},
},
{
"go.mod with many replace",
map[string]string{
"go.mod": "module golang.org/a\ngo 1.18",
"a.go": "package a",
"b/go.mod": "module golang.org/b\ngo 1.18\nrequire golang.org/a v1.2.3\nreplace golang.org/a => ../",
"b/b.go": "package b",
},
[]folderSummary{{dir: "b"}},
[]string{"a/a.go", "b/b.go"},
[]viewSummary{{GoModView, "b", nil}},
},
{
"go.mod with replace directive; workspace replace off",
map[string]string{
"go.mod": "module golang.org/a\n require golang.org/b v1.2.3\nreplace example.com/b => ./b",
"a.go": "package a",
"b/go.mod": "module golang.org/b\ngo 1.18\n",
"b/b.go": "package b",
},
[]folderSummary{{
dir: ".",
options: func(string) map[string]any {
return map[string]any{
"includeReplaceInWorkspace": false,
}
},
}},
[]string{"a/a.go", "b/b.go"},
[]viewSummary{{GoModView, ".", nil}, {GoModView, "b", nil}},
},
}

for _, test := range tests {
Expand Down
26 changes: 22 additions & 4 deletions gopls/internal/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,10 @@ type viewDefinition struct {

// workspaceModFiles holds the set of mod files active in this snapshot.
//
// This is either empty, a single entry for the workspace go.mod file, or the
// set of mod files used by the workspace go.work file.
// For a go.work workspace, this is the set of workspace modfiles. For a
// go.mod workspace, this contains the go.mod file defining the workspace
// root, as well as any locally replaced modules (if
// "includeReplaceInWorkspace" is set).
//
// TODO(rfindley): should we just run `go list -m` to compute this set?
workspaceModFiles map[protocol.DocumentURI]struct{}
Expand Down Expand Up @@ -939,6 +941,22 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forFile fil
// But gopls is less strict, allowing GOPATH mode if GO111MODULE="", and
// AdHoc views if no module is found.

// gomodWorkspace is a helper to compute the correct set of workspace
// modfiles for a go.mod file, based on folder options.
gomodWorkspace := func() map[protocol.DocumentURI]unit {
modFiles := map[protocol.DocumentURI]struct{}{def.gomod: {}}
if folder.Options.IncludeReplaceInWorkspace {
includingReplace, err := goModModules(ctx, def.gomod, fs)
if err == nil {
modFiles = includingReplace
} else {
// If the go.mod file fails to parse, we don't know anything about
// replace directives, so fall back to a view of just the root module.
}
}
return modFiles
}

// Prefer a go.work file if it is available and contains the module relevant
// to forURI.
if def.adjustedGO111MODULE() != "off" && folder.Env.GOWORK != "off" && def.gowork != "" {
Expand All @@ -959,7 +977,7 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forFile fil
if _, ok := def.workspaceModFiles[def.gomod]; !ok {
def.typ = GoModView
def.root = def.gomod.Dir()
def.workspaceModFiles = map[protocol.DocumentURI]unit{def.gomod: {}}
def.workspaceModFiles = gomodWorkspace()
if def.envOverlay == nil {
def.envOverlay = make(map[string]string)
}
Expand All @@ -978,7 +996,7 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forFile fil
if def.adjustedGO111MODULE() != "off" && def.gomod != "" {
def.typ = GoModView
def.root = def.gomod.Dir()
def.workspaceModFiles = map[protocol.DocumentURI]struct{}{def.gomod: {}}
def.workspaceModFiles = gomodWorkspace()
return def, nil
}

Expand Down
56 changes: 47 additions & 9 deletions gopls/internal/cache/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ import (
"golang.org/x/tools/gopls/internal/protocol"
)

// TODO(rfindley): now that experimentalWorkspaceModule is gone, this file can
// be massively cleaned up and/or removed.
// isGoWork reports if uri is a go.work file.
func isGoWork(uri protocol.DocumentURI) bool {
return filepath.Base(uri.Path()) == "go.work"
}

// goWorkModules returns the URIs of go.mod files named by the go.work file.
func goWorkModules(ctx context.Context, gowork protocol.DocumentURI, fs file.Source) (map[protocol.DocumentURI]unit, error) {
Expand All @@ -34,26 +36,62 @@ func goWorkModules(ctx context.Context, gowork protocol.DocumentURI, fs file.Sou
if err != nil {
return nil, fmt.Errorf("parsing go.work: %w", err)
}
modFiles := make(map[protocol.DocumentURI]unit)
var usedDirs []string
for _, use := range workFile.Use {
modDir := filepath.FromSlash(use.Path)
usedDirs = append(usedDirs, use.Path)
}
return localModFiles(dir, usedDirs), nil
}

// localModFiles builds a set of local go.mod files referenced by
// goWorkOrModPaths, which is a slice of paths as contained in a go.work 'use'
// directive or go.mod 'replace' directive (and which therefore may use either
// '/' or '\' as a path separator).
func localModFiles(relativeTo string, goWorkOrModPaths []string) map[protocol.DocumentURI]unit {
modFiles := make(map[protocol.DocumentURI]unit)
for _, path := range goWorkOrModPaths {
modDir := filepath.FromSlash(path)
if !filepath.IsAbs(modDir) {
modDir = filepath.Join(dir, modDir)
modDir = filepath.Join(relativeTo, modDir)
}
modURI := protocol.URIFromPath(filepath.Join(modDir, "go.mod"))
modFiles[modURI] = unit{}
}
return modFiles, nil
return modFiles
}

// isGoMod reports if uri is a go.mod file.
func isGoMod(uri protocol.DocumentURI) bool {
return filepath.Base(uri.Path()) == "go.mod"
}

// isGoWork reports if uri is a go.work file.
func isGoWork(uri protocol.DocumentURI) bool {
return filepath.Base(uri.Path()) == "go.work"
// goModModules returns the URIs of "workspace" go.mod files defined by a
// go.mod file. This set is defined to be the given go.mod file itself, as well
// as the modfiles of any locally replaced modules in the go.mod file.
func goModModules(ctx context.Context, gomod protocol.DocumentURI, fs file.Source) (map[protocol.DocumentURI]unit, error) {
fh, err := fs.ReadFile(ctx, gomod)
if err != nil {
return nil, err // canceled
}
content, err := fh.Content()
if err != nil {
return nil, err
}
filename := gomod.Path()
dir := filepath.Dir(filename)
modFile, err := modfile.Parse(filename, content, nil)
if err != nil {
return nil, err
}
var localReplaces []string
for _, replace := range modFile.Replace {
if modfile.IsDirectoryPath(replace.New.Path) {
localReplaces = append(localReplaces, replace.New.Path)
}
}
modFiles := localModFiles(dir, localReplaces)
modFiles[gomod] = unit{}
return modFiles, nil
}

// fileExists reports whether the file has a Content (which may be empty).
Expand Down
1 change: 1 addition & 0 deletions gopls/internal/settings/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func DefaultOptions(overrides ...func(*Options)) *Options {
ReportAnalysisProgressAfter: 5 * time.Second,
TelemetryPrompt: false,
LinkifyShowMessage: false,
IncludeReplaceInWorkspace: true,
},
Hooks: Hooks{
URLRegexp: urlRegexp(),
Expand Down
10 changes: 10 additions & 0 deletions gopls/internal/settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,12 @@ type InternalOptions struct {
// LinkifyShowMessage controls whether the client wants gopls
// to linkify links in showMessage. e.g. [go.dev](https://go.dev).
LinkifyShowMessage bool

// IncludeReplaceInWorkspace controls whether locally replaced modules in a
// go.mod file are treated like workspace modules.
// Or in other words, if a go.mod file with local replaces behaves like a
// go.work file.
IncludeReplaceInWorkspace bool
}

type SubdirWatchPatterns string
Expand Down Expand Up @@ -1146,9 +1152,13 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})

case "telemetryPrompt":
result.setBool(&o.TelemetryPrompt)

case "linkifyShowMessage":
result.setBool(&o.LinkifyShowMessage)

case "includeReplaceInWorkspace":
result.setBool(&o.IncludeReplaceInWorkspace)

// Replaced settings.
case "experimentalDisabledAnalyses":
result.deprecated("analyses")
Expand Down
49 changes: 49 additions & 0 deletions gopls/internal/test/integration/workspace/zero_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,52 @@ const C = 0
)
})
}

func TestGoModReplace(t *testing.T) {
// This test checks that we treat locally replaced modules as workspace
// modules, according to the "includeReplaceInWorkspace" setting.
const files = `
-- moda/go.mod --
module golang.org/a
require golang.org/b v1.2.3
replace golang.org/b => ../modb
go 1.20
-- moda/a.go --
package a
import "golang.org/b"
const A = b.B
-- modb/go.mod --
module golang.org/b
go 1.20
-- modb/b.go --
package b
const B = 1
`

for useReplace, expectation := range map[bool]Expectation{
true: FileWatchMatching("modb"),
false: NoFileWatchMatching("modb"),
} {
WithOptions(
WorkspaceFolders("moda"),
Settings{
"includeReplaceInWorkspace": useReplace,
},
).Run(t, files, func(t *testing.T, env *Env) {
env.OnceMet(
InitialWorkspaceLoad,
expectation,
)
})
}
}

0 comments on commit 9619683

Please sign in to comment.