Skip to content

Commit e48fac3

Browse files
committed
internal/lsp: change workspace symbols to use session config for matcher
WorkspaceSymbols matches symbols across views using the given query, according to the matcher Matcher. The workspace symbol method is defined in the spec as follows: > The workspace symbol request is sent from the client to the server to > list project-wide symbols matching the query string. It is unclear what "project-wide" means here, but given the parameters of workspace/symbol do not include any workspace identifier, then it has to be assumed that "project-wide" means "across all workspaces". Hence why WorkspaceSymbols receives the views []View. However, it then becomes unclear what it would mean to call WorkspaceSymbols with a different configured Matcher per View. Therefore we assume that Session level configuration will define the Matcher to be used for the WorkspaceSymbols method. As part of this change we also tidy up lsp_test.go and source_test.go to remove some repetition. Change-Id: I444f9a78303ac9d2c8d8ac6496603b5758e4aafd Reviewed-on: https://go-review.googlesource.com/c/tools/+/228763 Run-TryBot: Paul Jolly <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
1 parent da4261a commit e48fac3

File tree

4 files changed

+51
-95
lines changed

4 files changed

+51
-95
lines changed

internal/lsp/lsp_test.go

Lines changed: 18 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -820,71 +820,41 @@ func (r *runner) Symbols(t *testing.T, uri span.URI, expectedSymbols []protocol.
820820
}
821821

822822
func (r *runner) WorkspaceSymbols(t *testing.T, query string, expectedSymbols []protocol.SymbolInformation, dirs map[string]struct{}) {
823-
got := r.callWorkspaceSymbols(t, query, func(opts *source.Options) {
824-
opts.Matcher = source.CaseInsensitive
825-
})
826-
got = tests.FilterWorkspaceSymbols(got, dirs)
827-
if len(got) != len(expectedSymbols) {
828-
t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
829-
return
830-
}
831-
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
832-
t.Error(diff)
833-
}
823+
r.callWorkspaceSymbols(t, query, source.CaseInsensitive, dirs, expectedSymbols)
834824
}
835825

836826
func (r *runner) FuzzyWorkspaceSymbols(t *testing.T, query string, expectedSymbols []protocol.SymbolInformation, dirs map[string]struct{}) {
837-
got := r.callWorkspaceSymbols(t, query, func(opts *source.Options) {
838-
opts.Matcher = source.Fuzzy
839-
})
840-
got = tests.FilterWorkspaceSymbols(got, dirs)
841-
if len(got) != len(expectedSymbols) {
842-
t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
843-
return
844-
}
845-
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
846-
t.Error(diff)
847-
}
827+
r.callWorkspaceSymbols(t, query, source.Fuzzy, dirs, expectedSymbols)
848828
}
849829

850830
func (r *runner) CaseSensitiveWorkspaceSymbols(t *testing.T, query string, expectedSymbols []protocol.SymbolInformation, dirs map[string]struct{}) {
851-
got := r.callWorkspaceSymbols(t, query, func(opts *source.Options) {
852-
opts.Matcher = source.CaseSensitive
853-
})
854-
got = tests.FilterWorkspaceSymbols(got, dirs)
855-
if len(got) != len(expectedSymbols) {
856-
t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
857-
return
858-
}
859-
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
860-
t.Error(diff)
861-
}
831+
r.callWorkspaceSymbols(t, query, source.CaseSensitive, dirs, expectedSymbols)
862832
}
863833

864-
func (r *runner) callWorkspaceSymbols(t *testing.T, query string, options func(*source.Options)) []protocol.SymbolInformation {
834+
func (r *runner) callWorkspaceSymbols(t *testing.T, query string, matcher source.Matcher, dirs map[string]struct{}, expectedSymbols []protocol.SymbolInformation) {
865835
t.Helper()
866836

867-
for _, view := range r.server.session.Views() {
868-
original := view.Options()
869-
modified := original
870-
options(&modified)
871-
var err error
872-
view, err = view.SetOptions(r.ctx, modified)
873-
if err != nil {
874-
t.Error(err)
875-
return nil
876-
}
877-
defer view.SetOptions(r.ctx, original)
878-
}
837+
original := r.server.session.Options()
838+
modified := original
839+
modified.Matcher = matcher
840+
r.server.session.SetOptions(modified)
841+
defer r.server.session.SetOptions(original)
879842

880843
params := &protocol.WorkspaceSymbolParams{
881844
Query: query,
882845
}
883-
symbols, err := r.server.Symbol(r.ctx, params)
846+
got, err := r.server.Symbol(r.ctx, params)
884847
if err != nil {
885848
t.Fatal(err)
886849
}
887-
return symbols
850+
got = tests.FilterWorkspaceSymbols(got, dirs)
851+
if len(got) != len(expectedSymbols) {
852+
t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
853+
return
854+
}
855+
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
856+
t.Error(diff)
857+
}
888858
}
889859

890860
func (r *runner) SignatureHelp(t *testing.T, spn span.Span, want *protocol.SignatureHelp) {

internal/lsp/source/source_test.go

Lines changed: 11 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -805,37 +805,23 @@ func (r *runner) Symbols(t *testing.T, uri span.URI, expectedSymbols []protocol.
805805
}
806806

807807
func (r *runner) WorkspaceSymbols(t *testing.T, query string, expectedSymbols []protocol.SymbolInformation, dirs map[string]struct{}) {
808-
got := r.callWorkspaceSymbols(t, query, func(opts *source.Options) {
809-
opts.Matcher = source.CaseInsensitive
810-
})
811-
got = tests.FilterWorkspaceSymbols(got, dirs)
812-
if len(got) != len(expectedSymbols) {
813-
t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
814-
return
815-
}
816-
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
817-
t.Error(diff)
818-
}
808+
r.callWorkspaceSymbols(t, query, source.CaseInsensitive, dirs, expectedSymbols)
819809
}
820810

