Skip to content

Commit

Permalink
fix: don't modify slices
Browse files Browse the repository at this point in the history
  • Loading branch information
qvalentin committed May 24, 2024
1 parent d6e8731 commit 62ca24c
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 41 deletions.
2 changes: 1 addition & 1 deletion internal/charts/values_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}})
Expand Down
2 changes: 1 addition & 1 deletion internal/handler/hover_main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand Down
7 changes: 6 additions & 1 deletion internal/lsp/symbol_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:]
}
Expand Down Expand Up @@ -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) {
Expand Down
58 changes: 33 additions & 25 deletions internal/util/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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]]
Expand All @@ -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]}
}

Expand Down
7 changes: 5 additions & 2 deletions internal/util/values_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
14 changes: 5 additions & 9 deletions internal/util/yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -48,24 +49,19 @@ 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:])
}
}
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)
Expand Down
7 changes: 5 additions & 2 deletions internal/util/yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
Expand All @@ -68,14 +68,17 @@ 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)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.expected, result)
}
assert.Equal(t, queryCopy, tt.query)
})
}
}

0 comments on commit 62ca24c

Please sign in to comment.