feat(yamlls): keep only text nodes when trimming yaml

.github/workflows/tests.yml

@@ -13,7 +13,7 @@ jobs:
     strategy:
       matrix:
         go-version: [1.21.5]
-        os: [ubuntu-latest, macos-latest]
+        os: [ubuntu-latest, macos-latest, windows-2022]
     runs-on: ${{ matrix.os }}
     steps:
 
@@ -27,30 +27,3 @@ jobs:
 
     - name: Run tests
       run: make test
-
-  tests-windows:
-    name: tests
-    strategy:
-      matrix:
-        go-version: [1.21.5]
-        os: [windows-2019]
-    runs-on: ${{ matrix.os }}
-    steps:
-
-    - name: Install Go
-      uses: actions/setup-go@v4
-      with:
-        go-version: ${{ matrix.go-version }}
-
-    - name: Set up MinGW
-      uses: egor-tensin/setup-mingw@v2
-      with:
-        version: 12.2.0
-
-    - name: Checkout code
-      uses: actions/checkout@v4
-
-    - name: Run tests
-      run: make test
-      env:
-        CC: "cc"

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "testdata/charts"]
+	path = testdata/charts
+	url = https://github.com/qvalentin/charts.git

Makefile

@@ -55,7 +55,16 @@ install-metalinter:
 	@$(GO) get -v github.com/golangci/golangci-lint/cmd/[email protected]
 	@$(GO) install -v github.com/golangci/golangci-lint/cmd/[email protected]
 
+install-yamlls:
+	npm install --global yaml-language-server
+
+integration-test-deps:
+	@YAMLLS_BIN=$$(command -v yaml-language-server) || { echo "yaml-language-server command not found! Installing..." && $(MAKE) install-yamlls; };
+	git submodule init
+	git submodule update --depth 1
+
 test:
+	$(MAKE) integration-test-deps
 	@$(GO) test ./... -v -race
 
 coverage:

internal/adapter/yamlls/diagnostics.go

