Skip to content

Commit

Permalink
Centralize diagnostics publishing (#1361)
Browse files Browse the repository at this point in the history
* Introduce different source for diagnostics

* Add reference validation job

* Run terraform validate as a job

* Track validation job states

* Review feedback

* Pass pathCtx to reference validation funcs

* Rename early validation job

* Bump hcl-lang to `29034e`

* Introduce VarsDiagnosticsState

* Fix tests
  • Loading branch information
dbanck committed Sep 27, 2023
1 parent 086576c commit 66e8b06
Show file tree
Hide file tree
Showing 18 changed files with 445 additions and 259 deletions.
7 changes: 0 additions & 7 deletions internal/decoder/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ import (
"context"

"github.com/hashicorp/hcl-lang/decoder"
"github.com/hashicorp/hcl-lang/lang"
"github.com/hashicorp/hcl-lang/reference"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform-ls/internal/codelens"
"github.com/hashicorp/terraform-ls/internal/decoder/validations"
ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
lsp "github.com/hashicorp/terraform-ls/internal/protocol"
"github.com/hashicorp/terraform-ls/internal/state"
Expand Down Expand Up @@ -93,10 +91,5 @@ func DecoderContext(ctx context.Context) decoder.DecoderContext {
}
}

validations := []lang.ValidationFunc{
validations.UnreferencedOrigins,
}
dCtx.Validations = append(dCtx.Validations, validations...)

return dCtx
}
7 changes: 1 addition & 6 deletions internal/decoder/validations/unreferenced_origin.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,9 @@ import (
"github.com/hashicorp/hcl/v2"
)

func UnreferencedOrigins(ctx context.Context) lang.DiagnosticsMap {
func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang.DiagnosticsMap {
diagsMap := make(lang.DiagnosticsMap)

pathCtx, err := decoder.PathCtx(ctx)
if err != nil {
return diagsMap
}

for _, origin := range pathCtx.ReferenceOrigins {
matchableOrigin, ok := origin.(reference.MatchableOrigin)
if !ok {
Expand Down
4 changes: 1 addition & 3 deletions internal/decoder/validations/unreferenced_origin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ func TestUnreferencedOrigins(t *testing.T) {
ReferenceOrigins: tt.origins,
}

ctx = decoder.WithPathContext(ctx, pathCtx)

diags := UnreferencedOrigins(ctx)
diags := UnreferencedOrigins(ctx, pathCtx)
if diff := cmp.Diff(tt.want["test.tf"], diags["test.tf"]); diff != "" {
t.Fatalf("unexpected diagnostics: %s", diff)
}
Expand Down
19 changes: 17 additions & 2 deletions internal/indexer/document_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,20 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand
}
ids = append(ids, metaId)

// TODO! check if early validation setting is enabled
_, err = idx.jobStore.EnqueueJob(ctx, job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
return module.SchemaValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
},
Type: op.OpTypeSchemaValidation.String(),
DependsOn: job.IDs{metaId},
IgnoreState: ignoreState,
})
if err != nil {
return ids, err
}

refTargetsId, err := idx.jobStore.EnqueueJob(ctx, job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
Expand All @@ -135,12 +149,13 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand
}
ids = append(ids, refTargetsId)

// TODO! check if early validation setting is enabled
_, err = idx.jobStore.EnqueueJob(ctx, job.Job{
Dir: modHandle,
Func: func(ctx context.Context) error {
return module.EarlyValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
return module.ReferenceValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path())
},
Type: op.OpTypeEarlyValidation.String(),
Type: op.OpTypeReferenceValidation.String(),
DependsOn: job.IDs{metaId, refTargetsId},
IgnoreState: ignoreState,
})
Expand Down
15 changes: 7 additions & 8 deletions internal/langserver/diagnostics/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/hashicorp/hcl/v2"
ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
lsp "github.com/hashicorp/terraform-ls/internal/protocol"
"github.com/hashicorp/terraform-ls/internal/terraform/ast"
"github.com/hashicorp/terraform-ls/internal/uri"
)

Expand All @@ -21,8 +22,6 @@ type diagContext struct {
diags []lsp.Diagnostic
}

type DiagnosticSource string

