Skip to content

Commit 351c04c

Browse files
committed
/internal/lsp/source: apply directory filters to workspace symbols
Apply Options.DirectoryFilters when searching for workspace symbols. The natural way to implement it would lead to an import loop, so the working code was moved from cache to source. Fixes golang/go#48939 Change-Id: Iccf32bc8327ba7845505a6a3de621db8946063f5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/359514 Run-TryBot: Peter Weinberger <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Peter Weinberger <[email protected]> Reviewed-by: Suzy Mueller <[email protected]>
1 parent a6c6f4b commit 351c04c

File tree

5 files changed

+85
-15
lines changed

5 files changed

+85
-15
lines changed

gopls/internal/regtest/workspace/workspace_test.go

+34
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@ import (
99
"fmt"
1010
"io/ioutil"
1111
"path/filepath"
12+
"sort"
1213
"strings"
1314
"testing"
1415

1516
"golang.org/x/tools/gopls/internal/hooks"
1617
. "golang.org/x/tools/internal/lsp/regtest"
18+
"golang.org/x/tools/internal/lsp/source"
1719

1820
"golang.org/x/tools/internal/lsp/command"
1921
"golang.org/x/tools/internal/lsp/fake"
@@ -137,6 +139,38 @@ func TestReferences(t *testing.T) {
137139
}
138140
}
139141

142+
// make sure that directory filters work
143+
func TestFilters(t *testing.T) {
144+
for _, tt := range []struct {
145+
name, rootPath string
146+
}{
147+
{
148+
name: "module root",
149+
rootPath: "pkg",
150+
},
151+
} {
152+
t.Run(tt.name, func(t *testing.T) {
153+
opts := []RunOption{ProxyFiles(workspaceProxy)}
154+
if tt.rootPath != "" {
155+
opts = append(opts, WorkspaceFolders(tt.rootPath))
156+
}
157+
f := func(o *source.Options) {
158+
o.DirectoryFilters = append(o.DirectoryFilters, "-inner")
159+
}
160+
opts = append(opts, Options(f))
161+
WithOptions(opts...).Run(t, workspaceModule, func(t *testing.T, env *Env) {
162+
syms := env.WorkspaceSymbol("Hi")
163+
sort.Slice(syms, func(i, j int) bool { return syms[i].ContainerName < syms[j].ContainerName })
164+
for i, s := range syms {
165+
if strings.Contains(s.ContainerName, "/inner") {
166+
t.Errorf("%s %v %s %s %d\n", s.Name, s.Kind, s.ContainerName, tt.name, i)
167+
}
168+
}
169+
})
170+
})
171+
}
172+
}
173+
140174
// Make sure that analysis diagnostics are cleared for the whole package when
141175
// the only opened file is closed. This test was inspired by the experience in
142176
// VS Code, where clicking on a reference result triggers a

internal/lsp/cache/view.go

+1-15
Original file line numberDiff line numberDiff line change
@@ -1060,23 +1060,9 @@ func pathExcludedByFilterFunc(root, gomodcache string, opts *source.Options) fun
10601060
func pathExcludedByFilter(path, root, gomodcache string, opts *source.Options) bool {
10611061
path = strings.TrimPrefix(filepath.ToSlash(path), "/")
10621062
gomodcache = strings.TrimPrefix(filepath.ToSlash(strings.TrimPrefix(gomodcache, root)), "/")
1063-
1064-
excluded := false
10651063
filters := opts.DirectoryFilters
10661064
if gomodcache != "" {
10671065
filters = append(filters, "-"+gomodcache)
10681066
}
1069-
for _, filter := range filters {
1070-
op, prefix := filter[0], filter[1:]
1071-
// Non-empty prefixes have to be precise directory matches.
1072-
if prefix != "" {
1073-
prefix = prefix + "/"
1074-
path = path + "/"
1075-
}
1076-
if !strings.HasPrefix(path, prefix) {
1077-
continue
1078-
}
1079-
excluded = op == '-'
1080-
}
1081-
return excluded
1067+
return source.FiltersDisallow(path, filters)
10821068
}

internal/lsp/fake/editor.go

+10
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,16 @@ func (e *Editor) AcceptCompletion(ctx context.Context, path string, pos Pos, ite
10681068
}, item.AdditionalTextEdits...)))
10691069
}
10701070

1071+
// Symbols executes a workspace/symbols request on the server.
1072+
func (e *Editor) Symbols(ctx context.Context, sym string) ([]protocol.SymbolInformation, error) {
1073+
if e.Server == nil {
1074+
return nil, nil
1075+
}
1076+
params := &protocol.WorkspaceSymbolParams{Query: sym}
1077+
ans, err := e.Server.Symbol(ctx, params)
1078+
return ans, err
1079+
}
1080+
10711081
// References executes a reference request on the server.
10721082
func (e *Editor) References(ctx context.Context, path string, pos Pos) ([]protocol.Location, error) {
10731083
if e.Server == nil {

internal/lsp/regtest/wrappers.go

+10
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,16 @@ func (e *Env) ExecuteCommand(params *protocol.ExecuteCommandParams, result inter
358358
}
359359
}
360360

361+
// WorkspaceSymbol calls workspace/symbol
362+
func (e *Env) WorkspaceSymbol(sym string) []protocol.SymbolInformation {
363+
e.T.Helper()
364+
ans, err := e.Editor.Symbols(e.Ctx, sym)
365+
if err != nil {
366+
e.T.Fatal(err)
367+
}
368+
return ans
369+
}
370+
361371
// References calls textDocument/references for the given path at the given
362372
// position.
363373
func (e *Env) References(path string, pos fake.Pos) []protocol.Location {

internal/lsp/source/workspace_symbol.go

+30
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"context"
99
"fmt"
1010
"go/types"
11+
"path/filepath"
1112
"runtime"
1213
"sort"
1314
"strings"
@@ -316,7 +317,14 @@ func (sc *symbolCollector) walk(ctx context.Context, views []View) ([]protocol.S
316317
return nil, err
317318
}
318319

320+
filters := v.Options().DirectoryFilters
321+
folder := filepath.ToSlash(v.Folder().Filename())
319322
for uri, syms := range psyms {
323+
norm := filepath.ToSlash(uri.Filename())
324+
nm := strings.TrimPrefix(norm, folder)
325+
if FiltersDisallow(nm, filters) {
326+
continue
327+
}
320328
// Only scan each file once.
321329
if _, ok := files[uri]; ok {
322330
continue
@@ -364,6 +372,28 @@ func (sc *symbolCollector) walk(ctx context.Context, views []View) ([]protocol.S
364372
return sc.results(), nil
365373
}
366374

375+
// FilterDisallow is code from the body of cache.pathExcludedByFilter in cache/view.go
376+
// Exporting and using that function would cause an import cycle.
377+
// Moving it here and exporting it would leave behind view_test.go.
378+
// (This code is exported and used in the body of cache.pathExcludedByFilter)
379+
func FiltersDisallow(path string, filters []string) bool {
380+
path = strings.TrimPrefix(path, "/")
381+
var excluded bool
382+
for _, filter := range filters {
383+
op, prefix := filter[0], filter[1:]
384+
// Non-empty prefixes have to be precise directory matches.
385+
if prefix != "" {
386+
prefix = prefix + "/"
387+
path = path + "/"
388+
}
389+
if !strings.HasPrefix(path, prefix) {
390+
continue
391+
}
392+
excluded = op == '-'
393+
}
394+
return excluded
395+
}
396+
367397
// symbolFile holds symbol information for a single file.
368398
type symbolFile struct {
369399
uri span.URI

0 commit comments

Comments
 (0)