From 351c04c5a14afe1131e549831fcb29a0ca1df982 Mon Sep 17 00:00:00 2001 From: Peter Weinberger Date: Thu, 28 Oct 2021 14:39:26 -0400 Subject: [PATCH] /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 gopls-CI: kokoro TryBot-Result: Go Bot Trust: Peter Weinberger Reviewed-by: Suzy Mueller --- .../regtest/workspace/workspace_test.go | 34 +++++++++++++++++++ internal/lsp/cache/view.go | 16 +-------- internal/lsp/fake/editor.go | 10 ++++++ internal/lsp/regtest/wrappers.go | 10 ++++++ internal/lsp/source/workspace_symbol.go | 30 ++++++++++++++++ 5 files changed, 85 insertions(+), 15 deletions(-) diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go index 456a5d19fc..11ab507708 100644 --- a/gopls/internal/regtest/workspace/workspace_test.go +++ b/gopls/internal/regtest/workspace/workspace_test.go @@ -9,11 +9,13 @@ import ( "fmt" "io/ioutil" "path/filepath" + "sort" "strings" "testing" "golang.org/x/tools/gopls/internal/hooks" . "golang.org/x/tools/internal/lsp/regtest" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/command" "golang.org/x/tools/internal/lsp/fake" @@ -137,6 +139,38 @@ func TestReferences(t *testing.T) { } } +// make sure that directory filters work +func TestFilters(t *testing.T) { + for _, tt := range []struct { + name, rootPath string + }{ + { + name: "module root", + rootPath: "pkg", + }, + } { + t.Run(tt.name, func(t *testing.T) { + opts := []RunOption{ProxyFiles(workspaceProxy)} + if tt.rootPath != "" { + opts = append(opts, WorkspaceFolders(tt.rootPath)) + } + f := func(o *source.Options) { + o.DirectoryFilters = append(o.DirectoryFilters, "-inner") + } + opts = append(opts, Options(f)) + WithOptions(opts...).Run(t, workspaceModule, func(t *testing.T, env *Env) { + syms := env.WorkspaceSymbol("Hi") + sort.Slice(syms, func(i, j int) bool { return syms[i].ContainerName < syms[j].ContainerName }) + for i, s := range syms { + if strings.Contains(s.ContainerName, "/inner") { + t.Errorf("%s %v %s %s %d\n", s.Name, s.Kind, s.ContainerName, tt.name, i) + } + } + }) + }) + } +} + // Make sure that analysis diagnostics are cleared for the whole package when // the only opened file is closed. This test was inspired by the experience in // VS Code, where clicking on a reference result triggers a diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index b54210ef67..db99b775f3 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -1060,23 +1060,9 @@ func pathExcludedByFilterFunc(root, gomodcache string, opts *source.Options) fun func pathExcludedByFilter(path, root, gomodcache string, opts *source.Options) bool { path = strings.TrimPrefix(filepath.ToSlash(path), "/") gomodcache = strings.TrimPrefix(filepath.ToSlash(strings.TrimPrefix(gomodcache, root)), "/") - - excluded := false filters := opts.DirectoryFilters if gomodcache != "" { filters = append(filters, "-"+gomodcache) } - for _, filter := range filters { - op, prefix := filter[0], filter[1:] - // Non-empty prefixes have to be precise directory matches. - if prefix != "" { - prefix = prefix + "/" - path = path + "/" - } - if !strings.HasPrefix(path, prefix) { - continue - } - excluded = op == '-' - } - return excluded + return source.FiltersDisallow(path, filters) } diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index c9780b8bbd..f92ef09f23 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -1068,6 +1068,16 @@ func (e *Editor) AcceptCompletion(ctx context.Context, path string, pos Pos, ite }, item.AdditionalTextEdits...))) } +// Symbols executes a workspace/symbols request on the server. +func (e *Editor) Symbols(ctx context.Context, sym string) ([]protocol.SymbolInformation, error) { + if e.Server == nil { + return nil, nil + } + params := &protocol.WorkspaceSymbolParams{Query: sym} + ans, err := e.Server.Symbol(ctx, params) + return ans, err +} + // References executes a reference request on the server. func (e *Editor) References(ctx context.Context, path string, pos Pos) ([]protocol.Location, error) { if e.Server == nil { diff --git a/internal/lsp/regtest/wrappers.go b/internal/lsp/regtest/wrappers.go index 96844e3b6d..9031e71f1f 100644 --- a/internal/lsp/regtest/wrappers.go +++ b/internal/lsp/regtest/wrappers.go @@ -358,6 +358,16 @@ func (e *Env) ExecuteCommand(params *protocol.ExecuteCommandParams, result inter } } +// WorkspaceSymbol calls workspace/symbol +func (e *Env) WorkspaceSymbol(sym string) []protocol.SymbolInformation { + e.T.Helper() + ans, err := e.Editor.Symbols(e.Ctx, sym) + if err != nil { + e.T.Fatal(err) + } + return ans +} + // References calls textDocument/references for the given path at the given // position. func (e *Env) References(path string, pos fake.Pos) []protocol.Location { diff --git a/internal/lsp/source/workspace_symbol.go b/internal/lsp/source/workspace_symbol.go index 1f6fd20895..fe38c9356c 100644 --- a/internal/lsp/source/workspace_symbol.go +++ b/internal/lsp/source/workspace_symbol.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "go/types" + "path/filepath" "runtime" "sort" "strings" @@ -316,7 +317,14 @@ func (sc *symbolCollector) walk(ctx context.Context, views []View) ([]protocol.S return nil, err } + filters := v.Options().DirectoryFilters + folder := filepath.ToSlash(v.Folder().Filename()) for uri, syms := range psyms { + norm := filepath.ToSlash(uri.Filename()) + nm := strings.TrimPrefix(norm, folder) + if FiltersDisallow(nm, filters) { + continue + } // Only scan each file once. if _, ok := files[uri]; ok { continue @@ -364,6 +372,28 @@ func (sc *symbolCollector) walk(ctx context.Context, views []View) ([]protocol.S return sc.results(), nil } +// FilterDisallow is code from the body of cache.pathExcludedByFilter in cache/view.go +// Exporting and using that function would cause an import cycle. +// Moving it here and exporting it would leave behind view_test.go. +// (This code is exported and used in the body of cache.pathExcludedByFilter) +func FiltersDisallow(path string, filters []string) bool { + path = strings.TrimPrefix(path, "/") + var excluded bool + for _, filter := range filters { + op, prefix := filter[0], filter[1:] + // Non-empty prefixes have to be precise directory matches. + if prefix != "" { + prefix = prefix + "/" + path = path + "/" + } + if !strings.HasPrefix(path, prefix) { + continue + } + excluded = op == '-' + } + return excluded +} + // symbolFile holds symbol information for a single file. type symbolFile struct { uri span.URI