Rootmodule diagnostics

internal/filesystem/document.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 
 	"github.com/hashicorp/terraform-ls/internal/source"
+	"github.com/hashicorp/terraform-ls/internal/uri"
 	"github.com/spf13/afero"
 )
 
@@ -30,7 +31,7 @@ func (d *document) Filename() string {
 }
 
 func (d *document) URI() string {
-	return URIFromPath(d.meta.dh.FullPath())
+	return uri.FromPath(d.meta.dh.FullPath())
 }
 
 func (d *document) Lines() source.Lines {

internal/filesystem/filesystem_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform-ls/internal/uri"
 )
 
 func TestFilesystem_Change_notOpen(t *testing.T) {
@@ -454,7 +455,7 @@ func TempDir(t *testing.T) string {
 }
 
 func testHandlerFromPath(path string) DocumentHandler {
-	return &testHandler{uri: URIFromPath(path), fullPath: path}
+	return &testHandler{uri: uri.FromPath(path), fullPath: path}
 }
 
 type testHandler struct {

internal/lsp/diagnostics.go

@@ -17,3 +17,26 @@ func HCLSeverityToLSP(severity hcl.DiagnosticSeverity) lsp.DiagnosticSeverity {
 	}
 	return sev
 }
+
+func HCLDiagsToLSP(hclDiags hcl.Diagnostics, source string) []lsp.Diagnostic {
+	diags := []lsp.Diagnostic{}
+
+	for _, hclDiag := range hclDiags {
+		msg := hclDiag.Summary
+		if hclDiag.Detail != "" {
+			msg += ": " + hclDiag.Detail
+		}
+		var rnge lsp.Range
+		if hclDiag.Subject != nil {
+			rnge = HCLRangeToLSP(*hclDiag.Subject)
+		}
+		diags = append(diags, lsp.Diagnostic{
+			Range:    rnge,
+			Severity: HCLSeverityToLSP(hclDiag.Severity),
+			Source:   source,
+			Message:  msg,
+		})
+
+	}
+	return diags
+}

internal/lsp/diagnostics_test.go

@@ -0,0 +1,28 @@
+package lsp
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+)
+
+func TestHCLDiagsToLSP_NeverReturnsNil(t *testing.T) {
+	diags := HCLDiagsToLSP(nil, "test")
+	if diags == nil {
+		t.Fatal("diags should not be nil")
+	}
+
+	diags = HCLDiagsToLSP(hcl.Diagnostics{}, "test")
+	if diags == nil {
+		t.Fatal("diags should not be nil")
+	}
+
+	diags = HCLDiagsToLSP(hcl.Diagnostics{
+		{
+			Severity: hcl.DiagError,
+		},
+	}, "source")
+	if diags == nil {
+		t.Fatal("diags should not be nil")
+	}
+}

internal/lsp/file_handler.go

@@ -5,7 +5,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/hashicorp/terraform-ls/internal/filesystem"
+	"github.com/hashicorp/terraform-ls/internal/uri"
 	"github.com/sourcegraph/go-lsp"
 )
 
@@ -95,12 +95,12 @@ func (fh *versionedFileHandler) Version() int {
 }
 
 func FileHandlerFromPath(path string) *fileHandler {
-	return &fileHandler{uri: filesystem.URIFromPath(path)}
+	return &fileHandler{uri: uri.FromPath(path)}
 }
 
 func FileHandlerFromDirPath(dirPath string) *fileHandler {
 	// Dir URIs are usually without trailing separator in LSP
 	dirPath = filepath.Clean(dirPath)
 
-	return &fileHandler{uri: filesystem.URIFromPath(dirPath), isDir: true}
+	return &fileHandler{uri: uri.FromPath(dirPath), isDir: true}
 }

internal/lsp/range.go

