diff --git a/internal/charts/values_files.go b/internal/charts/values_files.go index ba9bf9b5..4f5ce580 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(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/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..fe7f5a49 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: fmt.Sprintf("### %s\n%s\n\n", filepath.Join("..", "..", "testdata", "example", "values.yaml"), "value"), + expectedError: nil, + }, { desc: "Test hover on template context with variables", position: lsp.Position{ 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) 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/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..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,32 +93,59 @@ 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]} - } + return rangeArrayLookup(v3, path) + } + if nestedValues, ok := v2.(map[string]interface{}); ok { + return rangeMappingLookup(nestedValues, path) + } + + 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 v3[0], nil + return nestedValues[k], nil } - if vv, ok := v3[0].(map[string]interface{}); ok { - return pathLookup(vv, path[1:]) + 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]} } 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..3672307e 100644 --- a/internal/util/values_test.go +++ b/internal/util/values_test.go @@ -64,3 +64,16 @@ 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": doubleNested} + values := map[string]interface{}{"global": nested} + + 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 c2c23ed3..ec2b9fea 100644 --- a/internal/util/yaml.go +++ b/internal/util/yaml.go @@ -14,36 +14,65 @@ 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 = append([]string{}, query...) + 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 getPositionOfNodeAfterRange(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 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) + } + 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..210c32d7 100644 --- a/internal/util/yaml_test.go +++ b/internal/util/yaml_test.go @@ -11,97 +11,74 @@ 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 + query []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) + data, err := os.ReadFile("./yaml_test_input.yaml") + if err != nil { + t.Fatalf("error reading test input file: %v", err) + } - err = yaml.Unmarshal([]byte(" "), &node) + var node yaml.Node + err = yaml.Unmarshal(data, &node) if err != nil { - print(fmt.Sprint(err)) - t.Errorf("error yml parsing") + t.Fatalf("error parsing YAML: %v", err) } - result, err = GetPositionOfNode(&node, []string{"list[]"}) - expected = lsp.Position{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + queryCopy := append([]string{}, tt.query...) - assert.Error(t, err) - assert.Equal(t, expected, result) + result, err := GetPositionOfNode(&node, tt.query) + + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + assert.Equal(t, queryCopy, tt.query) + }) + } } 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 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