From 9619683231abea86b02c3e2af49a1f5ee9fc5d1a Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Fri, 9 Feb 2024 15:33:01 +0000 Subject: [PATCH] gopls/internal/cache: treat local replaces as workspace modules 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 LUCI-TryBot-Result: Go LUCI --- gopls/internal/cache/session_test.go | 85 +++++++++++++++++++ gopls/internal/cache/view.go | 26 +++++- gopls/internal/cache/workspace.go | 56 ++++++++++-- gopls/internal/settings/default.go | 1 + gopls/internal/settings/settings.go | 10 +++ .../integration/workspace/zero_config_test.go | 49 +++++++++++ 6 files changed, 214 insertions(+), 13 deletions(-) diff --git a/gopls/internal/cache/session_test.go b/gopls/internal/cache/session_test.go index cc6f664060a..a3bd8ce5800 100644 --- a/gopls/internal/cache/session_test.go +++ b/gopls/internal/cache/session_test.go @@ -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 { diff --git a/gopls/internal/cache/view.go b/gopls/internal/cache/view.go index 78e85a18b0c..ed52646f31d 100644 --- a/gopls/internal/cache/view.go +++ b/gopls/internal/cache/view.go @@ -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{} @@ -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 != "" { @@ -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) } @@ -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 } diff --git a/gopls/internal/cache/workspace.go b/gopls/internal/cache/workspace.go index 4f01ebf0364..07134b3da00 100644 --- a/gopls/internal/cache/workspace.go +++ b/gopls/internal/cache/workspace.go @@ -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) { @@ -34,16 +36,28 @@ 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. @@ -51,9 +65,33 @@ 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). diff --git a/gopls/internal/settings/default.go b/gopls/internal/settings/default.go index 74b98c82555..bb248273d4d 100644 --- a/gopls/internal/settings/default.go +++ b/gopls/internal/settings/default.go @@ -115,6 +115,7 @@ func DefaultOptions(overrides ...func(*Options)) *Options { ReportAnalysisProgressAfter: 5 * time.Second, TelemetryPrompt: false, LinkifyShowMessage: false, + IncludeReplaceInWorkspace: true, }, Hooks: Hooks{ URLRegexp: urlRegexp(), diff --git a/gopls/internal/settings/settings.go b/gopls/internal/settings/settings.go index 0f1f1779982..8010dcd6e5a 100644 --- a/gopls/internal/settings/settings.go +++ b/gopls/internal/settings/settings.go @@ -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 @@ -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") diff --git a/gopls/internal/test/integration/workspace/zero_config_test.go b/gopls/internal/test/integration/workspace/zero_config_test.go index 51fd9a7dbd2..87c7c087844 100644 --- a/gopls/internal/test/integration/workspace/zero_config_test.go +++ b/gopls/internal/test/integration/workspace/zero_config_test.go @@ -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, + ) + }) + } +}