@@ -6,7 +6,7 @@ import (
 )
 
 func HCLRangeToLSP(hclRng hcl.Range) lsp.Range {
-	return lsp.Range{
+	r := lsp.Range{
 		Start: lsp.Position{
 			Character: hclRng.Start.Column - 1,
 			Line:      hclRng.Start.Line - 1,
@@ -16,6 +16,8 @@ func HCLRangeToLSP(hclRng hcl.Range) lsp.Range {
 			Line:      hclRng.End.Line - 1,
 		},
 	}
+
+	return r
 }
 
 func lspRangeToHCL(lspRng lsp.Range, f File) (*hcl.Range, error) {

internal/terraform/rootmodule/root_module.go

@@ -72,7 +72,7 @@ type rootModule struct {
 	isParsed    bool
 	isParsedMu  *sync.RWMutex
 	pFilesMap   map[string]*hcl.File
-	parsedDiags hcl.Diagnostics
+	parsedDiags map[string]hcl.Diagnostics
 	parserMu    *sync.RWMutex
 	filesystem  filesystem.Filesystem
 }
@@ -430,7 +430,7 @@ func (rm *rootModule) ParseFiles() error {
 	defer rm.parserMu.Unlock()
 
 	files := make(map[string]*hcl.File, 0)
-	var diags hcl.Diagnostics
+	diags := make(map[string]hcl.Diagnostics, 0)
 
 	infos, err := rm.filesystem.ReadDir(rm.Path())
 	if err != nil {
@@ -459,7 +459,7 @@ func (rm *rootModule) ParseFiles() error {
 
 		rm.logger.Printf("parsing file %q", name)
 		f, pDiags := hclsyntax.ParseConfig(src, name, hcl.InitialPos)
-		diags = append(diags, pDiags...)
+		diags[name] = pDiags
 		if f != nil {
 			files[name] = f
 		}
@@ -472,7 +472,7 @@ func (rm *rootModule) ParseFiles() error {
 	return nil
 }
 
-func (rm *rootModule) ParsedDiagnostics() hcl.Diagnostics {
+func (rm *rootModule) ParsedDiagnostics() map[string]hcl.Diagnostics {
 	rm.parserMu.Lock()
 	defer rm.parserMu.Unlock()
 	return rm.parsedDiags

internal/terraform/rootmodule/types.go

@@ -76,7 +76,7 @@ type RootModule interface {
 	MergedSchema() (*schema.BodySchema, error)
 	IsParsed() bool
 	ParseFiles() error
-	ParsedDiagnostics() hcl.Diagnostics
+	ParsedDiagnostics() map[string]hcl.Diagnostics
 	IsCoreSchemaLoaded() bool
 	TerraformFormatter() (exec.Formatter, error)
 	HasTerraformDiscoveryFinished() bool

internal/filesystem/uri.go → internal/uri/uri.go

@@ -1,11 +1,11 @@
-package filesystem
+package uri
 
 import (
 	"net/url"
 	"path/filepath"
 )
 
-func URIFromPath(path string) string {
+func FromPath(path string) string {
 	p := filepath.ToSlash(path)
 	p = wrapPath(p)
 

internal/filesystem/uri_unix.go → internal/uri/uri_unix.go

@@ -1,6 +1,6 @@
 // +build !windows
 
-package filesystem
+package uri
 
 // wrapPath is no-op for unix-style paths
 func wrapPath(path string) string {

internal/filesystem/uri_unix_test.go → internal/uri/uri_unix_test.go

@@ -1,14 +1,14 @@
 // +build !windows
 
-package filesystem
+package uri
 
 import (
 	"testing"
 )
 
 func TestURIFromPath(t *testing.T) {
 	path := "/random/path"
-	uri := URIFromPath(path)
+	uri := FromPath(path)
 
 	expectedURI := "file:///random/path"
 	if uri != expectedURI {

internal/filesystem/uri_windows.go → internal/uri/uri_windows.go

@@ -1,4 +1,4 @@
-package filesystem
+package uri
 
 // wrapPath prepends Windows-style paths (C:\path)
 // with an additional slash to account for an empty hostname

internal/filesystem/uri_windows_test.go → internal/uri/uri_windows_test.go

@@ -1,12 +1,12 @@
-package filesystem
+package uri
 
 import (
 	"testing"
 )
 
-func TestURIFromPath(t *testing.T) {
+func TestFromPath(t *testing.T) {
 	path := `C:\Users\With Space\file.tf`
-	uri := URIFromPath(path)
+	uri := FromPath(path)
 
 	expectedURI := "file:///C:/Users/With%20Space/file.tf"
 	if uri != expectedURI {

langserver/diagnostics/diagnostics.go

@@ -3,81 +3,64 @@ package diagnostics
 import (
 	"context"
 	"log"
+	"path/filepath"
 	"sync"
 
 	"github.com/creachadair/jrpc2"
-	"github.com/hashicorp/hcl/v2/hclparse"
+	"github.com/hashicorp/hcl/v2"
 	ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
+	"github.com/hashicorp/terraform-ls/internal/uri"
 	"github.com/sourcegraph/go-lsp"
 )
 
-// documentContext encapsulates the data needed to diagnose the file and push diagnostics to the client
-type documentContext struct {
-	ctx  context.Context
-	uri  lsp.DocumentURI
-	text []byte
+type diagContext struct {
+	ctx    context.Context
+	uri    lsp.DocumentURI
+	diags  hcl.Diagnostics
+	source string
 }
 
-// Notifier is a type responsible for processing documents and pushing diagnostics to the client
+// Notifier is a type responsible for queueing hcl diagnostics to be converted
+// and sent to the client
 type Notifier struct {
-	sessCtx          context.Context
-	hclDocs          chan documentContext
-	closeHclDocsOnce sync.Once
+	sessCtx        context.Context
+	diags          chan diagContext
+	closeDiagsOnce sync.Once
 }
 
 func NewNotifier(sessCtx context.Context, logger *log.Logger) *Notifier {
-	hclDocs := make(chan documentContext, 10)
-	go hclDiags(hclDocs, logger)
-	return &Notifier{hclDocs: hclDocs, sessCtx: sessCtx}
+	diags := make(chan diagContext, 50)
+	go notify(diags, logger)
+	return &Notifier{sessCtx: sessCtx, diags: diags}
 }
 
-// DiagnoseHCL enqueues the document for HCL parsing. Documents will be parsed and notifications delivered in order that
-// they are enqueued. Files that are actively changing should be enqueued in order, so that diagnostics remain insync with
-// the current content of the file. This is the responsibility of the caller.
-func (n *Notifier) DiagnoseHCL(ctx context.Context, uri lsp.DocumentURI, text []byte) {
+// Publish accepts a map of diagnostics per file and queues them for publishing
+func (n *Notifier) Publish(ctx context.Context, rmDir string, diags map[string]hcl.Diagnostics, source string) {
 	select {
 	case <-n.sessCtx.Done():
-		n.closeHclDocsOnce.Do(func() {
-			close(n.hclDocs)
+		n.closeDiagsOnce.Do(func() {
+			close(n.diags)
 		})
 		return
 	default:
 	}
-	n.hclDocs <- documentContext{ctx: ctx, uri: uri, text: text}
-}
 
-func hclParse(doc documentContext) []lsp.Diagnostic {
-	diags := []lsp.Diagnostic{}
+	if source == "" {
+		source = "Terraform"
+	}
 
-	_, hclDiags := hclparse.NewParser().ParseHCL(doc.text, string(doc.uri))
-	for _, hclDiag := range hclDiags {
-		// only process diagnostics with an attributable spot in the code
-		if hclDiag.Subject != nil {
-			msg := hclDiag.Summary
-			if hclDiag.Detail != "" {
-				msg += ": " + hclDiag.Detail
-			}
-			diags = append(diags, lsp.Diagnostic{
-				Range:    ilsp.HCLRangeToLSP(*hclDiag.Subject),
-				Severity: ilsp.HCLSeverityToLSP(hclDiag.Severity),
-				Source:   "HCL",
-				Message:  msg,
-			})
-		}
+	for path, ds := range diags {
+		n.diags <- diagContext{ctx: ctx, diags: ds, source: source, uri: lsp.DocumentURI(uri.FromPath(filepath.Join(rmDir, path)))}
 	}
-	return diags
 }
 
-func hclDiags(docs <-chan documentContext, logger *log.Logger) {
-	for doc := range docs {
-		// always push diagnostics, even if the slice is empty, this is how previous diagnostics are cleared
-		// any push error will result in a panic since this is executing in its own thread and we can't bubble
-		// an error to a jrpc response
-		if err := jrpc2.PushNotify(doc.ctx, "textDocument/publishDiagnostics", lsp.PublishDiagnosticsParams{
-			URI:         doc.uri,
-			Diagnostics: hclParse(doc),
+func notify(diags <-chan diagContext, logger *log.Logger) {
+	for d := range diags {
+		if err := jrpc2.PushNotify(d.ctx, "textDocument/publishDiagnostics", lsp.PublishDiagnosticsParams{
+			URI:         d.uri,
+			Diagnostics: ilsp.HCLDiagsToLSP(d.diags, d.source),
 		}); err != nil {
-			logger.Printf("Error pushing hcl diagnostics: %s", err)
+			logger.Printf("Error pushing diagnostics: %s", err)
 		}
 	}
 }