Skip to content

Commit

Permalink
gopls/internal/lsp/cache: remove validBuildConfiguration
Browse files Browse the repository at this point in the history
The validBuildConfiguration helper never had a well-defined meaning, and
now just means that the view is an ad-hoc view. Delete it and check the
view type directly.

Also, revisit the log message formatting view information.

For golang/go#57979

Change-Id: Ia09f697dd96c1930f1c97c74f08a81698ad17f30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/553095
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 a863a4f commit 2e53332
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 57 deletions.
15 changes: 3 additions & 12 deletions gopls/internal/lsp/cache/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (s *Snapshot) load(ctx context.Context, allowNetwork bool, scopes ...loadSc
case viewLoadScope:
// If we are outside of GOPATH, a module, or some other known
// build system, don't load subdirectories.
if !s.validBuildConfiguration() {
if s.view.typ == AdHocView {
query = append(query, "./")
} else {
query = append(query, "./...")
Expand Down Expand Up @@ -329,22 +329,13 @@ func (s *Snapshot) workspaceLayoutError(ctx context.Context) (error, []*Diagnost
// If the snapshot does not have a valid build configuration, it may be
// that the user has opened a directory that contains multiple modules.
// Check for that an warn about it.
if !s.validBuildConfiguration() {
var msg string
if s.view.folder.Env.GoVersion >= 18 {
msg = `gopls was not able to find modules in your workspace.
if s.view.typ == AdHocView {
msg := `gopls was not able to find modules in your workspace.
When outside of GOPATH, gopls needs to know which modules you are working on.
You can fix this by opening your workspace to a folder inside a Go module, or
by using a go.work file to specify multiple modules.
See the documentation for more information on setting up your workspace:
https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.`
} else {
msg = `gopls requires a module at the root of your workspace.
You can work with multiple modules by upgrading to Go 1.18 or later, and using
go workspaces (go.work files).
See the documentation for more information on setting up your workspace:
https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.`
}
return fmt.Errorf(msg), s.applyCriticalErrorToFiles(ctx, msg, openFiles)
}

Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cache/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ type (
dir string // dir containing the go.mod file
modulePath string // parsed module path
}
viewLoadScope protocol.DocumentURI // load the workspace
viewLoadScope struct{} // load the workspace
)

// Implement the loadScope interface.
Expand Down
29 changes: 4 additions & 25 deletions gopls/internal/lsp/cache/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,26 +351,6 @@ func (s *Snapshot) Templates() map[protocol.DocumentURI]file.Handle {
return tmpls
}

func (s *Snapshot) validBuildConfiguration() bool {
// Since we only really understand the `go` command, if the user has a
// different GOPACKAGESDRIVER, assume that their configuration is valid.
if s.view.typ == GoPackagesDriverView {
return true
}

// Check if the user is working within a module or if we have found
// multiple modules in the workspace.
if len(s.view.workspaceModFiles) > 0 {
return true
}

if s.view.typ == GOPATHView {
return true
}

return false
}

// config returns the configuration used for the snapshot's interaction with
// the go/packages API. It uses the given working directory.
//
Expand Down Expand Up @@ -1431,7 +1411,7 @@ If you are using modules, please open your editor to a directory in your module.
If you believe this warning is incorrect, please file an issue: https://github.com/golang/go/issues/new.`

func shouldShowAdHocPackagesWarning(snapshot *Snapshot, active []*metadata.Package) string {
if !snapshot.validBuildConfiguration() {
if snapshot.view.typ == AdHocView {
for _, mp := range active {
// A blank entry in DepsByImpPath
// indicates a missing dependency.
Expand Down Expand Up @@ -1528,10 +1508,9 @@ func (s *Snapshot) reloadWorkspace(ctx context.Context) error {
return nil
}

// If the view's build configuration is invalid, we cannot reload by
// package path. Just reload the directory instead.
if !s.validBuildConfiguration() {
scopes = []loadScope{viewLoadScope("LOAD_INVALID_VIEW")}
// For an ad-hoc view, we cannot reload by package path. Just reload the view.
if s.view.typ == AdHocView {
scopes = []loadScope{viewLoadScope{}}
}

err := s.load(ctx, false, scopes...)
Expand Down
28 changes: 10 additions & 18 deletions gopls/internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,34 +410,26 @@ func (s *Session) UpdateFolders(ctx context.Context, newFolders []*Folder) error
// viewEnv returns a string describing the environment of a newly created view.
//
// It must not be called concurrently with any other view methods.
//
// TODO(golang/go#57979): revisit this function and its uses once the dust
// settles.
// TODO(rfindley): rethink this function, or inline sole call.
func viewEnv(v *View) string {
env := v.folder.Options.EnvSlice()
buildFlags := append([]string{}, v.folder.Options.BuildFlags...)

var buf bytes.Buffer
fmt.Fprintf(&buf, `go info for %v
(go dir %s)
(view type %v)
(root dir %s)
(go version %s)
(valid build configuration = %v)
(build flags: %v)
(go env: %+v)
(env overlay: %v)
`,
v.folder.Dir.Path(),
v.typ,
v.root.Path(),
strings.TrimRight(v.folder.Env.GoVersionOutput, "\n"),
v.snapshot.validBuildConfiguration(),
buildFlags,
v.folder.Options.BuildFlags,
*v.snapshot.view.folder.Env,
v.snapshot.view.envOverlay,
)

for _, v := range env {
s := strings.SplitN(v, "=", 2)
if len(s) != 2 {
continue
}
}

return buf.String()
}

Expand Down Expand Up @@ -715,7 +707,7 @@ func (s *Snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) (loadEr
scopes = append(scopes, moduleLoadScope{dir: moduleDir, modulePath: parsed.File.Module.Mod.Path})
}
} else {
scopes = append(scopes, viewLoadScope("LOAD_VIEW"))
scopes = append(scopes, viewLoadScope{})
}

// If we're loading anything, ensure we also load builtin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ package main
// Confirm that the build configuration is seen as valid,
// even though there are technically multiple go.mod files in the
// worskpace.
LogMatching(protocol.Info, ".*valid build configuration = true.*", 1, false),
LogMatching(protocol.Info, ".*view type GoModView.*", 1, false),
)
})
}
Expand Down

0 comments on commit 2e53332

Please sign in to comment.