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 Aug 29, 2023
1 parent 825c426 commit 31c677f
Show file tree
Hide file tree
Showing 19 changed files with 441 additions and 260 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/hashicorp/go-uuid v1.0.3
github.com/hashicorp/go-version v1.6.0
github.com/hashicorp/hc-install v0.5.2
github.com/hashicorp/hcl-lang v0.0.0-20230823131918-b6a3f8271038
github.com/hashicorp/hcl-lang v0.0.0-20230825134805-29034e9d534d
github.com/hashicorp/hcl/v2 v2.17.0
github.com/hashicorp/terraform-exec v0.18.1
github.com/hashicorp/terraform-json v0.17.1
Expand All @@ -27,7 +27,7 @@ require (
github.com/pmezard/go-difflib v1.0.0
github.com/stretchr/testify v1.8.4
github.com/vektra/mockery/v2 v2.33.0
github.com/zclconf/go-cty v1.13.2
github.com/zclconf/go-cty v1.13.3
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b
go.bobheadxi.dev/gobenchdata v1.3.1
go.opentelemetry.io/otel/trace v1.16.0
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ github.com/hashicorp/hc-install v0.5.2 h1:SfwMFnEXVVirpwkDuSF5kymUOhrUxrTq3udEse
github.com/hashicorp/hc-install v0.5.2/go.mod h1:9QISwe6newMWIfEiXpzuu1k9HAGtQYgnSH8H9T8wmoI=
github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
github.com/hashicorp/hcl-lang v0.0.0-20230823131918-b6a3f8271038 h1:fhyfQoAyYFvt+AqIFlTHrstlC4OOT4KB/F/KatqJUfI=
github.com/hashicorp/hcl-lang v0.0.0-20230823131918-b6a3f8271038/go.mod h1:xlYq00R/OYgpAmScjO1zkE9NCjl6zuMex1VVcUU0pjQ=
github.com/hashicorp/hcl-lang v0.0.0-20230825134805-29034e9d534d h1:kr/f+fJGnFGe2al/H55/blLb1Ne5BAZWe4qY/BxA2D0=
github.com/hashicorp/hcl-lang v0.0.0-20230825134805-29034e9d534d/go.mod h1:X9wA1+6Zr0nVspFxRvhVZnDhBMWhvb0uuAjp4bGujuc=
github.com/hashicorp/hcl/v2 v2.17.0 h1:z1XvSUyXd1HP10U4lrLg5e0JMVz6CPaJvAgxM0KNZVY=
github.com/hashicorp/hcl/v2 v2.17.0/go.mod h1:gJyW2PTShkJqQBKpAmPO3yxMxIuoXkOF2TpqXzrQyx4=
github.com/hashicorp/terraform-exec v0.18.1 h1:LAbfDvNQU1l0NOQlTuudjczVhHj061fNX5H8XZxHlH4=
Expand Down Expand Up @@ -378,8 +378,8 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de
github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8=
github.com/zclconf/go-cty v1.13.2 h1:4GvrUxe/QUDYuJKAav4EYqdM47/kZa672LwmXFmEKT0=
github.com/zclconf/go-cty v1.13.2/go.mod h1:YKQzy/7pZ7iq2jNFzy5go57xdxdWoLLpaEp4u238AE0=
github.com/zclconf/go-cty v1.13.3 h1:m+b9q3YDbg6Bec5rr+KGy1MzEVzY/jC2X+YX4yqKtHI=
github.com/zclconf/go-cty v1.13.3/go.mod h1:YKQzy/7pZ7iq2jNFzy5go57xdxdWoLLpaEp4u238AE0=
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI=
github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8=
go.bobheadxi.dev/gobenchdata v1.3.1 h1:3Pts2nPUZdgFSU63nWzvfs2xRbK8WVSNeJ2H9e/Ypew=
Expand Down
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
21 changes: 18 additions & 3 deletions internal/indexer/document_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,20 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand
}
ids = append(ids, eSchemaId)

// 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{eSchemaId},
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 Down Expand Up @@ -124,13 +138,14 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand
}
ids = append(ids, refOriginsId)

// 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(),
DependsOn: job.IDs{eSchemaId, refTargetsId, refOriginsId},
Type: op.OpTypeReferenceValidation.String(),
DependsOn: job.IDs{refTargetsId, refOriginsId},
IgnoreState: ignoreState,
})
if err != nil {
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 31c677f

Please sign in to comment.