From b1a9763a29b90a24c557ffef8a4c42f5f0d503f3 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Tue, 7 Jan 2025 21:04:41 -0500 Subject: [PATCH] Fix symbols crashing (#183) Closes https://github.com/grafana/jsonnet-language-server/issues/166 Symbols are now always strings. If field names are complex nodes, then we use the actual jsonnet --- pkg/ast/processing/find_field.go | 2 +- pkg/ast/processing/object_range.go | 41 +++++-- pkg/cache/cache.go | 6 +- pkg/server/hover_test.go | 2 +- pkg/server/symbols.go | 17 +-- pkg/server/symbols_test.go | 106 ++++++++++++++++++ .../testdata/conditional-fields.jsonnet | 6 + 7 files changed, 157 insertions(+), 23 deletions(-) create mode 100644 pkg/server/testdata/conditional-fields.jsonnet diff --git a/pkg/ast/processing/find_field.go b/pkg/ast/processing/find_field.go index 5d5d5b7..a010c4c 100644 --- a/pkg/ast/processing/find_field.go +++ b/pkg/ast/processing/find_field.go @@ -96,7 +96,7 @@ func (p *Processor) extractObjectRangesFromDesugaredObjs(desugaredObjs []*ast.De } if len(indexList) == 0 { for _, found := range foundFields { - ranges = append(ranges, FieldToRange(*found)) + ranges = append(ranges, p.FieldToRange(*found)) // If the field is not PlusSuper (field+: value), we stop there. Other previous values are not relevant // If partialMatchCurrentField is true, we can continue to look for other fields diff --git a/pkg/ast/processing/object_range.go b/pkg/ast/processing/object_range.go index 1d5be49..6873238 100644 --- a/pkg/ast/processing/object_range.go +++ b/pkg/ast/processing/object_range.go @@ -5,6 +5,8 @@ import ( "strings" "github.com/google/go-jsonnet/ast" + position "github.com/grafana/jsonnet-language-server/pkg/position_conversion" + "github.com/jdbaldry/go-language-server-protocol/lsp/protocol" ) type ObjectRange struct { @@ -15,7 +17,7 @@ type ObjectRange struct { Node ast.Node } -func FieldToRange(field ast.DesugaredObjectField) ObjectRange { +func (p *Processor) FieldToRange(field ast.DesugaredObjectField) ObjectRange { selectionRange := ast.LocationRange{ Begin: ast.Location{ Line: field.LocRange.Begin.Line, @@ -23,33 +25,48 @@ func FieldToRange(field ast.DesugaredObjectField) ObjectRange { }, End: ast.Location{ Line: field.LocRange.Begin.Line, - Column: field.LocRange.Begin.Column + len(FieldNameToString(field.Name)), + Column: field.LocRange.Begin.Column + len(p.FieldNameToString(field.Name)), }, } return ObjectRange{ Filename: field.LocRange.FileName, SelectionRange: selectionRange, FullRange: field.LocRange, - FieldName: FieldNameToString(field.Name), + FieldName: p.FieldNameToString(field.Name), Node: field.Body, } } -func FieldNameToString(fieldName ast.Node) string { - if fieldName, ok := fieldName.(*ast.LiteralString); ok { +func (p *Processor) FieldNameToString(fieldName ast.Node) string { + const unknown = "" + + switch fieldName := fieldName.(type) { + case *ast.LiteralString: return fieldName.Value - } - if fieldName, ok := fieldName.(*ast.Index); ok { + case *ast.Index: // We only want to wrap in brackets at the top level, so we trim at all step and then rewrap return fmt.Sprintf("[%s.%s]", - strings.Trim(FieldNameToString(fieldName.Target), "[]"), - strings.Trim(FieldNameToString(fieldName.Index), "[]"), + strings.Trim(p.FieldNameToString(fieldName.Target), "[]"), + strings.Trim(p.FieldNameToString(fieldName.Index), "[]"), ) - } - if fieldName, ok := fieldName.(*ast.Var); ok { + case *ast.Var: return string(fieldName.Id) + default: + loc := fieldName.Loc() + if loc == nil { + return unknown + } + fname := loc.FileName + if fname == "" { + return unknown + } + + content, err := p.cache.GetContents(protocol.URIFromPath(fname), position.RangeASTToProtocol(*loc)) + if err != nil { + return unknown + } + return content } - return "" } func LocalBindToRange(bind ast.LocalBind) ObjectRange { diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 804a976..dec59aa 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -107,7 +107,11 @@ func (c *Cache) GetContents(uri protocol.DocumentURI, position protocol.Range) ( for i := position.Start.Line; i <= position.End.Line; i++ { switch i { case position.Start.Line: - contentBuilder.WriteString(lines[i][position.Start.Character:]) + if i == position.End.Line { + contentBuilder.WriteString(lines[i][position.Start.Character:position.End.Character]) + } else { + contentBuilder.WriteString(lines[i][position.Start.Character:]) + } case position.End.Line: contentBuilder.WriteString(lines[i][:position.End.Character]) default: diff --git a/pkg/server/hover_test.go b/pkg/server/hover_test.go index 2622a77..d9db5ba 100644 --- a/pkg/server/hover_test.go +++ b/pkg/server/hover_test.go @@ -259,7 +259,7 @@ func TestHover(t *testing.T) { expectedContent: protocol.Hover{ Contents: protocol.MarkupContent{ Kind: protocol.Markdown, - Value: "```jsonnet\nbar: 'innerfoo',\n```\n", + Value: "```jsonnet\nbar: 'innerfoo'\n```\n", }, Range: protocol.Range{ Start: protocol.Position{Line: 9, Character: 5}, diff --git a/pkg/server/symbols.go b/pkg/server/symbols.go index 096f5bf..97b544b 100644 --- a/pkg/server/symbols.go +++ b/pkg/server/symbols.go @@ -27,7 +27,8 @@ func (s *Server) DocumentSymbol(_ context.Context, params *protocol.DocumentSymb return nil, nil } - symbols := buildDocumentSymbols(doc.AST) + processor := processing.NewProcessor(s.cache, nil) + symbols := s.buildDocumentSymbols(processor, doc.AST) result := make([]interface{}, len(symbols)) for i, symbol := range symbols { @@ -37,13 +38,13 @@ func (s *Server) DocumentSymbol(_ context.Context, params *protocol.DocumentSymb return result, nil } -func buildDocumentSymbols(node ast.Node) []protocol.DocumentSymbol { +func (s *Server) buildDocumentSymbols(processor *processing.Processor, node ast.Node) []protocol.DocumentSymbol { var symbols []protocol.DocumentSymbol switch node := node.(type) { case *ast.Binary: - symbols = append(symbols, buildDocumentSymbols(node.Left)...) - symbols = append(symbols, buildDocumentSymbols(node.Right)...) + symbols = append(symbols, s.buildDocumentSymbols(processor, node.Left)...) + symbols = append(symbols, s.buildDocumentSymbols(processor, node.Right)...) case *ast.Local: for _, bind := range node.Binds { objectRange := processing.LocalBindToRange(bind) @@ -55,21 +56,21 @@ func buildDocumentSymbols(node ast.Node) []protocol.DocumentSymbol { Detail: symbolDetails(bind.Body), }) } - symbols = append(symbols, buildDocumentSymbols(node.Body)...) + symbols = append(symbols, s.buildDocumentSymbols(processor, node.Body)...) case *ast.DesugaredObject: for _, field := range node.Fields { kind := protocol.Field if field.Hide == ast.ObjectFieldHidden { kind = protocol.Property } - fieldRange := processing.FieldToRange(field) + fieldRange := processor.FieldToRange(field) symbols = append(symbols, protocol.DocumentSymbol{ - Name: processing.FieldNameToString(field.Name), + Name: processor.FieldNameToString(field.Name), Kind: kind, Range: position.RangeASTToProtocol(fieldRange.FullRange), SelectionRange: position.RangeASTToProtocol(fieldRange.SelectionRange), Detail: symbolDetails(field.Body), - Children: buildDocumentSymbols(field.Body), + Children: s.buildDocumentSymbols(processor, field.Body), }) } } diff --git a/pkg/server/symbols_test.go b/pkg/server/symbols_test.go index fd688e6..09a0df8 100644 --- a/pkg/server/symbols_test.go +++ b/pkg/server/symbols_test.go @@ -266,6 +266,112 @@ func TestSymbols(t *testing.T) { }, }, }, + { + name: "Conditional fields", + filename: "testdata/conditional-fields.jsonnet", + expectSymbols: []interface{}{ + protocol.DocumentSymbol{ + Name: "flag", + Detail: "Boolean", + Kind: protocol.Variable, + Range: protocol.Range{ + Start: protocol.Position{ + Line: 0, + Character: 6, + }, + End: protocol.Position{ + Line: 0, + Character: 17, + }, + }, + SelectionRange: protocol.Range{ + Start: protocol.Position{ + Line: 0, + Character: 6, + }, + End: protocol.Position{ + Line: 0, + Character: 10, + }, + }, + }, + protocol.DocumentSymbol{ + Name: "if flag then 'hello'", + Detail: "String", + Kind: protocol.Field, + Range: protocol.Range{ + Start: protocol.Position{ + Line: 2, + Character: 2, + }, + End: protocol.Position{ + Line: 2, + Character: 34, + }, + }, + SelectionRange: protocol.Range{ + Start: protocol.Position{ + Line: 2, + Character: 2, + }, + End: protocol.Position{ + Line: 2, + Character: 22, + }, + }, + }, + protocol.DocumentSymbol{ + Name: "if flag then 'hello1' else 'hello2'", + Detail: "String", + Kind: protocol.Field, + Range: protocol.Range{ + Start: protocol.Position{ + Line: 3, + Character: 2, + }, + End: protocol.Position{ + Line: 3, + Character: 49, + }, + }, + SelectionRange: protocol.Range{ + Start: protocol.Position{ + Line: 3, + Character: 2, + }, + End: protocol.Position{ + Line: 3, + Character: 37, + }, + }, + }, + protocol.DocumentSymbol{ + Name: "if false == flag then 'hello3' else (function() 'test')()", + Detail: "String", + Kind: protocol.Field, + Range: protocol.Range{ + Start: protocol.Position{ + Line: 4, + Character: 2, + }, + End: protocol.Position{ + Line: 4, + Character: 71, + }, + }, + SelectionRange: protocol.Range{ + Start: protocol.Position{ + Line: 4, + Character: 2, + }, + End: protocol.Position{ + Line: 4, + Character: 59, + }, + }, + }, + }, + }, } { t.Run(tc.name, func(t *testing.T) { params := &protocol.DocumentSymbolParams{ diff --git a/pkg/server/testdata/conditional-fields.jsonnet b/pkg/server/testdata/conditional-fields.jsonnet new file mode 100644 index 0000000..0cec24c --- /dev/null +++ b/pkg/server/testdata/conditional-fields.jsonnet @@ -0,0 +1,6 @@ +local flag = true; +{ + [if flag then 'hello']: 'world!', + [if flag then 'hello1' else 'hello2']: 'world!', + [if false == flag then 'hello3' else (function() 'test')()]: 'world!', +}