@@ -54,20 +54,26 @@ func diagnisticIsRelevant(diagnostic lsp.Diagnostic, node *sitter.Node) bool {
 	switch diagnostic.Message {
 	case "Map keys must be unique":
 		return !lsplocal.IsInElseBranch(node)
-	case "All mapping items must start at the same column",
-		"Implicit map keys need to be followed by map values",
-		"Implicit keys need to be on a single line",
-		"A block sequence may not be used as an implicit map key":
-		// TODO: could add a check if is is caused by includes
+	case "All mapping items must start at the same column":
+		// unknown what exactly this is, only causes one error in bitnami/charts
 		return false
+	case "Implicit map keys need to be followed by map values", "A block sequence may not be used as an implicit map key", "Implicit keys need to be on a single line":
+		// still breaks with
+		// params:
+		// {{- range $key, $value := .params }}
+		// {{ $key }}:
+		//   {{- range $value }}
+		//   - {{ . | quote }}
+		//   {{- end }}
+		// {{- end }}
+		return false && !lsplocal.IsInElseBranch(node)
 	case "Block scalars with more-indented leading empty lines must use an explicit indentation indicator":
-		return false
 		// TODO: check if this is a false positive, probably requires parsing the yaml with tree-sitter injections
 		// smtp-password: |
 		//   {{- if not .Values.existingSecret }}
 		//   test: dsa
 		//   {{- end }}
-
+		return false
 	default:
 		return true
 	}

internal/adapter/yamlls/diagnostics_integration_test.go

@@ -0,0 +1,164 @@
+package yamlls
+
+import (
+	"encoding/json"
+	"fmt"
+	"log"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+	"time"
+
+	lsplocal "github.com/mrjosh/helm-ls/internal/lsp"
+	"github.com/mrjosh/helm-ls/internal/util"
+	"github.com/stretchr/testify/assert"
+	"go.lsp.dev/jsonrpc2"
+	lsp "go.lsp.dev/protocol"
+	"go.lsp.dev/uri"
+)
+
+// must be relative to this file
+var TEST_DATA_DIR = "../../../testdata/charts/bitnami/"
+
+type jsonRpcDiagnostics struct {
+	Params  lsp.PublishDiagnosticsParams `json:"params"`
+	Jsonrpc string                       `json:"jsonrpc"`
+	Method  string                       `json:"method"`
+}
+
+type readWriteCloseMock struct {
+	diagnosticsChan chan lsp.PublishDiagnosticsParams
+}
+
+func (proc readWriteCloseMock) Read(p []byte) (int, error) {
+	return 1, nil
+}
+
+func (proc readWriteCloseMock) Write(p []byte) (int, error) {
+	if strings.HasPrefix(string(p), "Content-Length: ") {
+		return 1, nil
+	}
+	var diagnostics jsonRpcDiagnostics
+	json.NewDecoder(strings.NewReader(string(p))).Decode(&diagnostics)
+
+	proc.diagnosticsChan <- diagnostics.Params
+	return 1, nil
+}
+
+func (proc readWriteCloseMock) Close() error {
+	return nil
+}
+
+func readTestFiles(dir string, channel chan<- lsp.DidOpenTextDocumentParams, doneChan chan<- int) {
+	libRegEx, e := regexp.Compile(".*(/|\\\\)templates(/|\\\\).*\\.ya?ml")
+	if e != nil {
+		log.Fatal(e)
+		return
+	}
+	if _, err := os.Stat(dir); os.IsNotExist(err) {
+		log.Fatal(err)
+		return
+	}
+
+	count := 0
+	filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error {
+		if d.Type().IsRegular() && libRegEx.MatchString(path) {
+			contentBytes, _ := os.ReadFile(path)
+			count++
+			channel <- lsp.DidOpenTextDocumentParams{
+				TextDocument: lsp.TextDocumentItem{
+					URI:        uri.File(path),
+					LanguageID: "",
+					Version:    0,
+					Text:       string(contentBytes),
+				},
+			}
+		}
+		return nil
+	})
+	doneChan <- count
+}
+
+func sendTestFilesToYamlls(documents *lsplocal.DocumentStore, yamllsConnector *Connector,
+	doneReadingFilesChan <-chan int,
+	doneSendingFilesChan chan<- int,
+	filesChan <-chan lsp.DidOpenTextDocumentParams,
+) {
+	ownCount := 0
+	for {
+		select {
+		case d := <-filesChan:
+			documents.DidOpen(d, util.DefaultConfig)
+			tree := lsplocal.ParseAst(nil, d.TextDocument.Text)
+			yamllsConnector.DocumentDidOpen(tree, d)
+			ownCount++
+		case count := <-doneReadingFilesChan:
+			if count != ownCount {
+				log.Fatal("Count mismatch: ", count, " != ", ownCount)
+			}
+			doneSendingFilesChan <- count
+			return
+		}
+	}
+}
+
+func TestYamllsDiagnosticsIntegration(t *testing.T) {
+	diagnosticsChan := make(chan lsp.PublishDiagnosticsParams)
+	doneReadingFilesChan := make(chan int)
+	doneSendingFilesChan := make(chan int)
+
+	dir := t.TempDir()
+	documents := lsplocal.NewDocumentStore()
+	con := jsonrpc2.NewConn(jsonrpc2.NewStream(readWriteCloseMock{diagnosticsChan}))
+	config := util.DefaultConfig.YamllsConfiguration
+
+	yamllsSettings := util.DefaultYamllsSettings
+	// disabling yamlls schema store improves performance and
+	// removes all schema diagnostics that are not caused by the yaml trimming
+	yamllsSettings.Schemas = make(map[string]string)
+	yamllsSettings.YamllsSchemaStoreSettings = util.YamllsSchemaStoreSettings{
+		Enable: false,
+	}
+	config.YamllsSettings = yamllsSettings
+	yamllsConnector := NewConnector(config, con, documents)
+
+	if yamllsConnector.Conn == nil {
+		t.Fatal("Could not connect to yaml-language-server")
+	}
+
+	yamllsConnector.CallInitialize(uri.File(dir))
+
+	didOpenChan := make(chan lsp.DidOpenTextDocumentParams)
+	go readTestFiles(TEST_DATA_DIR, didOpenChan, doneReadingFilesChan)
+	go sendTestFilesToYamlls(documents,
+		yamllsConnector, doneReadingFilesChan, doneSendingFilesChan, didOpenChan)
+
+	sentCount, diagnosticsCount := 0, 0
+	receivedDiagnostics := make(map[uri.URI]lsp.PublishDiagnosticsParams)
+
+	afterCh := time.After(600 * time.Second)
+	for {
+		if sentCount != 0 && len(receivedDiagnostics) == sentCount {
+			fmt.Println("All files checked")
+			break
+		}
+		select {
+		case d := <-diagnosticsChan:
+			receivedDiagnostics[d.URI] = d
+			if len(d.Diagnostics) > 0 {
+				diagnosticsCount++
+				fmt.Printf("Got diagnostic in %s diagnostics: %v \n", d.URI.Filename(), d.Diagnostics)
+			}
+		case <-afterCh:
+			t.Fatal("Timed out waiting for diagnostics")
+		case count := <-doneSendingFilesChan:
+			sentCount = count
+		}
+	}
+
+	fmt.Printf("Checked %d files, found %d diagnostics\n", sentCount, diagnosticsCount)
+	assert.LessOrEqual(t, diagnosticsCount, 23)
+	assert.Equal(t, 2368, sentCount, "Count of files in test data not equal to actual count")
+}

