From 27bd9018fbbc66cc40991ca8786540e402816e16 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Mon, 20 May 2024 13:21:26 +0200 Subject: [PATCH 1/5] feat: support values lookup for range on mapping --- internal/lsp/symbol_table_test.go | 9 +++++++++ internal/util/values.go | 21 ++++++++++++++++++++- internal/util/values_test.go | 10 ++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/internal/lsp/symbol_table_test.go b/internal/lsp/symbol_table_test.go index 302ccc04..c81a0867 100644 --- a/internal/lsp/symbol_table_test.go +++ b/internal/lsp/symbol_table_test.go @@ -284,6 +284,15 @@ func TestSymbolTableForValuesSingleTests(t *testing.T) { }, foundContextsLen: 2, }, + { + template: `{{- range $type, $config := .Values.deployments }} {{ .test }} {{ end }} `, + path: []string{"Values", "deployments[]", "test"}, + startPoint: sitter.Point{ + Row: 0, + Column: 55, + }, + foundContextsLen: 3, + }, } for _, v := range testCases { diff --git a/internal/util/values.go b/internal/util/values.go index d13496cb..a69a5a0c 100644 --- a/internal/util/values.go +++ b/internal/util/values.go @@ -110,6 +110,21 @@ func arrayLookup(v chartutil.Values, path []string) (interface{}, error) { } return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} } + if nestedValues, ok := v2.(map[string]interface{}); ok { + if len(nestedValues) == 0 { + return chartutil.Values{}, ErrEmpytMapping{path[0]} + } + + for k := range nestedValues { + if len(path) == 1 { + return nestedValues[k], nil + } + if nestedValues, ok := (nestedValues[k]).(map[string]interface{}); ok { + return pathLookup(nestedValues, path[1:]) + } + } + return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} + } return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} } @@ -117,8 +132,12 @@ func arrayLookup(v chartutil.Values, path []string) (interface{}, error) { type ErrEmpytArray struct { Key string } +type ErrEmpytMapping struct { + Key string +} -func (e ErrEmpytArray) Error() string { return fmt.Sprintf("%q is an empyt array", e.Key) } +func (e ErrEmpytArray) Error() string { return fmt.Sprintf("%q is an empyt array", e.Key) } +func (e ErrEmpytMapping) Error() string { return fmt.Sprintf("%q is an empyt mapping", e.Key) } func builCompletionItem(value interface{}, variable string) lsp.CompletionItem { var ( diff --git a/internal/util/values_test.go b/internal/util/values_test.go index 6beb1188..5cf2c390 100644 --- a/internal/util/values_test.go +++ b/internal/util/values_test.go @@ -64,3 +64,13 @@ func TestValuesListNested(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "1", result) } + +func TestValuesRangeLookupOnMapping(t *testing.T) { + doubleNested := map[string]interface{}{"a": 1} + nested := map[string]interface{}{"nested": doubleNested, "other": "value"} + values := map[string]interface{}{"global": nested} + + result, err := GetTableOrValueForSelector(values, []string{"global[]"}) + assert.NoError(t, err) + assert.Equal(t, "a: 1\n", result) +} From 16a7e0adbed0180b86ed05e09c52a0d914e853dc Mon Sep 17 00:00:00 2001 From: qvalentin Date: Mon, 20 May 2024 19:10:19 +0200 Subject: [PATCH 2/5] fix: add recover to ApplyChanges --- internal/lsp/document.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/lsp/document.go b/internal/lsp/document.go index 99b8172c..70265950 100644 --- a/internal/lsp/document.go +++ b/internal/lsp/document.go @@ -2,6 +2,7 @@ package lsp import ( "bytes" + "fmt" "strings" "github.com/mrjosh/helm-ls/internal/util" @@ -24,6 +25,12 @@ type Document struct { // ApplyChanges updates the content of the document from LSP textDocument/didChange events. func (d *Document) ApplyChanges(changes []lsp.TextDocumentContentChangeEvent) { + defer func() { + if r := recover(); r != nil { + logger.Error(fmt.Sprintf("Recovered in ApplyChanges for %s, the document may be corrupted ", d.URI), r) + } + }() + content := []byte(d.Content) for _, change := range changes { start, end := util.PositionToIndex(change.Range.Start, content), util.PositionToIndex(change.Range.End, content) From 85c685403e5605fb5e23046d2687ab8756797bdd Mon Sep 17 00:00:00 2001 From: qvalentin Date: Wed, 22 May 2024 17:27:15 +0200 Subject: [PATCH 3/5] test: range over mapping test --- internal/handler/completion_main_test.go | 4 +++- internal/handler/hover_main_test.go | 9 +++++++++ testdata/example/templates/deployment.yaml | 3 +++ testdata/example/values.yaml | 7 +++++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/handler/completion_main_test.go b/internal/handler/completion_main_test.go index 8f49aeec..ba5de282 100644 --- a/internal/handler/completion_main_test.go +++ b/internal/handler/completion_main_test.go @@ -177,6 +177,8 @@ func TestCompletionMainSingleLines(t *testing.T) { {`Test completion on {{ range .Values.ingress.hosts }} {{ .ho^ }} {{ end }}`, []string{"host", "paths"}, []string{}, nil}, {`Test completion on {{ range .Values.ingress.hosts }} {{ range .paths }} {{ .^ }} {{ end }} {{ end }}`, []string{"pathType", "path"}, []string{}, nil}, {`Test completion on {{ root := . }} {{ $root.test.^ }}`, []string{}, []string{}, errors.New("[$root test ] is no valid template context for helm")}, + {`Test completion on {{ range $type, $config := $.Values.deployments }} {{ .^ }} {{ end }}`, []string{"some"}, []string{}, nil}, + {`Test completion on {{ range $type, $config := $.Values.deployments }} {{ .s^ }} {{ end }}`, []string{"some"}, []string{}, nil}, } for _, tt := range testCases { @@ -200,10 +202,10 @@ func TestCompletionMainSingleLines(t *testing.T) { for _, item := range result.Items { insertTexts = append(insertTexts, item.InsertText) } + for _, expectedInsertText := range tt.expectedInsertTexts { assert.Contains(t, insertTexts, expectedInsertText) } - for _, notExpectedInsertText := range tt.notExpectedInsertTexts { assert.NotContains(t, insertTexts, notExpectedInsertText) } diff --git a/internal/handler/hover_main_test.go b/internal/handler/hover_main_test.go index 2e3959fa..41887171 100644 --- a/internal/handler/hover_main_test.go +++ b/internal/handler/hover_main_test.go @@ -23,6 +23,15 @@ func TestHoverMain(t *testing.T) { expected string expectedError error }{ + { + desc: "Test hover on template context in range over mapping", + position: lsp.Position{ + Line: 85, + Character: 26, + }, + expected: "### ../../testdata/example/values.yaml\nvalue\n\n", + expectedError: nil, + }, { desc: "Test hover on template context with variables", position: lsp.Position{ diff --git a/testdata/example/templates/deployment.yaml b/testdata/example/templates/deployment.yaml index b8935401..50d3c002 100644 --- a/testdata/example/templates/deployment.yaml +++ b/testdata/example/templates/deployment.yaml @@ -82,3 +82,6 @@ spec: test: {{ $.Values.ingress.hosts }} --- {{- end }} + {{- range $type, $config := $.Values.deployments }} + test: {{ toYaml .some | nindent 4}} + {{- end }} diff --git a/testdata/example/values.yaml b/testdata/example/values.yaml index 7e405f4a..515e40d7 100644 --- a/testdata/example/values.yaml +++ b/testdata/example/values.yaml @@ -105,3 +105,10 @@ nodeSelector: {} tolerations: [] affinity: {} + + +deployments: + first: + some: value + second: + some: value From d6e87313cb3b3c405b8b73dcfc86183978f95538 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Wed, 22 May 2024 18:22:38 +0200 Subject: [PATCH 4/5] feat: support values location lookup for range on mapping --- internal/charts/values_files.go | 8 ++- internal/util/yaml.go | 51 +++++++++++--- internal/util/yaml_test.go | 106 +++++++++++------------------ internal/util/yaml_test_input.yaml | 6 ++ 4 files changed, 94 insertions(+), 77 deletions(-) diff --git a/internal/charts/values_files.go b/internal/charts/values_files.go index ba9bf9b5..4c37004d 100644 --- a/internal/charts/values_files.go +++ b/internal/charts/values_files.go @@ -1,6 +1,7 @@ package charts import ( + "fmt" "path/filepath" "github.com/mrjosh/helm-ls/internal/util" @@ -69,11 +70,14 @@ func (v *ValuesFiles) AllValuesFiles() []*ValuesFile { } func (v *ValuesFiles) GetPositionsForValue(query []string) []lsp.Location { + logger.Debug(fmt.Sprintf("GetPositionsForValue with query %v", query)) result := []lsp.Location{} for _, value := range v.AllValuesFiles() { - pos, err := util.GetPositionOfNode(&value.ValueNode, query) + queryCopy := append([]string{}, query...) + pos, err := util.GetPositionOfNode(&value.ValueNode, queryCopy) if err != nil { - logger.Error("Error getting position for value", value, query, err) + yaml, _ := value.Values.YAML() + logger.Error("Error getting position for value in yaml file %", yaml, query, err) continue } result = append(result, lsp.Location{URI: value.URI, Range: lsp.Range{Start: pos, End: pos}}) diff --git a/internal/util/yaml.go b/internal/util/yaml.go index c2c23ed3..91350ef5 100644 --- a/internal/util/yaml.go +++ b/internal/util/yaml.go @@ -14,36 +14,69 @@ 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) } + if node.Kind == 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) + } + if len(query) == 0 { return lsp.Position{Line: uint32(node.Line) - 1, Character: uint32(node.Column) - 1}, nil } - query[0] = strings.TrimSuffix(query[0], "[]") + isRange := false - 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) + if strings.HasSuffix(query[0], "[]") { + query[0] = strings.TrimSuffix(query[0], "[]") + isRange = true + } + + kind := node.Kind + switch kind { case yamlv3.SequenceNode: if len(node.Content) > 0 { return GetPositionOfNode(node.Content[0], query) } } + checkNested := []string{} for index, nestedNode := range node.Content { + checkNested = append(checkNested, nestedNode.Value) if nestedNode.Value == query[0] { if len(query) == 1 { return GetPositionOfNode(nestedNode, query[1:]) } if len(node.Content) < index+1 { - return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml", query) + return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.", query) + } + if isRange { + return getPositionOfNodeAfterMapping(node.Content[index+1], query[1:]) } 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) + return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml. Found no match. Possible values %v. Kind is %d", query, checkNested, kind) +} + +func getPositionOfNodeAfterMapping(node *yamlv3.Node, query []string) (lsp.Position, error) { + kind := node.Kind + + println(fmt.Sprintf("Kind is %d, len %d", kind, len(node.Content))) + println(fmt.Sprintf("ChildKind is %d, len %d", node.Content[0].Kind, len(node.Content[0].Content))) + + switch kind { + case yamlv3.SequenceNode: + if len(node.Content) > 0 { + return GetPositionOfNode(node.Content[0], query) + } + case yamlv3.MappingNode: + if len(node.Content) > 1 { + return GetPositionOfNode(node.Content[1], query) + } + } + + return lsp.Position{}, fmt.Errorf("could not find Position of %s in values. Found no match", query) } // ReadYamlFileToNode will parse a YAML file into a yaml Node. diff --git a/internal/util/yaml_test.go b/internal/util/yaml_test.go index e54c4373..8f1d1038 100644 --- a/internal/util/yaml_test.go +++ b/internal/util/yaml_test.go @@ -11,97 +11,71 @@ import ( "gopkg.in/yaml.v3" ) -func TestGetPositionOfNode(t *testing.T) { - data, err := os.ReadFile("./yaml_test_input.yaml") - if err != nil { - print(fmt.Sprint(err)) - t.Errorf("error yml parsing") - } - +func TestGetPositionOfNodeInEmptyDocument(t *testing.T) { var node yaml.Node - err = yaml.Unmarshal(data, &node) + err := yaml.Unmarshal([]byte(""), &node) if err != nil { print(fmt.Sprint(err)) t.Errorf("error yml parsing") } - result, err := GetPositionOfNode(&node, []string{"replicaCount"}) - expected := lsp.Position{Line: 5, Character: 0} - 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{"list[]"}) + expected := lsp.Position{} - 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 { - print(fmt.Sprint(err)) - t.Errorf("error yml parsing") - } - - var node yaml.Node - err = yaml.Unmarshal(data, &node) + err = yaml.Unmarshal([]byte(" "), &node) if err != nil { print(fmt.Sprint(err)) t.Errorf("error yml parsing") } - result, err := GetPositionOfNode(&node, []string{"list[]"}) - expected := lsp.Position{Line: 32, Character: 0} - - assert.NoError(t, err) - assert.Equal(t, expected, result) - - result, err = GetPositionOfNode(&node, []string{"list[]", "first"}) - expected = lsp.Position{Line: 33, Character: 4} - - assert.NoError(t, err) - assert.Equal(t, expected, result) - - result, err = GetPositionOfNode(&node, []string{"notExistingList[]", "first"}) + result, err = GetPositionOfNode(&node, []string{"list[]"}) 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 { - print(fmt.Sprint(err)) - t.Errorf("error yml parsing") +func TestGetPositionOfNodeTable(t *testing.T) { + tests := []struct { + name string + keys []string + expected lsp.Position + expectErr bool + }{ + {"replicaCount", []string{"replicaCount"}, lsp.Position{Line: 5, Character: 0}, false}, + {"image, repository", []string{"image", "repository"}, lsp.Position{Line: 8, Character: 2}, false}, + {"service, test, nested, value", []string{"service", "test", "nested", "value"}, lsp.Position{Line: 30, Character: 6}, false}, + {"service, test, wrong, value", []string{"service", "test", "wrong", "value"}, lsp.Position{}, true}, + {"list[]", []string{"list[]"}, lsp.Position{Line: 32, Character: 0}, false}, + {"list[], first", []string{"list[]", "first"}, lsp.Position{Line: 33, Character: 4}, false}, + {"notExistingList[], first", []string{"notExistingList[]", "first"}, lsp.Position{}, true}, + {"mapping[], something", []string{"mapping[]", "something"}, lsp.Position{Line: 40, Character: 4}, false}, } - result, err := GetPositionOfNode(&node, []string{"list[]"}) - expected := lsp.Position{} - - assert.Error(t, err) - assert.Equal(t, expected, result) - - err = yaml.Unmarshal([]byte(" "), &node) + data, err := os.ReadFile("./yaml_test_input.yaml") if err != nil { - print(fmt.Sprint(err)) - t.Errorf("error yml parsing") + t.Fatalf("error reading test input file: %v", err) } - result, err = GetPositionOfNode(&node, []string{"list[]"}) - expected = lsp.Position{} + var node yaml.Node + err = yaml.Unmarshal(data, &node) + if err != nil { + t.Fatalf("error parsing YAML: %v", err) + } - assert.Error(t, err) - assert.Equal(t, expected, result) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := GetPositionOfNode(&node, tt.keys) + + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } } diff --git a/internal/util/yaml_test_input.yaml b/internal/util/yaml_test_input.yaml index decd4ef8..65ac07bc 100644 --- a/internal/util/yaml_test_input.yaml +++ b/internal/util/yaml_test_input.yaml @@ -35,3 +35,9 @@ list: nested: 1 - second: nested: 2 + +mapping: + a: + something: value + b: + something: value From 62ca24c1131e1d83a2bb7c1266173651d3c9c549 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Wed, 22 May 2024 18:43:21 +0200 Subject: [PATCH 5/5] fix: don't modify slices --- internal/charts/values_files.go | 2 +- internal/handler/hover_main_test.go | 2 +- internal/lsp/symbol_table.go | 7 +++- internal/util/values.go | 58 ++++++++++++++++------------- internal/util/values_test.go | 7 +++- internal/util/yaml.go | 14 +++---- internal/util/yaml_test.go | 7 +++- 7 files changed, 56 insertions(+), 41 deletions(-) diff --git a/internal/charts/values_files.go b/internal/charts/values_files.go index 4c37004d..4f5ce580 100644 --- a/internal/charts/values_files.go +++ b/internal/charts/values_files.go @@ -77,7 +77,7 @@ func (v *ValuesFiles) GetPositionsForValue(query []string) []lsp.Location { pos, err := util.GetPositionOfNode(&value.ValueNode, queryCopy) if err != nil { yaml, _ := value.Values.YAML() - logger.Error("Error getting position for value in yaml file %", yaml, query, err) + logger.Error(fmt.Sprintf("Error getting position for value in yaml file %s with query %v ", yaml, query), err) continue } result = append(result, lsp.Location{URI: value.URI, Range: lsp.Range{Start: pos, End: pos}}) diff --git a/internal/handler/hover_main_test.go b/internal/handler/hover_main_test.go index 41887171..fe7f5a49 100644 --- a/internal/handler/hover_main_test.go +++ b/internal/handler/hover_main_test.go @@ -29,7 +29,7 @@ func TestHoverMain(t *testing.T) { Line: 85, Character: 26, }, - expected: "### ../../testdata/example/values.yaml\nvalue\n\n", + expected: fmt.Sprintf("### %s\n%s\n\n", filepath.Join("..", "..", "testdata", "example", "values.yaml"), "value"), expectedError: nil, }, { diff --git a/internal/lsp/symbol_table.go b/internal/lsp/symbol_table.go index 88ef5731..83ea3340 100644 --- a/internal/lsp/symbol_table.go +++ b/internal/lsp/symbol_table.go @@ -13,6 +13,10 @@ func (t TemplateContext) Format() string { return strings.Join(t, ".") } +func (t TemplateContext) Copy() TemplateContext { + return append(TemplateContext{}, t...) +} + func (t TemplateContext) Tail() TemplateContext { return t[1:] } @@ -65,7 +69,8 @@ func (s *SymbolTable) GetTemplateContext(pointRange sitter.Range) (TemplateConte if !ok { return result, fmt.Errorf("no template context found") } - return result, nil + // return a copy to never modify the original + return result.Copy(), nil } func (s *SymbolTable) AddIncludeDefinition(symbol string, pointRange sitter.Range) { diff --git a/internal/util/values.go b/internal/util/values.go index a69a5a0c..b446b1a6 100644 --- a/internal/util/values.go +++ b/internal/util/values.go @@ -64,7 +64,7 @@ func valuesLookup(values chartutil.Values, splittedVar []string) (chartutil.Valu return chartutil.Values{}, chartutil.ErrNoTable{Key: splittedVar[0]} } -// PathValue takes a path that traverses a YAML structure and returns the value at the end of that path. +// pathLookup takes a path that traverses a YAML structure and returns the value at the end of that path. // The path starts at the root of the YAML structure and is comprised of YAML keys separated by periods. // Given the following YAML data the value at path "chapter.one.title" is "Loomings". The path can also // include array indexes as in "chapters[].title" which will use the first element of the array. @@ -77,7 +77,7 @@ func pathLookup(v chartutil.Values, path []string) (interface{}, error) { return v, nil } if strings.HasSuffix(path[0], "[]") { - return arrayLookup(v, path) + return rangePathLookup(v, path) } // if exists must be root key not table value, ok := v[path[0]] @@ -93,39 +93,47 @@ func pathLookup(v chartutil.Values, path []string) (interface{}, error) { return nil, chartutil.ErrNoTable{Key: path[0]} } -func arrayLookup(v chartutil.Values, path []string) (interface{}, error) { +func rangePathLookup(v chartutil.Values, path []string) (interface{}, error) { v2, ok := v[path[0][:(len(path[0])-2)]] if !ok { return v, chartutil.ErrNoTable{Key: fmt.Sprintf("Yaml key %s does not exist", path[0])} } if v3, ok := v2.([]interface{}); ok { - if len(v3) == 0 { - return chartutil.Values{}, ErrEmpytArray{path[0]} - } - if len(path) == 1 { - return v3[0], nil - } - if vv, ok := v3[0].(map[string]interface{}); ok { - return pathLookup(vv, path[1:]) - } - return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} + return rangeArrayLookup(v3, path) } if nestedValues, ok := v2.(map[string]interface{}); ok { - if len(nestedValues) == 0 { - return chartutil.Values{}, ErrEmpytMapping{path[0]} - } + return rangeMappingLookup(nestedValues, path) + } - for k := range nestedValues { - if len(path) == 1 { - return nestedValues[k], nil - } - if nestedValues, ok := (nestedValues[k]).(map[string]interface{}); ok { - return pathLookup(nestedValues, path[1:]) - } - } - return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} + return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} +} + +func rangeArrayLookup(v3 []interface{}, path []string) (interface{}, error) { + if len(v3) == 0 { + return chartutil.Values{}, ErrEmpytArray{path[0]} } + if len(path) == 1 { + return v3[0], nil + } + if vv, ok := v3[0].(map[string]interface{}); ok { + return pathLookup(vv, path[1:]) + } + return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} +} +func rangeMappingLookup(nestedValues map[string]interface{}, path []string) (interface{}, error) { + if len(nestedValues) == 0 { + return chartutil.Values{}, ErrEmpytMapping{path[0]} + } + + for k := range nestedValues { + if len(path) == 1 { + return nestedValues[k], nil + } + if nestedValues, ok := (nestedValues[k]).(map[string]interface{}); ok { + return pathLookup(nestedValues, path[1:]) + } + } return chartutil.Values{}, chartutil.ErrNoTable{Key: path[0]} } diff --git a/internal/util/values_test.go b/internal/util/values_test.go index 5cf2c390..3672307e 100644 --- a/internal/util/values_test.go +++ b/internal/util/values_test.go @@ -67,10 +67,13 @@ func TestValuesListNested(t *testing.T) { func TestValuesRangeLookupOnMapping(t *testing.T) { doubleNested := map[string]interface{}{"a": 1} - nested := map[string]interface{}{"nested": doubleNested, "other": "value"} + nested := map[string]interface{}{"nested": doubleNested, "other": doubleNested} values := map[string]interface{}{"global": nested} - result, err := GetTableOrValueForSelector(values, []string{"global[]"}) + input := []string{"global[]"} + inputCopy := append([]string{}, input...) + result, err := GetTableOrValueForSelector(values, input) assert.NoError(t, err) assert.Equal(t, "a: 1\n", result) + assert.Equal(t, inputCopy, input) } diff --git a/internal/util/yaml.go b/internal/util/yaml.go index 91350ef5..ec2b9fea 100644 --- a/internal/util/yaml.go +++ b/internal/util/yaml.go @@ -28,6 +28,7 @@ func GetPositionOfNode(node *yamlv3.Node, query []string) (lsp.Position, error) isRange := false if strings.HasSuffix(query[0], "[]") { + query = append([]string{}, query...) query[0] = strings.TrimSuffix(query[0], "[]") isRange = true } @@ -48,10 +49,10 @@ func GetPositionOfNode(node *yamlv3.Node, query []string) (lsp.Position, error) return GetPositionOfNode(nestedNode, query[1:]) } if len(node.Content) < index+1 { - return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.", query) + return lsp.Position{}, fmt.Errorf("could not find Position of %s in values", query) } if isRange { - return getPositionOfNodeAfterMapping(node.Content[index+1], query[1:]) + return getPositionOfNodeAfterRange(node.Content[index+1], query[1:]) } return GetPositionOfNode(node.Content[index+1], query[1:]) } @@ -59,13 +60,8 @@ func GetPositionOfNode(node *yamlv3.Node, query []string) (lsp.Position, error) return lsp.Position{}, fmt.Errorf("could not find Position of %s in values.yaml. Found no match. Possible values %v. Kind is %d", query, checkNested, kind) } -func getPositionOfNodeAfterMapping(node *yamlv3.Node, query []string) (lsp.Position, error) { - kind := node.Kind - - println(fmt.Sprintf("Kind is %d, len %d", kind, len(node.Content))) - println(fmt.Sprintf("ChildKind is %d, len %d", node.Content[0].Kind, len(node.Content[0].Content))) - - switch kind { +func getPositionOfNodeAfterRange(node *yamlv3.Node, query []string) (lsp.Position, error) { + switch node.Kind { case yamlv3.SequenceNode: if len(node.Content) > 0 { return GetPositionOfNode(node.Content[0], query) diff --git a/internal/util/yaml_test.go b/internal/util/yaml_test.go index 8f1d1038..210c32d7 100644 --- a/internal/util/yaml_test.go +++ b/internal/util/yaml_test.go @@ -41,7 +41,7 @@ func TestGetPositionOfNodeInEmptyDocument(t *testing.T) { func TestGetPositionOfNodeTable(t *testing.T) { tests := []struct { name string - keys []string + query []string expected lsp.Position expectErr bool }{ @@ -68,7 +68,9 @@ func TestGetPositionOfNodeTable(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := GetPositionOfNode(&node, tt.keys) + queryCopy := append([]string{}, tt.query...) + + result, err := GetPositionOfNode(&node, tt.query) if tt.expectErr { assert.Error(t, err) @@ -76,6 +78,7 @@ func TestGetPositionOfNodeTable(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tt.expected, result) } + assert.Equal(t, queryCopy, tt.query) }) } }