Skip to content

Commit

Permalink
gopls/internal/lsp/cache: don't scan for modules when defining a view
Browse files Browse the repository at this point in the history
With zero-config gopls, we no longer need to scan for modules when
defining the default view for a folder. If there is no go.mod or go.work
file in a parent directory, just use an ad-hoc view until the first file
is opened.

Delete tests that were explicitly testing the view narrowing logic, and
so no longer make sense.

For golang/go#57979

Change-Id: Ib2ff96068b2e17d652f24d5ec05e1f2335a7f222
Reviewed-on: https://go-review.googlesource.com/c/tools/+/553096
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 Jan 5, 2024
1 parent 2e53332 commit 88ea935
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 117 deletions.
31 changes: 3 additions & 28 deletions gopls/internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,22 +883,9 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forFile fil

// When deriving the best view for a given file, we only want to search
// up the directory hierarchy for modfiles.
//
// If forURI is unset, we still use the legacy heuristic of scanning for
// nested modules (this will be removed as part of golang/go#57979).
if forFile != nil {
def.gomod, err = findRootPattern(ctx, dirURI, "go.mod", fs)
if err != nil {
return nil, err
}
} else {
// filterFunc is the path filter function for this workspace folder. Notably,
// it is relative to folder (which is specified by the user), not root.
filterFunc := relPathExcludedByFilterFunc(folder.Dir.Path(), folder.Env.GOMODCACHE, folder.Options.DirectoryFilters)
def.gomod, err = findWorkspaceModFile(ctx, folder.Dir, fs, filterFunc)
if err != nil {
return nil, err
}
def.gomod, err = findRootPattern(ctx, dirURI, "go.mod", fs)
if err != nil {
return nil, err
}

// Determine how we load and where to load package information for this view
Expand Down Expand Up @@ -1345,18 +1332,6 @@ func allFilesExcluded(files []string, filterFunc func(protocol.DocumentURI) bool
return true
}

// relPathExcludedByFilterFunc returns a func that filters paths relative to the
// given folder according the given GOMODCACHE value and directory filters (see
// settings.BuildOptions.DirectoryFilters).
//
// The resulting func returns true if the directory should be skipped.
func relPathExcludedByFilterFunc(folder, gomodcache string, directoryFilters []string) func(string) bool {
filterer := buildFilterer(folder, gomodcache, directoryFilters)
return func(path string) bool {
return relPathExcludedByFilter(path, filterer)
}
}

func relPathExcludedByFilter(path string, filterer *Filterer) bool {
path = strings.TrimPrefix(filepath.ToSlash(path), "/")
return filterer.Disallow(path)
Expand Down
7 changes: 4 additions & 3 deletions gopls/internal/test/integration/modfile/modfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
"testing"

"golang.org/x/tools/gopls/internal/hooks"
. "golang.org/x/tools/gopls/internal/test/integration"
"golang.org/x/tools/gopls/internal/test/compare"
. "golang.org/x/tools/gopls/internal/test/integration"
"golang.org/x/tools/gopls/internal/util/bug"

"golang.org/x/tools/gopls/internal/lsp/protocol"
Expand Down Expand Up @@ -427,8 +427,9 @@ func main() {
{"default", WithOptions(ProxyFiles(proxy), WorkspaceFolders("a"))},
{"nested", WithOptions(ProxyFiles(proxy))},
}.Run(t, mod, func(t *testing.T, env *Env) {
env.OnceMet(
InitialWorkspaceLoad,
// With zero-config gopls, we must open a/main.go to have a View including a/go.mod.
env.OpenFile("a/main.go")
env.AfterChange(
Diagnostics(env.AtRegexp("a/go.mod", "require")),
)
env.RunGoCommandInDir("a", "mod", "tidy")
Expand Down
48 changes: 0 additions & 48 deletions gopls/internal/test/integration/workspace/directoryfilters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,54 +90,6 @@ const X = 1
})
}

func TestDirectoryFiltersWorkspaceModules(t *testing.T) {
// Define a module include.com which should be in the workspace, plus a
// module exclude.com which should be excluded and therefore come from
// the proxy.
const files = `
-- include/go.mod --
module include.com
go 1.12
require exclude.com v1.0.0
-- include/go.sum --
exclude.com v1.0.0 h1:Q5QSfDXY5qyNCBeUiWovUGqcLCRZKoTs9XdBeVz+w1I=
exclude.com v1.0.0/go.mod h1:hFox2uDlNB2s2Jfd9tHlQVfgqUiLVTmh6ZKat4cvnj4=
-- include/include.go --
package include
import "exclude.com"
var _ = exclude.X // satisfied only by the workspace version
-- exclude/go.mod --
module exclude.com
go 1.12
-- exclude/exclude.go --
package exclude
const X = 1
`
const proxy = `
-- exclude.com@v1.0.0/go.mod --
module exclude.com
go 1.12
-- exclude.com@v1.0.0/exclude.go --
package exclude
`
WithOptions(
Modes(Experimental),
ProxyFiles(proxy),
Settings{"directoryFilters": []string{"-exclude"}},
).Run(t, files, func(t *testing.T, env *Env) {
env.Await(Diagnostics(env.AtRegexp("include/include.go", `exclude.(X)`)))
})
}

// Test for golang/go#46438: support for '**' in directory filters.
func TestDirectoryFilters_Wildcard(t *testing.T) {
filters := []string{"-**/bye"}
Expand Down
38 changes: 0 additions & 38 deletions gopls/internal/test/integration/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package workspace
import (
"context"
"fmt"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -975,43 +974,6 @@ func main() {
})
}

// Sometimes users may have their module cache within the workspace.
// We shouldn't consider any module in the module cache to be in the workspace.
func TestGOMODCACHEInWorkspace(t *testing.T) {
const mod = `
-- a/go.mod --
module a.com
go 1.12
-- a/a.go --
package a
func _() {}
-- a/c/c.go --
package c
-- gopath/src/b/b.go --
package b
-- gopath/pkg/mod/example.com/go.mod --
module example.com
go 1.12
-- gopath/pkg/mod/example.com/main.go --
package main
`
WithOptions(
EnvVars{"GOPATH": filepath.FromSlash("$SANDBOX_WORKDIR/gopath")},
Modes(Default),
).Run(t, mod, func(t *testing.T, env *Env) {
// Because logs are asynchronous, this test can't use OnceMet.
env.Await(
// Confirm that the build configuration is seen as valid,
// even though there are technically multiple go.mod files in the
// worskpace.
LogMatching(protocol.Info, ".*view type GoModView.*", 1, false),
)
})
}

// Tests the fix for golang/go#52500.
func TestChangeTestVariant_Issue52500(t *testing.T) {
const src = `
Expand Down

0 comments on commit 88ea935

Please sign in to comment.