From d5645a81b8dea9577c125f357983966c08946666 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Wed, 25 Nov 2020 17:30:30 -0500 Subject: [PATCH 01/11] Implement module validation on save --- go.mod | 6 +- go.sum | 12 +++- .../langserver/diagnostics/diagnostics.go | 55 ++++++++++++----- .../diagnostics/diagnostics_test.go | 10 +-- internal/langserver/handlers/did_change.go | 3 +- internal/langserver/handlers/did_open.go | 3 +- internal/langserver/handlers/did_save.go | 53 ++++++++++++++++ internal/langserver/handlers/service.go | 10 +++ internal/terraform/exec/exec.go | 13 ++++ internal/terraform/exec/mock/executor.go | 23 +++++++ internal/terraform/exec/types.go | 1 + internal/terraform/rootmodule/root_module.go | 61 +++++++++++++++++++ internal/terraform/rootmodule/types.go | 2 + 13 files changed, 225 insertions(+), 27 deletions(-) create mode 100644 internal/langserver/handlers/did_save.go diff --git a/go.mod b/go.mod index 5e8bcdac0..745fe16a1 100644 --- a/go.mod +++ b/go.mod @@ -7,13 +7,13 @@ require ( github.com/creachadair/jrpc2 v0.11.0 github.com/fsnotify/fsnotify v1.4.9 github.com/gammazero/workerpool v1.0.0 - github.com/google/go-cmp v0.5.1 + github.com/google/go-cmp v0.5.2 github.com/google/uuid v1.1.2 github.com/hashicorp/go-multierror v1.1.0 github.com/hashicorp/go-version v1.2.1 github.com/hashicorp/hcl-lang v0.0.0-20201116081236-948e43712a65 github.com/hashicorp/hcl/v2 v2.6.0 - github.com/hashicorp/terraform-exec v0.11.1-0.20201007122305-ea2094d52cb5 + github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8 github.com/hashicorp/terraform-json v0.6.0 github.com/hashicorp/terraform-schema v0.0.0-20201204171308-0c9744a02c65 github.com/mh-cbon/go-fmt-fail v0.0.0-20160815164508-67765b3fbcb5 @@ -24,6 +24,6 @@ require ( github.com/shurcooL/httpfs v0.0.0-20190707220628-8d4bc4ba7749 // indirect github.com/shurcooL/vfsgen v0.0.0-20200824052919-0d455de96546 github.com/spf13/afero v1.3.2 - github.com/stretchr/testify v1.4.0 + github.com/stretchr/testify v1.6.1 github.com/vektra/mockery/v2 v2.3.0 ) diff --git a/go.sum b/go.sum index ae62f4c17..1c3d52c2e 100644 --- a/go.sum +++ b/go.sum @@ -128,6 +128,8 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k= github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= @@ -196,8 +198,10 @@ github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2p github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= github.com/hashicorp/terraform-config-inspect v0.0.0-20201102131242-0c45ba392e51 h1:SEGO1vz/pFLfKy4QpABIMCe7wffmtsOiWO4yc1E87cU= github.com/hashicorp/terraform-config-inspect v0.0.0-20201102131242-0c45ba392e51/go.mod h1:Z0Nnk4+3Cy89smEbrq+sl1bxc9198gIP4I7wcQF6Kqs= -github.com/hashicorp/terraform-exec v0.11.1-0.20201007122305-ea2094d52cb5 h1:P+lBGicJEG3ijvOrDdQf/Oo8UrG4QAJbdY3g9OGBnr0= -github.com/hashicorp/terraform-exec v0.11.1-0.20201007122305-ea2094d52cb5/go.mod h1:eQdBvA0Xr/ZJNilY8TzrtePLSqLyexk9PSwVwzzHTjY= +github.com/hashicorp/terraform-exec v0.11.1-0.20201126033416-f9d65bf2bca2 h1:WNNF6oK2BXG0nGalTXDjP2XwdWxARRtGuN2Ih1ECIn8= +github.com/hashicorp/terraform-exec v0.11.1-0.20201126033416-f9d65bf2bca2/go.mod h1:162lr1MQN7XkrqevlStXPonCGMP5yvyG9zWnlmGdQLI= +github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8 h1:sJXjDnxrrpmmJcAiPqX1HL7uksBgVXjY+3VX6k09zkA= +github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8/go.mod h1:162lr1MQN7XkrqevlStXPonCGMP5yvyG9zWnlmGdQLI= github.com/hashicorp/terraform-json v0.5.0 h1:7TV3/F3y7QVSuN4r9BEXqnWqrAyeOtON8f0wvREtyzs= github.com/hashicorp/terraform-json v0.5.0/go.mod h1:eAbqb4w0pSlRmdvl8fOyHAi/+8jnkVYN28gJkSJrLhU= github.com/hashicorp/terraform-json v0.6.0 h1:nMTj4t9ysC7xJ72rvVsDqhUccvbUINrjhPqafeUeREk= @@ -351,6 +355,8 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= @@ -551,6 +557,8 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/internal/langserver/diagnostics/diagnostics.go b/internal/langserver/diagnostics/diagnostics.go index 55bfdd37f..09e548b7c 100644 --- a/internal/langserver/diagnostics/diagnostics.go +++ b/internal/langserver/diagnostics/diagnostics.go @@ -16,26 +16,28 @@ import ( type diagContext struct { ctx context.Context uri lsp.DocumentURI - diags hcl.Diagnostics source string + diags []lsp.Diagnostic } // Notifier is a type responsible for queueing hcl diagnostics to be converted // and sent to the client type Notifier struct { + logger *log.Logger sessCtx context.Context diags chan diagContext + diagsCache map[string]map[string][]lsp.Diagnostic closeDiagsOnce sync.Once } func NewNotifier(sessCtx context.Context, logger *log.Logger) *Notifier { - diags := make(chan diagContext, 50) - go notify(diags, logger) - return &Notifier{sessCtx: sessCtx, diags: diags} + n := &Notifier{logger: logger, sessCtx: sessCtx, diags: make(chan diagContext, 50), diagsCache: make(map[string]map[string][]lsp.Diagnostic)} + go n.notify() + return n } // 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) { +func (n *Notifier) Publish(ctx context.Context, dir string, diags map[string][]lsp.Diagnostic, source string) { select { case <-n.sessCtx.Done(): n.closeDiagsOnce.Do(func() { @@ -45,22 +47,45 @@ func (n *Notifier) Publish(ctx context.Context, rmDir string, diags map[string]h default: } - if source == "" { - source = "Terraform" - } - - for path, ds := range diags { - n.diags <- diagContext{ctx: ctx, diags: ds, source: source, uri: lsp.DocumentURI(uri.FromPath(filepath.Join(rmDir, path)))} + for filename, ds := range diags { + n.diags <- diagContext{ctx: ctx, source: source, diags: ds, uri: lsp.DocumentURI(uri.FromPath(filepath.Join(dir, filename)))} } } -func notify(diags <-chan diagContext, logger *log.Logger) { - for d := range diags { +func (n *Notifier) notify() { + for d := range n.diags { if err := jrpc2.PushNotify(d.ctx, "textDocument/publishDiagnostics", lsp.PublishDiagnosticsParams{ URI: d.uri, - Diagnostics: ilsp.HCLDiagsToLSP(d.diags, d.source), + Diagnostics: n.mergeDiags(string(d.uri), d.source, d.diags), }); err != nil { - logger.Printf("Error pushing diagnostics: %s", err) + n.logger.Printf("Error pushing diagnostics: %s", err) } } } + +// mergeDiags will return all diags from all cached sources for a given uri. +// the passed diags overwrites the cached entry for the passed source key +// even if empty +func (n *Notifier) mergeDiags(uri string, source string, diags []lsp.Diagnostic) []lsp.Diagnostic { + fileDiags, ok := n.diagsCache[uri] + if !ok { + fileDiags = make(map[string][]lsp.Diagnostic) + } + + fileDiags[source] = diags + n.diagsCache[uri] = fileDiags + + all := []lsp.Diagnostic{} + for _, diags := range fileDiags { + all = append(all, diags...) + } + return all +} + +func FromHCLMap(hclDiags map[string]hcl.Diagnostics) map[string][]lsp.Diagnostic { + diags := make(map[string][]lsp.Diagnostic, len(hclDiags)) + for file, ds := range hclDiags { + diags[file] = ilsp.HCLDiagsToLSP(ds, "HCL") + } + return diags +} diff --git a/internal/langserver/diagnostics/diagnostics_test.go b/internal/langserver/diagnostics/diagnostics_test.go index d45a31880..9f11a947d 100644 --- a/internal/langserver/diagnostics/diagnostics_test.go +++ b/internal/langserver/diagnostics/diagnostics_test.go @@ -6,7 +6,7 @@ import ( "log" "testing" - "github.com/hashicorp/hcl/v2" + lsp "github.com/hashicorp/terraform-ls/internal/protocol" ) var discardLogger = log.New(ioutil.Discard, "", 0) @@ -16,10 +16,10 @@ func TestDiags_Closes(t *testing.T) { n := NewNotifier(ctx, discardLogger) cancel() - n.Publish(context.Background(), "", map[string]hcl.Diagnostics{ + n.Publish(context.Background(), "", map[string][]lsp.Diagnostic{ "test": { { - Severity: hcl.DiagError, + Severity: lsp.SeverityError, }, }, }, "test") @@ -40,10 +40,10 @@ func TestPublish_DoesNotSendAfterClose(t *testing.T) { n := NewNotifier(ctx, discardLogger) cancel() - n.Publish(context.Background(), "", map[string]hcl.Diagnostics{ + n.Publish(context.Background(), "", map[string][]lsp.Diagnostic{ "test": { { - Severity: hcl.DiagError, + Severity: lsp.SeverityError, }, }, }, "test") diff --git a/internal/langserver/handlers/did_change.go b/internal/langserver/handlers/did_change.go index 85d510d56..e88bbe52a 100644 --- a/internal/langserver/handlers/did_change.go +++ b/internal/langserver/handlers/did_change.go @@ -5,6 +5,7 @@ import ( "fmt" lsctx "github.com/hashicorp/terraform-ls/internal/context" + "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" lsp "github.com/hashicorp/terraform-ls/internal/protocol" ) @@ -67,7 +68,7 @@ func TextDocumentDidChange(ctx context.Context, params lsp.DidChangeTextDocument if err != nil { return err } - diags.Publish(ctx, rm.Path(), rm.ParsedDiagnostics(), "HCL") + diags.Publish(ctx, rm.Path(), diagnostics.FromHCLMap(rm.ParsedDiagnostics()), "hcl") return nil } diff --git a/internal/langserver/handlers/did_open.go b/internal/langserver/handlers/did_open.go index b37154ba6..8ec897c73 100644 --- a/internal/langserver/handlers/did_open.go +++ b/internal/langserver/handlers/did_open.go @@ -14,6 +14,7 @@ import ( ilsp "github.com/hashicorp/terraform-ls/internal/lsp" lsp "github.com/hashicorp/terraform-ls/internal/protocol" "github.com/hashicorp/terraform-ls/internal/terraform/rootmodule" + "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" ) func (lh *logHandler) TextDocumentDidOpen(ctx context.Context, params lsp.DidOpenTextDocumentParams) error { @@ -70,7 +71,7 @@ func (lh *logHandler) TextDocumentDidOpen(ctx context.Context, params lsp.DidOpe if err != nil { return err } - diags.Publish(ctx, rm.Path(), rm.ParsedDiagnostics(), "HCL") + diags.Publish(ctx, rm.Path(), diagnostics.FromHCLMap(rm.ParsedDiagnostics()), "hcl") candidates := rmm.RootModuleCandidatesByPath(f.Dir()) diff --git a/internal/langserver/handlers/did_save.go b/internal/langserver/handlers/did_save.go new file mode 100644 index 000000000..978c725c4 --- /dev/null +++ b/internal/langserver/handlers/did_save.go @@ -0,0 +1,53 @@ +package handlers + +import ( + "context" + + lsctx "github.com/hashicorp/terraform-ls/internal/context" + "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" + ilsp "github.com/hashicorp/terraform-ls/internal/lsp" + lsp "github.com/hashicorp/terraform-ls/internal/protocol" +) + +func (h *logHandler) TextDocumentDidSave(ctx context.Context, params lsp.DidSaveTextDocumentParams) error { + fs, err := lsctx.DocumentStorage(ctx) + if err != nil { + return err + } + + rmf, err := lsctx.RootModuleFinder(ctx) + if err != nil { + return err + } + + file, err := fs.GetDocument(ilsp.FileHandlerFromDocumentURI(params.TextDocument.URI)) + if err != nil { + return err + } + + rm, err := rmf.RootModuleByPath(file.Dir()) + if err != nil { + return err + } + + wasInit, err := rm.WasInitialized() + if err != nil { + h.logger.Printf("error checking if rootmodule was initialized: %s", err) + } + if !wasInit { + return nil + } + + diags, err := lsctx.Diagnostics(ctx) + if err != nil { + return err + } + + hclDiags, err := rm.ExecuteTerraformValidate(ctx) + if err != nil { + return err + } + diags.Publish(ctx, rm.Path(), diagnostics.FromHCLMap(hclDiags), "validate") + + return nil +} diff --git a/internal/langserver/handlers/service.go b/internal/langserver/handlers/service.go index 4f043dbce..ef735190e 100644 --- a/internal/langserver/handlers/service.go +++ b/internal/langserver/handlers/service.go @@ -214,6 +214,16 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { ctx = lsctx.WithDocumentStorage(ctx, svc.fs) return handle(ctx, req, TextDocumentDidClose) }, + "textDocument/didSave": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { + err := session.CheckInitializationIsConfirmed() + if err != nil { + return nil, err + } + ctx = lsctx.WithDiagnostics(ctx, diags) + ctx = lsctx.WithDocumentStorage(ctx, svc.fs) + ctx = lsctx.WithRootModuleFinder(ctx, svc.modMgr) + return handle(ctx, req, lh.TextDocumentDidSave) + }, "textDocument/documentSymbol": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { err := session.CheckInitializationIsConfirmed() if err != nil { diff --git a/internal/terraform/exec/exec.go b/internal/terraform/exec/exec.go index 8a78a187e..bda7c8071 100644 --- a/internal/terraform/exec/exec.go +++ b/internal/terraform/exec/exec.go @@ -114,6 +114,19 @@ func (e *Executor) Format(ctx context.Context, input []byte) ([]byte, error) { return buf.Bytes(), e.contextfulError(ctx, "Format", err) } +func (e *Executor) Validate(ctx context.Context) ([]tfexec.Diagnostic, error) { + ctx, cancel := e.withTimeout(ctx) + defer cancel() + err := e.setLogPath("Validate") + if err != nil { + return []tfexec.Diagnostic{}, err + } + + validation, err := e.tf.Validate(ctx) + + return validation.Diagnostics, e.contextfulError(ctx, "Validate", err) +} + func (e *Executor) Version(ctx context.Context) (*version.Version, map[string]*version.Version, error) { ctx, cancel := e.withTimeout(ctx) defer cancel() diff --git a/internal/terraform/exec/mock/executor.go b/internal/terraform/exec/mock/executor.go index c8183cf0b..b844c5e7b 100644 --- a/internal/terraform/exec/mock/executor.go +++ b/internal/terraform/exec/mock/executor.go @@ -128,6 +128,29 @@ func (_m *Executor) SetTimeout(duration time.Duration) { _m.Called(duration) } +// Validate provides a mock function with given fields: ctx +func (_m *Executor) Validate(ctx context.Context) ([]tfexec.Diagnostic, error) { + ret := _m.Called(ctx) + + var r0 []tfexec.Diagnostic + if rf, ok := ret.Get(0).(func(context.Context) []tfexec.Diagnostic); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]tfexec.Diagnostic) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Version provides a mock function with given fields: ctx func (_m *Executor) Version(ctx context.Context) (*version.Version, map[string]*version.Version, error) { ret := _m.Called(ctx) diff --git a/internal/terraform/exec/types.go b/internal/terraform/exec/types.go index 61cd787f3..ef1409cc6 100644 --- a/internal/terraform/exec/types.go +++ b/internal/terraform/exec/types.go @@ -26,5 +26,6 @@ type TerraformExecutor interface { Init(ctx context.Context, opts ...tfexec.InitOption) error Format(ctx context.Context, input []byte) ([]byte, error) Version(ctx context.Context) (*version.Version, map[string]*version.Version, error) + Validate(ctx context.Context) ([]tfexec.Diagnostic, error) ProviderSchemas(ctx context.Context) (*tfjson.ProviderSchemas, error) } diff --git a/internal/terraform/rootmodule/root_module.go b/internal/terraform/rootmodule/root_module.go index 0d4ed509d..4b84cc9b0 100644 --- a/internal/terraform/rootmodule/root_module.go +++ b/internal/terraform/rootmodule/root_module.go @@ -378,6 +378,67 @@ func (rm *rootModule) ExecuteTerraformInit(ctx context.Context) error { return rm.tfExec.Init(ctx) } +func (rm *rootModule) ExecuteTerraformValidate(ctx context.Context) (map[string]hcl.Diagnostics, error) { + diagsMap := make(map[string]hcl.Diagnostics) + + if !rm.IsTerraformAvailable() { + if err := rm.discoverTerraformExecutor(ctx); err != nil { + return diagsMap, err + } + } + + if !rm.IsParsed() { + if err := rm.ParseFiles(); err != nil { + return diagsMap, err + } + } + + // an entry for each file should exist, even if there are no diags + for filename := range rm.pFilesMap { + diagsMap[filename] = make(hcl.Diagnostics, 0) + } + + validationDiags, err := rm.tfExec.Validate(ctx) + if err != nil { + return diagsMap, err + } + + // tfexec.Diagnostic is a conversion of an internal diag to terraform core, + // tfdiags, which is effectively based on hcl.Diagnostic. + // This process is really just converting it back to hcl.Diagnotic + // since it is the defacto diagnostic type for our codebase currently + // https://github.com/hashicorp/terraform/blob/ae025248cc0712bf53c675dc2fe77af4276dd5cc/command/validate.go#L138 + for _, d := range validationDiags { + // the diagnostic must be tied to a file to exist in the map + if d.Range == nil || d.Range.Filename == "" { + continue + } + + diags := diagsMap[d.Range.Filename] + + var severity hcl.DiagnosticSeverity + if d.Severity == "error" { + severity = hcl.DiagError + } else if d.Severity == "warning" { + severity = hcl.DiagWarning + } + + diags = append(diags, &hcl.Diagnostic{ + Severity: severity, + Summary: d.Summary, + Detail: d.Detail, + Subject: &hcl.Range{ + Filename: d.Range.Filename, + Start: hcl.Pos(d.Range.Start), + End: hcl.Pos(d.Range.End), + }, + }) + diagsMap[d.Range.Filename] = diags + } + + return diagsMap, nil +} + func (rm *rootModule) discoverTerraformVersion(ctx context.Context) error { if rm.tfExec == nil { return errors.New("no terraform executor - unable to read version") diff --git a/internal/terraform/rootmodule/types.go b/internal/terraform/rootmodule/types.go index 277152f40..274d62773 100644 --- a/internal/terraform/rootmodule/types.go +++ b/internal/terraform/rootmodule/types.go @@ -82,8 +82,10 @@ type RootModule interface { HasTerraformDiscoveryFinished() bool IsTerraformAvailable() bool ExecuteTerraformInit(ctx context.Context) error + ExecuteTerraformValidate(ctx context.Context) (map[string]hcl.Diagnostics, error) Modules() []ModuleRecord HumanReadablePath(string) string + WasInitialized() (bool, error) } type RootModuleFactory func(context.Context, string) (*rootModule, error) From 47e01ff00ffd5e7f292f311f335787462a8fdee9 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Sun, 6 Dec 2020 14:46:57 -0500 Subject: [PATCH 02/11] go mod tidy --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index 1c3d52c2e..1ca8deb88 100644 --- a/go.sum +++ b/go.sum @@ -198,8 +198,6 @@ github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2p github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= github.com/hashicorp/terraform-config-inspect v0.0.0-20201102131242-0c45ba392e51 h1:SEGO1vz/pFLfKy4QpABIMCe7wffmtsOiWO4yc1E87cU= github.com/hashicorp/terraform-config-inspect v0.0.0-20201102131242-0c45ba392e51/go.mod h1:Z0Nnk4+3Cy89smEbrq+sl1bxc9198gIP4I7wcQF6Kqs= -github.com/hashicorp/terraform-exec v0.11.1-0.20201126033416-f9d65bf2bca2 h1:WNNF6oK2BXG0nGalTXDjP2XwdWxARRtGuN2Ih1ECIn8= -github.com/hashicorp/terraform-exec v0.11.1-0.20201126033416-f9d65bf2bca2/go.mod h1:162lr1MQN7XkrqevlStXPonCGMP5yvyG9zWnlmGdQLI= github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8 h1:sJXjDnxrrpmmJcAiPqX1HL7uksBgVXjY+3VX6k09zkA= github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8/go.mod h1:162lr1MQN7XkrqevlStXPonCGMP5yvyG9zWnlmGdQLI= github.com/hashicorp/terraform-json v0.5.0 h1:7TV3/F3y7QVSuN4r9BEXqnWqrAyeOtON8f0wvREtyzs= From fcd6da6645b8c9bfe33bda4df7e5b881842604d9 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Sun, 6 Dec 2020 14:54:18 -0500 Subject: [PATCH 03/11] gofmt --- internal/langserver/handlers/did_open.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/langserver/handlers/did_open.go b/internal/langserver/handlers/did_open.go index 8ec897c73..e5e216cad 100644 --- a/internal/langserver/handlers/did_open.go +++ b/internal/langserver/handlers/did_open.go @@ -10,11 +10,11 @@ import ( "github.com/google/uuid" lsctx "github.com/hashicorp/terraform-ls/internal/context" "github.com/hashicorp/terraform-ls/internal/langserver/cmd" + "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" "github.com/hashicorp/terraform-ls/internal/langserver/handlers/command" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" lsp "github.com/hashicorp/terraform-ls/internal/protocol" "github.com/hashicorp/terraform-ls/internal/terraform/rootmodule" - "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" ) func (lh *logHandler) TextDocumentDidOpen(ctx context.Context, params lsp.DidOpenTextDocumentParams) error { From 31c50f96cf1d1a9046c4e96454720be5f9e4fe36 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Sun, 6 Dec 2020 17:17:34 -0500 Subject: [PATCH 04/11] Adapt validation to command --- .../langserver/diagnostics/diagnostics.go | 16 ++---- .../diagnostics/diagnostics_test.go | 10 ++-- .../langserver/handlers/command/validate.go | 49 +++++++++++++++++ internal/langserver/handlers/did_change.go | 3 +- internal/langserver/handlers/did_open.go | 3 +- internal/langserver/handlers/did_save.go | 53 ------------------ .../langserver/handlers/execute_command.go | 5 +- .../handlers/execute_command_validate_test.go | 54 +++++++++++++++++++ internal/langserver/handlers/service.go | 10 ---- 9 files changed, 118 insertions(+), 85 deletions(-) create mode 100644 internal/langserver/handlers/command/validate.go delete mode 100644 internal/langserver/handlers/did_save.go create mode 100644 internal/langserver/handlers/execute_command_validate_test.go diff --git a/internal/langserver/diagnostics/diagnostics.go b/internal/langserver/diagnostics/diagnostics.go index 09e548b7c..fe5585485 100644 --- a/internal/langserver/diagnostics/diagnostics.go +++ b/internal/langserver/diagnostics/diagnostics.go @@ -36,8 +36,10 @@ func NewNotifier(sessCtx context.Context, logger *log.Logger) *Notifier { return n } -// Publish accepts a map of diagnostics per file and queues them for publishing -func (n *Notifier) Publish(ctx context.Context, dir string, diags map[string][]lsp.Diagnostic, source string) { +// PublishHCLDiags accepts a map of hcl diagnostics per file and queues them for publishing. +// A dir path is passed which is joined with the filename keys of the map, to form a file URI. +// A source string is passed and set for each diagnostic, this is typically displayed in the client UI. +func (n *Notifier) PublishHCLDiags(ctx context.Context, dir string, diags map[string]hcl.Diagnostics, source string) { select { case <-n.sessCtx.Done(): n.closeDiagsOnce.Do(func() { @@ -48,7 +50,7 @@ func (n *Notifier) Publish(ctx context.Context, dir string, diags map[string][]l } for filename, ds := range diags { - n.diags <- diagContext{ctx: ctx, source: source, diags: ds, uri: lsp.DocumentURI(uri.FromPath(filepath.Join(dir, filename)))} + n.diags <- diagContext{ctx: ctx, source: source, diags: ilsp.HCLDiagsToLSP(ds, source), uri: lsp.DocumentURI(uri.FromPath(filepath.Join(dir, filename)))} } } @@ -81,11 +83,3 @@ func (n *Notifier) mergeDiags(uri string, source string, diags []lsp.Diagnostic) } return all } - -func FromHCLMap(hclDiags map[string]hcl.Diagnostics) map[string][]lsp.Diagnostic { - diags := make(map[string][]lsp.Diagnostic, len(hclDiags)) - for file, ds := range hclDiags { - diags[file] = ilsp.HCLDiagsToLSP(ds, "HCL") - } - return diags -} diff --git a/internal/langserver/diagnostics/diagnostics_test.go b/internal/langserver/diagnostics/diagnostics_test.go index 9f11a947d..ca79aea5e 100644 --- a/internal/langserver/diagnostics/diagnostics_test.go +++ b/internal/langserver/diagnostics/diagnostics_test.go @@ -6,7 +6,7 @@ import ( "log" "testing" - lsp "github.com/hashicorp/terraform-ls/internal/protocol" + "github.com/hashicorp/hcl/v2" ) var discardLogger = log.New(ioutil.Discard, "", 0) @@ -16,10 +16,10 @@ func TestDiags_Closes(t *testing.T) { n := NewNotifier(ctx, discardLogger) cancel() - n.Publish(context.Background(), "", map[string][]lsp.Diagnostic{ + n.PublishHCLDiags(context.Background(), "", map[string]hcl.Diagnostics{ "test": { { - Severity: lsp.SeverityError, + Severity: hcl.DiagError, }, }, }, "test") @@ -40,10 +40,10 @@ func TestPublish_DoesNotSendAfterClose(t *testing.T) { n := NewNotifier(ctx, discardLogger) cancel() - n.Publish(context.Background(), "", map[string][]lsp.Diagnostic{ + n.PublishHCLDiags(context.Background(), "", map[string]hcl.Diagnostics{ "test": { { - Severity: lsp.SeverityError, + Severity: hcl.DiagError, }, }, }, "test") diff --git a/internal/langserver/handlers/command/validate.go b/internal/langserver/handlers/command/validate.go new file mode 100644 index 000000000..199df1abb --- /dev/null +++ b/internal/langserver/handlers/command/validate.go @@ -0,0 +1,49 @@ +package command + +import ( + "context" + "fmt" + + "github.com/creachadair/jrpc2/code" + lsctx "github.com/hashicorp/terraform-ls/internal/context" + "github.com/hashicorp/terraform-ls/internal/langserver/cmd" + ilsp "github.com/hashicorp/terraform-ls/internal/lsp" + lsp "github.com/hashicorp/terraform-ls/internal/protocol" +) + +func TerraformValidateHandler(ctx context.Context, args cmd.CommandArgs) (interface{}, error) { + dirUri, ok := args.GetString("uri") + if !ok || dirUri == "" { + return nil, fmt.Errorf("%w: expected dir uri argument to be set", code.InvalidParams.Err()) + } + + fh := ilsp.FileHandlerFromDirURI(lsp.DocumentURI(dirUri)) + + cf, err := lsctx.RootModuleFinder(ctx) + if err != nil { + return nil, err + } + + rm, err := cf.RootModuleByPath(fh.Dir()) + if err != nil { + return nil, err + } + + wasInit, _ := rm.WasInitialized() + if !wasInit { + return nil, fmt.Errorf("%s is not an initialized module, terraform validate cannot be called", dirUri) + } + + diags, err := lsctx.Diagnostics(ctx) + if err != nil { + return nil, err + } + + hclDiags, err := rm.ExecuteTerraformValidate(ctx) + if err != nil { + return nil, err + } + diags.PublishHCLDiags(ctx, rm.Path(), hclDiags, "terraform validate") + + return nil, nil +} diff --git a/internal/langserver/handlers/did_change.go b/internal/langserver/handlers/did_change.go index e88bbe52a..ec0dbe096 100644 --- a/internal/langserver/handlers/did_change.go +++ b/internal/langserver/handlers/did_change.go @@ -5,7 +5,6 @@ import ( "fmt" lsctx "github.com/hashicorp/terraform-ls/internal/context" - "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" lsp "github.com/hashicorp/terraform-ls/internal/protocol" ) @@ -68,7 +67,7 @@ func TextDocumentDidChange(ctx context.Context, params lsp.DidChangeTextDocument if err != nil { return err } - diags.Publish(ctx, rm.Path(), diagnostics.FromHCLMap(rm.ParsedDiagnostics()), "hcl") + diags.PublishHCLDiags(ctx, rm.Path(), rm.ParsedDiagnostics(), "HCL") return nil } diff --git a/internal/langserver/handlers/did_open.go b/internal/langserver/handlers/did_open.go index e5e216cad..b712c2ac6 100644 --- a/internal/langserver/handlers/did_open.go +++ b/internal/langserver/handlers/did_open.go @@ -10,7 +10,6 @@ import ( "github.com/google/uuid" lsctx "github.com/hashicorp/terraform-ls/internal/context" "github.com/hashicorp/terraform-ls/internal/langserver/cmd" - "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" "github.com/hashicorp/terraform-ls/internal/langserver/handlers/command" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" lsp "github.com/hashicorp/terraform-ls/internal/protocol" @@ -71,7 +70,7 @@ func (lh *logHandler) TextDocumentDidOpen(ctx context.Context, params lsp.DidOpe if err != nil { return err } - diags.Publish(ctx, rm.Path(), diagnostics.FromHCLMap(rm.ParsedDiagnostics()), "hcl") + diags.PublishHCLDiags(ctx, rm.Path(), rm.ParsedDiagnostics(), "HCL") candidates := rmm.RootModuleCandidatesByPath(f.Dir()) diff --git a/internal/langserver/handlers/did_save.go b/internal/langserver/handlers/did_save.go deleted file mode 100644 index 978c725c4..000000000 --- a/internal/langserver/handlers/did_save.go +++ /dev/null @@ -1,53 +0,0 @@ -package handlers - -import ( - "context" - - lsctx "github.com/hashicorp/terraform-ls/internal/context" - "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" - ilsp "github.com/hashicorp/terraform-ls/internal/lsp" - lsp "github.com/hashicorp/terraform-ls/internal/protocol" -) - -func (h *logHandler) TextDocumentDidSave(ctx context.Context, params lsp.DidSaveTextDocumentParams) error { - fs, err := lsctx.DocumentStorage(ctx) - if err != nil { - return err - } - - rmf, err := lsctx.RootModuleFinder(ctx) - if err != nil { - return err - } - - file, err := fs.GetDocument(ilsp.FileHandlerFromDocumentURI(params.TextDocument.URI)) - if err != nil { - return err - } - - rm, err := rmf.RootModuleByPath(file.Dir()) - if err != nil { - return err - } - - wasInit, err := rm.WasInitialized() - if err != nil { - h.logger.Printf("error checking if rootmodule was initialized: %s", err) - } - if !wasInit { - return nil - } - - diags, err := lsctx.Diagnostics(ctx) - if err != nil { - return err - } - - hclDiags, err := rm.ExecuteTerraformValidate(ctx) - if err != nil { - return err - } - diags.Publish(ctx, rm.Path(), diagnostics.FromHCLMap(hclDiags), "validate") - - return nil -} diff --git a/internal/langserver/handlers/execute_command.go b/internal/langserver/handlers/execute_command.go index a8d9079db..035fa9823 100644 --- a/internal/langserver/handlers/execute_command.go +++ b/internal/langserver/handlers/execute_command.go @@ -12,8 +12,9 @@ import ( ) var handlers = cmd.Handlers{ - cmd.Name("rootmodules"): command.RootModulesHandler, - cmd.Name("terraform.init"): command.TerraformInitHandler, + cmd.Name("rootmodules"): command.RootModulesHandler, + cmd.Name("terraform.init"): command.TerraformInitHandler, + cmd.Name("terraform.validate"): command.TerraformValidateHandler, } func (lh *logHandler) WorkspaceExecuteCommand(ctx context.Context, params lsp.ExecuteCommandParams) (interface{}, error) { diff --git a/internal/langserver/handlers/execute_command_validate_test.go b/internal/langserver/handlers/execute_command_validate_test.go new file mode 100644 index 000000000..ca12ce750 --- /dev/null +++ b/internal/langserver/handlers/execute_command_validate_test.go @@ -0,0 +1,54 @@ +package handlers + +import ( + "fmt" + "testing" + + "github.com/creachadair/jrpc2/code" + "github.com/hashicorp/terraform-ls/internal/langserver" + "github.com/hashicorp/terraform-ls/internal/langserver/cmd" + "github.com/hashicorp/terraform-ls/internal/terraform/rootmodule" +) + +func TestLangServer_workspaceExecuteCommand_validate_argumentError(t *testing.T) { + tmpDir := TempDir(t) + testFileURI := fmt.Sprintf("%s/main.tf", tmpDir.URI()) + + ls := langserver.NewLangServerMock(t, NewMockSession(&MockSessionInput{ + RootModules: map[string]*rootmodule.RootModuleMock{ + tmpDir.Dir(): { + TfExecFactory: validTfMockCalls(), + }, + }, + })) + stop := ls.Start(t) + defer stop() + + ls.Call(t, &langserver.CallRequest{ + Method: "initialize", + ReqParams: fmt.Sprintf(`{ + "capabilities": {}, + "rootUri": %q, + "processId": 12345 + }`, tmpDir.URI())}) + ls.Notify(t, &langserver.CallRequest{ + Method: "initialized", + ReqParams: "{}", + }) + ls.Call(t, &langserver.CallRequest{ + Method: "textDocument/didOpen", + ReqParams: fmt.Sprintf(`{ + "textDocument": { + "version": 0, + "languageId": "terraform", + "text": "provider \"github\" {}", + "uri": %q + } + }`, testFileURI)}) + + ls.CallAndExpectError(t, &langserver.CallRequest{ + Method: "workspace/executeCommand", + ReqParams: fmt.Sprintf(`{ + "command": %q + }`, cmd.Name("terraform.validate"))}, code.InvalidParams.Err()) +} diff --git a/internal/langserver/handlers/service.go b/internal/langserver/handlers/service.go index ef735190e..4f043dbce 100644 --- a/internal/langserver/handlers/service.go +++ b/internal/langserver/handlers/service.go @@ -214,16 +214,6 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { ctx = lsctx.WithDocumentStorage(ctx, svc.fs) return handle(ctx, req, TextDocumentDidClose) }, - "textDocument/didSave": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { - err := session.CheckInitializationIsConfirmed() - if err != nil { - return nil, err - } - ctx = lsctx.WithDiagnostics(ctx, diags) - ctx = lsctx.WithDocumentStorage(ctx, svc.fs) - ctx = lsctx.WithRootModuleFinder(ctx, svc.modMgr) - return handle(ctx, req, lh.TextDocumentDidSave) - }, "textDocument/documentSymbol": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { err := session.CheckInitializationIsConfirmed() if err != nil { From a6f9a4643ffa87909955715bc076b58c1ec6984e Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Mon, 7 Dec 2020 20:19:58 -0500 Subject: [PATCH 05/11] Use tfjson diagnostic type Bump tfjson and terraform-schema --- go.mod | 6 +++--- go.sum | 14 ++++++-------- internal/terraform/exec/exec.go | 4 ++-- internal/terraform/exec/mock/executor.go | 10 +++++----- internal/terraform/exec/types.go | 2 +- 5 files changed, 17 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index 745fe16a1..186717612 100644 --- a/go.mod +++ b/go.mod @@ -13,9 +13,9 @@ require ( github.com/hashicorp/go-version v1.2.1 github.com/hashicorp/hcl-lang v0.0.0-20201116081236-948e43712a65 github.com/hashicorp/hcl/v2 v2.6.0 - github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8 - github.com/hashicorp/terraform-json v0.6.0 - github.com/hashicorp/terraform-schema v0.0.0-20201204171308-0c9744a02c65 + github.com/hashicorp/terraform-exec v0.11.1-0.20201207223938-9186a7c3bb24 + github.com/hashicorp/terraform-json v0.7.0 + github.com/hashicorp/terraform-schema v0.0.0-20201208004742-b5e321a36f41 github.com/mh-cbon/go-fmt-fail v0.0.0-20160815164508-67765b3fbcb5 github.com/mitchellh/cli v1.1.1 github.com/mitchellh/go-homedir v1.1.0 diff --git a/go.sum b/go.sum index 1ca8deb88..924772719 100644 --- a/go.sum +++ b/go.sum @@ -198,14 +198,12 @@ github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2p github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= github.com/hashicorp/terraform-config-inspect v0.0.0-20201102131242-0c45ba392e51 h1:SEGO1vz/pFLfKy4QpABIMCe7wffmtsOiWO4yc1E87cU= github.com/hashicorp/terraform-config-inspect v0.0.0-20201102131242-0c45ba392e51/go.mod h1:Z0Nnk4+3Cy89smEbrq+sl1bxc9198gIP4I7wcQF6Kqs= -github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8 h1:sJXjDnxrrpmmJcAiPqX1HL7uksBgVXjY+3VX6k09zkA= -github.com/hashicorp/terraform-exec v0.11.1-0.20201201215927-6e0fd8a457b8/go.mod h1:162lr1MQN7XkrqevlStXPonCGMP5yvyG9zWnlmGdQLI= -github.com/hashicorp/terraform-json v0.5.0 h1:7TV3/F3y7QVSuN4r9BEXqnWqrAyeOtON8f0wvREtyzs= -github.com/hashicorp/terraform-json v0.5.0/go.mod h1:eAbqb4w0pSlRmdvl8fOyHAi/+8jnkVYN28gJkSJrLhU= -github.com/hashicorp/terraform-json v0.6.0 h1:nMTj4t9ysC7xJ72rvVsDqhUccvbUINrjhPqafeUeREk= -github.com/hashicorp/terraform-json v0.6.0/go.mod h1:eAbqb4w0pSlRmdvl8fOyHAi/+8jnkVYN28gJkSJrLhU= -github.com/hashicorp/terraform-schema v0.0.0-20201204171308-0c9744a02c65 h1:CFZI/uaZCoAN0SI4A9edORfj+tnq+Hg0v9PH2AZ+1tA= -github.com/hashicorp/terraform-schema v0.0.0-20201204171308-0c9744a02c65/go.mod h1:tU5zEQwdtUJbMQXh5+s0TnBOFshXSAcdAx8Ki8p2S4w= +github.com/hashicorp/terraform-exec v0.11.1-0.20201207223938-9186a7c3bb24 h1:kMl+qKoUCH6Uj8wzlT+ovHMbJAFJgY1Zw6ek1yHwWyo= +github.com/hashicorp/terraform-exec v0.11.1-0.20201207223938-9186a7c3bb24/go.mod h1:o1PZgrOpDMiDeKT0Hcr2oo6KoVKyaeTN867jcaXMg4E= +github.com/hashicorp/terraform-json v0.7.0 h1:DgkfLARKMQ/xmzVtSRX9Vz/fzPCL3vskHIgj6s+SQwQ= +github.com/hashicorp/terraform-json v0.7.0/go.mod h1:3defM4kkMfttwiE7VakJDwCd4R+umhSQnvJwORXbprE= +github.com/hashicorp/terraform-schema v0.0.0-20201208004742-b5e321a36f41 h1:FvVpjaQJXT9AH70Vs9i90QDTLz92BsL69N9kaLGK8qE= +github.com/hashicorp/terraform-schema v0.0.0-20201208004742-b5e321a36f41/go.mod h1:eRHMO4QL4TTka07aC7fH+AXvi/tYlv6udrA8nSFOl6g= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734/go.mod h1:kNDNcF7sN4DocDLBkQYz73HGKwN1ANB1blq4lIYLYvg= github.com/imdario/mergo v0.3.9 h1:UauaLniWCFHWd+Jp9oCEkTBj8VO/9DKg3PV3VCNMDIg= diff --git a/internal/terraform/exec/exec.go b/internal/terraform/exec/exec.go index bda7c8071..e0b3a8b1f 100644 --- a/internal/terraform/exec/exec.go +++ b/internal/terraform/exec/exec.go @@ -114,12 +114,12 @@ func (e *Executor) Format(ctx context.Context, input []byte) ([]byte, error) { return buf.Bytes(), e.contextfulError(ctx, "Format", err) } -func (e *Executor) Validate(ctx context.Context) ([]tfexec.Diagnostic, error) { +func (e *Executor) Validate(ctx context.Context) ([]tfjson.Diagnostic, error) { ctx, cancel := e.withTimeout(ctx) defer cancel() err := e.setLogPath("Validate") if err != nil { - return []tfexec.Diagnostic{}, err + return []tfjson.Diagnostic{}, err } validation, err := e.tf.Validate(ctx) diff --git a/internal/terraform/exec/mock/executor.go b/internal/terraform/exec/mock/executor.go index b844c5e7b..7c58507e3 100644 --- a/internal/terraform/exec/mock/executor.go +++ b/internal/terraform/exec/mock/executor.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.4.0-beta. DO NOT EDIT. +// Code generated by mockery v2.4.0. DO NOT EDIT. package mock @@ -129,15 +129,15 @@ func (_m *Executor) SetTimeout(duration time.Duration) { } // Validate provides a mock function with given fields: ctx -func (_m *Executor) Validate(ctx context.Context) ([]tfexec.Diagnostic, error) { +func (_m *Executor) Validate(ctx context.Context) ([]tfjson.Diagnostic, error) { ret := _m.Called(ctx) - var r0 []tfexec.Diagnostic - if rf, ok := ret.Get(0).(func(context.Context) []tfexec.Diagnostic); ok { + var r0 []tfjson.Diagnostic + if rf, ok := ret.Get(0).(func(context.Context) []tfjson.Diagnostic); ok { r0 = rf(ctx) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]tfexec.Diagnostic) + r0 = ret.Get(0).([]tfjson.Diagnostic) } } diff --git a/internal/terraform/exec/types.go b/internal/terraform/exec/types.go index ef1409cc6..d7c50d433 100644 --- a/internal/terraform/exec/types.go +++ b/internal/terraform/exec/types.go @@ -26,6 +26,6 @@ type TerraformExecutor interface { Init(ctx context.Context, opts ...tfexec.InitOption) error Format(ctx context.Context, input []byte) ([]byte, error) Version(ctx context.Context) (*version.Version, map[string]*version.Version, error) - Validate(ctx context.Context) ([]tfexec.Diagnostic, error) + Validate(ctx context.Context) ([]tfjson.Diagnostic, error) ProviderSchemas(ctx context.Context) (*tfjson.ProviderSchemas, error) } From 1d790b50fc48b2a5ab432709cf577360b7fe4e21 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Mon, 7 Dec 2020 20:21:35 -0500 Subject: [PATCH 06/11] update ref --- internal/terraform/rootmodule/root_module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/terraform/rootmodule/root_module.go b/internal/terraform/rootmodule/root_module.go index 4b84cc9b0..4b9ae0b11 100644 --- a/internal/terraform/rootmodule/root_module.go +++ b/internal/terraform/rootmodule/root_module.go @@ -403,7 +403,7 @@ func (rm *rootModule) ExecuteTerraformValidate(ctx context.Context) (map[string] return diagsMap, err } - // tfexec.Diagnostic is a conversion of an internal diag to terraform core, + // tfjson.Diagnostic is a conversion of an internal diag to terraform core, // tfdiags, which is effectively based on hcl.Diagnostic. // This process is really just converting it back to hcl.Diagnotic // since it is the defacto diagnostic type for our codebase currently From b0f51cb6b1fec223c4c95a20876e7d8b5817d63e Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Mon, 7 Dec 2020 20:53:58 -0500 Subject: [PATCH 07/11] Add tests regarding diag source cache --- .../diagnostics/diagnostics_test.go | 71 +++++++++++++++++++ .../langserver/handlers/command/validate.go | 4 +- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/internal/langserver/diagnostics/diagnostics_test.go b/internal/langserver/diagnostics/diagnostics_test.go index ca79aea5e..0589324e2 100644 --- a/internal/langserver/diagnostics/diagnostics_test.go +++ b/internal/langserver/diagnostics/diagnostics_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/hashicorp/hcl/v2" + lsp "github.com/hashicorp/terraform-ls/internal/protocol" ) var discardLogger = log.New(ioutil.Discard, "", 0) @@ -48,3 +49,73 @@ func TestPublish_DoesNotSendAfterClose(t *testing.T) { }, }, "test") } + +func TestMergeDiags_CachesMultipleSourcesPerURI(t *testing.T) { + uri := "test.tf" + + n := NewNotifier(context.Background(), discardLogger) + + all := n.mergeDiags(uri, "source1", []lsp.Diagnostic{ + { + Severity: lsp.SeverityError, + Message: "diag1", + }, + }) + if len(all) != 1 { + t.Fatalf("returns diags is incorrect length: expected %d, got %d", 1, len(all)) + } + + all = n.mergeDiags(uri, "source2", []lsp.Diagnostic{ + { + Severity: lsp.SeverityError, + Message: "diag2", + }, + }) + if len(all) != 2 { + t.Fatalf("returns diags is incorrect length: expected %d, got %d", 2, len(all)) + } +} + +func TestMergeDiags_OverwritesSource_EvenWithEmptySlice(t *testing.T) { + uri := "test.tf" + + n := NewNotifier(context.Background(), discardLogger) + + all := n.mergeDiags(uri, "source1", []lsp.Diagnostic{ + { + Severity: lsp.SeverityError, + Message: "diag1", + }, + }) + if len(all) != 1 { + t.Fatalf("returns diags is incorrect length: expected %d, got %d", 1, len(all)) + } + + all = n.mergeDiags(uri, "source1", []lsp.Diagnostic{ + { + Severity: lsp.SeverityError, + Message: "diagOverwritten", + }, + }) + if len(all) != 1 { + t.Fatalf("returns diags is incorrect length: expected %d, got %d", 1, len(all)) + } + if all[0].Message != "diagOverwritten" { + t.Fatalf("diag has incorrect message: expected %s, got %s", "diagOverwritten", all[0].Message) + } + + all = n.mergeDiags(uri, "source2", []lsp.Diagnostic{ + { + Severity: lsp.SeverityError, + Message: "diag2", + }, + }) + if len(all) != 2 { + t.Fatalf("returns diags is incorrect length: expected %d, got %d", 2, len(all)) + } + + all = n.mergeDiags(uri, "source2", []lsp.Diagnostic{}) + if len(all) != 1 { + t.Fatalf("returns diags is incorrect length: expected %d, got %d", 1, len(all)) + } +} diff --git a/internal/langserver/handlers/command/validate.go b/internal/langserver/handlers/command/validate.go index 199df1abb..155bf5d3f 100644 --- a/internal/langserver/handlers/command/validate.go +++ b/internal/langserver/handlers/command/validate.go @@ -17,14 +17,14 @@ func TerraformValidateHandler(ctx context.Context, args cmd.CommandArgs) (interf return nil, fmt.Errorf("%w: expected dir uri argument to be set", code.InvalidParams.Err()) } - fh := ilsp.FileHandlerFromDirURI(lsp.DocumentURI(dirUri)) + dh := ilsp.FileHandlerFromDirURI(lsp.DocumentURI(dirUri)) cf, err := lsctx.RootModuleFinder(ctx) if err != nil { return nil, err } - rm, err := cf.RootModuleByPath(fh.Dir()) + rm, err := cf.RootModuleByPath(dh.Dir()) if err != nil { return nil, err } From 185e2efac009fb9be65c529a000d21bd8501ee72 Mon Sep 17 00:00:00 2001 From: appilon Date: Tue, 8 Dec 2020 11:44:42 -0500 Subject: [PATCH 08/11] Apply suggestions from code review Co-authored-by: Radek Simko --- internal/langserver/diagnostics/diagnostics.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/langserver/diagnostics/diagnostics.go b/internal/langserver/diagnostics/diagnostics.go index fe5585485..c29696b1b 100644 --- a/internal/langserver/diagnostics/diagnostics.go +++ b/internal/langserver/diagnostics/diagnostics.go @@ -31,7 +31,12 @@ type Notifier struct { } func NewNotifier(sessCtx context.Context, logger *log.Logger) *Notifier { - n := &Notifier{logger: logger, sessCtx: sessCtx, diags: make(chan diagContext, 50), diagsCache: make(map[string]map[string][]lsp.Diagnostic)} + n := &Notifier{ + logger: logger, + sessCtx: sessCtx, + diags: make(chan diagContext, 50), + diagsCache: make(map[string]map[string][]lsp.Diagnostic), + } go n.notify() return n } @@ -39,7 +44,7 @@ func NewNotifier(sessCtx context.Context, logger *log.Logger) *Notifier { // PublishHCLDiags accepts a map of hcl diagnostics per file and queues them for publishing. // A dir path is passed which is joined with the filename keys of the map, to form a file URI. // A source string is passed and set for each diagnostic, this is typically displayed in the client UI. -func (n *Notifier) PublishHCLDiags(ctx context.Context, dir string, diags map[string]hcl.Diagnostics, source string) { +func (n *Notifier) PublishHCLDiags(ctx context.Context, dirPath string, diags map[string]hcl.Diagnostics, source string) { select { case <-n.sessCtx.Done(): n.closeDiagsOnce.Do(func() { @@ -50,7 +55,11 @@ func (n *Notifier) PublishHCLDiags(ctx context.Context, dir string, diags map[st } for filename, ds := range diags { - n.diags <- diagContext{ctx: ctx, source: source, diags: ilsp.HCLDiagsToLSP(ds, source), uri: lsp.DocumentURI(uri.FromPath(filepath.Join(dir, filename)))} + n.diags <- diagContext{ + ctx: ctx, source: source, + diags: ilsp.HCLDiagsToLSP(ds, source), + uri: lsp.DocumentURI(uri.FromPath(filepath.Join(dir, filename))), + } } } From 7a3b1d6e957ab1d55d38958bb80d6ad38c0b3585 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Tue, 8 Dec 2020 11:45:45 -0500 Subject: [PATCH 09/11] fix var --- internal/langserver/diagnostics/diagnostics.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/langserver/diagnostics/diagnostics.go b/internal/langserver/diagnostics/diagnostics.go index c29696b1b..4a8354a2d 100644 --- a/internal/langserver/diagnostics/diagnostics.go +++ b/internal/langserver/diagnostics/diagnostics.go @@ -32,9 +32,9 @@ type Notifier struct { func NewNotifier(sessCtx context.Context, logger *log.Logger) *Notifier { n := &Notifier{ - logger: logger, - sessCtx: sessCtx, - diags: make(chan diagContext, 50), + logger: logger, + sessCtx: sessCtx, + diags: make(chan diagContext, 50), diagsCache: make(map[string]map[string][]lsp.Diagnostic), } go n.notify() @@ -58,7 +58,7 @@ func (n *Notifier) PublishHCLDiags(ctx context.Context, dirPath string, diags ma n.diags <- diagContext{ ctx: ctx, source: source, diags: ilsp.HCLDiagsToLSP(ds, source), - uri: lsp.DocumentURI(uri.FromPath(filepath.Join(dir, filename))), + uri: lsp.DocumentURI(uri.FromPath(filepath.Join(dirPath, filename))), } } } From 68dd0869fb7b5cbeac16a89c962d03610a22e9f0 Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Tue, 8 Dec 2020 11:46:34 -0500 Subject: [PATCH 10/11] bump terraform-schema --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 186717612..5801bd79a 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/hashicorp/hcl/v2 v2.6.0 github.com/hashicorp/terraform-exec v0.11.1-0.20201207223938-9186a7c3bb24 github.com/hashicorp/terraform-json v0.7.0 - github.com/hashicorp/terraform-schema v0.0.0-20201208004742-b5e321a36f41 + github.com/hashicorp/terraform-schema v0.0.0-20201208163444-44d0347ab290 github.com/mh-cbon/go-fmt-fail v0.0.0-20160815164508-67765b3fbcb5 github.com/mitchellh/cli v1.1.1 github.com/mitchellh/go-homedir v1.1.0 diff --git a/go.sum b/go.sum index 924772719..2d6ad92a1 100644 --- a/go.sum +++ b/go.sum @@ -204,6 +204,8 @@ github.com/hashicorp/terraform-json v0.7.0 h1:DgkfLARKMQ/xmzVtSRX9Vz/fzPCL3vskHI github.com/hashicorp/terraform-json v0.7.0/go.mod h1:3defM4kkMfttwiE7VakJDwCd4R+umhSQnvJwORXbprE= github.com/hashicorp/terraform-schema v0.0.0-20201208004742-b5e321a36f41 h1:FvVpjaQJXT9AH70Vs9i90QDTLz92BsL69N9kaLGK8qE= github.com/hashicorp/terraform-schema v0.0.0-20201208004742-b5e321a36f41/go.mod h1:eRHMO4QL4TTka07aC7fH+AXvi/tYlv6udrA8nSFOl6g= +github.com/hashicorp/terraform-schema v0.0.0-20201208163444-44d0347ab290 h1:kAs5ZG+cgtWy3+81Z7G/Blj2imiDLfFBRaqmNs8mD4o= +github.com/hashicorp/terraform-schema v0.0.0-20201208163444-44d0347ab290/go.mod h1:eRHMO4QL4TTka07aC7fH+AXvi/tYlv6udrA8nSFOl6g= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734/go.mod h1:kNDNcF7sN4DocDLBkQYz73HGKwN1ANB1blq4lIYLYvg= github.com/imdario/mergo v0.3.9 h1:UauaLniWCFHWd+Jp9oCEkTBj8VO/9DKg3PV3VCNMDIg= From 87018ac365d7196fb81fdb5b468c2597fb86295a Mon Sep 17 00:00:00 2001 From: Alex Pilon Date: Tue, 8 Dec 2020 15:40:09 -0500 Subject: [PATCH 11/11] clean up internals with typedefs, surface WasInitialized error --- internal/langserver/diagnostics/diagnostics.go | 17 +++++++++++------ .../langserver/diagnostics/diagnostics_test.go | 4 ++-- .../langserver/handlers/command/validate.go | 5 ++++- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/internal/langserver/diagnostics/diagnostics.go b/internal/langserver/diagnostics/diagnostics.go index 4a8354a2d..fe14c290d 100644 --- a/internal/langserver/diagnostics/diagnostics.go +++ b/internal/langserver/diagnostics/diagnostics.go @@ -20,13 +20,17 @@ type diagContext struct { diags []lsp.Diagnostic } +type diagnosticSource string + +type fileDiagnostics map[diagnosticSource][]lsp.Diagnostic + // Notifier is a type responsible for queueing hcl diagnostics to be converted // and sent to the client type Notifier struct { logger *log.Logger sessCtx context.Context diags chan diagContext - diagsCache map[string]map[string][]lsp.Diagnostic + diagsCache map[lsp.DocumentURI]fileDiagnostics closeDiagsOnce sync.Once } @@ -35,7 +39,7 @@ func NewNotifier(sessCtx context.Context, logger *log.Logger) *Notifier { logger: logger, sessCtx: sessCtx, diags: make(chan diagContext, 50), - diagsCache: make(map[string]map[string][]lsp.Diagnostic), + diagsCache: make(map[lsp.DocumentURI]fileDiagnostics), } go n.notify() return n @@ -67,7 +71,7 @@ func (n *Notifier) notify() { for d := range n.diags { if err := jrpc2.PushNotify(d.ctx, "textDocument/publishDiagnostics", lsp.PublishDiagnosticsParams{ URI: d.uri, - Diagnostics: n.mergeDiags(string(d.uri), d.source, d.diags), + Diagnostics: n.mergeDiags(d.uri, d.source, d.diags), }); err != nil { n.logger.Printf("Error pushing diagnostics: %s", err) } @@ -77,13 +81,14 @@ func (n *Notifier) notify() { // mergeDiags will return all diags from all cached sources for a given uri. // the passed diags overwrites the cached entry for the passed source key // even if empty -func (n *Notifier) mergeDiags(uri string, source string, diags []lsp.Diagnostic) []lsp.Diagnostic { +func (n *Notifier) mergeDiags(uri lsp.DocumentURI, source string, diags []lsp.Diagnostic) []lsp.Diagnostic { + fileDiags, ok := n.diagsCache[uri] if !ok { - fileDiags = make(map[string][]lsp.Diagnostic) + fileDiags = make(fileDiagnostics) } - fileDiags[source] = diags + fileDiags[diagnosticSource(source)] = diags n.diagsCache[uri] = fileDiags all := []lsp.Diagnostic{} diff --git a/internal/langserver/diagnostics/diagnostics_test.go b/internal/langserver/diagnostics/diagnostics_test.go index 0589324e2..3812e8ca5 100644 --- a/internal/langserver/diagnostics/diagnostics_test.go +++ b/internal/langserver/diagnostics/diagnostics_test.go @@ -51,7 +51,7 @@ func TestPublish_DoesNotSendAfterClose(t *testing.T) { } func TestMergeDiags_CachesMultipleSourcesPerURI(t *testing.T) { - uri := "test.tf" + uri := lsp.DocumentURI("test.tf") n := NewNotifier(context.Background(), discardLogger) @@ -77,7 +77,7 @@ func TestMergeDiags_CachesMultipleSourcesPerURI(t *testing.T) { } func TestMergeDiags_OverwritesSource_EvenWithEmptySlice(t *testing.T) { - uri := "test.tf" + uri := lsp.DocumentURI("test.tf") n := NewNotifier(context.Background(), discardLogger) diff --git a/internal/langserver/handlers/command/validate.go b/internal/langserver/handlers/command/validate.go index 155bf5d3f..b215f449e 100644 --- a/internal/langserver/handlers/command/validate.go +++ b/internal/langserver/handlers/command/validate.go @@ -29,7 +29,10 @@ func TerraformValidateHandler(ctx context.Context, args cmd.CommandArgs) (interf return nil, err } - wasInit, _ := rm.WasInitialized() + wasInit, err := rm.WasInitialized() + if err != nil { + return nil, fmt.Errorf("error checking if %s was initialized: %s", dirUri, err) + } if !wasInit { return nil, fmt.Errorf("%s is not an initialized module, terraform validate cannot be called", dirUri) }