feat(lint): rework diagnostics

internal/adapter/yamlls/documentSync.go

@@ -91,7 +91,7 @@ func (yamllsConnector Connector) DocumentDidChangeFullSync(doc *lsplocal.Documen
 		return
 	}
 
-	logger.Println("Sending DocumentDidChange with full sync, current content:", doc.Content)
+	logger.Debug("Sending DocumentDidChange with full sync, current content:", doc.Content)
 	trimmedText := lsplocal.TrimTemplate(doc.Ast.Copy(), doc.Content)
 
 	params.ContentChanges = []lsp.TextDocumentContentChangeEvent{

internal/handler/diagnostics.go

@@ -0,0 +1,17 @@
+package handler
+
+import (
+	"context"
+
+	lsp "go.lsp.dev/protocol"
+)
+
+func (h *langHandler) publishDiagnostics(ctx context.Context, notifications []lsp.PublishDiagnosticsParams) {
+	for _, notification := range notifications {
+		logger.Debug("Publishing diagnostics notification", notification)
+		err := h.client.PublishDiagnostics(ctx, &notification)
+		if err != nil {
+			logger.Error("Error publishing diagnostics ", err)
+		}
+	}
+}

internal/handler/text_document.go

@@ -28,12 +28,14 @@ func (h *langHandler) DidOpen(ctx context.Context, params *lsp.DidOpenTextDocume
 	if err != nil {
 		logger.Error("Error getting chart info for file", doc.URI, err)
 	}
-	notification := lsplocal.GetDiagnosticsNotification(chart, doc)
+	notifications := lsplocal.GetDiagnosticsNotifications(chart, doc)
 
-	return h.client.PublishDiagnostics(ctx, notification)
+	defer h.publishDiagnostics(ctx, notifications)
+
+	return nil
 }
 
-func (h *langHandler) DidClose(ctx context.Context, params *lsp.DidCloseTextDocumentParams) (err error) {
+func (h *langHandler) DidClose(_ context.Context, _ *lsp.DidCloseTextDocumentParams) (err error) {
 	return nil
 }
 
@@ -48,9 +50,11 @@ func (h *langHandler) DidSave(ctx context.Context, params *lsp.DidSaveTextDocume
 	}
 
 	h.yamllsConnector.DocumentDidSave(doc, *params)
-	notification := lsplocal.GetDiagnosticsNotification(chart, doc)
+	notifications := lsplocal.GetDiagnosticsNotifications(chart, doc)
 
-	return h.client.PublishDiagnostics(ctx, notification)
+	defer h.publishDiagnostics(ctx, notifications)
+
+	return nil
 }
 
 func (h *langHandler) DidChange(ctx context.Context, params *lsp.DidChangeTextDocumentParams) (err error) {

internal/lsp/lint.go

@@ -2,6 +2,7 @@ package lsp
 
 import (
 	"fmt"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -13,126 +14,130 @@ import (
 	lsp "go.lsp.dev/protocol"
 	"go.lsp.dev/uri"
 	"helm.sh/helm/v3/pkg/action"
-	// "helm.sh/helm/v3/pkg/lint/rules"
+
 	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/lint/support"
 )
 
 var logger = log.GetLogger()
 
-func GetDiagnosticsNotification(chart *charts.Chart, doc *Document) *lsp.PublishDiagnosticsParams {
+func GetDiagnosticsNotifications(chart *charts.Chart, doc *Document) []lsp.PublishDiagnosticsParams {
 	vals := chart.ValuesFiles.MainValuesFile.Values
 	if chart.ValuesFiles.OverlayValuesFile != nil {
 		vals = chartutil.CoalesceTables(chart.ValuesFiles.OverlayValuesFile.Values, chart.ValuesFiles.MainValuesFile.Values)
 	}
 
-	diagnostics := GetDiagnostics(doc.URI, vals)
-	doc.DiagnosticsCache.HelmDiagnostics = diagnostics
-
-	return &lsp.PublishDiagnosticsParams{
-		URI:         doc.URI,
-		Diagnostics: doc.DiagnosticsCache.GetMergedDiagnostics(),
+	diagnostics := GetDiagnostics(chart.RootURI, vals)
+
+	// Update the diagnostics cache only for the currently opened document
+	// as it will also get diagnostics from yamlls
+	// if currentDocDiagnostics is empty it means that all issues in that file have been fixed
+	// we need to send this to the client
+	currentDocDiagnostics := diagnostics[string(doc.URI.Filename())]
+	doc.DiagnosticsCache.HelmDiagnostics = currentDocDiagnostics
+	diagnostics[string(doc.URI.Filename())] = doc.DiagnosticsCache.GetMergedDiagnostics()
+
+	result := []lsp.PublishDiagnosticsParams{}
+
+	for diagnosticsURI, diagnostics := range diagnostics {
+		result = append(result,
+			lsp.PublishDiagnosticsParams{
+				URI:         uri.File(diagnosticsURI),
+				Diagnostics: diagnostics,
+			},
+		)
 	}
+
+	return result
 }
 
-// GetDiagnostics will run helm linter against the currect document URI using the given values
+// GetDiagnostics will run helm linter against the chart root URI using the given values
 // and converts the helm.support.Message to lsp.Diagnostics
-func GetDiagnostics(uri uri.URI, vals chartutil.Values) []lsp.Diagnostic {
-	var (
-		filename    = uri.Filename()
-		paths       = strings.Split(filename, "/")
-		dir         = strings.Join(paths, "/")
-		diagnostics = make([]lsp.Diagnostic, 0)
-	)
-
-	pathfile := ""
-
-	for i, p := range paths {
-		if p == "templates" {
-			dir = strings.Join(paths[0:i], "/")
-			pathfile = strings.Join(paths[i:], "/")
-		}
-	}
+func GetDiagnostics(rootURI uri.URI, vals chartutil.Values) map[string][]lsp.Diagnostic {
+	diagnostics := map[string][]lsp.Diagnostic{}
 
 	client := action.NewLint()
 
-	result := client.Run([]string{dir}, vals)
-	logger.Println(fmt.Sprintf("helm lint: result for file %s : %s", uri, result.Messages))
+	result := client.Run([]string{rootURI.Filename()}, vals)
 
 	for _, msg := range result.Messages {
-		d, filename, err := GetDiagnosticFromLinterErr(msg)
-		if err != nil {
-			continue
-		}
-		if filename != pathfile {
-			continue
+		d, relativeFilePath, _ := GetDiagnosticFromLinterErr(msg)
+		absoluteFilePath := filepath.Join(rootURI.Filename(), string(relativeFilePath))
+		if d != nil {
+			diagnostics[absoluteFilePath] = append(diagnostics[absoluteFilePath], *d)
 		}
-		diagnostics = append(diagnostics, *d)
 	}
-	logger.Println(fmt.Sprintf("helm lint: result for file %s : %v", uri, diagnostics))
+	logger.Println(fmt.Sprintf("helm lint: result for chart %s : %v", rootURI.Filename(), diagnostics))
 
 	return diagnostics
 }
 
 func GetDiagnosticFromLinterErr(supMsg support.Message) (*lsp.Diagnostic, string, error) {
-	var (
-		err      error
-		msg      string
-		line     = 1
-		severity lsp.DiagnosticSeverity
-		filename = getFilePathFromLinterErr(supMsg)
-	)
-
-	switch supMsg.Severity {
-	case support.ErrorSev:
-
-		severity = lsp.DiagnosticSeverityError
-
-		// if superr, ok := supMsg.Err.(*rules.YAMLToJSONParseError); ok {
-
-		// line = superr.Line
-		// msg = superr.Error()
-		//
-		// } else {
+	severity := parseSeverity(supMsg)
 
-		fileLine := util.BetweenStrings(supMsg.Error(), "(", ")")
-		fileLineArr := strings.Split(fileLine, ":")
-		if len(fileLineArr) < 2 {
-			return nil, filename, errors.Errorf("linter Err contains no position information")
-		}
-		lineStr := fileLineArr[1]
-		line, err = strconv.Atoi(lineStr)
+	if strings.HasPrefix(supMsg.Path, "templates") {
+		message, err := parseTemplatesMessage(supMsg, severity)
+		path := getFilePathFromLinterErr(supMsg)
 		if err != nil {
-			return nil, filename, err
+			return nil, "", err
 		}
-		msgStr := util.AfterStrings(supMsg.Error(), "):")
-		msg = strings.TrimSpace(msgStr)
-
-		// }
-
-	case support.WarningSev:
-
-		// severity = lsp.DiagnosticSeverityWarning
-		// if err, ok := supMsg.Err.(*rules.MetadataError); ok {
-		// 	line = 1
-		// 	msg = err.Details().Error()
-		// }
+		return &message, path, nil
+	}
 
-	case support.InfoSev:
+	message := string(supMsg.Err.Error())
+	// NOTE: The diagnostics may not be shown correctly in the Chart.yaml file in neovim
+	// because the lsp is not active for that file
+	if supMsg.Path == "Chart.yaml" || strings.Contains(message, "chart metadata") {
+		return &lsp.Diagnostic{
+			Severity: severity,
+			Source:   "Helm lint",
+			Message:  message,
+		}, "Chart.yaml", nil
+	}
 
-		severity = lsp.DiagnosticSeverityInformation
-		msg = supMsg.Err.Error()
+	return nil, "", nil
+}
 
+func parseTemplatesMessage(supMsg support.Message, severity lsp.DiagnosticSeverity) (lsp.Diagnostic, error) {
+	var (
+		err         error
+		line        int
+		fileLine    = util.BetweenStrings(supMsg.Error(), "(", ")")
+		fileLineArr = strings.Split(fileLine, ":")
+	)
+	if len(fileLineArr) < 2 {
+		return lsp.Diagnostic{}, errors.Errorf("linter Err contains no position information")
+	}
+	lineStr := fileLineArr[1]
+	line, err = strconv.Atoi(lineStr)
+	if err != nil {
+		return lsp.Diagnostic{}, err
 	}
+	msgStr := util.AfterStrings(supMsg.Error(), "):")
+	msg := strings.TrimSpace(msgStr)
 
-	return &lsp.Diagnostic{
+	return lsp.Diagnostic{
 		Range: lsp.Range{
 			Start: lsp.Position{Line: uint32(line - 1)},
 			End:   lsp.Position{Line: uint32(line - 1)},
 		},
 		Severity: severity,
+		Source:   "Helm lint",
 		Message:  msg,
-	}, filename, nil
+	}, nil
+}
+
+func parseSeverity(supMsg support.Message) lsp.DiagnosticSeverity {
+	var severity lsp.DiagnosticSeverity
+	switch supMsg.Severity {
+	case support.ErrorSev:
+		severity = lsp.DiagnosticSeverityError
+	case support.WarningSev:
+		severity = lsp.DiagnosticSeverityWarning
+	case support.InfoSev:
+		severity = lsp.DiagnosticSeverityInformation
+	}
+	return severity
 }
 
 func getFilePathFromLinterErr(msg support.Message) string {

internal/lsp/lint_test.go

@@ -3,12 +3,63 @@ package lsp
 import (
 	"testing"
 
+	"github.com/mrjosh/helm-ls/internal/charts"
 	"github.com/stretchr/testify/assert"
 	"go.lsp.dev/uri"
 	"helm.sh/helm/v3/pkg/chartutil"
 )
 
 func TestLint(t *testing.T) {
-	diagnostics := GetDiagnostics(uri.File("../../testdata/example/templates/lint.yaml"), chartutil.Values{})
+	diagnostics := GetDiagnostics(uri.File("../../testdata/example"), chartutil.Values{})
 	assert.NotEmpty(t, diagnostics)
+	assert.Len(t, diagnostics, 2)
+	assert.Len(t, diagnostics[uri.File("../../testdata/example/Chart.yaml").Filename()], 1)
+}
+
+func TestLintNotifications(t *testing.T) {
+	chart := charts.Chart{
+		RootURI: uri.File("../../testdata/example"),
+		ValuesFiles: &charts.ValuesFiles{
+			MainValuesFile:        &charts.ValuesFile{},
+			OverlayValuesFile:     &charts.ValuesFile{},
+			AdditionalValuesFiles: []*charts.ValuesFile{},
+		},
+	}
+	diagnostics := GetDiagnosticsNotifications(&chart, &Document{URI: uri.File("../../testdata/example/templates/deployment-no-templates.yaml")})
+	assert.NotEmpty(t, diagnostics)
+	assert.Len(t, diagnostics, 3)
+
+	uris := []string{}
+	for _, notification := range diagnostics {
+		uris = append(uris, notification.URI.Filename())
+	}
+	assert.Contains(t, uris, uri.File("../../testdata/example/templates/deployment-no-templates.yaml").Filename())
+	for _, notification := range diagnostics {
+		if notification.URI.Filename() == uri.File("../../testdata/example/templates/deployment-no-templates.yaml").Filename() {
+			assert.Empty(t, notification.Diagnostics)
+		}
+	}
+}
+
+func TestLintNotificationsIncludesEmptyDiagnosticsForFixedIssues(t *testing.T) {
+	chart := charts.Chart{
+		RootURI: uri.File("../../testdata/example"),
+		ValuesFiles: &charts.ValuesFiles{
+			MainValuesFile:        &charts.ValuesFile{},
+			OverlayValuesFile:     &charts.ValuesFile{},
+			AdditionalValuesFiles: []*charts.ValuesFile{},
+		},
+	}
+	diagnostics := GetDiagnosticsNotifications(&chart, &Document{URI: uri.File("../../testdata/example/templates/deployment-no-templates.yaml")})
+
+	uris := []string{}
+	for _, notification := range diagnostics {
+		uris = append(uris, notification.URI.Filename())
+	}
+	assert.Contains(t, uris, uri.File("../../testdata/example/templates/deployment-no-templates.yaml").Filename())
+	for _, notification := range diagnostics {
+		if notification.URI.Filename() == uri.File("../../testdata/example/templates/deployment-no-templates.yaml").Filename() {
+			assert.Empty(t, notification.Diagnostics)
+		}
+	}
 }

internal/util/values.go

@@ -66,7 +66,8 @@ func valuesLookup(values chartutil.Values, splittedVar []string) (chartutil.Valu
 
 // PathValue 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".
+// 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.
 //
 //	chapter:
 //	  one:
@@ -152,8 +153,7 @@ func builCompletionItem(value interface{}, variable string) lsp.CompletionItem {
 }
 
 func FormatToYAML(field reflect.Value, fieldName string) string {
-	x := field.Kind()
-	switch x {
+	switch field.Kind() {
 	case reflect.String:
 		return field.String()
 	case reflect.Map: