From be0da057c5e3c2df569a2c25cd280149b7d7e7d0 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Thu, 29 Aug 2019 19:03:23 -0400 Subject: [PATCH] internal/lsp: return only multiline ranges when lineFoldingOnly If the client registers with foldingRange.lineFoldingOnly = true, only return folding ranges that span multiple lines. Do this as they are computed, so that if other filtering is applied later, we do not include ranges that would go unused by the client anyway. Change-Id: I27ea24428d25f180e26892de0f6d16c211225bf7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/192477 Run-TryBot: Suzy Mueller TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/folding_range.go | 2 +- internal/lsp/general.go | 2 + internal/lsp/lsp_test.go | 95 +++++++++------ internal/lsp/server.go | 1 + internal/lsp/source/folding_range.go | 5 +- internal/lsp/source/source_test.go | 94 ++++++++------- internal/lsp/testdata/folding/a.go.golden | 140 ++++++++++++++++++++++ 7 files changed, 257 insertions(+), 82 deletions(-) diff --git a/internal/lsp/folding_range.go b/internal/lsp/folding_range.go index 572ae8e966..4dcc572847 100644 --- a/internal/lsp/folding_range.go +++ b/internal/lsp/folding_range.go @@ -20,7 +20,7 @@ func (s *Server) foldingRange(ctx context.Context, params *protocol.FoldingRange return nil, err } - ranges, err := source.FoldingRange(ctx, view, f) + ranges, err := source.FoldingRange(ctx, view, f, s.lineFoldingOnly) if err != nil { return nil, err } diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 82cb13c62c..3dcbb85e8b 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -158,6 +158,8 @@ func (s *Server) setClientCapabilities(caps protocol.ClientCapabilities) { if len(caps.TextDocument.Hover.ContentFormat) > 0 { s.preferredContentFormat = caps.TextDocument.Hover.ContentFormat[0] } + // Check if the client supports only line folding. + s.lineFoldingOnly = caps.TextDocument.FoldingRange.LineFoldingOnly } func (s *Server) initialized(ctx context.Context, params *protocol.InitializedParams) error { diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index b0cbeb0fad..ba05f31578 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -268,8 +268,9 @@ func summarizeCompletionItems(i int, want []source.CompletionItem, got []protoco func (r *runner) FoldingRange(t *testing.T, data tests.FoldingRanges) { for _, spn := range data { uri := spn.URI() - filename := uri.Filename() + // Test all folding ranges. + r.server.lineFoldingOnly = false ranges, err := r.server.FoldingRange(r.ctx, &protocol.FoldingRangeParams{ TextDocument: protocol.TextDocumentIdentifier{ URI: protocol.NewURI(uri), @@ -279,63 +280,79 @@ func (r *runner) FoldingRange(t *testing.T, data tests.FoldingRanges) { t.Error(err) continue } + r.foldingRanges(t, "foldingRange", uri, ranges) - f, err := getGoFile(r.ctx, r.server.session.ViewOf(uri), uri) + // Test folding ranges with lineFoldingOnly = true. + r.server.lineFoldingOnly = true + ranges, err = r.server.FoldingRange(r.ctx, &protocol.FoldingRangeParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: protocol.NewURI(uri), + }, + }) if err != nil { - t.Fatal(err) + t.Error(err) + continue } - m, err := getMapper(r.ctx, f) + r.foldingRanges(t, "foldingRange-lineFolding", uri, ranges) + + } +} + +func (r *runner) foldingRanges(t *testing.T, prefix string, uri span.URI, ranges []protocol.FoldingRange) { + f, err := getGoFile(r.ctx, r.server.session.ViewOf(uri), uri) + if err != nil { + t.Fatal(err) + } + m, err := getMapper(r.ctx, f) + if err != nil { + t.Fatal(err) + } + + // Fold all ranges. + nonOverlapping := nonOverlappingRanges(ranges) + for i, rngs := range nonOverlapping { + got, err := foldRanges(m, string(m.Content), rngs) if err != nil { - t.Fatal(err) + t.Error(err) + continue + } + tag := fmt.Sprintf("%s-%d", prefix, i) + want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) { + return []byte(got), nil + })) + + if want != got { + t.Errorf("%s: foldingRanges failed for %s, expected:\n%v\ngot:\n%v", tag, uri.Filename(), want, got) + } + } + + // Filter by kind. + kinds := []protocol.FoldingRangeKind{protocol.Imports, protocol.Comment} + for _, kind := range kinds { + var kindOnly []protocol.FoldingRange + for _, fRng := range ranges { + if fRng.Kind == string(kind) { + kindOnly = append(kindOnly, fRng) + } } - // Fold all ranges. - nonOverlapping := nonOverlappingRanges(ranges) + nonOverlapping := nonOverlappingRanges(kindOnly) for i, rngs := range nonOverlapping { got, err := foldRanges(m, string(m.Content), rngs) if err != nil { t.Error(err) continue } - tag := fmt.Sprintf("foldingRange-%d", i) - want := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) { + tag := fmt.Sprintf("%s-%s-%d", prefix, kind, i) + want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) { return []byte(got), nil })) if want != got { - t.Errorf("%s: foldingRanges failed for %s, expected:\n%v\ngot:\n%v", tag, filename, want, got) + t.Errorf("%s: foldingRanges failed for %s, expected:\n%v\ngot:\n%v", tag, uri.Filename(), want, got) } } - // Filter by kind. - kinds := []protocol.FoldingRangeKind{protocol.Imports, protocol.Comment} - for _, kind := range kinds { - var kindOnly []protocol.FoldingRange - for _, fRng := range ranges { - if fRng.Kind == string(kind) { - kindOnly = append(kindOnly, fRng) - } - } - - nonOverlapping := nonOverlappingRanges(kindOnly) - for i, rngs := range nonOverlapping { - got, err := foldRanges(m, string(m.Content), rngs) - if err != nil { - t.Error(err) - continue - } - tag := fmt.Sprintf("foldingRange-%s-%d", kind, i) - want := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) { - return []byte(got), nil - })) - - if want != got { - t.Errorf("%s: foldingRanges failed for %s, expected:\n%v\ngot:\n%v", tag, filename, want, got) - } - } - - } - } } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 0aadbc30bd..aedbcb7b21 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -92,6 +92,7 @@ type Server struct { preferredContentFormat protocol.MarkupKind disabledAnalyses map[string]struct{} wantSuggestedFixes bool + lineFoldingOnly bool supportedCodeActions map[source.FileKind]map[protocol.CodeActionKind]bool diff --git a/internal/lsp/source/folding_range.go b/internal/lsp/source/folding_range.go index 1ceccbdaeb..22da1eaf3e 100644 --- a/internal/lsp/source/folding_range.go +++ b/internal/lsp/source/folding_range.go @@ -16,7 +16,7 @@ type FoldingRangeInfo struct { } // FoldingRange gets all of the folding range for f. -func FoldingRange(ctx context.Context, view View, f GoFile) (ranges []FoldingRangeInfo, err error) { +func FoldingRange(ctx context.Context, view View, f GoFile, lineFoldingOnly bool) (ranges []FoldingRangeInfo, err error) { // TODO(suzmue): consider limiting the number of folding ranges returned, and // implement a way to prioritize folding ranges in that case. file, err := f.GetAST(ctx, ParseFull) @@ -54,6 +54,9 @@ func FoldingRange(ctx context.Context, view View, f GoFile) (ranges []FoldingRan } if start.IsValid() && end.IsValid() { + if lineFoldingOnly && f.FileSet().Position(start).Line == f.FileSet().Position(end).Line { + return true + } ranges = append(ranges, FoldingRangeInfo{ Range: span.NewRange(f.FileSet(), start, end), Kind: kind, diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 00e71b0b85..75d8501135 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -271,71 +271,83 @@ func summarizeCompletionItems(i int, want []source.CompletionItem, got []source. func (r *runner) FoldingRange(t *testing.T, data tests.FoldingRanges) { for _, spn := range data { uri := spn.URI() - filename := uri.Filename() f, err := r.view.GetFile(r.ctx, uri) if err != nil { t.Fatalf("failed for %v: %v", spn, err) } - - ranges, err := source.FoldingRange(r.ctx, r.view, f.(source.GoFile)) - if err != nil { - t.Error(err) - continue - } data, _, err := f.Handle(r.ctx).Read(r.ctx) if err != nil { t.Error(err) continue } - // Fold all ranges. - nonOverlapping := nonOverlappingRanges(ranges) + // Test all folding ranges. + ranges, err := source.FoldingRange(r.ctx, r.view, f.(source.GoFile), false) + if err != nil { + t.Error(err) + continue + } + r.foldingRanges(t, "foldingRange", uri, string(data), ranges) + + // Test folding ranges with lineFoldingOnly + ranges, err = source.FoldingRange(r.ctx, r.view, f.(source.GoFile), true) + if err != nil { + t.Error(err) + continue + } + r.foldingRanges(t, "foldingRange-lineFolding", uri, string(data), ranges) + + } +} + +func (r *runner) foldingRanges(t *testing.T, prefix string, uri span.URI, data string, ranges []source.FoldingRangeInfo) { + t.Helper() + // Fold all ranges. + nonOverlapping := nonOverlappingRanges(ranges) + for i, rngs := range nonOverlapping { + got, err := foldRanges(string(data), rngs) + if err != nil { + t.Error(err) + continue + } + tag := fmt.Sprintf("%s-%d", prefix, i) + want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) { + return []byte(got), nil + })) + + if want != got { + t.Errorf("%s: foldingRanges failed for %s, expected:\n%v\ngot:\n%v", tag, uri.Filename(), want, got) + } + } + + // Filter by kind. + kinds := []protocol.FoldingRangeKind{protocol.Imports, protocol.Comment} + for _, kind := range kinds { + var kindOnly []source.FoldingRangeInfo + for _, fRng := range ranges { + if fRng.Kind == kind { + kindOnly = append(kindOnly, fRng) + } + } + + nonOverlapping := nonOverlappingRanges(kindOnly) for i, rngs := range nonOverlapping { got, err := foldRanges(string(data), rngs) if err != nil { t.Error(err) continue } - tag := fmt.Sprintf("foldingRange-%d", i) - want := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) { + tag := fmt.Sprintf("%s-%s-%d", prefix, kind, i) + want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) { return []byte(got), nil })) if want != got { - t.Errorf("%s: foldingRanges failed for %s, expected:\n%v\ngot:\n%v", tag, filename, want, got) + t.Errorf("%s: failed for %s, expected:\n%v\ngot:\n%v", tag, uri.Filename(), want, got) } } - // Filter by kind. - kinds := []protocol.FoldingRangeKind{protocol.Imports, protocol.Comment} - for _, kind := range kinds { - var kindOnly []source.FoldingRangeInfo - for _, fRng := range ranges { - if fRng.Kind == kind { - kindOnly = append(kindOnly, fRng) - } - } - - nonOverlapping := nonOverlappingRanges(kindOnly) - for i, rngs := range nonOverlapping { - got, err := foldRanges(string(data), rngs) - if err != nil { - t.Error(err) - continue - } - tag := fmt.Sprintf("foldingRange-%s-%d", kind, i) - want := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) { - return []byte(got), nil - })) - - if want != got { - t.Errorf("%s: failed for %s, expected:\n%v\ngot:\n%v", tag, filename, want, got) - } - } - - } - } } diff --git a/internal/lsp/testdata/folding/a.go.golden b/internal/lsp/testdata/folding/a.go.golden index 2c09f381f3..15ad786f44 100644 --- a/internal/lsp/testdata/folding/a.go.golden +++ b/internal/lsp/testdata/folding/a.go.golden @@ -168,3 +168,143 @@ is not indented` } +-- foldingRange-lineFolding-0 -- +package folding //@fold("package") + +import (<>) + +import _ "os" + +// bar is a function.<> +func bar() string {<>} + +-- foldingRange-lineFolding-1 -- +package folding //@fold("package") + +import ( + "fmt" + _ "log" +) + +import _ "os" + +// bar is a function. +// With a multiline doc comment. +func bar() string { + switch {<>} + + return ` +this string +is not indented` + +} + +-- foldingRange-lineFolding-2 -- +package folding //@fold("package") + +import ( + "fmt" + _ "log" +) + +import _ "os" + +// bar is a function. +// With a multiline doc comment. +func bar() string { + switch { + case true:<> + case false:<> + default:<> + } + + return ` +this string +is not indented` + +} + +-- foldingRange-lineFolding-3 -- +package folding //@fold("package") + +import ( + "fmt" + _ "log" +) + +import _ "os" + +// bar is a function. +// With a multiline doc comment. +func bar() string { + switch { + case true: + if true {<>} + case false: + fmt.Println("false") + default: + fmt.Println("default") + } + + return ` +this string +is not indented` + +} + +-- foldingRange-lineFolding-comment-0 -- +package folding //@fold("package") + +import ( + "fmt" + _ "log" +) + +import _ "os" + +// bar is a function.<> +func bar() string { + switch { + case true: + if true { + fmt.Println("true") + } + case false: + fmt.Println("false") + default: + fmt.Println("default") + } + + return ` +this string +is not indented` + +} + +-- foldingRange-lineFolding-imports-0 -- +package folding //@fold("package") + +import (<>) + +import _ "os" + +// bar is a function. +// With a multiline doc comment. +func bar() string { + switch { + case true: + if true { + fmt.Println("true") + } + case false: + fmt.Println("false") + default: + fmt.Println("default") + } + + return ` +this string +is not indented` + +} +