From da5d643084b8b4a98b2418f806982af468dd90a4 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Sun, 19 May 2024 18:47:05 +0200 Subject: [PATCH 1/2] feat(symbol-table): read in template context with variables --- internal/handler/hover_main_test.go | 28 ++++++++----- internal/lsp/symbol_table.go | 2 +- internal/lsp/symbol_table_template_context.go | 15 +++++-- .../lsp/symbol_table_template_context_test.go | 40 +++++++++++++++++++ internal/lsp/symbol_table_test.go | 22 ++++++++++ internal/lsp/visitor.go | 6 ++- testdata/example/templates/deployment.yaml | 1 + 7 files changed, 99 insertions(+), 15 deletions(-) create mode 100644 internal/lsp/symbol_table_template_context_test.go diff --git a/internal/handler/hover_main_test.go b/internal/handler/hover_main_test.go index b31509da..2e3959fa 100644 --- a/internal/handler/hover_main_test.go +++ b/internal/handler/hover_main_test.go @@ -2,7 +2,6 @@ package handler import ( "context" - "errors" "fmt" "os" "path/filepath" @@ -24,6 +23,24 @@ func TestHoverMain(t *testing.T) { expected string expectedError error }{ + { + desc: "Test hover on template context with variables", + position: lsp.Position{ + Line: 74, + Character: 50, + }, + expected: "$root.Values.deployments", + expectedError: nil, + }, + { + desc: "Test hover on template context with variables in range loop", + position: lsp.Position{ + Line: 80, + Character: 35, + }, + expected: "$config.hpa.minReplicas", + expectedError: nil, + }, { desc: "Test hover on dot", position: lsp.Position{ @@ -114,15 +131,6 @@ func TestHoverMain(t *testing.T) { expected: fmt.Sprintf("### %s\n%s\n\n", filepath.Join("..", "..", "testdata", "example", "values.yaml"), "1"), expectedError: nil, }, - { - desc: "Test hover on template context with variables", - position: lsp.Position{ - Line: 74, - Character: 50, - }, - expected: "", - expectedError: errors.New("no template context found"), - }, } for _, tt := range testCases { t.Run(tt.desc, func(t *testing.T) { diff --git a/internal/lsp/symbol_table.go b/internal/lsp/symbol_table.go index 8588c57e..d5227111 100644 --- a/internal/lsp/symbol_table.go +++ b/internal/lsp/symbol_table.go @@ -18,7 +18,7 @@ func (t TemplateContext) Tail() TemplateContext { } func (t TemplateContext) IsVariable() bool { - return len(t) > 0 && t[0] == "$" + return len(t) > 0 && strings.HasPrefix(t[0], "$") } func (t TemplateContext) AppendSuffix(suffix string) TemplateContext { diff --git a/internal/lsp/symbol_table_template_context.go b/internal/lsp/symbol_table_template_context.go index b77cdcbf..18688dcb 100644 --- a/internal/lsp/symbol_table_template_context.go +++ b/internal/lsp/symbol_table_template_context.go @@ -67,8 +67,12 @@ func (v *TemplateContextVisitor) Enter(node *sitter.Node) { GetRangeForNode(node.Child(int(node.ChildCount())-1))) case gotemplate.NodeTypeSelectorExpression: operandNode := node.ChildByFieldName("operand") - if operandNode.Type() == gotemplate.NodeTypeVariable && operandNode.Content(v.content) == "$" { + if operandNode.Type() == gotemplate.NodeTypeVariable { v.StashContext() + if operandNode.Content(v.content) != "$" { + v.symbolTable.AddTemplateContext(append(v.currentContext, operandNode.Content(v.content)), GetRangeForNode(operandNode)) + v.PushContext(operandNode.Content(v.content)) + } } } } @@ -77,7 +81,10 @@ func (v *TemplateContextVisitor) Exit(node *sitter.Node) { switch node.Type() { case gotemplate.NodeTypeSelectorExpression, gotemplate.NodeTypeUnfinishedSelectorExpression: operandNode := node.ChildByFieldName("operand") - if operandNode.Type() == gotemplate.NodeTypeVariable && operandNode.Content(v.content) == "$" { + if operandNode.Type() == gotemplate.NodeTypeVariable { + if operandNode.Content(v.content) != "$" { + v.PopContext() + } v.RestoreStashedContext() } } @@ -97,7 +104,9 @@ func (v *TemplateContextVisitor) EnterContextShift(node *sitter.Node, suffix str s = s.AppendSuffix(suffix) if s.IsVariable() { v.StashContext() - s = s.Tail() + if s[0] == "$" { + s = s.Tail() + } } } v.PushContextMany(s) diff --git a/internal/lsp/symbol_table_template_context_test.go b/internal/lsp/symbol_table_template_context_test.go new file mode 100644 index 00000000..a912e13b --- /dev/null +++ b/internal/lsp/symbol_table_template_context_test.go @@ -0,0 +1,40 @@ +package lsp + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetContextForSelectorExpression(t *testing.T) { + testCases := []struct { + desc string + template string + nodeContent string + expected TemplateContext + }{ + { + desc: "Selects single selector expression correctly", + template: `{{ .Values.test }}`, + nodeContent: ".Values.test", + expected: TemplateContext{"Values", "test"}, + }, + { + desc: "Selects selector expression with variable correctly", + template: `{{ $x.test }}`, + nodeContent: "$x.test", + expected: TemplateContext{"$x", "test"}, + }, + } + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + ast := ParseAst(nil, tC.template) + node := ast.RootNode().Child(1) + + assert.Equal(t, tC.nodeContent, node.Content([]byte(tC.template))) + result := getContextForSelectorExpression(node, []byte(tC.template)) + + assert.Equal(t, tC.expected, result) + }) + } +} diff --git a/internal/lsp/symbol_table_test.go b/internal/lsp/symbol_table_test.go index 530bf7e1..2b088431 100644 --- a/internal/lsp/symbol_table_test.go +++ b/internal/lsp/symbol_table_test.go @@ -223,6 +223,28 @@ func TestSymbolTableForValuesSingleTests(t *testing.T) { } testCases := []testCase{ + { + template: ` + {{- $root := . -}} + {{- range $type, $config := $root.Values.deployments }} + {{- .InLoop }} + {{- end }} + {{ .Values.test }} +`, + path: []string{"$root", "Values"}, + startPoint: sitter.Point{ + Row: 2, + Column: 40, + }, + }, + { + template: `{{ $x := .Values }}{{ $x.test }}{{ .Values.test }}`, + path: []string{"$x", "test"}, + startPoint: sitter.Point{ + Row: 0, + Column: 25, + }, + }, { template: `{{ if (and .Values. ) }} {{ end }} `, path: []string{"Values"}, diff --git a/internal/lsp/visitor.go b/internal/lsp/visitor.go index 7c96e639..8fdd9945 100644 --- a/internal/lsp/visitor.go +++ b/internal/lsp/visitor.go @@ -43,7 +43,11 @@ func (v *Visitors) visitNodesRecursiveWithScopeShift(node *sitter.Node) { case gotemplate.NodeTypeRangeAction: rangeNode := node.ChildByFieldName("range") if rangeNode == nil { - break // range is optional (e.g. {{ range $index, $element := pipeline }}) + rangeNode = node.NamedChild(0).ChildByFieldName("range") + if rangeNode == nil { + logger.Error("Could not find range node") + break + } } v.visitNodesRecursiveWithScopeShift(rangeNode) for _, visitor := range v.visitors { diff --git a/testdata/example/templates/deployment.yaml b/testdata/example/templates/deployment.yaml index d77fd2d8..b3ceb552 100644 --- a/testdata/example/templates/deployment.yaml +++ b/testdata/example/templates/deployment.yaml @@ -79,5 +79,6 @@ spec: name: my-app-{{ $type }} spec: replicas: {{ $config.hpa.minReplicas }} + {{ $.Values.ingress.hosts }} --- {{- end }} From 69e2d30727ea0fb054115d69874c6bc49eed6ab1 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Mon, 20 May 2024 11:41:32 +0200 Subject: [PATCH 2/2] fix: handle root variable ($) at different stage --- Makefile | 2 +- internal/lsp/symbol_table.go | 7 ++- internal/lsp/symbol_table_template_context.go | 14 ++---- .../lsp/symbol_table_template_context_test.go | 26 +++++++++- internal/lsp/symbol_table_test.go | 48 ++++++++++++++----- internal/lsp/visitor.go | 2 + testdata/example/templates/deployment.yaml | 2 +- 7 files changed, 75 insertions(+), 26 deletions(-) diff --git a/Makefile b/Makefile index 9880237e..a721d6be 100644 --- a/Makefile +++ b/Makefile @@ -68,7 +68,7 @@ test: @$(GO) test ./... -v -race -tags=integration coverage: - @$(GO) test -coverprofile=.coverage -tags=integration ./internal/... && go tool cover -html=.coverage + @$(GO) test -coverprofile=.coverage -tags=integration -coverpkg=./internal/... ./internal/... && go tool cover -html=.coverage .PHONY: build-release build-release: diff --git a/internal/lsp/symbol_table.go b/internal/lsp/symbol_table.go index d5227111..88ef5731 100644 --- a/internal/lsp/symbol_table.go +++ b/internal/lsp/symbol_table.go @@ -45,7 +45,12 @@ func NewSymbolTable(ast *sitter.Tree, content []byte) *SymbolTable { } func (s *SymbolTable) AddTemplateContext(templateContext TemplateContext, pointRange sitter.Range) { - s.contexts[templateContext.Format()] = append(s.contexts[strings.Join(templateContext, ".")], pointRange) + if templateContext.IsVariable() && templateContext[0] == "$" { + // $ is a special variable that resolves to the root context + // we can just remove it from the template context + templateContext = templateContext.Tail() + } + s.contexts[templateContext.Format()] = append(s.contexts[templateContext.Format()], pointRange) sliceCopy := make(TemplateContext, len(templateContext)) copy(sliceCopy, templateContext) s.contextsReversed[pointRange] = sliceCopy diff --git a/internal/lsp/symbol_table_template_context.go b/internal/lsp/symbol_table_template_context.go index 18688dcb..c83199b7 100644 --- a/internal/lsp/symbol_table_template_context.go +++ b/internal/lsp/symbol_table_template_context.go @@ -60,7 +60,7 @@ func (v *TemplateContextVisitor) Enter(node *sitter.Node) { v.symbolTable.AddTemplateContext(append(v.currentContext, content), GetRangeForNode(node.ChildByFieldName("name"))) case gotemplate.NodeTypeUnfinishedSelectorExpression: operandNode := node.ChildByFieldName("operand") - if operandNode.Type() == gotemplate.NodeTypeVariable && operandNode.Content(v.content) == "$" { + if operandNode.Type() == gotemplate.NodeTypeVariable { v.StashContext() } v.symbolTable.AddTemplateContext(append(getContextForSelectorExpression(operandNode, v.content), ""), @@ -69,10 +69,7 @@ func (v *TemplateContextVisitor) Enter(node *sitter.Node) { operandNode := node.ChildByFieldName("operand") if operandNode.Type() == gotemplate.NodeTypeVariable { v.StashContext() - if operandNode.Content(v.content) != "$" { - v.symbolTable.AddTemplateContext(append(v.currentContext, operandNode.Content(v.content)), GetRangeForNode(operandNode)) - v.PushContext(operandNode.Content(v.content)) - } + v.PushContext(operandNode.Content(v.content)) } } } @@ -82,9 +79,7 @@ func (v *TemplateContextVisitor) Exit(node *sitter.Node) { case gotemplate.NodeTypeSelectorExpression, gotemplate.NodeTypeUnfinishedSelectorExpression: operandNode := node.ChildByFieldName("operand") if operandNode.Type() == gotemplate.NodeTypeVariable { - if operandNode.Content(v.content) != "$" { - v.PopContext() - } + v.PopContext() v.RestoreStashedContext() } } @@ -104,9 +99,6 @@ func (v *TemplateContextVisitor) EnterContextShift(node *sitter.Node, suffix str s = s.AppendSuffix(suffix) if s.IsVariable() { v.StashContext() - if s[0] == "$" { - s = s.Tail() - } } } v.PushContextMany(s) diff --git a/internal/lsp/symbol_table_template_context_test.go b/internal/lsp/symbol_table_template_context_test.go index a912e13b..e9e12312 100644 --- a/internal/lsp/symbol_table_template_context_test.go +++ b/internal/lsp/symbol_table_template_context_test.go @@ -14,17 +14,41 @@ func TestGetContextForSelectorExpression(t *testing.T) { expected TemplateContext }{ { - desc: "Selects single selector expression correctly", + desc: "Selects simple selector expression correctly", template: `{{ .Values.test }}`, nodeContent: ".Values.test", expected: TemplateContext{"Values", "test"}, }, + { + desc: "Selects unfinished selector expression correctly", + template: `{{ .Values.test. }}`, + nodeContent: ".Values.test.", + expected: TemplateContext{"Values", "test"}, + }, + { + desc: "Selects selector expression with $ correctly", + template: `{{ $.Values.test }}`, + nodeContent: "$.Values.test", + expected: TemplateContext{"$", "Values", "test"}, + }, + { + desc: "Selects unfinished selector expression with $ correctly", + template: `{{ $.Values.test. }}`, + nodeContent: "$.Values.test.", + expected: TemplateContext{"$", "Values", "test"}, + }, { desc: "Selects selector expression with variable correctly", template: `{{ $x.test }}`, nodeContent: "$x.test", expected: TemplateContext{"$x", "test"}, }, + { + desc: "Selects unfinished selector expression with variable correctly", + template: `{{ $x.test. }}`, + nodeContent: "$x.test.", + expected: TemplateContext{"$x", "test"}, + }, } for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { diff --git a/internal/lsp/symbol_table_test.go b/internal/lsp/symbol_table_test.go index 2b088431..302ccc04 100644 --- a/internal/lsp/symbol_table_test.go +++ b/internal/lsp/symbol_table_test.go @@ -217,9 +217,10 @@ func TestSymbolTableForValuesTestFile(t *testing.T) { func TestSymbolTableForValuesSingleTests(t *testing.T) { type testCase struct { - template string - path []string - startPoint sitter.Point + template string + path []string + startPoint sitter.Point + foundContextsLen int } testCases := []testCase{ @@ -236,6 +237,7 @@ func TestSymbolTableForValuesSingleTests(t *testing.T) { Row: 2, Column: 40, }, + foundContextsLen: 6, }, { template: `{{ $x := .Values }}{{ $x.test }}{{ .Values.test }}`, @@ -244,6 +246,25 @@ func TestSymbolTableForValuesSingleTests(t *testing.T) { Row: 0, Column: 25, }, + foundContextsLen: 3, + }, + { + template: `{{ $x.test }}`, + path: []string{"$x", "test"}, + startPoint: sitter.Point{ + Row: 0, + Column: 6, + }, + foundContextsLen: 1, + }, + { + template: `{{ $x.test. }}`, + path: []string{"$x", "test", ""}, + startPoint: sitter.Point{ + Row: 0, + Column: 10, + }, + foundContextsLen: 2, }, { template: `{{ if (and .Values. ) }} {{ end }} `, @@ -252,6 +273,7 @@ func TestSymbolTableForValuesSingleTests(t *testing.T) { Row: 0, Column: 12, }, + foundContextsLen: 2, }, { template: `{{ if (and .Values. ) }} {{ end }} `, @@ -260,17 +282,21 @@ func TestSymbolTableForValuesSingleTests(t *testing.T) { Row: 0, Column: 18, }, + foundContextsLen: 2, }, } for _, v := range testCases { - ast := ParseAst(nil, v.template) - symbolTable := NewSymbolTable(ast, []byte(v.template)) - values := symbolTable.GetTemplateContextRanges(v.path) - points := []sitter.Point{} - for _, v := range values { - points = append(points, v.StartPoint) - } - assert.Contains(t, points, v.startPoint) + t.Run(v.template, func(t *testing.T) { + ast := ParseAst(nil, v.template) + symbolTable := NewSymbolTable(ast, []byte(v.template)) + values := symbolTable.GetTemplateContextRanges(v.path) + points := []sitter.Point{} + for _, v := range values { + points = append(points, v.StartPoint) + } + assert.Contains(t, points, v.startPoint) + assert.Len(t, symbolTable.contexts, v.foundContextsLen) + }) } } diff --git a/internal/lsp/visitor.go b/internal/lsp/visitor.go index 8fdd9945..fbfaf5b9 100644 --- a/internal/lsp/visitor.go +++ b/internal/lsp/visitor.go @@ -43,6 +43,8 @@ func (v *Visitors) visitNodesRecursiveWithScopeShift(node *sitter.Node) { case gotemplate.NodeTypeRangeAction: rangeNode := node.ChildByFieldName("range") if rangeNode == nil { + // for {{- range $type, $config := $root.Values.deployments }} the range node is in the + // range_variable_definition node an not in the range_action node rangeNode = node.NamedChild(0).ChildByFieldName("range") if rangeNode == nil { logger.Error("Could not find range node") diff --git a/testdata/example/templates/deployment.yaml b/testdata/example/templates/deployment.yaml index b3ceb552..b8935401 100644 --- a/testdata/example/templates/deployment.yaml +++ b/testdata/example/templates/deployment.yaml @@ -79,6 +79,6 @@ spec: name: my-app-{{ $type }} spec: replicas: {{ $config.hpa.minReplicas }} - {{ $.Values.ingress.hosts }} + test: {{ $.Values.ingress.hosts }} --- {{- end }}