821811
func (r *runner) FuzzyWorkspaceSymbols(t *testing.T, query string, expectedSymbols []protocol.SymbolInformation, dirs map[string]struct{}) {
822-
got := r.callWorkspaceSymbols(t, query, func(opts *source.Options) {
823-
opts.Matcher = source.Fuzzy
824-
})
825-
got = tests.FilterWorkspaceSymbols(got, dirs)
826-
if len(got) != len(expectedSymbols) {
827-
t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
828-
return
829-
}
830-
if diff := tests.DiffWorkspaceSymbols(expectedSymbols, got); diff != "" {
831-
t.Error(diff)
832-
}
812+
r.callWorkspaceSymbols(t, query, source.Fuzzy, dirs, expectedSymbols)
833813
}
834814

835815
func (r *runner) CaseSensitiveWorkspaceSymbols(t *testing.T, query string, expectedSymbols []protocol.SymbolInformation, dirs map[string]struct{}) {
836-
got := r.callWorkspaceSymbols(t, query, func(opts *source.Options) {
837-
opts.Matcher = source.CaseSensitive
838-
})
816+
r.callWorkspaceSymbols(t, query, source.CaseSensitive, dirs, expectedSymbols)
817+
}
818+
819+
func (r *runner) callWorkspaceSymbols(t *testing.T, query string, matcher source.Matcher, dirs map[string]struct{}, expectedSymbols []protocol.SymbolInformation) {
820+
t.Helper()
821+
got, err := source.WorkspaceSymbols(r.ctx, matcher, []source.View{r.view}, query)
822+
if err != nil {
823+
t.Fatal(err)
824+
}
839825
got = tests.FilterWorkspaceSymbols(got, dirs)
840826
if len(got) != len(expectedSymbols) {
841827
t.Errorf("want %d symbols, got %d", len(expectedSymbols), len(got))
@@ -846,25 +832,6 @@ func (r *runner) CaseSensitiveWorkspaceSymbols(t *testing.T, query string, expec
846832
}
847833
}
848834

849-
func (r *runner) callWorkspaceSymbols(t *testing.T, query string, options func(*source.Options)) []protocol.SymbolInformation {
850-
t.Helper()
851-
852-
original := r.view.Options()
853-
modified := original
854-
options(&modified)
855-
view, err := r.view.SetOptions(r.ctx, modified)
856-
if err != nil {
857-
t.Fatal(err)
858-
}
859-
defer r.view.SetOptions(r.ctx, original)
860-
861-
got, err := source.WorkspaceSymbols(r.ctx, []source.View{view}, query)
862-
if err != nil {
863-
t.Fatal(err)
864-
}
865-
return got
866-
}
867-
868835
func (r *runner) SignatureHelp(t *testing.T, spn span.Span, want *protocol.SignatureHelp) {
869836
_, rng, err := spanToRange(r.data, spn)
870837
if err != nil {

internal/lsp/source/workspace_symbol.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,31 @@ import (
1818

1919
const maxSymbols = 100
2020

21-
func WorkspaceSymbols(ctx context.Context, views []View, query string) ([]protocol.SymbolInformation, error) {
21+
// WorkspaceSymbols matches symbols across views using the given query,
22+
// according to the matcher Matcher.
23+
//
24+
// The workspace symbol method is defined in the spec as follows:
25+
//
26+
// > The workspace symbol request is sent from the client to the server to
27+
// > list project-wide symbols matching the query string.
28+
//
29+
// It is unclear what "project-wide" means here, but given the parameters of
30+
// workspace/symbol do not include any workspace identifier, then it has to be
31+
// assumed that "project-wide" means "across all workspaces". Hence why
32+
// WorkspaceSymbols receives the views []View.
33+
//
34+
// However, it then becomes unclear what it would mean to call WorkspaceSymbols
35+
// with a different configured Matcher per View. Therefore we assume that
36+
// Session level configuration will define the Matcher to be used for the
37+
// WorkspaceSymbols method.
38+
func WorkspaceSymbols(ctx context.Context, matcherType Matcher, views []View, query string) ([]protocol.SymbolInformation, error) {
2239
ctx, done := event.Start(ctx, "source.WorkspaceSymbols")
2340
defer done()
2441
if query == "" {
2542
return nil, nil
2643
}
2744

45+
matcher := makeMatcher(matcherType, query)
2846
seen := make(map[string]struct{})
2947
var symbols []protocol.SymbolInformation
3048
outer:
@@ -33,7 +51,6 @@ outer:
3351
if err != nil {
3452
return nil, err
3553
}
36-
matcher := makeMatcher(view.Options().Matcher, query)
3754
for _, ph := range knownPkgs {
3855
pkg, err := ph.Check(ctx)
3956
if err != nil {

internal/lsp/workspace_symbol.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,7 @@ func (s *Server) symbol(ctx context.Context, params *protocol.WorkspaceSymbolPar
1616
ctx, done := event.Start(ctx, "lsp.Server.symbol")
1717
defer done()
1818

19-
return source.WorkspaceSymbols(ctx, s.session.Views(), params.Query)
19+
views := s.session.Views()
20+
matcher := s.session.Options().Matcher
21+
return source.WorkspaceSymbols(ctx, matcher, views, params.Query)
2022
}

0 commit comments

Comments
 (0)