type ClientNotifier interface {
Notify(ctx context.Context, method string, params interface{}) error
}
Expand Down Expand Up @@ -61,7 +60,7 @@ func (n *Notifier) PublishHCLDiags(ctx context.Context, dirPath string, diags Di
for filename, ds := range diags {
fileDiags := make([]lsp.Diagnostic, 0)
for source, diags := range ds {
fileDiags = append(fileDiags, ilsp.HCLDiagsToLSP(diags, string(source))...)
fileDiags = append(fileDiags, ilsp.HCLDiagsToLSP(diags, source.String())...)
}

n.diags <- diagContext{
Expand All @@ -83,7 +82,7 @@ func (n *Notifier) notify() {
}
}

type Diagnostics map[string]map[DiagnosticSource]hcl.Diagnostics
type Diagnostics map[string]map[ast.DiagnosticSource]hcl.Diagnostics

func NewDiagnostics() Diagnostics {
return make(Diagnostics, 0)
Expand All @@ -92,16 +91,16 @@ func NewDiagnostics() Diagnostics {
// EmptyRootDiagnostic allows emptying any diagnostics for
// the whole directory which were published previously.
func (d Diagnostics) EmptyRootDiagnostic() Diagnostics {
d[""] = make(map[DiagnosticSource]hcl.Diagnostics, 0)
d[""] = make(map[ast.DiagnosticSource]hcl.Diagnostics, 0)
return d
}

func (d Diagnostics) Append(src string, diagsMap map[string]hcl.Diagnostics) Diagnostics {
func (d Diagnostics) Append(src ast.DiagnosticSource, diagsMap map[string]hcl.Diagnostics) Diagnostics {
for uri, uriDiags := range diagsMap {
if _, ok := d[uri]; !ok {
d[uri] = make(map[DiagnosticSource]hcl.Diagnostics, 0)
d[uri] = make(map[ast.DiagnosticSource]hcl.Diagnostics, 0)
}
d[uri][DiagnosticSource(src)] = uriDiags
d[uri][src] = uriDiags
}

return d
Expand Down
23 changes: 12 additions & 11 deletions internal/langserver/diagnostics/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform-ls/internal/terraform/ast"
)

var discardLogger = log.New(ioutil.Discard, "", 0)
Expand All @@ -19,8 +20,8 @@ func TestDiags_Closes(t *testing.T) {
n := NewNotifier(noopNotifier{}, discardLogger)

diags := NewDiagnostics()
diags.Append("test", map[string]hcl.Diagnostics{
"test": {
diags.Append(ast.HCLParsingSource, map[string]hcl.Diagnostics{
ast.HCLParsingSource.String(): {
{
Severity: hcl.DiagError,
},
Expand All @@ -46,8 +47,8 @@ func TestPublish_DoesNotSendAfterClose(t *testing.T) {
n := NewNotifier(noopNotifier{}, discardLogger)

diags := NewDiagnostics()
diags.Append("test", map[string]hcl.Diagnostics{
"test": {
diags.Append(ast.TerraformValidateSource, map[string]hcl.Diagnostics{
ast.TerraformValidateSource.String(): {
{
Severity: hcl.DiagError,
},
Expand All @@ -61,7 +62,7 @@ func TestPublish_DoesNotSendAfterClose(t *testing.T) {

func TestDiagnostics_Append(t *testing.T) {
diags := NewDiagnostics()
diags.Append("foo", map[string]hcl.Diagnostics{
diags.Append(ast.HCLParsingSource, map[string]hcl.Diagnostics{
"first.tf": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Expand All @@ -79,7 +80,7 @@ func TestDiagnostics_Append(t *testing.T) {
},
},
})
diags.Append("bar", map[string]hcl.Diagnostics{
diags.Append(ast.SchemaValidationSource, map[string]hcl.Diagnostics{
"first.tf": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Expand All @@ -96,8 +97,8 @@ func TestDiagnostics_Append(t *testing.T) {
})

expectedDiags := Diagnostics{
"first.tf": map[DiagnosticSource]hcl.Diagnostics{
DiagnosticSource("foo"): {
"first.tf": map[ast.DiagnosticSource]hcl.Diagnostics{
ast.HCLParsingSource: {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Something went wrong",
Expand All @@ -113,16 +114,16 @@ func TestDiagnostics_Append(t *testing.T) {
},
},
},
DiagnosticSource("bar"): {
ast.SchemaValidationSource: {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Something else went wrong",
Detail: "Testing detail",
},
},
},
"second.tf": map[DiagnosticSource]hcl.Diagnostics{
DiagnosticSource("bar"): {
"second.tf": map[ast.DiagnosticSource]hcl.Diagnostics{
ast.SchemaValidationSource: {
&hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Beware",
Expand Down
59 changes: 11 additions & 48 deletions internal/langserver/handlers/command/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@ import (
"fmt"

"github.com/creachadair/jrpc2"
lsctx "github.com/hashicorp/terraform-ls/internal/context"
"github.com/hashicorp/terraform-ls/internal/document"
"github.com/hashicorp/terraform-ls/internal/job"
"github.com/hashicorp/terraform-ls/internal/langserver/cmd"
"github.com/hashicorp/terraform-ls/internal/langserver/diagnostics"
"github.com/hashicorp/terraform-ls/internal/langserver/errors"
"github.com/hashicorp/terraform-ls/internal/langserver/progress"
"github.com/hashicorp/terraform-ls/internal/state"
"github.com/hashicorp/terraform-ls/internal/terraform/module"
op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation"
"github.com/hashicorp/terraform-ls/internal/uri"
)

Expand All @@ -31,51 +28,17 @@ func (h *CmdHandler) TerraformValidateHandler(ctx context.Context, args cmd.Comm

dirHandle := document.DirHandleFromURI(dirUri)

mod, err := h.StateStore.Modules.ModuleByPath(dirHandle.Path())
if err != nil {
if state.IsModuleNotFound(err) {
err = h.StateStore.Modules.Add(dirHandle.Path())
if err != nil {
return nil, err
}
mod, err = h.StateStore.Modules.ModuleByPath(dirHandle.Path())
if err != nil {
return nil, err
}
} else {
return nil, err
}
}

tfExec, err := module.TerraformExecutorForModule(ctx, mod.Path)
if err != nil {
return nil, errors.EnrichTfExecError(err)
}

notifier, err := lsctx.DiagnosticsNotifier(ctx)
id, err := h.StateStore.JobStore.EnqueueJob(ctx, job.Job{
Dir: dirHandle,
Func: func(ctx context.Context) error {
return module.TerraformValidate(ctx, h.StateStore.Modules, dirHandle.Path())
},
Type: op.OpTypeTerraformValidate.String(),
IgnoreState: true,
})
if err != nil {
return nil, err
}

progress.Begin(ctx, "Validating")
defer func() {
progress.End(ctx, "Finished")
}()
progress.Report(ctx, "Running terraform validate ...")
jsonDiags, err := tfExec.Validate(ctx)
if err != nil {
return nil, err
}

diags := diagnostics.NewDiagnostics()
validateDiags := diagnostics.HCLDiagsFromJSON(jsonDiags)
diags.EmptyRootDiagnostic()
diags.Append("terraform validate", validateDiags)
diags.Append("early validation", mod.ValidationDiagnostics)
diags.Append("HCL", mod.ModuleDiagnostics.AutoloadedOnly().AsMap())
diags.Append("HCL", mod.VarsDiagnostics.AutoloadedOnly().AsMap())

notifier.PublishHCLDiags(ctx, mod.Path, diags)

return nil, nil
return nil, h.StateStore.JobStore.WaitForJobs(ctx, id)
}
13 changes: 7 additions & 6 deletions internal/langserver/handlers/hooks_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,14 @@ func updateDiagnostics(dNotifier *diagnostics.Notifier) notifier.Hook {
diags := diagnostics.NewDiagnostics()
diags.EmptyRootDiagnostic()

defer dNotifier.PublishHCLDiags(ctx, mod.Path, diags)

if mod != nil {
diags.Append("early validation", mod.ValidationDiagnostics)
diags.Append("HCL", mod.ModuleDiagnostics.AutoloadedOnly().AsMap())
diags.Append("HCL", mod.VarsDiagnostics.AutoloadedOnly().AsMap())
for source, dm := range mod.ModuleDiagnostics {
diags.Append(source, dm.AutoloadedOnly().AsMap())
}
for source, dm := range mod.VarsDiagnostics {
diags.Append(source, dm.AutoloadedOnly().AsMap())
}

dNotifier.PublishHCLDiags(ctx, mod.Path, diags)
}
return nil
}
Expand Down
Loading

0 comments on commit 66e8b06

Please sign in to comment.