Skip to content

Commit

Permalink
/internal/lsp/source: apply directory filters to workspace symbols
Browse files Browse the repository at this point in the history
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 <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
  • Loading branch information
pjweinbgo committed Oct 30, 2021
1 parent a6c6f4b commit 351c04c
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 15 deletions.
34 changes: 34 additions & 0 deletions gopls/internal/regtest/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
16 changes: 1 addition & 15 deletions internal/lsp/cache/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
10 changes: 10 additions & 0 deletions internal/lsp/fake/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions internal/lsp/regtest/wrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions internal/lsp/source/workspace_symbol.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"fmt"
"go/types"
"path/filepath"
"runtime"
"sort"
"strings"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 351c04c

Please sign in to comment.