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 <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

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
 	}

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 {

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)
-				}
-			}
-
-		}
-
 	}
 }
 

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
 

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,

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)
-				}
-			}
-
-		}
-
 	}
 }
 

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`
+
+}
+