Skip to content

Folding Ranges implementation #1326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Folding Ranges implementation #1326

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f1616c3
porting folding
navya9singh Jun 23, 2025
c32bbb7
Merge branch 'main' of https://github.com/navya9singh/typescript-go-r…
navya9singh Jun 23, 2025
c691b7d
adding all tests
navya9singh Jun 30, 2025
02723d4
Merge branch 'main' of https://github.com/navya9singh/typescript-go-r…
navya9singh Jun 30, 2025
1938da6
code clean up
navya9singh Jun 30, 2025
86df138
cleanup
navya9singh Jun 30, 2025
37babb6
Merge branch 'main' of https://github.com/navya9singh/typescript-go-r…
navya9singh Jun 30, 2025
18a2737
adding check for binary expression
navya9singh Jul 1, 2025
b7b020f
addressing pr comments
navya9singh Jul 3, 2025
dd3f93d
Merge branch 'main' of https://github.com/navya9singh/typescript-go-r…
navya9singh Jul 3, 2025
7435726
fixing test
navya9singh Jul 3, 2025
3527adb
using sourceFile.endOfFileToken
navya9singh Jul 3, 2025
3a1f476
Merge branch 'main' of https://github.com/navya9singh/typescript-go-r…
navya9singh Jul 7, 2025
08fbeda
addressing pr comments
navya9singh Jul 7, 2025
30f9df7
addressing comments
navya9singh Jul 8, 2025
f68abaa
Merge branch 'main' of https://github.com/navya9singh/typescript-go-r…
navya9singh Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 1 addition & 44 deletions internal/ast/utilities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,49 +663,6 @@ func isDeclarationStatementKind(kind Kind) bool {
return false
}

func isDeclarationKind(kind Kind) bool {
switch kind {
case KindArrowFunction,
KindBindingElement,
KindClassDeclaration,
KindClassExpression,
KindClassStaticBlockDeclaration,
KindConstructor,
KindEnumDeclaration,
KindEnumMember,
KindExportSpecifier,
KindFunctionDeclaration,
KindFunctionExpression,
KindGetAccessor,
KindImportClause,
KindImportEqualsDeclaration,
KindImportSpecifier,
KindInterfaceDeclaration,
KindJsxAttribute,
KindMethodDeclaration,
KindMethodSignature,
KindModuleDeclaration,
KindNamespaceExportDeclaration,
KindNamespaceExport,
KindNamespaceImport,
KindParameter,
KindPropertyAssignment,
KindPropertyDeclaration,
KindPropertySignature,
KindSetAccessor,
KindShorthandPropertyAssignment,
KindTypeAliasDeclaration,
KindTypeParameter,
KindVariableDeclaration,
KindJSDocTypedefTag,
KindJSDocCallbackTag,
KindJSDocPropertyTag,
KindNamedTupleMember:
return true
}
return false
}

// Determines whether a node is a DeclarationStatement. Ideally this does not use Parent pointers, but it may use them
// to rule out a Block node that is part of `try` or `catch` or is the Block-like body of a function.
//
Expand Down Expand Up @@ -1308,7 +1265,7 @@ func IsDeclaration(node *Node) bool {
if node.Kind == KindTypeParameter {
return node.Parent != nil
}
return isDeclarationKind(node.Kind)
return IsDeclarationNode(node)
}

