From c75f5b723702d6dcb447582ccfa39c1a23324a18 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Sat, 30 Mar 2024 20:23:22 +0100 Subject: [PATCH] fix(go-to-definition): support range values test(definition): fix tests --- internal/handler/definition.go | 9 ++- internal/handler/definition_test.go | 48 +++++++++++- internal/handler/generic_usecase.go | 1 + internal/handler/hover.go | 3 +- internal/handler/hover_main_test.go | 22 +++++- internal/handler/hover_test.go | 4 +- internal/util/strings.go | 5 +- internal/util/yaml.go | 40 ++++++---- internal/util/yaml_test.go | 87 +++++++++++++++++----- internal/util/yaml_test_input.yaml | 6 +- testdata/example/templates/deployment.yaml | 1 + 11 files changed, 179 insertions(+), 47 deletions(-) create mode 100644 internal/handler/generic_usecase.go diff --git a/internal/handler/definition.go b/internal/handler/definition.go index 408b26af..b7fa372e 100644 --- a/internal/handler/definition.go +++ b/internal/handler/definition.go @@ -46,7 +46,8 @@ func (h *langHandler) definitionAstParsing(chart *charts.Chart, doc *lsplocal.Do relevantChildNode = lsplocal.FindRelevantChildNode(currentNode, pointToLoopUp) ) - switch relevantChildNode.Type() { + nodeType := relevantChildNode.Type() + switch nodeType { case gotemplate.NodeTypeIdentifier: if relevantChildNode.Parent().Type() == gotemplate.NodeTypeVariable { return h.getDefinitionForVariable(relevantChildNode, doc) @@ -63,7 +64,7 @@ func (h *langHandler) getDefinitionForVariable(node *sitter.Node, doc *lsplocal. variableName := node.Content([]byte(doc.Content)) defintionNode := lsplocal.GetVariableDefinition(variableName, node.Parent(), doc.Content) if defintionNode == nil { - return []lsp.Location{}, fmt.Errorf("Could not find definition for %s", variableName) + return []lsp.Location{}, fmt.Errorf("Could not find definition for %s. Variable definition not found", variableName) } return []lsp.Location{{URI: doc.URI, Range: lsp.Range{Start: util.PointToPosition(defintionNode.StartPoint())}}}, nil } @@ -87,7 +88,7 @@ func (h *langHandler) getDefinitionForFixedIdentifier(chart *charts.Chart, node nil } - return []lsp.Location{}, fmt.Errorf("Could not find definition for %s", name) + return []lsp.Location{}, fmt.Errorf("Could not find definition for %s. Fixed identifier not found", name) } func (h *langHandler) getDefinitionForValue(chart *charts.Chart, node *sitter.Node, doc *lsplocal.Document) ([]lsp.Location, error) { @@ -122,7 +123,7 @@ func (h *langHandler) getDefinitionForValue(chart *charts.Chart, node *sitter.No } return locations, nil } - return []lsp.Location{}, fmt.Errorf("Could not find definition for %s", yamlPath) + return []lsp.Location{}, fmt.Errorf("Could not find definition for %s. No definition found", yamlPath) } func getYamlPath(node *sitter.Node, doc *lsplocal.Document) string { diff --git a/internal/handler/definition_test.go b/internal/handler/definition_test.go index 812c53e0..2bb7616a 100644 --- a/internal/handler/definition_test.go +++ b/internal/handler/definition_test.go @@ -25,6 +25,10 @@ var testFileContent = ` {{ range $index, $element := pipeline }}{{ $index }}{{ $element }}{{ end }} # line 7 {{ .Values.foo }} # line 8 {{ .Values.something.nested }} # line 9 + +{{ range .Values.list }} +{{ . }} # line 12 +{{ end }} ` var ( @@ -35,6 +39,8 @@ var ( foo: bar something: nested: false +list: + - test ` ) @@ -145,7 +151,29 @@ func TestDefinitionValue(t *testing.T) { } // Input: -// {{ .Values.something.nested }} # line 9 +// {{ range .Values.list }} +// {{ . }} # line 12 +// ---| +func TestDefinitionValueInList(t *testing.T) { + genericDefinitionTest(t, lsp.Position{Line: 12, Character: 3}, []lsp.Location{ + { + URI: testValuesURI, + Range: lsp.Range{ + Start: lsp.Position{ + Line: 4, + Character: 0, + }, + End: lsp.Position{ + Line: 4, + Character: 0, + }, + }, + }, + }, nil) +} + +// Input: +// {{ . }} # line 9 // ----------------------| func TestDefinitionValueNested(t *testing.T) { genericDefinitionTest(t, lsp.Position{Line: 9, Character: 26}, []lsp.Location{ @@ -173,7 +201,11 @@ func TestDefinitionValueFile(t *testing.T) { URI: testValuesURI, Range: lsp.Range{ Start: lsp.Position{ - Line: 0, + Line: 1, + Character: 0, + }, + End: lsp.Position{ + Line: 1, Character: 0, }, }, @@ -237,7 +269,11 @@ func TestDefinitionValueFileMulitpleValues(t *testing.T) { URI: testValuesURI, Range: lsp.Range{ Start: lsp.Position{ - Line: 0, + Line: 1, + Character: 0, + }, + End: lsp.Position{ + Line: 1, Character: 0, }, }, @@ -245,7 +281,11 @@ func TestDefinitionValueFileMulitpleValues(t *testing.T) { URI: testOtherValuesURI, Range: lsp.Range{ Start: lsp.Position{ - Line: 0, + Line: 1, + Character: 0, + }, + End: lsp.Position{ + Line: 1, Character: 0, }, }, diff --git a/internal/handler/generic_usecase.go b/internal/handler/generic_usecase.go new file mode 100644 index 00000000..abeebd16 --- /dev/null +++ b/internal/handler/generic_usecase.go @@ -0,0 +1 @@ +package handler diff --git a/internal/handler/hover.go b/internal/handler/hover.go index 3ae3553f..a7323f75 100644 --- a/internal/handler/hover.go +++ b/internal/handler/hover.go @@ -11,6 +11,7 @@ import ( "github.com/mrjosh/helm-ls/internal/charts" lspinternal "github.com/mrjosh/helm-ls/internal/lsp" + "github.com/mrjosh/helm-ls/internal/tree-sitter/gotemplate" "github.com/mrjosh/helm-ls/internal/util" "github.com/mrjosh/helm-ls/pkg/chart" @@ -57,7 +58,7 @@ func (h *langHandler) Hover(ctx context.Context, params *lsp.HoverParams) (resul if (pt == "selector_expression" || pt == "field") && (ct == "identifier" || ct == "field_identifier") { word = lspinternal.GetFieldIdentifierPath(currentNode, doc) } - if ct == "dot" { + if ct == gotemplate.NodeTypeDot { word = lspinternal.TraverseIdentifierPathUp(currentNode, doc) } diff --git a/internal/handler/hover_main_test.go b/internal/handler/hover_main_test.go index c986b45c..632d9ab2 100644 --- a/internal/handler/hover_main_test.go +++ b/internal/handler/hover_main_test.go @@ -86,6 +86,24 @@ func TestHoverMain(t *testing.T) { expected: "", expectedError: nil, }, + { + desc: "Test hover values list", + position: lsp.Position{ + Line: 71, + Character: 35, + }, + expected: fmt.Sprintf("### %s\n%s\n\n", filepath.Join("..", "..", "testdata", "example", "values.yaml"), "ingress.hosts:\n- host: chart-example.local\n paths:\n - path: /\n pathType: ImplementationSpecific\n"), + expectedError: nil, + }, + { + desc: "Test not existing values list", + position: lsp.Position{ + Line: 101, + Character: 35, + }, + expected: "", + expectedError: fmt.Errorf("Could not parse ast correctly"), + }, } for _, tt := range testCases { t.Run(tt.desc, func(t *testing.T) { @@ -121,7 +139,9 @@ func TestHoverMain(t *testing.T) { }, }) assert.Equal(t, tt.expectedError, err) - assert.Equal(t, tt.expected, result.Contents.Value) + if err == nil { + assert.Equal(t, tt.expected, result.Contents.Value) + } }) } } diff --git a/internal/handler/hover_test.go b/internal/handler/hover_test.go index aabaed3c..301c0e0d 100644 --- a/internal/handler/hover_test.go +++ b/internal/handler/hover_test.go @@ -95,7 +95,9 @@ nested: value splittedVar: []string{"key"}, }, want: `### values.yaml -[map[nested:value]] +key: +- nested: value + `, wantErr: false, diff --git a/internal/util/strings.go b/internal/util/strings.go index 03545cf6..68f8b6f0 100644 --- a/internal/util/strings.go +++ b/internal/util/strings.go @@ -8,10 +8,7 @@ import ( "go.lsp.dev/protocol" ) -var ( - logger = log.GetLogger() - wordRegex = regexp.MustCompile(`[^ \t\n\f\r,;\[\]\"\']+`) -) +var wordRegex = regexp.MustCompile(`[^ \t\n\f\r,;\[\]\"\']+`) // BetweenStrings gets the substring between two strings. func BetweenStrings(value string, a string, b string) string { diff --git a/internal/util/yaml.go b/internal/util/yaml.go index dc40ca15..66f758ef 100644 --- a/internal/util/yaml.go +++ b/internal/util/yaml.go @@ -2,6 +2,7 @@ package util import ( "fmt" + "strings" lsp "go.lsp.dev/protocol" yamlv3 "gopkg.in/yaml.v3" @@ -12,23 +13,34 @@ func GetPositionOfNode(node *yamlv3.Node, query []string) (lsp.Position, error) return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml. Node was zero", query) } - for index, value := range node.Content { - if value.Value == "" { - result, err := GetPositionOfNode(value, query) - if err == nil { - return result, nil - } + if len(query) == 0 { + return lsp.Position{Line: uint32(node.Line) - 1, Character: uint32(node.Column) - 1}, nil + } + + query[0] = strings.TrimSuffix(query[0], "[0]") + + switch node.Kind { + case yamlv3.DocumentNode: + if len(node.Content) < 1 { + return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml. Document is empty", query) + } + return GetPositionOfNode(node.Content[0], query) + case yamlv3.SequenceNode: + if len(node.Content) > 0 { + return GetPositionOfNode(node.Content[0], query) } - if value.Value == query[0] { - if len(query) > 1 { - if len(node.Content) < index+1 { - return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml", query) - } - return GetPositionOfNode(node.Content[index+1], query[1:]) + } + + for index, nestedNode := range node.Content { + if nestedNode.Value == query[0] { + if len(query) == 1 { + return GetPositionOfNode(nestedNode, query[1:]) } - return lsp.Position{Line: uint32(value.Line) - 1, Character: uint32(value.Column) - 1}, nil + if len(node.Content) < index+1 { + return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml", query) + } + return GetPositionOfNode(node.Content[index+1], query[1:]) } } return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml. Found no match", query) - } diff --git a/internal/util/yaml_test.go b/internal/util/yaml_test.go index b8796c4e..56b5edb3 100644 --- a/internal/util/yaml_test.go +++ b/internal/util/yaml_test.go @@ -2,15 +2,16 @@ package util import ( "fmt" - lsp "go.lsp.dev/protocol" "os" "testing" + "github.com/stretchr/testify/assert" + lsp "go.lsp.dev/protocol" + "gopkg.in/yaml.v3" ) func TestGetPositionOfNode(t *testing.T) { - data, err := os.ReadFile("./yaml_test_input.yaml") if err != nil { print(fmt.Sprint(err)) @@ -19,7 +20,6 @@ func TestGetPositionOfNode(t *testing.T) { var node yaml.Node err = yaml.Unmarshal(data, &node) - if err != nil { print(fmt.Sprint(err)) t.Errorf("error yml parsing") @@ -27,28 +27,81 @@ func TestGetPositionOfNode(t *testing.T) { result, err := GetPositionOfNode(&node, []string{"replicaCount"}) expected := lsp.Position{Line: 5, Character: 0} - if err != nil { - t.Errorf("Result had error: %s", err) - } - if result != expected { - t.Errorf("Result was not expected Position %v but was %v", expected, result) - } + assert.NoError(t, err) + assert.Equal(t, expected, result) result, err = GetPositionOfNode(&node, []string{"image", "repository"}) expected = lsp.Position{Line: 8, Character: 2} + assert.NoError(t, err) + assert.Equal(t, expected, result) + + result, err = GetPositionOfNode(&node, []string{"service", "test", "nested", "value"}) + expected = lsp.Position{Line: 30, Character: 6} + assert.NoError(t, err) + assert.Equal(t, expected, result) + + result, err = GetPositionOfNode(&node, []string{"service", "test", "wrong", "value"}) + expected = lsp.Position{} + assert.Error(t, err) + assert.Equal(t, expected, result) +} + +func TestGetPositionOfNodeWithList(t *testing.T) { + data, err := os.ReadFile("./yaml_test_input.yaml") if err != nil { - t.Errorf("Result had error: %s", err) + print(fmt.Sprint(err)) + t.Errorf("error yml parsing") } - if result != expected { - t.Errorf("Result was not expected Position %v but was %v", expected, result) + + var node yaml.Node + err = yaml.Unmarshal(data, &node) + if err != nil { + print(fmt.Sprint(err)) + t.Errorf("error yml parsing") } - result, err = GetPositionOfNode(&node, []string{"service", "test", "nested", "value"}) - expected = lsp.Position{Line: 30, Character: 6} + result, err := GetPositionOfNode(&node, []string{"list[0]"}) + expected := lsp.Position{Line: 32, Character: 0} + + assert.NoError(t, err) + assert.Equal(t, expected, result) + + result, err = GetPositionOfNode(&node, []string{"list[0]", "first"}) + expected = lsp.Position{Line: 33, Character: 4} + + assert.NoError(t, err) + assert.Equal(t, expected, result) + + result, err = GetPositionOfNode(&node, []string{"notExistingList[0]", "first"}) + expected = lsp.Position{} + + assert.Error(t, err) + assert.Equal(t, expected, result) +} + +func TestGetPositionOfNodeInEmptyDocument(t *testing.T) { + var node yaml.Node + err := yaml.Unmarshal([]byte(""), &node) if err != nil { - t.Errorf("Result had error: %s", err) + print(fmt.Sprint(err)) + t.Errorf("error yml parsing") } - if result != expected { - t.Errorf("Result was not expected Position %v but was %v", expected, result) + + result, err := GetPositionOfNode(&node, []string{"list[0]"}) + expected := lsp.Position{} + + assert.Error(t, err) + assert.Equal(t, expected, result) + + err = yaml.Unmarshal([]byte(" "), &node) + if err != nil { + print(fmt.Sprint(err)) + t.Errorf("error yml parsing") } + + result, err = GetPositionOfNode(&node, []string{"list[0]"}) + expected = lsp.Position{} + + assert.Error(t, err) + assert.Equal(t, expected, result) } diff --git a/internal/util/yaml_test_input.yaml b/internal/util/yaml_test_input.yaml index 81ba0102..decd4ef8 100644 --- a/internal/util/yaml_test_input.yaml +++ b/internal/util/yaml_test_input.yaml @@ -30,4 +30,8 @@ service: nested: value: test - +list: + - first: + nested: 1 + - second: + nested: 2 diff --git a/testdata/example/templates/deployment.yaml b/testdata/example/templates/deployment.yaml index 67807b0f..6d2f3764 100644 --- a/testdata/example/templates/deployment.yaml +++ b/testdata/example/templates/deployment.yaml @@ -69,3 +69,4 @@ spec: {{- if $.Files.Glob "files/dags/*.py" }} {{- end }} {{- end }} + hosts: {{ .Values.ingress.hosts }}