diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index 07be50406af..a838c73df6b 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -593,6 +593,7 @@ Result: "Type": string, "Root": string, "Folder": string, + "EnvOverlay": []string, } ``` diff --git a/gopls/internal/lsp/cache/port.go b/gopls/internal/lsp/cache/port.go new file mode 100644 index 00000000000..e62ebe29903 --- /dev/null +++ b/gopls/internal/lsp/cache/port.go @@ -0,0 +1,204 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cache + +import ( + "bytes" + "go/build" + "go/parser" + "go/token" + "io" + "path/filepath" + "strings" + + "golang.org/x/tools/gopls/internal/util/bug" +) + +type port struct{ GOOS, GOARCH string } + +var ( + // preferredPorts holds GOOS/GOARCH combinations for which we dynamically + // create new Views, by setting GOOS=... and GOARCH=... on top of + // user-provided configuration when we detect that the default build + // configuration does not match an open file. Ports are matched in the order + // defined below, so that when multiple ports match a file we use the port + // occurring at a lower index in the slice. For that reason, we sort first + // class ports ahead of secondary ports, and (among first class ports) 64-bit + // ports ahead of the less common 32-bit ports. + preferredPorts = []port{ + // First class ports, from https://go.dev/wiki/PortingPolicy. + {"darwin", "amd64"}, + {"darwin", "arm64"}, + {"linux", "amd64"}, + {"linux", "arm64"}, + {"windows", "amd64"}, + {"linux", "arm"}, + {"linux", "386"}, + {"windows", "386"}, + + // Secondary ports, from GOROOT/src/internal/platform/zosarch.go. + // (First class ports are commented out.) + {"aix", "ppc64"}, + {"dragonfly", "amd64"}, + {"freebsd", "386"}, + {"freebsd", "amd64"}, + {"freebsd", "arm"}, + {"freebsd", "arm64"}, + {"illumos", "amd64"}, + {"linux", "ppc64"}, + {"linux", "ppc64le"}, + {"linux", "mips"}, + {"linux", "mipsle"}, + {"linux", "mips64"}, + {"linux", "mips64le"}, + {"linux", "riscv64"}, + {"linux", "s390x"}, + {"android", "386"}, + {"android", "amd64"}, + {"android", "arm"}, + {"android", "arm64"}, + {"ios", "arm64"}, + {"ios", "amd64"}, + {"js", "wasm"}, + {"netbsd", "386"}, + {"netbsd", "amd64"}, + {"netbsd", "arm"}, + {"netbsd", "arm64"}, + {"openbsd", "386"}, + {"openbsd", "amd64"}, + {"openbsd", "arm"}, + {"openbsd", "arm64"}, + {"openbsd", "mips64"}, + {"plan9", "386"}, + {"plan9", "amd64"}, + {"plan9", "arm"}, + {"solaris", "amd64"}, + {"windows", "arm"}, + {"windows", "arm64"}, + + {"aix", "ppc64"}, + {"android", "386"}, + {"android", "amd64"}, + {"android", "arm"}, + {"android", "arm64"}, + // {"darwin", "amd64"}, + // {"darwin", "arm64"}, + {"dragonfly", "amd64"}, + {"freebsd", "386"}, + {"freebsd", "amd64"}, + {"freebsd", "arm"}, + {"freebsd", "arm64"}, + {"freebsd", "riscv64"}, + {"illumos", "amd64"}, + {"ios", "amd64"}, + {"ios", "arm64"}, + {"js", "wasm"}, + // {"linux", "386"}, + // {"linux", "amd64"}, + // {"linux", "arm"}, + // {"linux", "arm64"}, + {"linux", "loong64"}, + {"linux", "mips"}, + {"linux", "mips64"}, + {"linux", "mips64le"}, + {"linux", "mipsle"}, + {"linux", "ppc64"}, + {"linux", "ppc64le"}, + {"linux", "riscv64"}, + {"linux", "s390x"}, + {"linux", "sparc64"}, + {"netbsd", "386"}, + {"netbsd", "amd64"}, + {"netbsd", "arm"}, + {"netbsd", "arm64"}, + {"openbsd", "386"}, + {"openbsd", "amd64"}, + {"openbsd", "arm"}, + {"openbsd", "arm64"}, + {"openbsd", "mips64"}, + {"openbsd", "ppc64"}, + {"openbsd", "riscv64"}, + {"plan9", "386"}, + {"plan9", "amd64"}, + {"plan9", "arm"}, + {"solaris", "amd64"}, + {"wasip1", "wasm"}, + // {"windows", "386"}, + // {"windows", "amd64"}, + {"windows", "arm"}, + {"windows", "arm64"}, + } +) + +// matches reports whether the port matches a file with the given absolute path +// and content. +// +// Note that this function accepts content rather than e.g. a file.Handle, +// because we trim content before matching for performance reasons, and +// therefore need to do this outside of matches when considering multiple ports. +func (p port) matches(path string, content []byte) bool { + ctxt := build.Default // make a copy + ctxt.UseAllFiles = false + dir, name := filepath.Split(path) + + // The only virtualized operation called by MatchFile is OpenFile. + ctxt.OpenFile = func(p string) (io.ReadCloser, error) { + if p != path { + return nil, bug.Errorf("unexpected file %q", p) + } + return io.NopCloser(bytes.NewReader(content)), nil + } + + ctxt.GOOS = p.GOOS + ctxt.GOARCH = p.GOARCH + ok, err := ctxt.MatchFile(dir, name) + return err == nil && ok +} + +// trimContentForPortMatch trims the given Go file content to a minimal file +// containing the same build constraints, if any. +// +// This is an unfortunate but necessary optimization, as matching build +// constraints using go/build has significant overhead, and involves parsing +// more than just the build constraint. +// +// TestMatchingPortsConsistency enforces consistency by comparing results +// without trimming content. +func trimContentForPortMatch(content []byte) []byte { + buildComment := buildComment(content) + return []byte(buildComment + "\npackage p") // package name does not matter +} + +// buildComment returns the first matching //go:build comment in the given +// content, or "" if none exists. +func buildComment(content []byte) string { + f, err := parser.ParseFile(token.NewFileSet(), "", content, parser.PackageClauseOnly|parser.ParseComments) + if err != nil { + return "" + } + + for _, cg := range f.Comments { + for _, c := range cg.List { + if isGoBuildComment(c.Text) { + return c.Text + } + } + } + return "" +} + +// Adapted from go/build/build.go. +// +// TODO(rfindley): use constraint.IsGoBuild once we are on 1.19+. +func isGoBuildComment(line string) bool { + const goBuildComment = "//go:build" + if !strings.HasPrefix(line, goBuildComment) { + return false + } + // Report whether //go:build is followed by a word boundary. + line = strings.TrimSpace(line) + rest := line[len(goBuildComment):] + return len(rest) == 0 || len(strings.TrimSpace(rest)) < len(rest) +} diff --git a/gopls/internal/lsp/cache/port_test.go b/gopls/internal/lsp/cache/port_test.go new file mode 100644 index 00000000000..96ba31846f8 --- /dev/null +++ b/gopls/internal/lsp/cache/port_test.go @@ -0,0 +1,126 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cache + +import ( + "os" + "testing" + + "github.com/google/go-cmp/cmp" + "golang.org/x/sync/errgroup" + "golang.org/x/tools/go/packages" + "golang.org/x/tools/gopls/internal/file" + "golang.org/x/tools/gopls/internal/lsp/protocol" + "golang.org/x/tools/gopls/internal/util/bug" + "golang.org/x/tools/internal/testenv" +) + +func TestMain(m *testing.M) { + bug.PanicOnBugs = true + os.Exit(m.Run()) +} + +func TestMatchingPortsStdlib(t *testing.T) { + // This test checks that we don't encounter a bug when matching ports, and + // sanity checks that the optimization to use trimmed/fake file content + // before delegating to go/build.Context.MatchFile does not affect + // correctness. + if testing.Short() { + t.Skip("skipping in short mode: takes to long on slow file systems") + } + + testenv.NeedsTool(t, "go") + + // Load, parse and type-check the program. + cfg := &packages.Config{ + Mode: packages.LoadFiles, + Tests: true, + } + pkgs, err := packages.Load(cfg, "std", "cmd") + if err != nil { + t.Fatal(err) + } + + var g errgroup.Group + packages.Visit(pkgs, nil, func(pkg *packages.Package) { + for _, f := range pkg.CompiledGoFiles { + f := f + g.Go(func() error { + content, err := os.ReadFile(f) + // We report errors via t.Error, not by returning, + // so that a single test can report multiple test failures. + if err != nil { + t.Errorf("failed to read %s: %v", f, err) + return nil + } + fh := makeFakeFileHandle(protocol.URIFromPath(f), content) + fastPorts := matchingPreferredPorts(t, fh, true) + slowPorts := matchingPreferredPorts(t, fh, false) + if diff := cmp.Diff(fastPorts, slowPorts); diff != "" { + t.Errorf("%s: ports do not match (-trimmed +untrimmed):\n%s", f, diff) + return nil + } + return nil + }) + } + }) + g.Wait() +} + +func matchingPreferredPorts(tb testing.TB, fh file.Handle, trimContent bool) map[port]unit { + content, err := fh.Content() + if err != nil { + tb.Fatal(err) + } + if trimContent { + content = trimContentForPortMatch(content) + } + path := fh.URI().Path() + matching := make(map[port]unit) + for _, port := range preferredPorts { + if port.matches(path, content) { + matching[port] = unit{} + } + } + return matching +} + +func BenchmarkMatchingPreferredPorts(b *testing.B) { + // Copy of robustio_posix.go + const src = ` +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !windows && !plan9 +// +build !windows,!plan9 + +// TODO(adonovan): use 'unix' tag when go1.19 can be assumed. + +package robustio + +import ( + "os" + "syscall" + "time" +) + +func getFileID(filename string) (FileID, time.Time, error) { + fi, err := os.Stat(filename) + if err != nil { + return FileID{}, time.Time{}, err + } + stat := fi.Sys().(*syscall.Stat_t) + return FileID{ + device: uint64(stat.Dev), // (int32 on darwin, uint64 on linux) + inode: stat.Ino, + }, fi.ModTime(), nil +} +` + fh := makeFakeFileHandle("file:///path/to/test/file.go", []byte(src)) + for i := 0; i < b.N; i++ { + _ = matchingPreferredPorts(b, fh, true) + } +} diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go index 719d91fd036..7436bcb60f2 100644 --- a/gopls/internal/lsp/cache/session.go +++ b/gopls/internal/lsp/cache/session.go @@ -99,7 +99,7 @@ func (s *Session) NewView(ctx context.Context, folder *Folder) (*View, *Snapshot } } - def, err := defineView(ctx, s, folder, "") + def, err := defineView(ctx, s, folder, nil) if err != nil { return nil, nil, nil, err } @@ -366,17 +366,14 @@ func (s *Session) viewOfLocked(ctx context.Context, uri protocol.DocumentURI) (* v, hit := s.viewMap[uri] if !hit { // Cache miss: compute (and memoize) the best view. - var defs []*viewDefinition - viewLookup := make(map[*viewDefinition]*View) - for _, v := range s.views { - defs = append(defs, v.viewDefinition) - viewLookup[v.viewDefinition] = v + fh, err := s.ReadFile(ctx, uri) + if err != nil { + return nil, err } - def, err := bestViewDefForURI(ctx, s, uri, defs) + v, err = bestView(ctx, s, fh, s.views) if err != nil { return nil, err } - v = viewLookup[def] // possibly nil if s.viewMap == nil { return nil, errors.New("session is shut down") } @@ -405,7 +402,7 @@ func selectViewDefs(ctx context.Context, fs file.Source, folders []*Folder, open // DidChangeWorkspaceFolders could introduce a path-dependent ordering on // folders. We should keep folders sorted, or sort them here. for _, folder := range folders { - def, err := defineView(ctx, fs, folder, "") + def, err := defineView(ctx, fs, folder, nil) if err != nil { return nil, err } @@ -436,16 +433,24 @@ checkFiles: if folder == nil { continue // only guess views for open files } - def, err := bestViewDefForURI(ctx, fs, uri, defs) + fh, err := fs.ReadFile(ctx, uri) if err != nil { return nil, err } + def, err := bestView(ctx, fs, fh, defs) + if err != nil { + // We should never call selectViewDefs with a cancellable context, so + // this should never fail. + return nil, bug.Errorf("failed to find best view for open file: %v", err) + } if def != nil { continue // file covered by an existing view } - def, err = defineView(ctx, fs, folder, uri) + def, err = defineView(ctx, fs, folder, fh) if err != nil { - return nil, err + // We should never call selectViewDefs with a cancellable context, so + // this should never fail. + return nil, bug.Errorf("failed to define view for open file: %v", err) } // It need not strictly be the case that the best view for a file is // distinct from other views, as the logic of getViewDefinition and @@ -464,80 +469,134 @@ checkFiles: return defs, nil } -// bestViewDefForURI returns the existing view that contains the existing file, -// or (nil, nil) if no matching view is found. +// The viewDefiner interface allows the bestView algorithm to operate on both +// Views and viewDefinitions. +type viewDefiner interface{ definition() *viewDefinition } + +// bestView returns the best View or viewDefinition that contains the +// given file, or (nil, nil) if no matching view is found. // -// The provided uri must be a file uri, not directory. +// bestView only returns an error in the event of context cancellation. // -// bestViewDefForURI only returns an error in the event of context cancellation. +// Making this function generic is convenient so that we can avoid mapping view +// definitions back to views inside Session.DidModifyFiles, where performance +// matters. It is, however, not the cleanest application of generics. // -// TODO(rfindley): if we pass a file's []constraint.Expr here, we can implement -// improved build tag support. -func bestViewDefForURI(ctx context.Context, fs file.Source, uri protocol.DocumentURI, views []*viewDefinition) (*viewDefinition, error) { +// Note: keep this function in sync with defineView. +func bestView[V viewDefiner](ctx context.Context, fs file.Source, fh file.Handle, views []V) (V, error) { + var zero V + if len(views) == 0 { - return nil, nil // avoid the call to findRootPattern + return zero, nil // avoid the call to findRootPattern } + uri := fh.URI() dir := uri.Dir() modURI, err := findRootPattern(ctx, dir, "go.mod", fs) if err != nil { - return nil, err + return zero, err } // Prefer GoWork > GoMod > GOPATH > GoPackages > AdHoc. var ( - goPackagesView *viewDefinition // prefer longest - gopathView *viewDefinition // prefer longest - adHocView *viewDefinition // exact match - modView *viewDefinition // exact match - // TODO(rfindley): should we also prefer the longest matching go.work view? - // If two go.work files contain a module, which one is more natural to use? + goPackagesViews []V // prefer longest + workViews []V // prefer longest + modViews []V // exact match + gopathViews []V // prefer longest + adHocViews []V // exact match ) + + // pushView updates the views slice with the matching view v, using the + // heuristic that views with a longer root are preferable. Accordingly, + // pushView may be a no op if v's root is shorter than the roots in the views + // slice. + // + // Invariant: the length of all roots in views is the same. + pushView := func(views *[]V, v V) { + if len(*views) == 0 { + *views = []V{v} + return + } + better := func(l, r V) bool { + return len(l.definition().root) > len(r.definition().root) + } + existing := (*views)[0] + switch { + case better(existing, v): + case better(v, existing): + *views = []V{v} + default: + *views = append(*views, v) + } + } + for _, view := range views { - switch view.Type() { + switch def := view.definition(); def.Type() { case GoPackagesDriverView: - if goPackagesView != nil && len(goPackagesView.root) > len(view.root) { - continue - } - if view.root.Encloses(dir) { - goPackagesView = view + if def.root.Encloses(dir) { + pushView(&goPackagesViews, view) } case GoWorkView: - if uri == view.gowork { - return view, nil - } - if _, ok := view.workspaceModFiles[modURI]; ok { - return view, nil + if _, ok := def.workspaceModFiles[modURI]; ok || uri == def.gowork { + pushView(&workViews, view) } case GoModView: - if modURI == view.gomod { - modView = view + if modURI == def.gomod { + modViews = append(modViews, view) } case GOPATHView: - if gopathView != nil && len(gopathView.root) > len(view.root) { - continue - } - if view.root.Encloses(dir) { - gopathView = view + if def.root.Encloses(dir) { + pushView(&gopathViews, view) } case AdHocView: - if view.root == dir { - adHocView = view + if def.root == dir { + adHocViews = append(adHocViews, view) } } } - if modView != nil { - return modView, nil - } - if gopathView != nil { - return gopathView, nil + + // Now that we've collected matching views, choose the best match, + // considering ports. + // + // We only consider one type of view, since the matching view created by + // defineView should be of the best type. + var bestViews []V + switch { + case len(workViews) > 0: + bestViews = workViews + case len(modViews) > 0: + bestViews = modViews + case len(gopathViews) > 0: + bestViews = gopathViews + case len(goPackagesViews) > 0: + bestViews = goPackagesViews + case len(adHocViews) > 0: + bestViews = adHocViews + default: + return zero, nil } - if goPackagesView != nil { - return goPackagesView, nil + + content, err := fh.Content() + // Port matching doesn't apply to non-go files, or files that no longer exist. + // Note that the behavior here on non-existent files shouldn't matter much, + // since there will be a subsequent failure. But it is simpler to preserve + // the invariant that bestView only fails on context cancellation. + if fileKind(fh) != file.Go || err != nil { + return bestViews[0], nil } - if adHocView != nil { - return adHocView, nil + + // Find the first view that matches constraints. + // Content trimming is nontrivial, so do this outside of the loop below. + path := fh.URI().Path() + content = trimContentForPortMatch(content) + for _, v := range bestViews { + def := v.definition() + viewPort := port{def.GOOS(), def.GOARCH()} + if viewPort.matches(path, content) { + return v, nil + } } - return nil, nil // no view found + + return zero, nil // no view found } // updateViewLocked recreates the view with the given options. @@ -617,7 +676,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio // This is done while holding viewMu because the set of open files affects // the set of views, and to prevent views from seeing updated file content // before they have processed invalidations. - overlays, err := s.updateOverlays(ctx, changes) + replaced, err := s.updateOverlays(ctx, changes) if err != nil { return nil, err } @@ -629,7 +688,8 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio changed := make(map[protocol.DocumentURI]file.Handle) for _, c := range changes { - changed[c.URI] = mustReadFile(ctx, s, c.URI) + fh := mustReadFile(ctx, s, c.URI) + changed[c.URI] = fh // Any change to the set of open files causes views to be recomputed. if c.Action == file.Open || c.Action == file.Close { @@ -650,6 +710,27 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio if isGoMod(c.URI) && c.Action != file.Change && c.Action != file.Save { checkViews = true } + + // Any change to the set of supported ports in a file may affect view + // selection. This is perhaps more subtle than it first seems: since the + // algorithm for selecting views considers open files in a deterministic + // order, a change in supported ports may cause a different port to be + // chosen, even if all open files still match an existing View! + // + // We endeavor to avoid that sort of path dependence, so must re-run the + // view selection algorithm whenever any input changes. + // + // However, extracting the build comment is nontrivial, so we don't want to + // pay this cost when e.g. processing a bunch of on-disk changes due to a + // branch change. Be careful to only do this if both files are open Go + // files. + if old, ok := replaced[c.URI]; ok && !checkViews && fileKind(fh) == file.Go { + if new, ok := fh.(*Overlay); ok { + if buildComment(old.content) != buildComment(new.content) { + checkViews = true + } + } + } } if checkViews { @@ -668,7 +749,7 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio } var openFiles []protocol.DocumentURI - for _, o := range overlays { + for _, o := range s.Overlays() { openFiles = append(openFiles, o.URI()) } // Sort for determinism. @@ -721,20 +802,11 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio } } - // Collect view definitions, for resolving the best view for each change. - var viewDefinitions []*viewDefinition - viewLookup := make(map[*viewDefinition]*View) - for _, view := range s.views { - viewDefinitions = append(viewDefinitions, view.viewDefinition) - viewLookup[view.viewDefinition] = view - } - // We only want to run fast-path diagnostics (i.e. diagnoseChangedFiles) once - // for each changed file, in its best view. Collect files into their best - // views. - viewsToDiagnose := make(map[*View][]protocol.DocumentURI) + // for each changed file, in its best view. + viewsToDiagnose := map[*View][]protocol.DocumentURI{} for _, mod := range changes { - def, err := bestViewDefForURI(ctx, s, mod.URI, viewDefinitions) + v, err := s.viewOfLocked(ctx, mod.URI) if err != nil { // bestViewForURI only returns an error in the event of context // cancellation. Since state changes should occur on an uncancellable @@ -742,14 +814,13 @@ func (s *Session) DidModifyFiles(ctx context.Context, changes []file.Modificatio bug.Reportf("finding best view for change: %v", err) continue } - if def != nil { - v := viewLookup[def] + if v != nil { viewsToDiagnose[v] = append(viewsToDiagnose[v], mod.URI) } } // ...but changes may be relevant to other views, for example if they are - // changes to a shared packaged. + // changes to a shared package. for _, v := range s.views { _, release, needsDiagnosis := v.Invalidate(ctx, StateChange{Files: changed}) release() @@ -810,14 +881,21 @@ func (s *Session) ExpandModificationsToDirectories(ctx context.Context, changes return result } +// updateOverlays updates the set of overlays and returns a map of any existing +// overlay values that were replaced. +// // Precondition: caller holds s.viewMu lock. // TODO(rfindley): move this to fs_overlay.go. -func (fs *overlayFS) updateOverlays(ctx context.Context, changes []file.Modification) ([]*Overlay, error) { +func (fs *overlayFS) updateOverlays(ctx context.Context, changes []file.Modification) (map[protocol.DocumentURI]*Overlay, error) { fs.mu.Lock() defer fs.mu.Unlock() + replaced := make(map[protocol.DocumentURI]*Overlay) for _, c := range changes { o, ok := fs.overlays[c.URI] + if ok { + replaced[c.URI] = o + } // If the file is not opened in an overlay and the change is on disk, // there's no need to update an overlay. If there is an overlay, we @@ -893,12 +971,7 @@ func (fs *overlayFS) updateOverlays(ctx context.Context, changes []file.Modifica fs.overlays[c.URI] = o } - var overlays []*Overlay - for _, o := range fs.overlays { - overlays = append(overlays, o) - } - - return overlays, nil + return replaced, nil } func mustReadFile(ctx context.Context, fs file.Source, uri protocol.DocumentURI) file.Handle { diff --git a/gopls/internal/lsp/cache/session_test.go b/gopls/internal/lsp/cache/session_test.go index 1ff92edb174..11046a21214 100644 --- a/gopls/internal/lsp/cache/session_test.go +++ b/gopls/internal/lsp/cache/session_test.go @@ -274,7 +274,7 @@ func TestZeroConfigAlgorithm(t *testing.T) { got = append(got, viewSummary{ Type: def.Type(), Root: rel.RelPath(def.root.Path()), - Env: def.envOverlay, + Env: def.EnvOverlay(), }) } if diff := cmp.Diff(test.want, got); diff != "" { diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go index a774dc96d86..14c06744475 100644 --- a/gopls/internal/lsp/cache/snapshot.go +++ b/gopls/internal/lsp/cache/snapshot.go @@ -273,7 +273,7 @@ func (s *Snapshot) View() *View { return s.view } -// FileKind returns the type of a file. +// FileKind returns the kind of a file. // // We can't reliably deduce the kind from the file name alone, // as some editors can be told to interpret a buffer as @@ -281,6 +281,28 @@ func (s *Snapshot) View() *View { // an .html file actually contains Go "html/template" syntax, // or even that a .go file contains Python. func (s *Snapshot) FileKind(fh file.Handle) file.Kind { + if k := fileKind(fh); k != file.UnknownKind { + return k + } + fext := filepath.Ext(fh.URI().Path()) + exts := s.Options().TemplateExtensions + for _, ext := range exts { + if fext == ext || fext == "."+ext { + return file.Tmpl + } + } + + // and now what? This should never happen, but it does for cgo before go1.15 + // + // TODO(rfindley): this doesn't look right. We should default to UnknownKind. + // Also, I don't understand the comment above, though I'd guess before go1.15 + // we encountered cgo files without the .go extension. + return file.Go +} + +// fileKind returns the default file kind for a file, before considering +// template file extensions. See [Snapshot.FileKind]. +func fileKind(fh file.Handle) file.Kind { // The kind of an unsaved buffer comes from the // TextDocumentItem.LanguageID field in the didChange event, // not from the file name. They may differ. @@ -301,14 +323,7 @@ func (s *Snapshot) FileKind(fh file.Handle) file.Kind { case ".work": return file.Work } - exts := s.Options().TemplateExtensions - for _, ext := range exts { - if fext == ext || fext == "."+ext { - return file.Tmpl - } - } - // and now what? This should never happen, but it does for cgo before go1.15 - return file.Go + return file.UnknownKind } // Options returns the options associated with this snapshot. @@ -512,7 +527,7 @@ func (s *Snapshot) goCommandInvocation(ctx context.Context, flags InvocationFlag s.Options().EnvSlice(), inv.Env, []string{"GO111MODULE=" + s.view.adjustedGO111MODULE()}, - s.view.envOverlay, + s.view.EnvOverlay(), ) inv.BuildFlags = append([]string{}, s.Options().BuildFlags...) cleanup = func() {} // fallback diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go index a37bf1d0b1b..05dce692f28 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -16,6 +16,7 @@ import ( "path" "path/filepath" "regexp" + "sort" "strings" "sync" "time" @@ -55,6 +56,8 @@ type Folder struct { type GoEnv struct { // Go environment variables. These correspond directly with the Go env var of // the same name. + GOOS string + GOARCH string GOCACHE string GOMODCACHE string GOPATH string @@ -128,6 +131,9 @@ type View struct { initializationSema chan struct{} } +// definition implements the viewDefiner interface. +func (v *View) definition() *viewDefinition { return v.viewDefinition } + // A viewDefinition is a logical build, i.e. configuration (Folder) along with // a build directory and possibly an environment overlay (e.g. GOWORK=off or // GOOS, GOARCH=...) to affect the build. @@ -158,21 +164,53 @@ type viewDefinition struct { workspaceModFilesErr error // error encountered computing workspaceModFiles // envOverlay holds additional environment to apply to this viewDefinition. - envOverlay []string + envOverlay map[string]string } +// definition implements the viewDefiner interface. +func (d *viewDefinition) definition() *viewDefinition { return d } + // Type returns the ViewType type, which determines how go/packages are loaded // for this View. -func (d viewDefinition) Type() ViewType { return d.typ } +func (d *viewDefinition) Type() ViewType { return d.typ } // Root returns the view root, which determines where packages are loaded from. -func (d viewDefinition) Root() protocol.DocumentURI { return d.root } +func (d *viewDefinition) Root() protocol.DocumentURI { return d.root } // GoMod returns the nearest go.mod file for this view's root, or "". -func (d viewDefinition) GoMod() protocol.DocumentURI { return d.gomod } +func (d *viewDefinition) GoMod() protocol.DocumentURI { return d.gomod } // GoWork returns the nearest go.work file for this view's root, or "". -func (d viewDefinition) GoWork() protocol.DocumentURI { return d.gowork } +func (d *viewDefinition) GoWork() protocol.DocumentURI { return d.gowork } + +// EnvOverlay returns a new sorted slice of environment variables (in the form +// "k=v") for this view definition's env overlay. +func (d *viewDefinition) EnvOverlay() []string { + var env []string + for k, v := range d.envOverlay { + env = append(env, fmt.Sprintf("%s=%s", k, v)) + } + sort.Strings(env) + return env +} + +// GOOS returns the effective GOOS value for this view definition, accounting +// for its env overlay. +func (d *viewDefinition) GOOS() string { + if goos, ok := d.envOverlay["GOOS"]; ok { + return goos + } + return d.folder.Env.GOOS +} + +// GOOS returns the effective GOARCH value for this view definition, accounting +// for its env overlay. +func (d *viewDefinition) GOARCH() string { + if goarch, ok := d.envOverlay["GOARCH"]; ok { + return goarch + } + return d.folder.Env.GOARCH +} // adjustedGO111MODULE is the value of GO111MODULE to use for loading packages. // It is adjusted to default to "auto" rather than "on", since if we are in @@ -787,24 +825,57 @@ func (v *View) Invalidate(ctx context.Context, changed StateChange) (*Snapshot, // // defineView only returns an error in the event of context cancellation. // +// Note: keep this function in sync with bestView. +// // TODO(rfindley): we should be able to remove the error return, as // findModules is going away, and all other I/O is memoized. // // TODO(rfindley): pass in a narrower interface for the file.Source // (e.g. fileExists func(DocumentURI) bool) to make clear that this // process depends only on directory information, not file contents. -func defineView(ctx context.Context, fs file.Source, folder *Folder, forURI protocol.DocumentURI) (*viewDefinition, error) { +func defineView(ctx context.Context, fs file.Source, folder *Folder, forFile file.Handle) (*viewDefinition, error) { if err := checkPathValid(folder.Dir.Path()); err != nil { return nil, fmt.Errorf("invalid workspace folder path: %w; check that the spelling of the configured workspace folder path agrees with the spelling reported by the operating system", err) } dir := folder.Dir.Path() - if forURI != "" { - dir = filepath.Dir(forURI.Path()) + if forFile != nil { + dir = filepath.Dir(forFile.URI().Path()) } def := new(viewDefinition) def.folder = folder + if forFile != nil && fileKind(forFile) == file.Go { + // If the file has GOOS/GOARCH build constraints that + // don't match the folder's environment (which comes from + // 'go env' in the folder, plus user options), + // add those constraints to the viewDefinition's environment. + + // Content trimming is nontrivial, so do this outside of the loop below. + // Keep this in sync with bestView. + path := forFile.URI().Path() + if content, err := forFile.Content(); err == nil { + // Note the err == nil condition above: by convention a non-existent file + // does not have any constraints. See the related note in bestView: this + // choice of behavior shouldn't actually matter. In this case, we should + // only call defineView with Overlays, which always have content. + content = trimContentForPortMatch(content) + viewPort := port{def.folder.Env.GOOS, def.folder.Env.GOARCH} + if !viewPort.matches(path, content) { + for _, p := range preferredPorts { + if p.matches(path, content) { + if def.envOverlay == nil { + def.envOverlay = make(map[string]string) + } + def.envOverlay["GOOS"] = p.GOOS + def.envOverlay["GOARCH"] = p.GOARCH + break + } + } + } + } + } + var err error dirURI := protocol.URIFromPath(dir) goworkFromEnv := false @@ -823,7 +894,7 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forURI prot // // 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 forURI != "" { + if forFile != nil { def.gomod, err = findRootPattern(ctx, dirURI, "go.mod", fs) if err != nil { return nil, err @@ -884,12 +955,15 @@ func defineView(ctx context.Context, fs file.Source, folder *Folder, forURI prot // If forURI is in a module but that module is not // included in the go.work file, use a go.mod view with GOWORK=off. - if forURI != "" && def.workspaceModFilesErr == nil && def.gomod != "" { + if forFile != nil && def.workspaceModFilesErr == nil && def.gomod != "" { if _, ok := def.workspaceModFiles[def.gomod]; !ok { def.typ = GoModView def.root = def.gomod.Dir() def.workspaceModFiles = map[protocol.DocumentURI]unit{def.gomod: {}} - def.envOverlay = []string{"GOWORK=off"} + if def.envOverlay == nil { + def.envOverlay = make(map[string]string) + } + def.envOverlay["GOWORK"] = "off" } } return def, nil @@ -946,6 +1020,8 @@ func FetchGoEnv(ctx context.Context, folder protocol.DocumentURI, opts *settings err error ) envvars := map[string]*string{ + "GOOS": &env.GOOS, + "GOARCH": &env.GOARCH, "GOCACHE": &env.GOCACHE, "GOPATH": &env.GOPATH, "GOPRIVATE": &env.GOPRIVATE, diff --git a/gopls/internal/lsp/command/interface.go b/gopls/internal/lsp/command/interface.go index 03de23aa93e..152387c2053 100644 --- a/gopls/internal/lsp/command/interface.go +++ b/gopls/internal/lsp/command/interface.go @@ -509,7 +509,8 @@ type DiagnoseFilesArgs struct { // A View holds summary information about a cache.View. type View struct { - Type string // view type (via cache.ViewType.String) - Root protocol.DocumentURI // root dir of the view (e.g. containing go.mod or go.work) - Folder protocol.DocumentURI // workspace folder associated with the view + Type string // view type (via cache.ViewType.String) + Root protocol.DocumentURI // root dir of the view (e.g. containing go.mod or go.work) + Folder protocol.DocumentURI // workspace folder associated with the view + EnvOverlay []string // environment variable overrides } diff --git a/gopls/internal/server/command.go b/gopls/internal/server/command.go index a569f26a883..330f0a8ba3e 100644 --- a/gopls/internal/server/command.go +++ b/gopls/internal/server/command.go @@ -1350,9 +1350,10 @@ func (c *commandHandler) Views(ctx context.Context) ([]command.View, error) { var summaries []command.View for _, view := range c.s.session.Views() { summaries = append(summaries, command.View{ - Type: view.Type().String(), - Root: view.Root(), - Folder: view.Folder().Dir, + Type: view.Type().String(), + Root: view.Root(), + Folder: view.Folder().Dir, + EnvOverlay: view.EnvOverlay(), }) } return summaries, nil diff --git a/gopls/internal/settings/api_json.go b/gopls/internal/settings/api_json.go index 0f69d2b2566..7f6760947d4 100644 --- a/gopls/internal/settings/api_json.go +++ b/gopls/internal/settings/api_json.go @@ -916,7 +916,7 @@ var GeneratedAPIJSON = &APIJSON{ Command: "gopls.views", Title: "List current Views on the server.", Doc: "This command is intended for use by gopls tests only.", - ResultDoc: "[]{\n\t\"Type\": string,\n\t\"Root\": string,\n\t\"Folder\": string,\n}", + ResultDoc: "[]{\n\t\"Type\": string,\n\t\"Root\": string,\n\t\"Folder\": string,\n\t\"EnvOverlay\": []string,\n}", }, { Command: "gopls.workspace_stats", diff --git a/gopls/internal/test/integration/workspace/workspace_test.go b/gopls/internal/test/integration/workspace/workspace_test.go index 09292493ff7..da935093574 100644 --- a/gopls/internal/test/integration/workspace/workspace_test.go +++ b/gopls/internal/test/integration/workspace/workspace_test.go @@ -11,10 +11,7 @@ import ( "strings" "testing" - "github.com/google/go-cmp/cmp" "golang.org/x/tools/gopls/internal/hooks" - "golang.org/x/tools/gopls/internal/lsp/cache" - "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/test/integration/fake" "golang.org/x/tools/gopls/internal/util/bug" @@ -1015,90 +1012,6 @@ package main }) } -func TestAddAndRemoveGoWork(t *testing.T) { - // TODO(golang/go#57979): update this test to assert that zero-config - // behavior means more than just a lack of diagnostics. - - // Use a workspace with a module in the root directory to exercise the case - // where a go.work is added to the existing root directory. This verifies - // that we're detecting changes to the module source, not just the root - // directory. - const nomod = ` --- go.mod -- -module a.com - -go 1.16 --- main.go -- -package main - -func main() {} --- b/go.mod -- -module b.com - -go 1.16 --- b/main.go -- -package main - -func main() {} -` - WithOptions( - Modes(Default), - ).Run(t, nomod, func(t *testing.T, env *Env) { - env.OpenFile("main.go") - env.OpenFile("b/main.go") - - summary := func(typ cache.ViewType, root, folder string) command.View { - return command.View{ - Type: typ.String(), - Root: env.Sandbox.Workdir.URI(root), - Folder: env.Sandbox.Workdir.URI(folder), - } - } - checkViews := func(want ...command.View) { - got := env.Views() - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("SummarizeViews() mismatch (-want +got):\n%s", diff) - } - } - - // Zero-config gopls makes this work. - env.AfterChange( - NoDiagnostics(ForFile("main.go")), - NoDiagnostics(env.AtRegexp("b/main.go", "package (main)")), - ) - checkViews(summary(cache.GoModView, ".", "."), summary(cache.GoModView, "b", ".")) - - env.WriteWorkspaceFile("go.work", `go 1.16 - -use ( - . - b -) -`) - env.AfterChange(NoDiagnostics()) - checkViews(summary(cache.GoWorkView, ".", ".")) - - // Removing the go.work file should put us back where we started. - env.RemoveWorkspaceFile("go.work") - - // Again, zero-config gopls makes this work. - env.AfterChange( - NoDiagnostics(ForFile("main.go")), - NoDiagnostics(env.AtRegexp("b/main.go", "package (main)")), - ) - checkViews(summary(cache.GoModView, ".", "."), summary(cache.GoModView, "b", ".")) - - // Close and reopen b, to ensure the views are adjusted accordingly. - env.CloseBuffer("b/main.go") - env.AfterChange() - checkViews(summary(cache.GoModView, ".", ".")) - - env.OpenFile("b/main.go") - env.AfterChange() - checkViews(summary(cache.GoModView, ".", "."), summary(cache.GoModView, "b", ".")) - }) -} - // Tests the fix for golang/go#52500. func TestChangeTestVariant_Issue52500(t *testing.T) { const src = ` diff --git a/gopls/internal/test/integration/workspace/zero_config_test.go b/gopls/internal/test/integration/workspace/zero_config_test.go new file mode 100644 index 00000000000..a1991b5930e --- /dev/null +++ b/gopls/internal/test/integration/workspace/zero_config_test.go @@ -0,0 +1,163 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package workspace + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "golang.org/x/tools/gopls/internal/lsp/cache" + "golang.org/x/tools/gopls/internal/lsp/command" + + . "golang.org/x/tools/gopls/internal/test/integration" +) + +func TestAddAndRemoveGoWork(t *testing.T) { + // Use a workspace with a module in the root directory to exercise the case + // where a go.work is added to the existing root directory. This verifies + // that we're detecting changes to the module source, not just the root + // directory. + const nomod = ` +-- go.mod -- +module a.com + +go 1.16 +-- main.go -- +package main + +func main() {} +-- b/go.mod -- +module b.com + +go 1.16 +-- b/main.go -- +package main + +func main() {} +` + WithOptions( + Modes(Default), + ).Run(t, nomod, func(t *testing.T, env *Env) { + env.OpenFile("main.go") + env.OpenFile("b/main.go") + + summary := func(typ cache.ViewType, root, folder string) command.View { + return command.View{ + Type: typ.String(), + Root: env.Sandbox.Workdir.URI(root), + Folder: env.Sandbox.Workdir.URI(folder), + } + } + checkViews := func(want ...command.View) { + got := env.Views() + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("SummarizeViews() mismatch (-want +got):\n%s", diff) + } + } + + // Zero-config gopls makes this work. + env.AfterChange( + NoDiagnostics(ForFile("main.go")), + NoDiagnostics(env.AtRegexp("b/main.go", "package (main)")), + ) + checkViews(summary(cache.GoModView, ".", "."), summary(cache.GoModView, "b", ".")) + + env.WriteWorkspaceFile("go.work", `go 1.16 + +use ( + . + b +) +`) + env.AfterChange(NoDiagnostics()) + checkViews(summary(cache.GoWorkView, ".", ".")) + + // Removing the go.work file should put us back where we started. + env.RemoveWorkspaceFile("go.work") + + // Again, zero-config gopls makes this work. + env.AfterChange( + NoDiagnostics(ForFile("main.go")), + NoDiagnostics(env.AtRegexp("b/main.go", "package (main)")), + ) + checkViews(summary(cache.GoModView, ".", "."), summary(cache.GoModView, "b", ".")) + + // Close and reopen b, to ensure the views are adjusted accordingly. + env.CloseBuffer("b/main.go") + env.AfterChange() + checkViews(summary(cache.GoModView, ".", ".")) + + env.OpenFile("b/main.go") + env.AfterChange() + checkViews(summary(cache.GoModView, ".", "."), summary(cache.GoModView, "b", ".")) + }) +} + +func TestOpenAndClosePorts(t *testing.T) { + // This test checks that as we open and close files requiring a different + // port, the set of Views is adjusted accordingly. + const files = ` +-- go.mod -- +module a.com/a + +go 1.20 + +-- a_linux.go -- +package a + +-- a_darwin.go -- +package a + +-- a_windows.go -- +package a +` + + WithOptions( + EnvVars{ + "GOOS": "linux", // assume that linux is the default GOOS + }, + ).Run(t, files, func(t *testing.T, env *Env) { + summary := func(envOverlay ...string) command.View { + return command.View{ + Type: cache.GoModView.String(), + Root: env.Sandbox.Workdir.URI("."), + Folder: env.Sandbox.Workdir.URI("."), + EnvOverlay: envOverlay, + } + } + checkViews := func(want ...command.View) { + got := env.Views() + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("SummarizeViews() mismatch (-want +got):\n%s", diff) + } + } + checkViews(summary()) + env.OpenFile("a_linux.go") + checkViews(summary()) + env.OpenFile("a_darwin.go") + checkViews( + summary(), + summary("GOARCH=amd64", "GOOS=darwin"), + ) + env.OpenFile("a_windows.go") + checkViews( + summary(), + summary("GOARCH=amd64", "GOOS=darwin"), + summary("GOARCH=amd64", "GOOS=windows"), + ) + env.CloseBuffer("a_darwin.go") + checkViews( + summary(), + summary("GOARCH=amd64", "GOOS=windows"), + ) + env.CloseBuffer("a_linux.go") + checkViews( + summary(), + summary("GOARCH=amd64", "GOOS=windows"), + ) + env.CloseBuffer("a_windows.go") + checkViews(summary()) + }) +} diff --git a/gopls/internal/test/marker/testdata/diagnostics/excludedfile.txt b/gopls/internal/test/marker/testdata/diagnostics/excludedfile.txt index 75f2030f6b8..ae3045b338d 100644 --- a/gopls/internal/test/marker/testdata/diagnostics/excludedfile.txt +++ b/gopls/internal/test/marker/testdata/diagnostics/excludedfile.txt @@ -1,10 +1,9 @@ This test demonstrates diagnostics for various forms of file exclusion. -Skip on plan9, an arbitrary GOOS, so that we can exercise GOOS exclusions -resulting from file suffixes. - --- flags -- --skip_goos=plan9 +Note: this test used to also check the errors when a file was excluded due to +an inactive module, or mismatching GOOS/GOARCH, comment, but with zero-config +gopls (golang/go#57979) and improved build tag support (golang/go#29202), we no +longer get these errors. -- go.work -- go 1.21 @@ -21,7 +20,7 @@ go 1.18 package a -- a/a_plan9.go -- -package a //@diag(re"package (a)", re"excluded due to its GOOS/GOARCH") +package a // Not excluded, due to improved build tag support. -- a/a_ignored.go -- //go:build skip diff --git a/gopls/internal/test/marker/testdata/zeroconfig/dynamicports.txt b/gopls/internal/test/marker/testdata/zeroconfig/dynamicports.txt new file mode 100644 index 00000000000..6dcdfe4cd7a --- /dev/null +++ b/gopls/internal/test/marker/testdata/zeroconfig/dynamicports.txt @@ -0,0 +1,118 @@ +This test checks that the zero-config algorithm selects Views to cover first +class ports. + +In this test, package a imports b, and b imports c. Package a contains files +constrained by go:build directives, package b contains files constrained by the +GOOS matching their file name, and package c is unconstrained. Various +assertions check that diagnostics and navigation work as expected. + +-- go.mod -- +module golang.org/lsptests + +-- a/a.go -- +package a + +import "golang.org/lsptests/b" + +var _ = b.F //@loc(F, "F") + +-- a/linux64.go -- +//go:build (linux && amd64) + +package a + +import "golang.org/lsptests/b" + +var _ int = 1<<32 -1 // OK on 64 bit platforms. Compare linux32.go below. + +var ( + _ = b.LinuxOnly //@def("LinuxOnly", LinuxOnly) + _ = b.DarwinOnly //@diag("DarwinOnly", re"(undefined|declared)") + _ = b.WindowsOnly //@diag("WindowsOnly", re"(undefined|declared)") +) + +-- a/linux32.go -- +//go:build (linux && 386) + +package a + +import "golang.org/lsptests/b" + +var _ int = 1<<32 -1 //@diag("1<<32", re"overflows") + +var ( + _ = b.LinuxOnly //@def("LinuxOnly", LinuxOnly) + _ = b.DarwinOnly //@diag("DarwinOnly", re"(undefined|declared)") + _ = b.WindowsOnly //@diag("WindowsOnly", re"(undefined|declared)") +) + +-- a/darwin64.go -- +//go:build (darwin && amd64) + +package a + +import "golang.org/lsptests/b" + +var ( + _ = b.LinuxOnly //@diag("LinuxOnly", re"(undefined|declared)") + _ = b.DarwinOnly //@def("DarwinOnly", DarwinOnly) + _ = b.WindowsOnly //@diag("WindowsOnly", re"(undefined|declared)") +) + +-- a/windows64.go -- +//go:build (windows && amd64) + +package a + +import "golang.org/lsptests/b" + +var ( + _ = b.LinuxOnly //@diag("LinuxOnly", re"(undefined|declared)") + _ = b.DarwinOnly //@diag("DarwinOnly", re"(undefined|declared)") + _ = b.WindowsOnly //@def("WindowsOnly", WindowsOnly) +) + +-- b/b_other.go -- +//go:build !linux && !darwin && !windows +package b + +func F() {} + +-- b/b_linux.go -- +package b + +import "golang.org/lsptests/c" + +func F() { //@refs("F", "F", F) + x := c.Common //@diag("x", re"not used"),def("Common", Common) +} + +const LinuxOnly = "darwin" //@loc(LinuxOnly, "LinuxOnly") + +-- b/b_darwin.go -- +package b + +import "golang.org/lsptests/c" + +func F() { //@refs("F", "F", F) + x := c.Common //@diag("x", re"not used"),def("Common", Common) +} + +const DarwinOnly = "darwin" //@loc(DarwinOnly, "DarwinOnly") + +-- b/b_windows.go -- +package b + +import "golang.org/lsptests/c" + +func F() { //@refs("F", "F", F) + x := c.Common //@diag("x", re"not used"),def("Common", Common) +} + +const WindowsOnly = "windows" //@loc(WindowsOnly, "WindowsOnly") + +-- c/c.go -- +package c + +const Common = 0 //@loc(Common, "Common") +