// True if `name` is the name of a declaration node
Expand Down
71 changes: 33 additions & 38 deletions internal/ls/folding.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,22 +232,20 @@ func addOutliningForLeadingCommentsForPos(pos int, sourceFile *ast.SourceFile, l
combineAndAddMultipleSingleLineComments := func() *lsproto.FoldingRange {
// Only outline spans of two or more consecutive single line comments
if singleLineCommentCount > 1 {

return createFoldingRangeFromBounds(
firstSingleLineCommentStart, lastSingleLineCommentEnd, foldingRangeKindComment, sourceFile, l)
return createFoldingRangeFromBounds(firstSingleLineCommentStart, lastSingleLineCommentEnd, foldingRangeKindComment, sourceFile, l)
}
return nil
}

sourceText := sourceFile.Text()
comments(func(comment ast.CommentRange) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that you can't, but why was this written this way instead of for comment := range GetLeadingCommentRanges?

pos := comment.Pos()
end := comment.End()
commentPos := comment.Pos()
commentEnd := comment.End()
// cancellationToken.throwIfCancellationRequested();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be rewritten in terms of ctx.Err()? I see other places where this has been written, so I'm okay with doing that as a separate PR.

switch comment.Kind {
case ast.KindSingleLineCommentTrivia:
// never fold region delimiters into single-line comment regions
commentText := sourceText[pos:end]
commentText := sourceText[commentPos:commentEnd]
if parseRegionDelimiter(commentText) != nil {
comments := combineAndAddMultipleSingleLineComments()
if comments != nil {
Expand All @@ -260,17 +258,17 @@ func addOutliningForLeadingCommentsForPos(pos int, sourceFile *ast.SourceFile, l
// For single line comments, combine consecutive ones (2 or more) into
// a single span from the start of the first till the end of the last
if singleLineCommentCount == 0 {
firstSingleLineCommentStart = pos
firstSingleLineCommentStart = commentPos
}
lastSingleLineCommentEnd = end
lastSingleLineCommentEnd = commentEnd
singleLineCommentCount++
break
case ast.KindMultiLineCommentTrivia:
comments := combineAndAddMultipleSingleLineComments()
if comments != nil {
foldingRange = append(foldingRange, comments)
}
foldingRange = append(foldingRange, createFoldingRangeFromBounds(pos, end, foldingRangeKindComment, sourceFile, l))
foldingRange = append(foldingRange, createFoldingRangeFromBounds(commentPos, commentEnd, foldingRangeKindComment, sourceFile, l))
singleLineCommentCount = 0
break
default:
Expand Down Expand Up @@ -321,43 +319,43 @@ func getOutliningSpanForNode(n *ast.Node, sourceFile *ast.SourceFile, l *Languag
// to be the entire span of the parent.
switch n.Parent.Kind {
case ast.KindDoStatement, ast.KindForInStatement, ast.KindForOfStatement, ast.KindForStatement, ast.KindIfStatement, ast.KindWhileStatement, ast.KindWithStatement, ast.KindCatchClause:
return spanForNode(n, n.Parent, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
case ast.KindTryStatement:
// Could be the try-block, or the finally-block.
tryStatement := n.Parent.AsTryStatement()
if tryStatement.TryBlock == n {
return spanForNode(n, n.Parent, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
} else if tryStatement.FinallyBlock == n {
node := findChildOfKind(n.Parent, ast.KindFinallyKeyword, sourceFile)
if node != nil {
return spanForNode(n, node, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
}
}
default:
// Block was a standalone block. In this case we want to only collapse
// the span of the block, independent of any parent span.
return createFoldingRange(l.createLspRangeFromNode(n, sourceFile), "", nil, "")
return createFoldingRange(l.createLspRangeFromNode(n, sourceFile), "", "")
}
case ast.KindModuleBlock:
return spanForNode(n, n.Parent, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
case ast.KindClassDeclaration, ast.KindClassExpression, ast.KindInterfaceDeclaration, ast.KindEnumDeclaration, ast.KindCaseBlock, ast.KindTypeLiteral, ast.KindObjectBindingPattern:
return spanForNode(n, n, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBraceToken, true /*useFullStart */, sourceFile, l)
case ast.KindTupleType:
return spanForNode(n, n, ast.KindOpenBracketToken, !ast.IsTupleTypeNode(n.Parent) /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBracketToken, !ast.IsTupleTypeNode(n.Parent) /*useFullStart */, sourceFile, l)
case ast.KindCaseClause, ast.KindDefaultClause:
return spanForNodeArray(n.AsCaseOrDefaultClause().Statements, sourceFile, l)
case ast.KindObjectLiteralExpression:
return spanForNode(n, n, ast.KindOpenBraceToken, !ast.IsArrayLiteralExpression(n.Parent) && !ast.IsCallExpression(n.Parent) /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBraceToken, !ast.IsArrayLiteralExpression(n.Parent) && !ast.IsCallExpression(n.Parent) /*useFullStart */, sourceFile, l)
case ast.KindArrayLiteralExpression:
return spanForNode(n, n, ast.KindOpenBracketToken, !ast.IsArrayLiteralExpression(n.Parent) && !ast.IsCallExpression(n.Parent) /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBracketToken, !ast.IsArrayLiteralExpression(n.Parent) && !ast.IsCallExpression(n.Parent) /*useFullStart */, sourceFile, l)
case ast.KindJsxElement, ast.KindJsxFragment:
return spanForJSXElement(n, sourceFile, l)
case ast.KindJsxSelfClosingElement, ast.KindJsxOpeningElement:
return spanForJSXAttributes(n, sourceFile, l)
case ast.KindTemplateExpression, ast.KindNoSubstitutionTemplateLiteral:
return spanForTemplateLiteral(n, sourceFile, l)
case ast.KindArrayBindingPattern:
return spanForNode(n, n, ast.KindOpenBracketToken, !ast.IsBindingElement(n.Parent) /*useFullStart */, sourceFile, l)
return spanForNode(n, ast.KindOpenBracketToken, !ast.IsBindingElement(n.Parent) /*useFullStart */, sourceFile, l)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of these useFullStart comments have a trailing space on the end. Can you clean that up?

case ast.KindArrowFunction:
return spanForArrowFunction(n, sourceFile, l)
case ast.KindCallExpression:
Expand Down Expand Up @@ -387,7 +385,7 @@ func spanForImportExportElements(node *ast.Node, sourceFile *ast.SourceFile, l *
if openToken == nil || closeToken == nil || printer.PositionsAreOnSameLine(openToken.Pos(), closeToken.Pos(), sourceFile) {
return nil
}
return rangeBetweenTokens(openToken, closeToken, node, sourceFile, false /*useFullStart*/, l)
return rangeBetweenTokens(openToken, closeToken, sourceFile, false /*useFullStart*/, l)
}

func spanForParenthesizedExpression(node *ast.Node, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
Expand All @@ -396,7 +394,7 @@ func spanForParenthesizedExpression(node *ast.Node, sourceFile *ast.SourceFile,
return nil
}
textRange := l.createLspRangeFromBounds(start, node.End(), sourceFile)
return createFoldingRange(textRange, "", l.createLspRangeFromNode(node, sourceFile), "")
return createFoldingRange(textRange, "", "")
}

func spanForCallExpression(node *ast.Node, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
Expand All @@ -409,7 +407,7 @@ func spanForCallExpression(node *ast.Node, sourceFile *ast.SourceFile, l *Langua
return nil
}

return rangeBetweenTokens(openToken, closeToken, node, sourceFile, true /*useFullStart*/, l)
return rangeBetweenTokens(openToken, closeToken, sourceFile, true /*useFullStart*/, l)
}

func spanForArrowFunction(node *ast.Node, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
Expand All @@ -418,7 +416,7 @@ func spanForArrowFunction(node *ast.Node, sourceFile *ast.SourceFile, l *Languag
return nil
}
textRange := l.createLspRangeFromBounds(arrowFunctionNode.Body.Pos(), arrowFunctionNode.Body.End(), sourceFile)
return createFoldingRange(textRange, "", l.createLspRangeFromNode(node, sourceFile), "")
return createFoldingRange(textRange, "", "")
}

func spanForTemplateLiteral(node *ast.Node, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
Expand Down Expand Up @@ -448,7 +446,7 @@ func spanForJSXElement(node *ast.Node, sourceFile *ast.SourceFile, l *LanguageSe
bannerText.WriteString("<>...</>")
}

return createFoldingRange(textRange, "", nil, bannerText.String())
return createFoldingRange(textRange, "", bannerText.String())
}

func spanForJSXAttributes(node *ast.Node, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
Expand All @@ -466,38 +464,35 @@ func spanForJSXAttributes(node *ast.Node, sourceFile *ast.SourceFile, l *Languag

func spanForNodeArray(statements *ast.NodeList, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
if statements != nil && len(statements.Nodes) != 0 {
return createFoldingRange(l.createLspRangeFromBounds(statements.Pos(), statements.End(), sourceFile), "", nil, "")
return createFoldingRange(l.createLspRangeFromBounds(statements.Pos(), statements.End(), sourceFile), "", "")
}
return nil
}

func spanForNode(node *ast.Node, hintSpanNode *ast.Node, open ast.Kind, useFullStart bool, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
close := ast.KindCloseBraceToken
func spanForNode(node *ast.Node, open ast.Kind, useFullStart bool, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
closeBrace := ast.KindCloseBraceToken
if open != ast.KindOpenBraceToken {
close = ast.KindCloseBracketToken
closeBrace = ast.KindCloseBracketToken
}
openToken := findChildOfKind(node, open, sourceFile)
closeToken := findChildOfKind(node, close, sourceFile)
closeToken := findChildOfKind(node, closeBrace, sourceFile)
if openToken != nil && closeToken != nil {
return rangeBetweenTokens(openToken, closeToken, hintSpanNode, sourceFile, useFullStart, l)
return rangeBetweenTokens(openToken, closeToken, sourceFile, useFullStart, l)
}
return nil
}

func rangeBetweenTokens(openToken *ast.Node, closeToken *ast.Node, hintSpanNode *ast.Node, sourceFile *ast.SourceFile, useFullStart bool, l *LanguageService) *lsproto.FoldingRange {
func rangeBetweenTokens(openToken *ast.Node, closeToken *ast.Node, sourceFile *ast.SourceFile, useFullStart bool, l *LanguageService) *lsproto.FoldingRange {
var textRange *lsproto.Range
if useFullStart {
textRange = l.createLspRangeFromBounds(openToken.Pos(), closeToken.End(), sourceFile)
} else {
textRange = l.createLspRangeFromBounds(astnav.GetStartOfNode(openToken, sourceFile, false /*includeJSDoc*/), closeToken.End(), sourceFile)
}
return createFoldingRange(textRange, "", l.createLspRangeFromNode(hintSpanNode, sourceFile), "")
return createFoldingRange(textRange, "", "")
}

func createFoldingRange(textRange *lsproto.Range, foldingRangeKind lsproto.FoldingRangeKind, hintRange *lsproto.Range, collapsedText string) *lsproto.FoldingRange {
if hintRange == nil {
hintRange = textRange
}
func createFoldingRange(textRange *lsproto.Range, foldingRangeKind lsproto.FoldingRangeKind, collapsedText string) *lsproto.FoldingRange {
if collapsedText == "" {
defaultText := "..."
collapsedText = defaultText
Expand All @@ -517,14 +512,14 @@ func createFoldingRange(textRange *lsproto.Range, foldingRangeKind lsproto.Foldi
}

func createFoldingRangeFromBounds(pos int, end int, foldingRangeKind lsproto.FoldingRangeKind, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
return createFoldingRange(l.createLspRangeFromBounds(pos, end, sourceFile), foldingRangeKind, nil, "")
return createFoldingRange(l.createLspRangeFromBounds(pos, end, sourceFile), foldingRangeKind, "")
}

func functionSpan(node *ast.Node, body *ast.Node, sourceFile *ast.SourceFile, l *LanguageService) *lsproto.FoldingRange {
openToken := tryGetFunctionOpenToken(node, body, sourceFile)
closeToken := findChildOfKind(body, ast.KindCloseBraceToken, sourceFile)
if openToken != nil && closeToken != nil {
return rangeBetweenTokens(openToken, closeToken, node, sourceFile, true /*useFullStart*/, l)
return rangeBetweenTokens(openToken, closeToken, sourceFile, true /*useFullStart*/, l)
}
return nil
}
Expand Down
10 changes: 1 addition & 9 deletions internal/ls/folding_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,7 @@ type B =[| [
}|]

interface IFoo[| {
[|// all consecutive single line comments should be in one block regardless of their number or empty lines/spaces inbetween
[|// all consecutive single line comments should be in one block regardless of their number or empty lines/spaces in between

// comment 2
// comment 3
Expand Down Expand Up @@ -880,8 +880,6 @@ type B =[| [
}|]

function Foo()[| {
[|// comment 1
// comment 2|]
this.method = function (param)[| {
}|]

Expand Down Expand Up @@ -1101,12 +1099,6 @@ const[| {
const foo = null;

function Foo()[| {
[|/**
* Description
*
* @param {string} param
* @returns
*/|]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this crash, and that's why it's being removed? Can we leave the comment in and make sure we don't crash?

If we do crash, the editor will pop up an error message.

Copy link
Member Author

@navya9singh navya9singh Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't crash, the range is just returned twice because of the IsDeclarationNode(node) change. So, in Strada this range would've returned once because we checked for isDeclarationKind(node.Kind) instead of IsDeclarationNode(node) returning true here. The behavior still remains the same in the editor even if it is returned twice, I tested it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha; that seems fine, though it seems like visitNode could just skip any JSDoc nodes too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason for the other change in that same commit.
Btw, why is it that we're testing IsDeclarationNode(node) and not isDeclarationKind(node.Kind)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new reparsed JS AST, JSDoc nodes are not actually declaration nodes anymore, as we should be inserting into the tree real nodes instead. But, that all is in flux.

Copy link
Member Author

@navya9singh navya9singh Jul 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I compared the debugging here with Strada, it's not exactly JSDoc, what's happening is that when the node is a BinaryExpression, then it returns true for IsDeclarationNode(node) which did not use to be the case in Strada for isDeclarationKind(node.Kind). This is causing the duplication of that range.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, bleh, because BinaryExpressions are currently Declarations for JSDoc reasons. Can you easily special case that for now until that is resolved?

this.method = function (param)[| {
}|]

Expand Down
6 changes: 1 addition & 5 deletions internal/lsp/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,11 +577,7 @@ func (s *Server) handleInitialize(req *lsproto.RequestMessage) {
MoreTriggerCharacter: &[]string{"}", ";", "\n"},
},
FoldingRangeProvider: &lsproto.BooleanOrFoldingRangeOptionsOrFoldingRangeRegistrationOptions{
FoldingRangeOptions: &lsproto.FoldingRangeOptions{
WorkDoneProgressOptions: lsproto.WorkDoneProgressOptions{
WorkDoneProgress: ptrTo(true),
},
},
Boolean: ptrTo(true),
},
},
})
Expand Down