Skip to content

Commit 8e9b185

Browse files
findleyrfindleyr
committed
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>
1 parent 4ac71c0 commit 8e9b185

File tree

10 files changed

+164
-13
lines changed

10 files changed

+164
-13
lines changed

‎gopls/doc/settings.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,22 @@ just "Foo.Field".
456456

457457
Default: `"Dynamic"`.
458458

459+
##### **symbolScope** *enum*
460+
461+
symbolScope controls which packages are searched for workspace/symbol
462+
requests. The default value, "workspace", searches only workspace
463+
packages. The legacy behavior, "all", causes all loaded packages to be
464+
searched, including dependencies; this is more expensive and may return
465+
unwanted results.
466+
467+
Must be one of:
468+
469+
* `"all"` matches symbols in any loaded package, including
470+
dependencies.
471+
* `"workspace"` matches symbols in workspace packages only.
472+
473+
Default: `"workspace"`.
474+
459475
#### **verboseOutput** *bool*
460476

461477
**This setting is for debugging purposes only.**

‎gopls/internal/lsp/cache/snapshot.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,18 +1113,26 @@ func (s *snapshot) WorkspaceMetadata(ctx context.Context) ([]*source.Metadata, e
11131113
// a loaded package. It awaits snapshot loading.
11141114
//
11151115
// TODO(rfindley): move this to the top of cache/symbols.go
1116-
func (s *snapshot) Symbols(ctx context.Context) (map[span.URI][]source.Symbol, error) {
1116+
func (s *snapshot) Symbols(ctx context.Context, workspaceOnly bool) (map[span.URI][]source.Symbol, error) {
11171117
if err := s.awaitLoaded(ctx); err != nil {
11181118
return nil, err
11191119
}
11201120

1121-
// Build symbols for all loaded Go files.
1122-
s.mu.Lock()
1123-
meta := s.meta
1124-
s.mu.Unlock()
1121+
var (
1122+
meta []*source.Metadata
1123+
err error
1124+
)
1125+
if workspaceOnly {
1126+
meta, err = s.WorkspaceMetadata(ctx)
1127+
} else {
1128+
meta, err = s.AllMetadata(ctx)
1129+
}
1130+
if err != nil {
1131+
return nil, fmt.Errorf("loading metadata: %v", err)
1132+
}
11251133

11261134
goFiles := make(map[span.URI]struct{})
1127-
for _, m := range meta.metadata {
1135+
for _, m := range meta {
11281136
for _, uri := range m.GoFiles {
11291137
goFiles[uri] = struct{}{}
11301138
}

‎gopls/internal/lsp/regtest/marker.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -882,14 +882,21 @@ func (run *markerTestRun) fmtPos(pos token.Pos) string {
882882
// archive-relative paths for files and including the line number in the full
883883
// archive file.
884884
func (run *markerTestRun) fmtLoc(loc protocol.Location) string {
885-
return run.fmtLocDetails(loc, true)
885+
formatted := run.fmtLocDetails(loc, true)
886+
if formatted == "" {
887+
run.env.T.Errorf("unable to find %s in test archive", loc)
888+
return "<invalid location>"
889+
}
890+
return formatted
886891
}
887892

888893
// See fmtLoc. If includeTxtPos is not set, the position in the full archive
889894
// file is omitted.
895+
//
896+
// If the location cannot be found within the archive, fmtLocDetails returns "".
890897
func (run *markerTestRun) fmtLocDetails(loc protocol.Location, includeTxtPos bool) string {
891898
if loc == (protocol.Location{}) {
892-
return "<missing location>"
899+
return ""
893900
}
894901
lines := bytes.Count(run.test.archive.Comment, []byte("\n"))
895902
var name string
@@ -903,8 +910,7 @@ func (run *markerTestRun) fmtLocDetails(loc protocol.Location, includeTxtPos boo
903910
lines += bytes.Count(f.Data, []byte("\n"))
904911
}
905912
if name == "" {
906-
run.env.T.Errorf("unable to find %s in test archive", loc)
907-
return "<invalid location>"
913+
return ""
908914
}
909915
m, err := run.env.Editor.Mapper(name)
910916
if err != nil {
@@ -1675,6 +1681,11 @@ func workspaceSymbolMarker(mark marker, query string, golden *Golden) {
16751681
// Omit the txtar position of the symbol location; otherwise edits to the
16761682
// txtar archive lead to unexpected failures.
16771683
loc := mark.run.fmtLocDetails(s.Location, false)
1684+
// TODO(rfindley): can we do better here, by detecting if the location is
1685+
// relative to GOROOT?
1686+
if loc == "" {
1687+
loc = "<unknown>"
1688+
}
16781689
fmt.Fprintf(&got, "%s %s %s\n", loc, s.Name, s.Kind)
16791690
}
16801691

‎gopls/internal/lsp/source/api_json.go

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎gopls/internal/lsp/source/options.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ func DefaultOptions() *Options {
143143
ImportShortcut: BothShortcuts,
144144
SymbolMatcher: SymbolFastFuzzy,
145145
SymbolStyle: DynamicSymbols,
146+
SymbolScope: WorkspaceSymbolScope,
146147
},
147148
CompletionOptions: CompletionOptions{
148149
Matcher: Fuzzy,
@@ -454,6 +455,13 @@ type NavigationOptions struct {
454455
// }
455456
// ```
456457
SymbolStyle SymbolStyle `status:"advanced"`
458+
459+
// SymbolScope controls which packages are searched for workspace/symbol
460+
// requests. The default value, "workspace", searches only workspace
461+
// packages. The legacy behavior, "all", causes all loaded packages to be
462+
// searched, including dependencies; this is more expensive and may return
463+
// unwanted results.
464+
SymbolScope SymbolScope
457465
}
458466

459467
// UserOptions holds custom Gopls configuration (not part of the LSP) that is
@@ -617,6 +625,8 @@ const (
617625
CaseSensitive Matcher = "CaseSensitive"
618626
)
619627

628+
// A SymbolMatcher controls the matching of symbols for workspace/symbol
629+
// requests.
620630
type SymbolMatcher string
621631

622632
const (
@@ -626,6 +636,7 @@ const (
626636
SymbolCaseSensitive SymbolMatcher = "CaseSensitive"
627637
)
628638

639+
// A SymbolStyle controls the formatting of symbols in workspace/symbol results.
629640
type SymbolStyle string
630641

631642
const (
@@ -642,6 +653,17 @@ const (
642653
DynamicSymbols SymbolStyle = "Dynamic"
643654
)
644655

656+
// A SymbolScope controls the search scope for workspace/symbol requests.
657+
type SymbolScope string
658+
659+
const (
660+
// WorkspaceSymbolScope matches symbols in workspace packages only.
661+
WorkspaceSymbolScope SymbolScope = "workspace"
662+
// AllSymbolScope matches symbols in any loaded package, including
663+
// dependencies.
664+
AllSymbolScope SymbolScope = "all"
665+
)
666+
645667
type HoverKind string
646668

647669
const (
@@ -969,6 +991,14 @@ func (o *Options) set(name string, value interface{}, seen map[string]struct{})
969991
o.SymbolStyle = SymbolStyle(s)
970992
}
971993

994+
case "symbolScope":
995+
if s, ok := result.asOneOf(
996+
string(WorkspaceSymbolScope),
997+
string(AllSymbolScope),
998+
); ok {
999+
o.SymbolScope = SymbolScope(s)
1000+
}
1001+
9721002
case "hoverKind":
9731003
if s, ok := result.asOneOf(
9741004
string(NoDocumentation),

‎gopls/internal/lsp/source/view.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ type Snapshot interface {
159159
CriticalError(ctx context.Context) *CriticalError
160160

161161
// Symbols returns all symbols in the snapshot.
162-
Symbols(ctx context.Context) (map[span.URI][]Symbol, error)
162+
//
163+
// If workspaceOnly is set, this only includes symbols from files in a
164+
// workspace package. Otherwise, it returns symbols from all loaded packages.
165+
Symbols(ctx context.Context, workspaceOnly bool) (map[span.URI][]Symbol, error)
163166

164167
// -- package metadata --
165168

‎gopls/internal/lsp/source/workspace_symbol.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,10 +317,16 @@ func collectSymbols(ctx context.Context, views []View, matcherType SymbolMatcher
317317
filters := v.Options().DirectoryFilters
318318
filterer := NewFilterer(filters)
319319
folder := filepath.ToSlash(v.Folder().Filename())
320-
symbols, err := snapshot.Symbols(ctx)
320+
321+
workspaceOnly := true
322+
if v.Options().SymbolScope == AllSymbolScope {
323+
workspaceOnly = false
324+
}
325+
symbols, err := snapshot.Symbols(ctx, workspaceOnly)
321326
if err != nil {
322327
return nil, err
323328
}
329+
324330
for uri, syms := range symbols {
325331
norm := filepath.ToSlash(uri.Filename())
326332
nm := strings.TrimPrefix(norm, folder)
@@ -496,6 +502,8 @@ func matchFile(store *symbolStore, symbolizer symbolizer, matcher matcherFunc, r
496502
}
497503
}
498504

505+
// TODO(rfindley): use metadata to determine if the file is in a workspace
506+
// package, rather than this heuristic.
499507
inWorkspace := false
500508
for _, root := range roots {
501509
if strings.HasPrefix(string(i.uri), root) {

‎gopls/internal/regtest/marker/testdata/workspacesymbol/allscope.txt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
This test verifies behavior when "symbolScope" is set to "all".
2+
3+
-- settings.json --
4+
{
5+
"symbolStyle": "full",
6+
"symbolMatcher": "casesensitive",
7+
"symbolScope": "all"
8+
}
9+
10+
-- go.mod --
11+
module mod.test/symbols
12+
13+
go 1.18
14+
15+
-- query.go --
16+
package symbols
17+
18+
//@workspacesymbol("fmt.Println", println)
19+
20+
-- fmt/fmt.go --
21+
package fmt
22+
23+
import "fmt"
24+
25+
func Println(s string) {
26+
fmt.Println(s)
27+
}
28+
-- @println --
29+
fmt/fmt.go:5:6-13 mod.test/symbols/fmt.Println Function
30+
<unknown> fmt.Println Function

‎gopls/internal/regtest/marker/testdata/workspacesymbol/wsscope.txt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
This test verifies behavior when "symbolScope" is set to the default value
2+
("workspace").
3+
4+
-- settings.json --
5+
{
6+
"symbolStyle": "full",
7+
"symbolMatcher": "casesensitive"
8+
}
9+
10+
-- go.mod --
11+
module mod.test/symbols
12+
13+
go 1.18
14+
15+
-- query.go --
16+
package symbols
17+
18+
//@workspacesymbol("fmt.Println", println)
19+
20+
-- fmt/fmt.go --
21+
package fmt
22+
23+
import "fmt"
24+
25+
func Println(s string) {
26+
fmt.Println(s)
27+
}
28+
-- @println --
29+
fmt/fmt.go:5:6-13 mod.test/symbols/fmt.Println Function

‎gopls/internal/regtest/misc/workspace_symbol_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ const (
7373
"Fooex", // shorter than Fooest, FooBar, lexically before Fooey
7474
"Fooey", // shorter than Fooest, Foobar
7575
"Fooest",
76-
"unsafe.Offsetof", // a very fuzzy match
7776
)
7877
})
7978
}

0 commit comments

Comments
 (0)