From c1575332d70f130ef6327dfb48d3fcd2e8a37fd1 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Sun, 19 May 2024 15:00:21 +0200 Subject: [PATCH] fix: prevent panic on empty or unsupported template context fixes https://github.com/mrjosh/helm-ls/issues/81 --- internal/handler/completion_main_test.go | 1 + internal/handler/hover_main_test.go | 10 ++++++++++ internal/language_features/built_in_objects.go | 10 ++++------ internal/language_features/template_context.go | 7 +++++-- testdata/example/templates/deployment.yaml | 11 +++++++++++ 5 files changed, 31 insertions(+), 8 deletions(-) diff --git a/internal/handler/completion_main_test.go b/internal/handler/completion_main_test.go index 481cbab5..8f49aeec 100644 --- a/internal/handler/completion_main_test.go +++ b/internal/handler/completion_main_test.go @@ -176,6 +176,7 @@ func TestCompletionMainSingleLines(t *testing.T) { {`Test completion on {{ range .Values.ingress.hosts }} {{ .^ }} {{ end }}`, []string{"host", "paths"}, []string{}, nil}, {`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")}, } for _, tt := range testCases { diff --git a/internal/handler/hover_main_test.go b/internal/handler/hover_main_test.go index 8eeba435..b31509da 100644 --- a/internal/handler/hover_main_test.go +++ b/internal/handler/hover_main_test.go @@ -2,6 +2,7 @@ package handler import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -113,6 +114,15 @@ 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/language_features/built_in_objects.go b/internal/language_features/built_in_objects.go index a80ebfca..dbeaf044 100644 --- a/internal/language_features/built_in_objects.go +++ b/internal/language_features/built_in_objects.go @@ -28,8 +28,8 @@ func (f *BuiltInObjectsFeature) AppropriateForNode() bool { allowedBuiltIns := []string{"Chart", "Values", "Files", "Template", "Release"} - templateContext, _ := f.getTemplateContext() - if len(templateContext) != 1 { + templateContext, err := f.getTemplateContext() + if err != nil || len(templateContext) != 1 { return false } for _, allowedBuiltIn := range allowedBuiltIns { @@ -41,10 +41,7 @@ func (f *BuiltInObjectsFeature) AppropriateForNode() bool { } func (f *BuiltInObjectsFeature) References() (result []lsp.Location, err error) { - templateContext, err := f.getTemplateContext() - if err != nil { - return []lsp.Location{}, err - } + templateContext, _ := f.getTemplateContext() locations := f.getReferencesFromSymbolTable(templateContext) return append(locations, f.getDefinitionLocations(templateContext)...), err @@ -68,6 +65,7 @@ func (f *BuiltInObjectsFeature) getDefinitionLocations(templateContext lsplocal. func (f *BuiltInObjectsFeature) Hover() (string, error) { templateContext, _ := f.getTemplateContext() + docs, err := f.builtInOjectDocsLookup(templateContext[0], helmdocs.BuiltInObjects) return docs.Doc, err } diff --git a/internal/language_features/template_context.go b/internal/language_features/template_context.go index 01da658e..cf088edf 100644 --- a/internal/language_features/template_context.go +++ b/internal/language_features/template_context.go @@ -35,7 +35,7 @@ func (f *TemplateContextFeature) AppropriateForNode() bool { func (f *TemplateContextFeature) References() (result []lsp.Location, err error) { templateContext, err := f.getTemplateContext() - if err != nil { + if err != nil || len(templateContext) == 0 { return []lsp.Location{}, err } @@ -45,7 +45,7 @@ func (f *TemplateContextFeature) References() (result []lsp.Location, err error) func (f *TemplateContextFeature) Definition() (result []lsp.Location, err error) { templateContext, err := f.getTemplateContext() - if err != nil { + if err != nil || len(templateContext) == 0 { return []lsp.Location{}, err } return f.getDefinitionLocations(templateContext), nil @@ -81,6 +81,9 @@ func (f *TemplateContextFeature) getDefinitionLocations(templateContext lsplocal func (f *TemplateContextFeature) Hover() (string, error) { templateContext, err := f.getTemplateContext() + if err != nil || len(templateContext) == 0 { + return "", err + } switch templateContext[0] { case "Values": diff --git a/testdata/example/templates/deployment.yaml b/testdata/example/templates/deployment.yaml index 6d2f3764..d77fd2d8 100644 --- a/testdata/example/templates/deployment.yaml +++ b/testdata/example/templates/deployment.yaml @@ -70,3 +70,14 @@ spec: {{- end }} {{- end }} hosts: {{ .Values.ingress.hosts }} + + {{- $root := . -}} + {{- range $type, $config := $root.Values.deployments }} + apiVersion: apps/v1 + kind: Deployment + metadata: + name: my-app-{{ $type }} + spec: + replicas: {{ $config.hpa.minReplicas }} + --- + {{- end }}