gopls/internal/lsp/source: add the "symbolScope" option
Add a new "symbolScope" option that controls whether matches are
restricted to workspace packages only. This is the new default behavior,
though the old behavior can be enabled by setting "symbolScope": "all".
Fixes golang/go#37236
Change-Id: Ic614b57005488e61ea0c018a68076785e667db16
Reviewed-on: https://go-review.googlesource.com/c/tools/+/490935
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md
index f15af0b..fa810f0 100644
--- a/gopls/doc/settings.md
+++ b/gopls/doc/settings.md
@@ -456,6 +456,22 @@
Default: `"Dynamic"`.
+##### **symbolScope** *enum*
+
+symbolScope controls which packages are searched for workspace/symbol
+requests. The default value, "workspace", searches only workspace
+packages. The legacy behavior, "all", causes all loaded packages to be
+searched, including dependencies; this is more expensive and may return
+unwanted results.
+
+Must be one of:
+
+* `"all"` matches symbols in any loaded package, including
+dependencies.
+* `"workspace"` matches symbols in workspace packages only.
+
+Default: `"workspace"`.
+
#### **verboseOutput** *bool*
**This setting is for debugging purposes only.**
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 62a84c5..6bd4be8 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -1113,18 +1113,26 @@
// a loaded package. It awaits snapshot loading.
//
// TODO(rfindley): move this to the top of cache/symbols.go
-func (s *snapshot) Symbols(ctx context.Context) (map[span.URI][]source.Symbol, error) {
+func (s *snapshot) Symbols(ctx context.Context, workspaceOnly bool) (map[span.URI][]source.Symbol, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}
- // Build symbols for all loaded Go files.
- s.mu.Lock()
- meta := s.meta
- s.mu.Unlock()
+ var (
+ meta []*source.Metadata
+ err error
+ )
+ if workspaceOnly {
+ meta, err = s.WorkspaceMetadata(ctx)
+ } else {
+ meta, err = s.AllMetadata(ctx)
+ }
+ if err != nil {
+ return nil, fmt.Errorf("loading metadata: %v", err)
+ }
goFiles := make(map[span.URI]struct{})
- for _, m := range meta.metadata {
+ for _, m := range meta {
for _, uri := range m.GoFiles {
goFiles[uri] = struct{}{}
}
diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go
index 98fe23f..b6b8bda 100644
--- a/gopls/internal/lsp/regtest/marker.go
+++ b/gopls/internal/lsp/regtest/marker.go
@@ -882,14 +882,21 @@
// archive-relative paths for files and including the line number in the full
// archive file.
func (run *markerTestRun) fmtLoc(loc protocol.Location) string {
- return run.fmtLocDetails(loc, true)
+ formatted := run.fmtLocDetails(loc, true)
+ if formatted == "" {
+ run.env.T.Errorf("unable to find %s in test archive", loc)
+ return "<invalid location>"
+ }
+ return formatted
}
// See fmtLoc. If includeTxtPos is not set, the position in the full archive
// file is omitted.
+//
+// If the location cannot be found within the archive, fmtLocDetails returns "".
func (run *markerTestRun) fmtLocDetails(loc protocol.Location, includeTxtPos bool) string {
if loc == (protocol.Location{}) {
- return "<missing location>"
+ return ""
}
lines := bytes.Count(run.test.archive.Comment, []byte("\n"))
var name string
@@ -903,8 +910,7 @@
lines += bytes.Count(f.Data, []byte("\n"))
}
if name == "" {
- run.env.T.Errorf("unable to find %s in test archive", loc)
- return "<invalid location>"
+ return ""
}
m, err := run.env.Editor.Mapper(name)
if err != nil {
@@ -1675,6 +1681,11 @@
// Omit the txtar position of the symbol location; otherwise edits to the
// txtar archive lead to unexpected failures.
loc := mark.run.fmtLocDetails(s.Location, false)
+ // TODO(rfindley): can we do better here, by detecting if the location is
+ // relative to GOROOT?
+ if loc == "" {
+ loc = "<unknown>"
+ }
fmt.Fprintf(&got, "%s %s %s\n", loc, s.Name, s.Kind)
}
diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go
index b564be1..2eb26d2 100644
--- a/gopls/internal/lsp/source/api_json.go
+++ b/gopls/internal/lsp/source/api_json.go
@@ -195,6 +195,23 @@
Hierarchy: "ui.navigation",
},
{
+ Name: "symbolScope",
+ Type: "enum",
+ Doc: "symbolScope controls which packages are searched for workspace/symbol\nrequests. The default value, \"workspace\", searches only workspace\npackages. The legacy behavior, \"all\", causes all loaded packages to be\nsearched, including dependencies; this is more expensive and may return\nunwanted results.\n",
+ EnumValues: []EnumValue{
+ {
+ Value: "\"all\"",
+ Doc: "`\"all\"` matches symbols in any loaded package, including\ndependencies.\n",
+ },
+ {
+ Value: "\"workspace\"",
+ Doc: "`\"workspace\"` matches symbols in workspace packages only.\n",
+ },
+ },
+ Default: "\"workspace\"",
+ Hierarchy: "ui.navigation",
+ },
+ {
Name: "analyses",
Type: "map[string]bool",
Doc: "analyses specify analyses that the user would like to enable or disable.\nA map of the names of analysis passes that should be enabled/disabled.\nA full list of analyzers that gopls uses can be found in\n[analyzers.md](https://github.com/golang/tools/blob/master/gopls/doc/analyzers.md).\n\nExample Usage:\n\n```json5\n...\n\"analyses\": {\n \"unreachable\": false, // Disable the unreachable analyzer.\n \"unusedparams\": true // Enable the unusedparams analyzer.\n}\n...\n```\n",
diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go
index 20a2903..4667b64 100644
--- a/gopls/internal/lsp/source/options.go
+++ b/gopls/internal/lsp/source/options.go
@@ -143,6 +143,7 @@
ImportShortcut: BothShortcuts,
SymbolMatcher: SymbolFastFuzzy,
SymbolStyle: DynamicSymbols,
+ SymbolScope: WorkspaceSymbolScope,
},
CompletionOptions: CompletionOptions{
Matcher: Fuzzy,
@@ -454,6 +455,13 @@
// }
// ```
SymbolStyle SymbolStyle `status:"advanced"`
+
+ // SymbolScope controls which packages are searched for workspace/symbol
+ // requests. The default value, "workspace", searches only workspace
+ // packages. The legacy behavior, "all", causes all loaded packages to be
+ // searched, including dependencies; this is more expensive and may return
+ // unwanted results.
+ SymbolScope SymbolScope
}
// UserOptions holds custom Gopls configuration (not part of the LSP) that is
@@ -617,6 +625,8 @@
CaseSensitive Matcher = "CaseSensitive"
)
+// A SymbolMatcher controls the matching of symbols for workspace/symbol
+// requests.
type SymbolMatcher string
const (
@@ -626,6 +636,7 @@
SymbolCaseSensitive SymbolMatcher = "CaseSensitive"
)
+// A SymbolStyle controls the formatting of symbols in workspace/symbol results.
type SymbolStyle string
const (
@@ -642,6 +653,17 @@
DynamicSymbols SymbolStyle = "Dynamic"
)
+// A SymbolScope controls the search scope for workspace/symbol requests.
+type SymbolScope string
+
+const (
+ // WorkspaceSymbolScope matches symbols in workspace packages only.
+ WorkspaceSymbolScope SymbolScope = "workspace"
+ // AllSymbolScope matches symbols in any loaded package, including
+ // dependencies.
+ AllSymbolScope SymbolScope = "all"
+)
+
type HoverKind string
const (
@@ -969,6 +991,14 @@
o.SymbolStyle = SymbolStyle(s)
}
+ case "symbolScope":
+ if s, ok := result.asOneOf(
+ string(WorkspaceSymbolScope),
+ string(AllSymbolScope),
+ ); ok {
+ o.SymbolScope = SymbolScope(s)
+ }
+
case "hoverKind":
if s, ok := result.asOneOf(
string(NoDocumentation),
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index a0a6fe9..a86874a 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -159,7 +159,10 @@
CriticalError(ctx context.Context) *CriticalError
// Symbols returns all symbols in the snapshot.
- Symbols(ctx context.Context) (map[span.URI][]Symbol, error)
+ //
+ // If workspaceOnly is set, this only includes symbols from files in a
+ // workspace package. Otherwise, it returns symbols from all loaded packages.
+ Symbols(ctx context.Context, workspaceOnly bool) (map[span.URI][]Symbol, error)
// -- package metadata --
diff --git a/gopls/internal/lsp/source/workspace_symbol.go b/gopls/internal/lsp/source/workspace_symbol.go
index a0ffe3f..4a65d52 100644
--- a/gopls/internal/lsp/source/workspace_symbol.go
+++ b/gopls/internal/lsp/source/workspace_symbol.go
@@ -317,10 +317,16 @@
filters := v.Options().DirectoryFilters
filterer := NewFilterer(filters)
folder := filepath.ToSlash(v.Folder().Filename())
- symbols, err := snapshot.Symbols(ctx)
+
+ workspaceOnly := true
+ if v.Options().SymbolScope == AllSymbolScope {
+ workspaceOnly = false
+ }
+ symbols, err := snapshot.Symbols(ctx, workspaceOnly)
if err != nil {
return nil, err
}
+
for uri, syms := range symbols {
norm := filepath.ToSlash(uri.Filename())
nm := strings.TrimPrefix(norm, folder)
@@ -496,6 +502,8 @@
}
}
+ // TODO(rfindley): use metadata to determine if the file is in a workspace
+ // package, rather than this heuristic.
inWorkspace := false
for _, root := range roots {
if strings.HasPrefix(string(i.uri), root) {
diff --git a/gopls/internal/regtest/marker/testdata/workspacesymbol/allscope.txt b/gopls/internal/regtest/marker/testdata/workspacesymbol/allscope.txt
new file mode 100644
index 0000000..18fe4e5
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/workspacesymbol/allscope.txt
@@ -0,0 +1,30 @@
+This test verifies behavior when "symbolScope" is set to "all".
+
+-- settings.json --
+{
+ "symbolStyle": "full",
+ "symbolMatcher": "casesensitive",
+ "symbolScope": "all"
+}
+
+-- go.mod --
+module mod.test/symbols
+
+go 1.18
+
+-- query.go --
+package symbols
+
+//@workspacesymbol("fmt.Println", println)
+
+-- fmt/fmt.go --
+package fmt
+
+import "fmt"
+
+func Println(s string) {
+ fmt.Println(s)
+}
+-- @println --
+fmt/fmt.go:5:6-13 mod.test/symbols/fmt.Println Function
+<unknown> fmt.Println Function
diff --git a/gopls/internal/regtest/marker/testdata/workspacesymbol/wsscope.txt b/gopls/internal/regtest/marker/testdata/workspacesymbol/wsscope.txt
new file mode 100644
index 0000000..023230d
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/workspacesymbol/wsscope.txt
@@ -0,0 +1,29 @@
+This test verifies behavior when "symbolScope" is set to the default value
+("workspace").
+
+-- settings.json --
+{
+ "symbolStyle": "full",
+ "symbolMatcher": "casesensitive"
+}
+
+-- go.mod --
+module mod.test/symbols
+
+go 1.18
+
+-- query.go --
+package symbols
+
+//@workspacesymbol("fmt.Println", println)
+
+-- fmt/fmt.go --
+package fmt
+
+import "fmt"
+
+func Println(s string) {
+ fmt.Println(s)
+}
+-- @println --
+fmt/fmt.go:5:6-13 mod.test/symbols/fmt.Println Function
diff --git a/gopls/internal/regtest/misc/workspace_symbol_test.go b/gopls/internal/regtest/misc/workspace_symbol_test.go
index 849743b..a99323d 100644
--- a/gopls/internal/regtest/misc/workspace_symbol_test.go
+++ b/gopls/internal/regtest/misc/workspace_symbol_test.go
@@ -73,7 +73,6 @@
"Fooex", // shorter than Fooest, FooBar, lexically before Fooey
"Fooey", // shorter than Fooest, Foobar
"Fooest",
- "unsafe.Offsetof", // a very fuzzy match
)
})
}