internal/adapter/yamlls/documentSync.go

@@ -24,11 +24,11 @@ func (yamllsConnector Connector) InitiallySyncOpenDocuments(docs []*lsplocal.Doc
 }
 
 func (yamllsConnector Connector) DocumentDidOpen(ast *sitter.Tree, params lsp.DidOpenTextDocumentParams) {
-	logger.Println("YamllsConnector DocumentDidOpen", params.TextDocument.URI)
+	logger.Debug("YamllsConnector DocumentDidOpen", params.TextDocument.URI)
 	if yamllsConnector.Conn == nil {
 		return
 	}
-	params.TextDocument.Text = trimTemplateForYamllsFromAst(ast, params.TextDocument.Text)
+	params.TextDocument.Text = lsplocal.TrimTemplate(ast, params.TextDocument.Text)
 
 	err := (*yamllsConnector.Conn).Notify(context.Background(), lsp.MethodTextDocumentDidOpen, params)
 	if err != nil {
@@ -40,24 +40,25 @@ func (yamllsConnector Connector) DocumentDidSave(doc *lsplocal.Document, params
 	if yamllsConnector.Conn == nil {
 		return
 	}
-	params.Text = trimTemplateForYamllsFromAst(doc.Ast, doc.Content)
+	params.Text = lsplocal.TrimTemplate(doc.Ast, doc.Content)
 
 	err := (*yamllsConnector.Conn).Notify(context.Background(), lsp.MethodTextDocumentDidSave, params)
 	if err != nil {
 		logger.Error("Error calling yamlls for didSave", err)
 	}
 
-	yamllsConnector.DocumentDidChangeFullSync(doc, lsp.DidChangeTextDocumentParams{TextDocument: lsp.VersionedTextDocumentIdentifier{
-		TextDocumentIdentifier: params.TextDocument,
-	},
+	yamllsConnector.DocumentDidChangeFullSync(doc, lsp.DidChangeTextDocumentParams{
+		TextDocument: lsp.VersionedTextDocumentIdentifier{
+			TextDocumentIdentifier: params.TextDocument,
+		},
 	})
 }
 
 func (yamllsConnector Connector) DocumentDidChange(doc *lsplocal.Document, params lsp.DidChangeTextDocumentParams) {
 	if yamllsConnector.Conn == nil {
 		return
 	}
-	var trimmedText = trimTemplateForYamllsFromAst(doc.Ast, doc.Content)
+	trimmedText := lsplocal.TrimTemplate(doc.Ast, doc.Content)
 
 	logger.Debug("Sending DocumentDidChange previous", params)
 	for i, change := range params.ContentChanges {
@@ -88,7 +89,7 @@ func (yamllsConnector Connector) DocumentDidChangeFullSync(doc *lsplocal.Documen
 	}
 
 	logger.Println("Sending DocumentDidChange with full sync, current content:", doc.Content)
-	var trimmedText = trimTemplateForYamllsFromAst(doc.Ast.Copy(), doc.Content)
+	trimmedText := lsplocal.TrimTemplate(doc.Ast.Copy(), doc.Content)
 
 	params.ContentChanges = []lsp.TextDocumentContentChangeEvent{
 		{