From a801ae7f2e3e2e50f11995c6119255444e319116 Mon Sep 17 00:00:00 2001 From: James Pogran Date: Tue, 8 Aug 2023 12:11:46 -0400 Subject: [PATCH 01/15] Introduce a new setting for early validation (#1353) Adds `validation.earlyValidation` as a new setting. I chose to use `validation` as a primary key as we expect to add future settings for this feature. --- internal/settings/settings.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/settings/settings.go b/internal/settings/settings.go index c93a68bb..f8638917 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -18,6 +18,10 @@ type ExperimentalFeatures struct { PrefillRequiredFields bool `mapstructure:"prefillRequiredFields"` } +type ValidationOptions struct { + EarlyValidation bool `mapstructure:"earlyValidation"` +} + type Indexing struct { IgnoreDirectoryNames []string `mapstructure:"ignoreDirectoryNames"` IgnorePaths []string `mapstructure:"ignorePaths"` @@ -36,6 +40,8 @@ type Options struct { // ExperimentalFeatures encapsulates experimental features users can opt into. ExperimentalFeatures ExperimentalFeatures `mapstructure:"experimentalFeatures"` + Validation ValidationOptions `mapstructure:"validation"` + IgnoreSingleFileWarning bool `mapstructure:"ignoreSingleFileWarning"` Terraform Terraform `mapstructure:"terraform"` From 2dca39b3ce5a7ca0e741dcef1610df621bced55f Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Thu, 24 Aug 2023 11:57:56 +0200 Subject: [PATCH 02/15] Introduce a new job for running early validation (#1346) * Implement unreferenced variable validation (#1357) Add the ability to use the collected origin and target references in early validation by providing a hook for validation funcs. This also adds a validator for unreferenced variables. Validation funcs will be provided by terraform-ls for now, but may be moved into hcl-lang in the future. * Introduce ValidationDiagnostics field to module * Publish early validation diagnostics * Include validation diagnotics in changes check * Introduce early validation job * Check ValidationDiagnosticsState when running validation * Run early validation job after collection jobs * Bump hcl-lang to `b6a3f8` * Update internal/terraform/module/module_ops.go Co-authored-by: Radek Simko --------- Co-authored-by: James Pogran Co-authored-by: Radek Simko --- internal/decoder/decoder.go | 7 + .../validations/unreferenced_origin.go | 60 +++++++++ .../validations/unreferenced_origin_test.go | 120 ++++++++++++++++++ internal/indexer/document_change.go | 13 ++ .../langserver/handlers/command/validate.go | 1 + internal/langserver/handlers/hooks_module.go | 1 + internal/state/module.go | 73 +++++++++-- internal/state/module_changes.go | 4 +- internal/terraform/module/module_ops.go | 39 ++++++ .../module/operation/op_type_string.go | 5 +- .../terraform/module/operation/operation.go | 1 + 11 files changed, 312 insertions(+), 12 deletions(-) create mode 100644 internal/decoder/validations/unreferenced_origin.go create mode 100644 internal/decoder/validations/unreferenced_origin_test.go diff --git a/internal/decoder/decoder.go b/internal/decoder/decoder.go index e951e7cf..74247e4e 100644 --- a/internal/decoder/decoder.go +++ b/internal/decoder/decoder.go @@ -7,9 +7,11 @@ 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" @@ -91,5 +93,10 @@ func DecoderContext(ctx context.Context) decoder.DecoderContext { } } + validations := []lang.ValidationFunc{ + validations.UnreferencedOrigins, + } + dCtx.Validations = append(dCtx.Validations, validations...) + return dCtx } diff --git a/internal/decoder/validations/unreferenced_origin.go b/internal/decoder/validations/unreferenced_origin.go new file mode 100644 index 00000000..e8871153 --- /dev/null +++ b/internal/decoder/validations/unreferenced_origin.go @@ -0,0 +1,60 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validations + +import ( + "context" + "fmt" + + "github.com/hashicorp/hcl-lang/decoder" + "github.com/hashicorp/hcl-lang/lang" + "github.com/hashicorp/hcl-lang/reference" + "github.com/hashicorp/hcl/v2" +) + +func UnreferencedOrigins(ctx context.Context) 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 { + // we don't report on other origins to avoid complexity for now + // other origins would need to be matched against other + // modules/directories and we cannot be sure the targets are + // available within the workspace or were parsed/decoded/collected + // at the time this event occurs + continue + } + + // we only initially validate variables + // resources and data sources can have unknown schema + // and will be researched at a later point + firstStep := matchableOrigin.Address()[0] + if firstStep.String() != "var" { + continue + } + + _, ok = pathCtx.ReferenceTargets.Match(matchableOrigin) + if !ok { + // target not found + fileName := origin.OriginRange().Filename + d := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("No declaration found for %q", matchableOrigin.Address()), + Subject: origin.OriginRange().Ptr(), + } + diagsMap[fileName] = diagsMap[fileName].Append(d) + + continue + } + + } + + return diagsMap +} diff --git a/internal/decoder/validations/unreferenced_origin_test.go b/internal/decoder/validations/unreferenced_origin_test.go new file mode 100644 index 00000000..420d4f24 --- /dev/null +++ b/internal/decoder/validations/unreferenced_origin_test.go @@ -0,0 +1,120 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package validations + +import ( + "context" + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl-lang/decoder" + "github.com/hashicorp/hcl-lang/lang" + "github.com/hashicorp/hcl-lang/reference" + "github.com/hashicorp/hcl/v2" +) + +func TestUnreferencedOrigins(t *testing.T) { + tests := []struct { + name string + origins reference.Origins + want lang.DiagnosticsMap + }{ + { + name: "undeclared variable", + origins: reference.Origins{ + reference.LocalOrigin{ + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{}, + End: hcl.Pos{}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "var"}, + lang.AttrStep{Name: "foo"}, + }, + }, + }, + want: lang.DiagnosticsMap{ + "test.tf": hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "No declaration found for \"var.foo\"", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{}, + End: hcl.Pos{}, + }, + }, + }, + }, + }, + { + name: "many undeclared variables", + origins: reference.Origins{ + reference.LocalOrigin{ + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 10, Byte: 10}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "var"}, + lang.AttrStep{Name: "foo"}, + }, + }, + reference.LocalOrigin{ + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 10, Byte: 10}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "var"}, + lang.AttrStep{Name: "wakka"}, + }, + }, + }, + want: lang.DiagnosticsMap{ + "test.tf": hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "No declaration found for \"var.foo\"", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 1, Column: 10, Byte: 10}, + }, + }, + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "No declaration found for \"var.wakka\"", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 10, Byte: 10}, + }, + }, + }, + }, + }, + } + + for i, tt := range tests { + t.Run(fmt.Sprintf("%2d-%s", i, tt.name), func(t *testing.T) { + ctx := context.Background() + + pathCtx := &decoder.PathContext{ + ReferenceOrigins: tt.origins, + } + + ctx = decoder.WithPathContext(ctx, pathCtx) + + diags := UnreferencedOrigins(ctx) + if diff := cmp.Diff(tt.want["test.tf"], diags["test.tf"]); diff != "" { + t.Fatalf("unexpected diagnostics: %s", diff) + } + }) + } +} diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index eb636bbf..74c68c33 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -135,6 +135,19 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand } ids = append(ids, refTargetsId) + _, 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()) + }, + Type: op.OpTypeEarlyValidation.String(), + DependsOn: job.IDs{metaId, refTargetsId}, + IgnoreState: ignoreState, + }) + if err != nil { + return ids, err + } + // This job may make an HTTP request, and we schedule it in // the low-priority queue, so we don't want to wait for it. _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ diff --git a/internal/langserver/handlers/command/validate.go b/internal/langserver/handlers/command/validate.go index 450e7a2b..8ad9d426 100644 --- a/internal/langserver/handlers/command/validate.go +++ b/internal/langserver/handlers/command/validate.go @@ -71,6 +71,7 @@ func (h *CmdHandler) TerraformValidateHandler(ctx context.Context, args cmd.Comm 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()) diff --git a/internal/langserver/handlers/hooks_module.go b/internal/langserver/handlers/hooks_module.go index f3669312..76ff0b7f 100644 --- a/internal/langserver/handlers/hooks_module.go +++ b/internal/langserver/handlers/hooks_module.go @@ -156,6 +156,7 @@ func updateDiagnostics(dNotifier *diagnostics.Notifier) notifier.Hook { 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()) } diff --git a/internal/state/module.go b/internal/state/module.go index 207f2c75..c9f3f986 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/go-memdb" "github.com/hashicorp/go-version" + "github.com/hashicorp/hcl-lang/lang" "github.com/hashicorp/hcl-lang/reference" "github.com/hashicorp/hcl/v2" tfaddr "github.com/hashicorp/terraform-registry-address" @@ -135,6 +136,9 @@ type Module struct { ModuleDiagnostics ast.ModDiags VarsDiagnostics ast.VarsDiags + + ValidationDiagnostics lang.DiagnosticsMap + ValidationDiagnosticsState op.OpState } func (m *Module) Copy() *Module { @@ -178,6 +182,8 @@ func (m *Module) Copy() *Module { ModuleParsingState: m.ModuleParsingState, VarsParsingState: m.VarsParsingState, + ValidationDiagnosticsState: m.ValidationDiagnosticsState, + Meta: m.Meta.Copy(), MetaErr: m.MetaErr, MetaState: m.MetaState, @@ -211,10 +217,15 @@ func (m *Module) Copy() *Module { newMod.ModuleDiagnostics = make(ast.ModDiags, len(m.ModuleDiagnostics)) for name, diags := range m.ModuleDiagnostics { newMod.ModuleDiagnostics[name] = make(hcl.Diagnostics, len(diags)) - for i, diag := range diags { - // hcl.Diagnostic is practically immutable once it comes out of parser - newMod.ModuleDiagnostics[name][i] = diag - } + copy(newMod.ModuleDiagnostics[name], diags) + } + } + + if m.ValidationDiagnostics != nil { + newMod.ValidationDiagnostics = make(lang.DiagnosticsMap, len(m.ValidationDiagnostics)) + for name, diags := range m.ValidationDiagnostics { + newMod.ValidationDiagnostics[name] = make(hcl.Diagnostics, len(diags)) + copy(newMod.ValidationDiagnostics[name], diags) } } @@ -222,10 +233,7 @@ func (m *Module) Copy() *Module { newMod.VarsDiagnostics = make(ast.VarsDiags, len(m.VarsDiagnostics)) for name, diags := range m.VarsDiagnostics { newMod.VarsDiagnostics[name] = make(hcl.Diagnostics, len(diags)) - for i, diag := range diags { - // hcl.Diagnostic is practically immutable once it comes out of parser - newMod.VarsDiagnostics[name][i] = diag - } + copy(newMod.VarsDiagnostics[name], diags) } } @@ -243,6 +251,7 @@ func newModule(modPath string) *Module { RefTargetsState: op.OpStateUnknown, ModuleParsingState: op.OpStateUnknown, MetaState: op.OpStateUnknown, + ValidationDiagnosticsState: op.OpStateUnknown, } } @@ -988,6 +997,54 @@ func (s *ModuleStore) UpdateModuleDiagnostics(path string, diags ast.ModDiags) e return nil } +func (s *ModuleStore) SetValidationDiagnosticsState(path string, state op.OpState) error { + txn := s.db.Txn(true) + defer txn.Abort() + + mod, err := moduleCopyByPath(txn, path) + if err != nil { + return err + } + + mod.ValidationDiagnosticsState = state + err = txn.Insert(s.tableName, mod) + if err != nil { + return err + } + + txn.Commit() + return nil +} + +func (s *ModuleStore) UpdateValidateDiagnostics(path string, diags lang.DiagnosticsMap) error { + txn := s.db.Txn(true) + txn.Defer(func() { + s.SetValidationDiagnosticsState(path, op.OpStateLoaded) + }) + defer txn.Abort() + + oldMod, err := moduleByPath(txn, path) + if err != nil { + return err + } + + mod := oldMod.Copy() + mod.ValidationDiagnostics = diags + + err = txn.Insert(s.tableName, mod) + if err != nil { + return err + } + + err = s.queueModuleChange(txn, oldMod, mod) + if err != nil { + return err + } + + txn.Commit() + return nil +} + func (s *ModuleStore) UpdateVarsDiagnostics(path string, diags ast.VarsDiags) error { txn := s.db.Txn(true) defer txn.Abort() diff --git a/internal/state/module_changes.go b/internal/state/module_changes.go index 7bac6b2a..c94e07d6 100644 --- a/internal/state/module_changes.go +++ b/internal/state/module_changes.go @@ -142,10 +142,10 @@ func (s *ModuleStore) queueModuleChange(txn *memdb.Txn, oldMod, newMod *Module) oldDiags, newDiags := 0, 0 if oldMod != nil { - oldDiags = oldMod.ModuleDiagnostics.Count() + oldMod.VarsDiagnostics.Count() + oldDiags = oldMod.ModuleDiagnostics.Count() + oldMod.VarsDiagnostics.Count() + oldMod.ValidationDiagnostics.Count() } if newMod != nil { - newDiags = newMod.ModuleDiagnostics.Count() + newMod.VarsDiagnostics.Count() + newDiags = newMod.ModuleDiagnostics.Count() + newMod.VarsDiagnostics.Count() + newMod.ValidationDiagnostics.Count() } // Comparing diagnostics accurately could be expensive // so we just treat any non-empty diags as a change diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 42d9e1cd..0b2288bf 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -699,6 +699,45 @@ func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, sche return rErr } +func EarlyValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if mod.ValidationDiagnosticsState != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetValidationDiagnosticsState(modPath, op.OpStateLoading) + if err != nil { + return err + } + + d := decoder.NewDecoder(&idecoder.PathReader{ + ModuleReader: modStore, + SchemaReader: schemaReader, + }) + d.SetContext(idecoder.DecoderContext(ctx)) + + moduleDecoder, err := d.Path(lang.Path{ + Path: modPath, + LanguageID: ilsp.Terraform.String(), + }) + if err != nil { + return err + } + + diags, rErr := moduleDecoder.Validate(ctx) + sErr := modStore.UpdateValidateDiagnostics(modPath, diags) + if sErr != nil { + return sErr + } + + return rErr +} + func GetModuleDataFromRegistry(ctx context.Context, regClient registry.Client, modStore *state.ModuleStore, modRegStore *state.RegistryModuleStore, modPath string) error { // loop over module calls calls, err := modStore.ModuleCalls(modPath) diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index c7271afb..a5370746 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -21,11 +21,12 @@ func _() { _ = x[OpTypeGetModuleDataFromRegistry-10] _ = x[OpTypeParseProviderVersions-11] _ = x[OpTypePreloadEmbeddedSchema-12] + _ = x[OpTypeEarlyValidation-13] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchema" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeEarlyValidation" -var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322} +var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 343} func (i OpType) String() string { if i >= OpType(len(_OpType_index)-1) { diff --git a/internal/terraform/module/operation/operation.go b/internal/terraform/module/operation/operation.go index 5a1d1e3d..908d04ea 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -30,4 +30,5 @@ const ( OpTypeGetModuleDataFromRegistry OpTypeParseProviderVersions OpTypePreloadEmbeddedSchema + OpTypeEarlyValidation ) From b0211911a5682140cb87d6b8ba241d4eeddb9ec5 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Tue, 29 Aug 2023 11:55:37 +0200 Subject: [PATCH 03/15] Centralize diagnostics publishing (#1361) * 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 --- internal/decoder/decoder.go | 7 - .../validations/unreferenced_origin.go | 7 +- .../validations/unreferenced_origin_test.go | 4 +- internal/indexer/document_change.go | 19 +- .../langserver/diagnostics/diagnostics.go | 15 +- .../diagnostics/diagnostics_test.go | 23 +- .../langserver/handlers/command/validate.go | 59 +---- internal/langserver/handlers/hooks_module.go | 13 +- internal/state/module.go | 157 +++++-------- internal/state/module_changes.go | 4 +- internal/state/module_test.go | 212 ++++++++++++++---- internal/terraform/ast/diagnostics.go | 46 ++++ internal/terraform/ast/module.go | 10 + internal/terraform/ast/variables.go | 10 + internal/terraform/module/module_ops.go | 99 ++++++-- internal/terraform/module/module_ops_test.go | 7 +- .../module/operation/op_type_string.go | 8 +- .../terraform/module/operation/operation.go | 4 +- 18 files changed, 445 insertions(+), 259 deletions(-) create mode 100644 internal/terraform/ast/diagnostics.go diff --git a/internal/decoder/decoder.go b/internal/decoder/decoder.go index 74247e4e..e951e7cf 100644 --- a/internal/decoder/decoder.go +++ b/internal/decoder/decoder.go @@ -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" @@ -93,10 +91,5 @@ func DecoderContext(ctx context.Context) decoder.DecoderContext { } } - validations := []lang.ValidationFunc{ - validations.UnreferencedOrigins, - } - dCtx.Validations = append(dCtx.Validations, validations...) - return dCtx } diff --git a/internal/decoder/validations/unreferenced_origin.go b/internal/decoder/validations/unreferenced_origin.go index e8871153..ad5f550d 100644 --- a/internal/decoder/validations/unreferenced_origin.go +++ b/internal/decoder/validations/unreferenced_origin.go @@ -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 { diff --git a/internal/decoder/validations/unreferenced_origin_test.go b/internal/decoder/validations/unreferenced_origin_test.go index 420d4f24..f36b6122 100644 --- a/internal/decoder/validations/unreferenced_origin_test.go +++ b/internal/decoder/validations/unreferenced_origin_test.go @@ -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) } diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index 74c68c33..500dcfee 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -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 { @@ -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, }) diff --git a/internal/langserver/diagnostics/diagnostics.go b/internal/langserver/diagnostics/diagnostics.go index 163f6214..b8fb2e8e 100644 --- a/internal/langserver/diagnostics/diagnostics.go +++ b/internal/langserver/diagnostics/diagnostics.go @@ -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" ) @@ -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 } @@ -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{ @@ -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) @@ -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 diff --git a/internal/langserver/diagnostics/diagnostics_test.go b/internal/langserver/diagnostics/diagnostics_test.go index 4a91f5a7..d3c95b68 100644 --- a/internal/langserver/diagnostics/diagnostics_test.go +++ b/internal/langserver/diagnostics/diagnostics_test.go @@ -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) @@ -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, }, @@ -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, }, @@ -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, @@ -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, @@ -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", @@ -113,7 +114,7 @@ func TestDiagnostics_Append(t *testing.T) { }, }, }, - DiagnosticSource("bar"): { + ast.SchemaValidationSource: { &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Something else went wrong", @@ -121,8 +122,8 @@ func TestDiagnostics_Append(t *testing.T) { }, }, }, - "second.tf": map[DiagnosticSource]hcl.Diagnostics{ - DiagnosticSource("bar"): { + "second.tf": map[ast.DiagnosticSource]hcl.Diagnostics{ + ast.SchemaValidationSource: { &hcl.Diagnostic{ Severity: hcl.DiagWarning, Summary: "Beware", diff --git a/internal/langserver/handlers/command/validate.go b/internal/langserver/handlers/command/validate.go index 8ad9d426..5b61e223 100644 --- a/internal/langserver/handlers/command/validate.go +++ b/internal/langserver/handlers/command/validate.go @@ -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" ) @@ -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) } diff --git a/internal/langserver/handlers/hooks_module.go b/internal/langserver/handlers/hooks_module.go index 76ff0b7f..a1bb6750 100644 --- a/internal/langserver/handlers/hooks_module.go +++ b/internal/langserver/handlers/hooks_module.go @@ -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 } diff --git a/internal/state/module.go b/internal/state/module.go index c9f3f986..b06765b3 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -9,7 +9,6 @@ import ( "github.com/hashicorp/go-memdb" "github.com/hashicorp/go-version" - "github.com/hashicorp/hcl-lang/lang" "github.com/hashicorp/hcl-lang/reference" "github.com/hashicorp/hcl/v2" tfaddr "github.com/hashicorp/terraform-registry-address" @@ -123,22 +122,19 @@ type Module struct { VarsRefOriginsErr error VarsRefOriginsState op.OpState - ParsedModuleFiles ast.ModFiles - ParsedVarsFiles ast.VarsFiles - ModuleParsingErr error - VarsParsingErr error - ModuleParsingState op.OpState - VarsParsingState op.OpState + ParsedModuleFiles ast.ModFiles + ParsedVarsFiles ast.VarsFiles + ModuleParsingErr error + VarsParsingErr error Meta ModuleMetadata MetaErr error MetaState op.OpState - ModuleDiagnostics ast.ModDiags - VarsDiagnostics ast.VarsDiags - - ValidationDiagnostics lang.DiagnosticsMap - ValidationDiagnosticsState op.OpState + ModuleDiagnostics ast.SourceModDiags + ModuleDiagnosticsState ast.DiagnosticSourceState + VarsDiagnostics ast.SourceVarsDiags + VarsDiagnosticsState ast.DiagnosticSourceState } func (m *Module) Copy() *Module { @@ -177,16 +173,15 @@ func (m *Module) Copy() *Module { VarsRefOriginsErr: m.VarsRefOriginsErr, VarsRefOriginsState: m.VarsRefOriginsState, - ModuleParsingErr: m.ModuleParsingErr, - VarsParsingErr: m.VarsParsingErr, - ModuleParsingState: m.ModuleParsingState, - VarsParsingState: m.VarsParsingState, - - ValidationDiagnosticsState: m.ValidationDiagnosticsState, + ModuleParsingErr: m.ModuleParsingErr, + VarsParsingErr: m.VarsParsingErr, Meta: m.Meta.Copy(), MetaErr: m.MetaErr, MetaState: m.MetaState, + + ModuleDiagnosticsState: m.ModuleDiagnosticsState.Copy(), + VarsDiagnosticsState: m.VarsDiagnosticsState.Copy(), } if m.InstalledProviders != nil { @@ -214,26 +209,28 @@ func (m *Module) Copy() *Module { } if m.ModuleDiagnostics != nil { - newMod.ModuleDiagnostics = make(ast.ModDiags, len(m.ModuleDiagnostics)) - for name, diags := range m.ModuleDiagnostics { - newMod.ModuleDiagnostics[name] = make(hcl.Diagnostics, len(diags)) - copy(newMod.ModuleDiagnostics[name], diags) - } - } + newMod.ModuleDiagnostics = make(ast.SourceModDiags, len(m.ModuleDiagnostics)) + + for source, modDiags := range m.ModuleDiagnostics { + newMod.ModuleDiagnostics[source] = make(ast.ModDiags, len(modDiags)) - if m.ValidationDiagnostics != nil { - newMod.ValidationDiagnostics = make(lang.DiagnosticsMap, len(m.ValidationDiagnostics)) - for name, diags := range m.ValidationDiagnostics { - newMod.ValidationDiagnostics[name] = make(hcl.Diagnostics, len(diags)) - copy(newMod.ValidationDiagnostics[name], diags) + for name, diags := range modDiags { + newMod.ModuleDiagnostics[source][name] = make(hcl.Diagnostics, len(diags)) + copy(newMod.ModuleDiagnostics[source][name], diags) + } } } if m.VarsDiagnostics != nil { - newMod.VarsDiagnostics = make(ast.VarsDiags, len(m.VarsDiagnostics)) - for name, diags := range m.VarsDiagnostics { - newMod.VarsDiagnostics[name] = make(hcl.Diagnostics, len(diags)) - copy(newMod.VarsDiagnostics[name], diags) + newMod.VarsDiagnostics = make(ast.SourceVarsDiags, len(m.VarsDiagnostics)) + + for source, varsDiags := range m.VarsDiagnostics { + newMod.VarsDiagnostics[source] = make(ast.VarsDiags, len(varsDiags)) + + for name, diags := range varsDiags { + newMod.VarsDiagnostics[source][name] = make(hcl.Diagnostics, len(diags)) + copy(newMod.VarsDiagnostics[source][name], diags) + } } } @@ -249,9 +246,19 @@ func newModule(modPath string) *Module { PreloadEmbeddedSchemaState: op.OpStateUnknown, InstalledProvidersState: op.OpStateUnknown, RefTargetsState: op.OpStateUnknown, - ModuleParsingState: op.OpStateUnknown, MetaState: op.OpStateUnknown, - ValidationDiagnosticsState: op.OpStateUnknown, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: op.OpStateUnknown, + ast.SchemaValidationSource: op.OpStateUnknown, + ast.ReferenceValidationSource: op.OpStateUnknown, + ast.TerraformValidateSource: op.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: op.OpStateUnknown, + ast.SchemaValidationSource: op.OpStateUnknown, + ast.ReferenceValidationSource: op.OpStateUnknown, + ast.TerraformValidateSource: op.OpStateUnknown, + }, } } @@ -824,49 +831,8 @@ func (s *ModuleStore) UpdateTerraformAndProviderVersions(modPath string, tfVer * return nil } -func (s *ModuleStore) SetModuleParsingState(path string, state op.OpState) error { - txn := s.db.Txn(true) - defer txn.Abort() - - mod, err := moduleCopyByPath(txn, path) - if err != nil { - return err - } - - mod.ModuleParsingState = state - err = txn.Insert(s.tableName, mod) - if err != nil { - return err - } - - txn.Commit() - return nil -} - -func (s *ModuleStore) SetVarsParsingState(path string, state op.OpState) error { - txn := s.db.Txn(true) - defer txn.Abort() - - mod, err := moduleCopyByPath(txn, path) - if err != nil { - return err - } - - mod.VarsParsingState = state - err = txn.Insert(s.tableName, mod) - if err != nil { - return err - } - - txn.Commit() - return nil -} - func (s *ModuleStore) UpdateParsedModuleFiles(path string, pFiles ast.ModFiles, pErr error) error { txn := s.db.Txn(true) - txn.Defer(func() { - s.SetModuleParsingState(path, op.OpStateLoaded) - }) defer txn.Abort() mod, err := moduleCopyByPath(txn, path) @@ -889,9 +855,6 @@ func (s *ModuleStore) UpdateParsedModuleFiles(path string, pFiles ast.ModFiles, func (s *ModuleStore) UpdateParsedVarsFiles(path string, vFiles ast.VarsFiles, vErr error) error { txn := s.db.Txn(true) - txn.Defer(func() { - s.SetVarsParsingState(path, op.OpStateLoaded) - }) defer txn.Abort() mod, err := moduleCopyByPath(txn, path) @@ -971,8 +934,11 @@ func (s *ModuleStore) UpdateMetadata(path string, meta *tfmod.Meta, mErr error) return nil } -func (s *ModuleStore) UpdateModuleDiagnostics(path string, diags ast.ModDiags) error { +func (s *ModuleStore) UpdateModuleDiagnostics(path string, source ast.DiagnosticSource, diags ast.ModDiags) error { txn := s.db.Txn(true) + txn.Defer(func() { + s.SetModuleDiagnosticsState(path, source, op.OpStateLoaded) + }) defer txn.Abort() oldMod, err := moduleByPath(txn, path) @@ -981,7 +947,10 @@ func (s *ModuleStore) UpdateModuleDiagnostics(path string, diags ast.ModDiags) e } mod := oldMod.Copy() - mod.ModuleDiagnostics = diags + if mod.ModuleDiagnostics == nil { + mod.ModuleDiagnostics = make(ast.SourceModDiags) + } + mod.ModuleDiagnostics[source] = diags err = txn.Insert(s.tableName, mod) if err != nil { @@ -997,7 +966,7 @@ func (s *ModuleStore) UpdateModuleDiagnostics(path string, diags ast.ModDiags) e return nil } -func (s *ModuleStore) SetValidationDiagnosticsState(path string, state op.OpState) error { +func (s *ModuleStore) SetModuleDiagnosticsState(path string, source ast.DiagnosticSource, state op.OpState) error { txn := s.db.Txn(true) defer txn.Abort() @@ -1005,8 +974,8 @@ func (s *ModuleStore) SetValidationDiagnosticsState(path string, state op.OpStat if err != nil { return err } + mod.ModuleDiagnosticsState[source] = state - mod.ValidationDiagnosticsState = state err = txn.Insert(s.tableName, mod) if err != nil { return err @@ -1016,10 +985,10 @@ func (s *ModuleStore) SetValidationDiagnosticsState(path string, state op.OpStat return nil } -func (s *ModuleStore) UpdateValidateDiagnostics(path string, diags lang.DiagnosticsMap) error { +func (s *ModuleStore) UpdateVarsDiagnostics(path string, source ast.DiagnosticSource, diags ast.VarsDiags) error { txn := s.db.Txn(true) txn.Defer(func() { - s.SetValidationDiagnosticsState(path, op.OpStateLoaded) + s.SetVarsDiagnosticsState(path, ast.HCLParsingSource, op.OpStateLoaded) }) defer txn.Abort() @@ -1029,7 +998,10 @@ func (s *ModuleStore) UpdateValidateDiagnostics(path string, diags lang.Diagnost } mod := oldMod.Copy() - mod.ValidationDiagnostics = diags + if mod.VarsDiagnostics == nil { + mod.VarsDiagnostics = make(ast.SourceVarsDiags) + } + mod.VarsDiagnostics[source] = diags err = txn.Insert(s.tableName, mod) if err != nil { @@ -1045,28 +1017,21 @@ func (s *ModuleStore) UpdateValidateDiagnostics(path string, diags lang.Diagnost return nil } -func (s *ModuleStore) UpdateVarsDiagnostics(path string, diags ast.VarsDiags) error { +func (s *ModuleStore) SetVarsDiagnosticsState(path string, source ast.DiagnosticSource, state op.OpState) error { txn := s.db.Txn(true) defer txn.Abort() - oldMod, err := moduleByPath(txn, path) + mod, err := moduleCopyByPath(txn, path) if err != nil { return err } - - mod := oldMod.Copy() - mod.VarsDiagnostics = diags + mod.VarsDiagnosticsState[source] = state err = txn.Insert(s.tableName, mod) if err != nil { return err } - err = s.queueModuleChange(txn, oldMod, mod) - if err != nil { - return err - } - txn.Commit() return nil } diff --git a/internal/state/module_changes.go b/internal/state/module_changes.go index c94e07d6..7bac6b2a 100644 --- a/internal/state/module_changes.go +++ b/internal/state/module_changes.go @@ -142,10 +142,10 @@ func (s *ModuleStore) queueModuleChange(txn *memdb.Txn, oldMod, newMod *Module) oldDiags, newDiags := 0, 0 if oldMod != nil { - oldDiags = oldMod.ModuleDiagnostics.Count() + oldMod.VarsDiagnostics.Count() + oldMod.ValidationDiagnostics.Count() + oldDiags = oldMod.ModuleDiagnostics.Count() + oldMod.VarsDiagnostics.Count() } if newMod != nil { - newDiags = newMod.ModuleDiagnostics.Count() + newMod.VarsDiagnostics.Count() + newMod.ValidationDiagnostics.Count() + newDiags = newMod.ModuleDiagnostics.Count() + newMod.VarsDiagnostics.Count() } // Comparing diagnostics accurately could be expensive // so we just treat any non-empty diags as a change diff --git a/internal/state/module_test.go b/internal/state/module_test.go index a45bb1c2..83b9e22b 100644 --- a/internal/state/module_test.go +++ b/internal/state/module_test.go @@ -77,6 +77,18 @@ func TestModuleStore_ModuleByPath(t *testing.T) { Path: modPath, TerraformVersion: tfVersion, TerraformVersionState: operation.OpStateLoaded, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, } if diff := cmp.Diff(expectedModule, mod); diff != "" { t.Fatalf("unexpected module: %s", diff) @@ -184,11 +196,35 @@ func TestModuleStore_CallersOfModule(t *testing.T) { Path: filepath.Join(tmpDir, "alpha"), ModManifest: alphaManifest, ModManifestState: operation.OpStateLoaded, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, }, { Path: filepath.Join(tmpDir, "gamma"), ModManifest: gammaManifest, ModManifestState: operation.OpStateLoaded, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, }, } @@ -223,9 +259,51 @@ func TestModuleStore_List(t *testing.T) { } expectedModules := []*Module{ - {Path: filepath.Join(tmpDir, "alpha")}, - {Path: filepath.Join(tmpDir, "beta")}, - {Path: filepath.Join(tmpDir, "gamma")}, + { + Path: filepath.Join(tmpDir, "alpha"), + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + }, + { + Path: filepath.Join(tmpDir, "beta"), + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + }, + { + Path: filepath.Join(tmpDir, "gamma"), + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + }, } if diff := cmp.Diff(expectedModules, modules, cmpOpts); diff != "" { @@ -283,6 +361,18 @@ func TestModuleStore_UpdateMetadata(t *testing.T) { }, }, MetaState: operation.OpStateLoaded, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, } if diff := cmp.Diff(expectedModule, mod, cmpOpts); diff != "" { @@ -319,6 +409,18 @@ func TestModuleStore_UpdateTerraformAndProviderVersions(t *testing.T) { TerraformVersion: testVersion(t, "0.12.4"), TerraformVersionState: operation.OpStateLoaded, TerraformVersionErr: vErr, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, + VarsDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateUnknown, + ast.SchemaValidationSource: operation.OpStateUnknown, + ast.ReferenceValidationSource: operation.OpStateUnknown, + ast.TerraformValidateSource: operation.OpStateUnknown, + }, } if diff := cmp.Diff(expectedModule, mod, cmpOpts); diff != "" { t.Fatalf("unexpected module data: %s", diff) @@ -427,37 +529,42 @@ provider "blah" { region = "london" `), "test.tf") - err = s.Modules.UpdateModuleDiagnostics(tmpDir, ast.ModDiagsFromMap(map[string]hcl.Diagnostics{ + err = s.Modules.UpdateModuleDiagnostics(tmpDir, ast.HCLParsingSource, ast.ModDiagsFromMap(map[string]hcl.Diagnostics{ "test.tf": diags, })) + if err != nil { + t.Fatal(err) + } mod, err := s.Modules.ModuleByPath(tmpDir) if err != nil { t.Fatal(err) } - expectedDiags := ast.ModDiagsFromMap(map[string]hcl.Diagnostics{ - "test.tf": { - { - Severity: hcl.DiagError, - Summary: "Unclosed configuration block", - Detail: "There is no closing brace for this block before the end of the file. This may be caused by incorrect brace nesting elsewhere in this file.", - Subject: &hcl.Range{ - Filename: "test.tf", - Start: hcl.Pos{ - Line: 2, - Column: 17, - Byte: 17, - }, - End: hcl.Pos{ - Line: 2, - Column: 18, - Byte: 18, + expectedDiags := ast.SourceModDiags{ + ast.HCLParsingSource: ast.ModDiagsFromMap(map[string]hcl.Diagnostics{ + "test.tf": { + { + Severity: hcl.DiagError, + Summary: "Unclosed configuration block", + Detail: "There is no closing brace for this block before the end of the file. This may be caused by incorrect brace nesting elsewhere in this file.", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{ + Line: 2, + Column: 17, + Byte: 17, + }, + End: hcl.Pos{ + Line: 2, + Column: 18, + Byte: 18, + }, }, }, }, - }, - }) + }), + } if diff := cmp.Diff(expectedDiags, mod.ModuleDiagnostics, cmpOpts); diff != "" { t.Fatalf("unexpected diagnostics: %s", diff) } @@ -481,37 +588,42 @@ dev = { region = "london" `), "test.tfvars") - err = s.Modules.UpdateVarsDiagnostics(tmpDir, ast.VarsDiagsFromMap(map[string]hcl.Diagnostics{ + err = s.Modules.UpdateVarsDiagnostics(tmpDir, ast.HCLParsingSource, ast.VarsDiagsFromMap(map[string]hcl.Diagnostics{ "test.tfvars": diags, })) + if err != nil { + t.Fatal(err) + } mod, err := s.Modules.ModuleByPath(tmpDir) if err != nil { t.Fatal(err) } - expectedDiags := ast.VarsDiagsFromMap(map[string]hcl.Diagnostics{ - "test.tfvars": { - { - Severity: hcl.DiagError, - Summary: "Missing expression", - Detail: "Expected the start of an expression, but found the end of the file.", - Subject: &hcl.Range{ - Filename: "test.tfvars", - Start: hcl.Pos{ - Line: 4, - Column: 1, - Byte: 29, - }, - End: hcl.Pos{ - Line: 4, - Column: 1, - Byte: 29, + expectedDiags := ast.SourceVarsDiags{ + ast.HCLParsingSource: ast.VarsDiagsFromMap(map[string]hcl.Diagnostics{ + "test.tfvars": { + { + Severity: hcl.DiagError, + Summary: "Missing expression", + Detail: "Expected the start of an expression, but found the end of the file.", + Subject: &hcl.Range{ + Filename: "test.tfvars", + Start: hcl.Pos{ + Line: 4, + Column: 1, + Byte: 29, + }, + End: hcl.Pos{ + Line: 4, + Column: 1, + Byte: 29, + }, }, }, }, - }, - }) + }), + } if diff := cmp.Diff(expectedDiags, mod.VarsDiagnostics, cmpOpts); diff != "" { t.Fatalf("unexpected diagnostics: %s", diff) } @@ -732,16 +844,20 @@ func BenchmarkModuleByPath(b *testing.B) { b.Fatal(err) } mDiags := ast.ModDiagsFromMap(diags) - err = s.Modules.UpdateModuleDiagnostics(modPath, mDiags) + err = s.Modules.UpdateModuleDiagnostics(modPath, ast.HCLParsingSource, mDiags) if err != nil { b.Fatal(err) } expectedMod := &Module{ - Path: modPath, - ParsedModuleFiles: mFiles, - ModuleParsingState: operation.OpStateLoaded, - ModuleDiagnostics: mDiags, + Path: modPath, + ParsedModuleFiles: mFiles, + ModuleDiagnostics: ast.SourceModDiags{ + ast.HCLParsingSource: mDiags, + }, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateLoaded, + }, } for n := 0; n < b.N; n++ { diff --git a/internal/terraform/ast/diagnostics.go b/internal/terraform/ast/diagnostics.go new file mode 100644 index 00000000..fa8ecb59 --- /dev/null +++ b/internal/terraform/ast/diagnostics.go @@ -0,0 +1,46 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ast + +import ( + "fmt" + + op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" +) + +// DiagnosticSource differentiates different sources of diagnostics. +type DiagnosticSource int + +const ( + HCLParsingSource DiagnosticSource = iota + SchemaValidationSource + ReferenceValidationSource + TerraformValidateSource +) + +func (d DiagnosticSource) String() string { + switch d { + case HCLParsingSource: + return "HCL" + case SchemaValidationSource: + return "early validation" + case ReferenceValidationSource: + return "early validation" + case TerraformValidateSource: + return "terraform validate" + default: + panic(fmt.Sprintf("Unknown diagnostic source %d", d)) + } +} + +type DiagnosticSourceState map[DiagnosticSource]op.OpState + +func (dss DiagnosticSourceState) Copy() DiagnosticSourceState { + newDiagnosticSourceState := make(DiagnosticSourceState, len(dss)) + for source, state := range dss { + newDiagnosticSourceState[source] = state + } + + return newDiagnosticSourceState +} diff --git a/internal/terraform/ast/module.go b/internal/terraform/ast/module.go index 3834321c..14751612 100644 --- a/internal/terraform/ast/module.go +++ b/internal/terraform/ast/module.go @@ -106,3 +106,13 @@ func (md ModDiags) Count() int { } return count } + +type SourceModDiags map[DiagnosticSource]ModDiags + +func (smd SourceModDiags) Count() int { + count := 0 + for _, diags := range smd { + count += diags.Count() + } + return count +} diff --git a/internal/terraform/ast/variables.go b/internal/terraform/ast/variables.go index a2f6aaf9..3486ba4e 100644 --- a/internal/terraform/ast/variables.go +++ b/internal/terraform/ast/variables.go @@ -87,3 +87,13 @@ func (vd VarsDiags) Count() int { } return count } + +type SourceVarsDiags map[DiagnosticSource]VarsDiags + +func (svd SourceVarsDiags) Count() int { + count := 0 + for _, diags := range svd { + count += diags.Count() + } + return count +} diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 0b2288bf..d3aa0237 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -21,8 +21,10 @@ import ( tfjson "github.com/hashicorp/terraform-json" lsctx "github.com/hashicorp/terraform-ls/internal/context" idecoder "github.com/hashicorp/terraform-ls/internal/decoder" + "github.com/hashicorp/terraform-ls/internal/decoder/validations" "github.com/hashicorp/terraform-ls/internal/document" "github.com/hashicorp/terraform-ls/internal/job" + "github.com/hashicorp/terraform-ls/internal/langserver/diagnostics" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" "github.com/hashicorp/terraform-ls/internal/registry" "github.com/hashicorp/terraform-ls/internal/schemas" @@ -34,7 +36,7 @@ import ( "github.com/hashicorp/terraform-ls/internal/uri" tfaddr "github.com/hashicorp/terraform-registry-address" "github.com/hashicorp/terraform-schema/earlydecoder" - "github.com/hashicorp/terraform-schema/module" + tfmodule "github.com/hashicorp/terraform-schema/module" tfregistry "github.com/hashicorp/terraform-schema/registry" tfschema "github.com/hashicorp/terraform-schema/schema" "github.com/zclconf/go-cty/cty" @@ -364,7 +366,7 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat // TODO: Avoid parsing if the content matches existing AST // Avoid parsing if it is already in progress or already known - if mod.ModuleParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) { + if mod.ModuleDiagnosticsState[ast.HCLParsingSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } @@ -372,9 +374,9 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat var diags ast.ModDiags rpcContext := lsctx.RPCContext(ctx) // Only parse the file that's being changed/opened, unless this is 1st-time parsing - if mod.ModuleParsingState == op.OpStateLoaded && rpcContext.IsDidChangeRequest() && lsctx.IsLanguageId(ctx, ilsp.Terraform.String()) { + if mod.ModuleDiagnosticsState[ast.HCLParsingSource] == op.OpStateLoaded && rpcContext.IsDidChangeRequest() && lsctx.IsLanguageId(ctx, ilsp.Terraform.String()) { // the file has already been parsed, so only examine this file and not the whole module - err = modStore.SetModuleParsingState(modPath, op.OpStateLoading) + err = modStore.SetModuleDiagnosticsState(modPath, ast.HCLParsingSource, op.OpStateLoading) if err != nil { return err } @@ -393,12 +395,17 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat existingFiles[ast.ModFilename(fileName)] = f files = existingFiles - existingDiags := mod.ModuleDiagnostics.Copy() + existingDiags, ok := mod.ModuleDiagnostics[ast.HCLParsingSource] + if !ok { + existingDiags = make(ast.ModDiags) + } else { + existingDiags = existingDiags.Copy() + } existingDiags[ast.ModFilename(fileName)] = fDiags diags = existingDiags } else { // this is the first time file is opened so parse the whole module - err = modStore.SetModuleParsingState(modPath, op.OpStateLoading) + err = modStore.SetModuleDiagnosticsState(modPath, ast.HCLParsingSource, op.OpStateLoading) if err != nil { return err } @@ -415,7 +422,7 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat return sErr } - sErr = modStore.UpdateModuleDiagnostics(modPath, diags) + sErr = modStore.UpdateModuleDiagnostics(modPath, ast.HCLParsingSource, diags) if sErr != nil { return sErr } @@ -434,11 +441,11 @@ func ParseVariables(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleSt // TODO: Only parse the file that's being changed/opened, unless this is 1st-time parsing // Avoid parsing if it is already in progress or already known - if mod.VarsParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) { + if mod.VarsDiagnosticsState[ast.HCLParsingSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } - err = modStore.SetVarsParsingState(modPath, op.OpStateLoading) + err = modStore.SetVarsDiagnosticsState(modPath, ast.HCLParsingSource, op.OpStateLoading) if err != nil { return err } @@ -450,7 +457,7 @@ func ParseVariables(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleSt return sErr } - sErr = modStore.UpdateVarsDiagnostics(modPath, diags) + sErr = modStore.UpdateVarsDiagnostics(modPath, ast.HCLParsingSource, diags) if sErr != nil { return sErr } @@ -559,7 +566,7 @@ func LoadModuleMetadata(ctx context.Context, modStore *state.ModuleStore, modPat } meta.ProviderRequirements = providerRequirements - providerRefs := make(map[module.ProviderRef]tfaddr.Provider, len(meta.ProviderReferences)) + providerRefs := make(map[tfmodule.ProviderRef]tfaddr.Provider, len(meta.ProviderReferences)) for localRef, pAddr := range meta.ProviderReferences { // TODO: check pAddr for migrations via Registry API? providerRefs[localRef] = pAddr @@ -699,18 +706,18 @@ func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, sche return rErr } -func EarlyValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { +func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { mod, err := modStore.ModuleByPath(modPath) if err != nil { return err } // Avoid validation if it is already in progress or already finished - if mod.ValidationDiagnosticsState != op.OpStateUnknown && !job.IgnoreState(ctx) { + if mod.ModuleDiagnosticsState[ast.SchemaValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } - err = modStore.SetValidationDiagnosticsState(modPath, op.OpStateLoading) + err = modStore.SetModuleDiagnosticsState(modPath, ast.SchemaValidationSource, op.OpStateLoading) if err != nil { return err } @@ -730,7 +737,7 @@ func EarlyValidation(ctx context.Context, modStore *state.ModuleStore, schemaRea } diags, rErr := moduleDecoder.Validate(ctx) - sErr := modStore.UpdateValidateDiagnostics(modPath, diags) + sErr := modStore.UpdateModuleDiagnostics(modPath, ast.SchemaValidationSource, ast.ModDiagsFromMap(diags)) if sErr != nil { return sErr } @@ -738,6 +745,68 @@ func EarlyValidation(ctx context.Context, modStore *state.ModuleStore, schemaRea return rErr } +func ReferenceValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if mod.ModuleDiagnosticsState[ast.ReferenceValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetModuleDiagnosticsState(modPath, ast.ReferenceValidationSource, op.OpStateLoading) + if err != nil { + return err + } + + pathReader := &idecoder.PathReader{ + ModuleReader: modStore, + SchemaReader: schemaReader, + } + pathCtx, err := pathReader.PathContext(lang.Path{ + Path: modPath, + LanguageID: ilsp.Terraform.String(), + }) + if err != nil { + return err + } + + diags := validations.UnreferencedOrigins(ctx, pathCtx) + return modStore.UpdateModuleDiagnostics(modPath, ast.ReferenceValidationSource, ast.ModDiagsFromMap(diags)) +} + +func TerraformValidate(ctx context.Context, modStore *state.ModuleStore, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if mod.ModuleDiagnosticsState[ast.TerraformValidateSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetModuleDiagnosticsState(modPath, ast.TerraformValidateSource, op.OpStateLoading) + if err != nil { + return err + } + + tfExec, err := TerraformExecutorForModule(ctx, mod.Path) + if err != nil { + return err + } + + jsonDiags, err := tfExec.Validate(ctx) + if err != nil { + return err + } + validateDiags := diagnostics.HCLDiagsFromJSON(jsonDiags) + + return modStore.UpdateModuleDiagnostics(modPath, ast.TerraformValidateSource, ast.ModDiagsFromMap(validateDiags)) +} + func GetModuleDataFromRegistry(ctx context.Context, regClient registry.Client, modStore *state.ModuleStore, modRegStore *state.RegistryModuleStore, modPath string) error { // loop over module calls calls, err := modStore.ModuleCalls(modPath) diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index 3bb0486f..9a84dfda 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -30,6 +30,7 @@ import ( ilsp "github.com/hashicorp/terraform-ls/internal/lsp" "github.com/hashicorp/terraform-ls/internal/registry" "github.com/hashicorp/terraform-ls/internal/state" + "github.com/hashicorp/terraform-ls/internal/terraform/ast" "github.com/hashicorp/terraform-ls/internal/terraform/exec" "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" "github.com/hashicorp/terraform-ls/internal/uri" @@ -1008,12 +1009,12 @@ func TestParseModuleConfiguration(t *testing.T) { } // examine diags should change for foo.tf - if before.ModuleDiagnostics["foo.tf"][0] == after.ModuleDiagnostics["foo.tf"][0] { + if before.ModuleDiagnostics[ast.HCLParsingSource]["foo.tf"][0] == after.ModuleDiagnostics[ast.HCLParsingSource]["foo.tf"][0] { t.Fatal("diags should mismatch") } // examine diags should change for main.tf - if before.ModuleDiagnostics["main.tf"][0] != after.ModuleDiagnostics["main.tf"][0] { + if before.ModuleDiagnostics[ast.HCLParsingSource]["main.tf"][0] != after.ModuleDiagnostics[ast.HCLParsingSource]["main.tf"][0] { t.Fatal("diags should match") } } @@ -1082,7 +1083,7 @@ func TestParseModuleConfiguration_ignore_tfvars(t *testing.T) { } // diags should be missing for example.tfvars - if _, ok := before.ModuleDiagnostics["example.tfvars"]; ok { + if _, ok := before.ModuleDiagnostics[ast.HCLParsingSource]["example.tfvars"]; ok { t.Fatal("there should be no diags for tfvars files right now") } } diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index a5370746..5b4ac874 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -21,12 +21,14 @@ func _() { _ = x[OpTypeGetModuleDataFromRegistry-10] _ = x[OpTypeParseProviderVersions-11] _ = x[OpTypePreloadEmbeddedSchema-12] - _ = x[OpTypeEarlyValidation-13] + _ = x[OpTypeSchemaValidation-13] + _ = x[OpTypeReferenceValidation-14] + _ = x[OpTypeTerraformValidate-15] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeEarlyValidation" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeSchemaValidationOpTypeReferenceValidationOpTypeTerraformValidate" -var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 343} +var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 344, 369, 392} func (i OpType) String() string { if i >= OpType(len(_OpType_index)-1) { diff --git a/internal/terraform/module/operation/operation.go b/internal/terraform/module/operation/operation.go index 908d04ea..fe408450 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -30,5 +30,7 @@ const ( OpTypeGetModuleDataFromRegistry OpTypeParseProviderVersions OpTypePreloadEmbeddedSchema - OpTypeEarlyValidation + OpTypeSchemaValidation + OpTypeReferenceValidation + OpTypeTerraformValidate ) From 00028de087deaabfe75e58891a39a0700dc272e1 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Tue, 29 Aug 2023 12:02:27 +0200 Subject: [PATCH 04/15] Check early validation setting before scheduling jobs (#1363) * Introduce new context for valiation settings * Check context before scheduling jobs --- internal/context/context.go | 27 ++++++++++- internal/indexer/document_change.go | 52 +++++++++++++--------- internal/langserver/handlers/initialize.go | 4 ++ internal/langserver/handlers/service.go | 7 +++ 4 files changed, 66 insertions(+), 24 deletions(-) diff --git a/internal/context/context.go b/internal/context/context.go index 9a60bc5d..857ef774 100644 --- a/internal/context/context.go +++ b/internal/context/context.go @@ -44,6 +44,7 @@ var ( ctxExperimentalFeatures = &contextKey{"experimental features"} ctxRPCContext = &contextKey{"rpc context"} ctxLanguageId = &contextKey{"language ID"} + ctxValidationOptions = &contextKey{"validation options"} ) func missingContextErr(ctxKey *contextKey) *MissingContextErr { @@ -188,6 +189,10 @@ func RPCContext(ctx context.Context) RPCContextData { return ctx.Value(ctxRPCContext).(RPCContextData) } +func (ctxData RPCContextData) IsDidChangeRequest() bool { + return ctxData.Method == "textDocument/didChange" +} + func WithLanguageId(ctx context.Context, languageId string) context.Context { return context.WithValue(ctx, ctxLanguageId, languageId) } @@ -200,6 +205,24 @@ func IsLanguageId(ctx context.Context, expectedLangId string) bool { return langId == expectedLangId } -func (ctxData RPCContextData) IsDidChangeRequest() bool { - return ctxData.Method == "textDocument/didChange" +func WithValidationOptions(ctx context.Context, validationOptions *settings.ValidationOptions) context.Context { + return context.WithValue(ctx, ctxValidationOptions, validationOptions) +} + +func SetValidationOptions(ctx context.Context, validationOptions settings.ValidationOptions) error { + e, ok := ctx.Value(ctxValidationOptions).(*settings.ValidationOptions) + if !ok { + return missingContextErr(ctxValidationOptions) + } + + *e = validationOptions + return nil +} + +func ValidationOptions(ctx context.Context) (settings.ValidationOptions, error) { + validationOptions, ok := ctx.Value(ctxValidationOptions).(*settings.ValidationOptions) + if !ok { + return settings.ValidationOptions{}, missingContextErr(ctxValidationOptions) + } + return *validationOptions, nil } diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index 500dcfee..2dc243ff 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -6,6 +6,7 @@ package indexer import ( "context" + 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/schemas" @@ -121,20 +122,26 @@ 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, - }) + validationOptions, err := lsctx.ValidationOptions(ctx) if err != nil { return ids, err } + if validationOptions.EarlyValidation { + _, 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 { @@ -149,18 +156,19 @@ 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.ReferenceValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) - }, - Type: op.OpTypeReferenceValidation.String(), - DependsOn: job.IDs{metaId, refTargetsId}, - IgnoreState: ignoreState, - }) - if err != nil { - return ids, err + if validationOptions.EarlyValidation { + _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + return module.ReferenceValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeReferenceValidation.String(), + DependsOn: job.IDs{metaId, refTargetsId}, + IgnoreState: ignoreState, + }) + if err != nil { + return ids, err + } } // This job may make an HTTP request, and we schedule it in diff --git a/internal/langserver/handlers/initialize.go b/internal/langserver/handlers/initialize.go index e9389648..14e8cf2d 100644 --- a/internal/langserver/handlers/initialize.go +++ b/internal/langserver/handlers/initialize.go @@ -107,6 +107,8 @@ func (svc *service) Initialize(ctx context.Context, params lsp.InitializeParams) // set experimental feature flags lsctx.SetExperimentalFeatures(ctx, out.Options.ExperimentalFeatures) + // set validation options for jobs + lsctx.SetValidationOptions(ctx, out.Options.Validation) if len(out.UnusedKeys) > 0 { jrpc2.ServerFromContext(ctx).Notify(ctx, "window/showMessage", &lsp.ShowMessageParams{ @@ -202,6 +204,7 @@ func getTelemetryProperties(out *settings.DecodedOptions) map[string]interface{} "options.terraform.path": false, "options.terraform.timeout": "", "options.terraform.logFilePath": false, + "options.validation.earlyValidation": false, "root_uri": "dir", "lsVersion": "", } @@ -217,6 +220,7 @@ func getTelemetryProperties(out *settings.DecodedOptions) map[string]interface{} properties["options.terraform.path"] = len(out.Options.Terraform.Path) > 0 properties["options.terraform.timeout"] = out.Options.Terraform.Timeout properties["options.terraform.logFilePath"] = len(out.Options.Terraform.LogFilePath) > 0 + properties["options.validation.earlyValidation"] = out.Options.Validation.EarlyValidation return properties } diff --git a/internal/langserver/handlers/service.go b/internal/langserver/handlers/service.go index 6eab9317..38240f36 100644 --- a/internal/langserver/handlers/service.go +++ b/internal/langserver/handlers/service.go @@ -120,6 +120,7 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { commandPrefix := "" clientName := "" var expFeatures settings.ExperimentalFeatures + var validationOptions settings.ValidationOptions m := map[string]rpch.Func{ "initialize": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { @@ -133,6 +134,7 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { ctx = lsctx.WithCommandPrefix(ctx, &commandPrefix) ctx = ilsp.ContextWithClientName(ctx, &clientName) ctx = lsctx.WithExperimentalFeatures(ctx, &expFeatures) + ctx = lsctx.WithValidationOptions(ctx, &validationOptions) version, ok := lsctx.LanguageServerVersion(svc.srvCtx) if ok { @@ -156,6 +158,8 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { if err != nil { return nil, err } + ctx = lsctx.WithValidationOptions(ctx, &validationOptions) + return handle(ctx, req, svc.TextDocumentDidChange) }, "textDocument/didOpen": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { @@ -163,6 +167,8 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { if err != nil { return nil, err } + ctx = lsctx.WithValidationOptions(ctx, &validationOptions) + return handle(ctx, req, svc.TextDocumentDidOpen) }, "textDocument/didClose": func(ctx context.Context, req *jrpc2.Request) (interface{}, error) { @@ -325,6 +331,7 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { if err != nil { return nil, err } + ctx = lsctx.WithValidationOptions(ctx, &validationOptions) return handle(ctx, req, svc.DidChangeWatchedFiles) }, From 8b08caed937043786f48eea7911030fbdc84d8db Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Thu, 31 Aug 2023 14:19:27 +0200 Subject: [PATCH 05/15] Only validate a single file on `didChange` (#1370) * Introduce new RPCContextData for storing rpc info * Attach RPCContextData to jobs * Add RPCContextData to walker ctx, fix params * Check RPCContext on SchemaValidation * Bump hcl-lang to `485c60` * Review feedback --- internal/terraform/module/module_ops.go | 34 ++++++- internal/terraform/module/module_ops_test.go | 88 +++++++++++++++++++ .../module/testdata/invalid-config/main.tf | 9 ++ .../testdata/invalid-config/variables.tf | 11 +++ 4 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 internal/terraform/module/testdata/invalid-config/main.tf create mode 100644 internal/terraform/module/testdata/invalid-config/variables.tf diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index d3aa0237..3617bc74 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -11,6 +11,7 @@ import ( "io" "io/fs" "log" + "path" "path/filepath" "time" @@ -18,6 +19,7 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/hcl-lang/decoder" "github.com/hashicorp/hcl-lang/lang" + "github.com/hashicorp/hcl/v2" tfjson "github.com/hashicorp/terraform-json" lsctx "github.com/hashicorp/terraform-ls/internal/context" idecoder "github.com/hashicorp/terraform-ls/internal/decoder" @@ -736,10 +738,34 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe return err } - diags, rErr := moduleDecoder.Validate(ctx) - sErr := modStore.UpdateModuleDiagnostics(modPath, ast.SchemaValidationSource, ast.ModDiagsFromMap(diags)) - if sErr != nil { - return sErr + var rErr error + rpcContext := lsctx.RPCContext(ctx) + isSingleFileChange := rpcContext.Method == "textDocument/didChange" + if isSingleFileChange { + filename := path.Base(rpcContext.URI) + // We only revalidate a single file that changed + var fileDiags hcl.Diagnostics + fileDiags, rErr = moduleDecoder.ValidateFile(ctx, filename) + + modDiags, ok := mod.ModuleDiagnostics[ast.SchemaValidationSource] + if !ok { + modDiags = make(ast.ModDiags) + } + modDiags[ast.ModFilename(filename)] = fileDiags + + sErr := modStore.UpdateModuleDiagnostics(modPath, ast.SchemaValidationSource, modDiags) + if sErr != nil { + return sErr + } + } else { + // We validate the whole module, e.g. on open + var diags lang.DiagnosticsMap + diags, rErr = moduleDecoder.Validate(ctx) + + sErr := modStore.UpdateModuleDiagnostics(modPath, ast.SchemaValidationSource, ast.ModDiagsFromMap(diags)) + if sErr != nil { + return sErr + } } return rErr diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index 9a84dfda..873a8d50 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -1123,3 +1123,91 @@ var randomSchemaJSON = `{ } } }` + +func TestSchemaValidation_FullModule(t *testing.T) { + ctx := context.Background() + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + testData, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + modPath := filepath.Join(testData, "invalid-config") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didOpen", + URI: "file:///test/variables.tf", + }) + err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 5 + diagsCount := mod.ModuleDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} + +func TestSchemaValidation_SingleFile(t *testing.T) { + ctx := context.Background() + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + testData, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + modPath := filepath.Join(testData, "invalid-config") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didChange", + URI: "file:///test/variables.tf", + }) + err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 3 + diagsCount := mod.ModuleDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} diff --git a/internal/terraform/module/testdata/invalid-config/main.tf b/internal/terraform/module/testdata/invalid-config/main.tf new file mode 100644 index 00000000..ce360b64 --- /dev/null +++ b/internal/terraform/module/testdata/invalid-config/main.tf @@ -0,0 +1,9 @@ +test {} + +variable { + +} + +locals { + test = 1 +} \ No newline at end of file diff --git a/internal/terraform/module/testdata/invalid-config/variables.tf b/internal/terraform/module/testdata/invalid-config/variables.tf new file mode 100644 index 00000000..c9421880 --- /dev/null +++ b/internal/terraform/module/testdata/invalid-config/variables.tf @@ -0,0 +1,11 @@ +variable { + +} + +variable "test" { + +} + +output { + +} \ No newline at end of file From a8816a0ffda10207f2bde288c26a42e2f90cc84f Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 1 Sep 2023 12:51:53 +0200 Subject: [PATCH 06/15] Add progress reporting for Terraform validate (#1373) --- internal/langserver/handlers/command/validate.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/langserver/handlers/command/validate.go b/internal/langserver/handlers/command/validate.go index 5b61e223..49aace90 100644 --- a/internal/langserver/handlers/command/validate.go +++ b/internal/langserver/handlers/command/validate.go @@ -11,6 +11,7 @@ import ( "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/progress" "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" @@ -28,6 +29,12 @@ func (h *CmdHandler) TerraformValidateHandler(ctx context.Context, args cmd.Comm dirHandle := document.DirHandleFromURI(dirUri) + progress.Begin(ctx, "Validating") + defer func() { + progress.End(ctx, "Finished") + }() + + progress.Report(ctx, "Running terraform validate ...") id, err := h.StateStore.JobStore.EnqueueJob(ctx, job.Job{ Dir: dirHandle, Func: func(ctx context.Context) error { From c4b7399926db244e9261e5e50873a2d882ce01a5 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 4 Sep 2023 15:53:40 +0100 Subject: [PATCH 07/15] validations: Limit ref. validation to only LocalOrigin(s) (#1379) * validations: Limit ref. validation to only LocalOrigin(s) * validations: add test for disabled origins --- .../validations/unreferenced_origin.go | 7 ++++++ .../validations/unreferenced_origin_test.go | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/internal/decoder/validations/unreferenced_origin.go b/internal/decoder/validations/unreferenced_origin.go index ad5f550d..aa19bfdf 100644 --- a/internal/decoder/validations/unreferenced_origin.go +++ b/internal/decoder/validations/unreferenced_origin.go @@ -27,6 +27,13 @@ func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang continue } + _, ok = origin.(reference.LocalOrigin) + if !ok { + // we avoid reporting on origins outside of the current module + // for now, to reduce complexity and reduce performance impact + continue + } + // we only initially validate variables // resources and data sources can have unknown schema // and will be researched at a later point diff --git a/internal/decoder/validations/unreferenced_origin_test.go b/internal/decoder/validations/unreferenced_origin_test.go index f36b6122..2f0457a9 100644 --- a/internal/decoder/validations/unreferenced_origin_test.go +++ b/internal/decoder/validations/unreferenced_origin_test.go @@ -50,6 +50,28 @@ func TestUnreferencedOrigins(t *testing.T) { }, }, }, + { + name: "unsupported path origins (module input)", + origins: reference.Origins{ + reference.PathOrigin{ + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{}, + End: hcl.Pos{}, + }, + TargetAddr: lang.Address{ + lang.RootStep{Name: "var"}, + lang.AttrStep{Name: "foo"}, + }, + TargetPath: lang.Path{ + Path: "./submodule", + LanguageID: "terraform", + }, + Constraints: reference.OriginConstraints{}, + }, + }, + want: lang.DiagnosticsMap{}, + }, { name: "many undeclared variables", origins: reference.Origins{ From 01c7ce9aea9130d5aa928ba8af1161461e64cb5c Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 4 Sep 2023 16:18:45 +0100 Subject: [PATCH 08/15] validations: Validate only first 2 segments of a reference (#1380) --- .../decoder/validations/unreferenced_origin.go | 14 ++++++++++++-- .../validations/unreferenced_origin_test.go | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/internal/decoder/validations/unreferenced_origin.go b/internal/decoder/validations/unreferenced_origin.go index aa19bfdf..046acc9a 100644 --- a/internal/decoder/validations/unreferenced_origin.go +++ b/internal/decoder/validations/unreferenced_origin.go @@ -34,10 +34,20 @@ func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang continue } + address := matchableOrigin.Address() + + if len(address) > 2 { + // We temporarily ignore references with more than 2 segments + // as these indicate references to complex types + // which we do not fully support yet. + // TODO: revisit as part of https://github.com/hashicorp/terraform-ls/issues/653 + continue + } + // we only initially validate variables // resources and data sources can have unknown schema // and will be researched at a later point - firstStep := matchableOrigin.Address()[0] + firstStep := address[0] if firstStep.String() != "var" { continue } @@ -48,7 +58,7 @@ func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang fileName := origin.OriginRange().Filename d := &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("No declaration found for %q", matchableOrigin.Address()), + Summary: fmt.Sprintf("No declaration found for %q", address), Subject: origin.OriginRange().Ptr(), } diagsMap[fileName] = diagsMap[fileName].Append(d) diff --git a/internal/decoder/validations/unreferenced_origin_test.go b/internal/decoder/validations/unreferenced_origin_test.go index 2f0457a9..fba76ecf 100644 --- a/internal/decoder/validations/unreferenced_origin_test.go +++ b/internal/decoder/validations/unreferenced_origin_test.go @@ -50,6 +50,24 @@ func TestUnreferencedOrigins(t *testing.T) { }, }, }, + { + name: "unsupported variable of complex type", + origins: reference.Origins{ + reference.LocalOrigin{ + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{}, + End: hcl.Pos{}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "var"}, + lang.AttrStep{Name: "obj"}, + lang.AttrStep{Name: "field"}, + }, + }, + }, + want: lang.DiagnosticsMap{}, + }, { name: "unsupported path origins (module input)", origins: reference.Origins{ From f65ee099b1e41923f81f7b04343d769f1d1c9b66 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 4 Sep 2023 16:46:58 +0100 Subject: [PATCH 09/15] validations: Enable reference validation of locals (#1381) * validations: Enable reference validation of locals * Update internal/decoder/validations/unreferenced_origin.go Co-authored-by: Daniel Banck * fix imports --------- Co-authored-by: Daniel Banck --- .../validations/unreferenced_origin.go | 8 +++-- .../validations/unreferenced_origin_test.go | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/internal/decoder/validations/unreferenced_origin.go b/internal/decoder/validations/unreferenced_origin.go index 046acc9a..1a99c1b1 100644 --- a/internal/decoder/validations/unreferenced_origin.go +++ b/internal/decoder/validations/unreferenced_origin.go @@ -6,6 +6,7 @@ package validations import ( "context" "fmt" + "slices" "github.com/hashicorp/hcl-lang/decoder" "github.com/hashicorp/hcl-lang/lang" @@ -44,11 +45,12 @@ func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang continue } - // we only initially validate variables + // we only initially validate variables & local values // resources and data sources can have unknown schema // and will be researched at a later point - firstStep := address[0] - if firstStep.String() != "var" { + supported := []string{"var", "local"} + firstStep := address[0].String() + if !slices.Contains(supported, firstStep) { continue } diff --git a/internal/decoder/validations/unreferenced_origin_test.go b/internal/decoder/validations/unreferenced_origin_test.go index fba76ecf..7c39cd6b 100644 --- a/internal/decoder/validations/unreferenced_origin_test.go +++ b/internal/decoder/validations/unreferenced_origin_test.go @@ -50,6 +50,35 @@ func TestUnreferencedOrigins(t *testing.T) { }, }, }, + { + name: "undeclared local value", + origins: reference.Origins{ + reference.LocalOrigin{ + Range: hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{}, + End: hcl.Pos{}, + }, + Addr: lang.Address{ + lang.RootStep{Name: "local"}, + lang.AttrStep{Name: "foo"}, + }, + }, + }, + want: lang.DiagnosticsMap{ + "test.tf": hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "No declaration found for \"local.foo\"", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{}, + End: hcl.Pos{}, + }, + }, + }, + }, + }, { name: "unsupported variable of complex type", origins: reference.Origins{ From 6d340eb86b28c5e7626529b8f434ffda10bfde88 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 11 Sep 2023 14:07:00 +0100 Subject: [PATCH 10/15] decoder: Update comments about validation of other Origin types (#1396) --- .../validations/unreferenced_origin.go | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/decoder/validations/unreferenced_origin.go b/internal/decoder/validations/unreferenced_origin.go index 1a99c1b1..7c2822f5 100644 --- a/internal/decoder/validations/unreferenced_origin.go +++ b/internal/decoder/validations/unreferenced_origin.go @@ -18,24 +18,24 @@ func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang diagsMap := make(lang.DiagnosticsMap) for _, origin := range pathCtx.ReferenceOrigins { - matchableOrigin, ok := origin.(reference.MatchableOrigin) + localOrigin, ok := origin.(reference.LocalOrigin) if !ok { - // we don't report on other origins to avoid complexity for now - // other origins would need to be matched against other - // modules/directories and we cannot be sure the targets are - // available within the workspace or were parsed/decoded/collected - // at the time this event occurs + // We avoid reporting on other origin types. + // + // DirectOrigin is represented as module's source + // and we already validate existence of the local module + // and avoiding linking to a non-existent module in terraform-schema + // https://github.com/hashicorp/terraform-schema/blob/b39f3de0/schema/module_schema.go#L212-L232 + // + // PathOrigin is represented as module inputs + // and we can validate module inputs more meaningfully + // as attributes in body (module block), e.g. raise that + // an input is required or unknown, rather than "reference" + // lacking a corresponding target. continue } - _, ok = origin.(reference.LocalOrigin) - if !ok { - // we avoid reporting on origins outside of the current module - // for now, to reduce complexity and reduce performance impact - continue - } - - address := matchableOrigin.Address() + address := localOrigin.Address() if len(address) > 2 { // We temporarily ignore references with more than 2 segments @@ -54,7 +54,7 @@ func UnreferencedOrigins(ctx context.Context, pathCtx *decoder.PathContext) lang continue } - _, ok = pathCtx.ReferenceTargets.Match(matchableOrigin) + _, ok = pathCtx.ReferenceTargets.Match(localOrigin) if !ok { // target not found fileName := origin.OriginRange().Filename From 1d0f73e41a9c947b84e909192c59e6574dba31f9 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Tue, 12 Sep 2023 14:19:26 +0200 Subject: [PATCH 11/15] Update diagnostic source to `Terraform` (#1399) --- internal/terraform/ast/diagnostics.go | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/internal/terraform/ast/diagnostics.go b/internal/terraform/ast/diagnostics.go index fa8ecb59..18f1cbb6 100644 --- a/internal/terraform/ast/diagnostics.go +++ b/internal/terraform/ast/diagnostics.go @@ -4,8 +4,6 @@ package ast import ( - "fmt" - op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" ) @@ -20,18 +18,7 @@ const ( ) func (d DiagnosticSource) String() string { - switch d { - case HCLParsingSource: - return "HCL" - case SchemaValidationSource: - return "early validation" - case ReferenceValidationSource: - return "early validation" - case TerraformValidateSource: - return "terraform validate" - default: - panic(fmt.Sprintf("Unknown diagnostic source %d", d)) - } + return "Terraform" } type DiagnosticSourceState map[DiagnosticSource]op.OpState From d0aba3e468ce8024508905989ba73215f84f8e2b Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 20 Sep 2023 12:41:52 +0100 Subject: [PATCH 12/15] early validation - bump `hcl-lang` and reflect upstream changes (#1416) * deps: bump hcl-lang to aa9b38d (#1414) * re-define decoupled validators --- internal/decoder/decoder.go | 1 + internal/decoder/validators.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 internal/decoder/validators.go diff --git a/internal/decoder/decoder.go b/internal/decoder/decoder.go index e951e7cf..318cd488 100644 --- a/internal/decoder/decoder.go +++ b/internal/decoder/decoder.go @@ -30,6 +30,7 @@ func modulePathContext(mod *state.Module, schemaReader state.SchemaReader, modRe ReferenceTargets: make(reference.Targets, 0), Files: make(map[string]*hcl.File, 0), Functions: coreFunctions(mod), + Validators: moduleValidators, } for _, origin := range mod.RefOrigins { diff --git a/internal/decoder/validators.go b/internal/decoder/validators.go new file mode 100644 index 00000000..7bb3bf7b --- /dev/null +++ b/internal/decoder/validators.go @@ -0,0 +1,19 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package decoder + +import ( + "github.com/hashicorp/hcl-lang/validator" +) + +var moduleValidators = []validator.Validator{ + validator.BlockLabelsLength{}, + validator.DeprecatedAttribute{}, + validator.DeprecatedBlock{}, + validator.MaxBlocks{}, + validator.MinBlocks{}, + validator.MissingRequiredAttribute{}, + validator.UnexpectedAttribute{}, + validator.UnexpectedBlock{}, +} From a79e6b10622633f6813a9be0c7af959be46b5e6c Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 25 Sep 2023 10:59:44 +0100 Subject: [PATCH 13/15] Introduce validation of `*.tfvars` files (#1413) * Introduce validation of tfvars files * clarify existing job name/type * add tests --- internal/decoder/decoder.go | 1 + internal/decoder/validators.go | 5 + internal/indexer/document_change.go | 24 +++- internal/indexer/document_open.go | 21 ++++ internal/state/module.go | 2 +- internal/terraform/module/module_ops.go | 69 ++++++++++- internal/terraform/module/module_ops_test.go | 116 +++++++++++++++++- .../module/operation/op_type_string.go | 11 +- .../terraform/module/operation/operation.go | 3 +- .../testdata/invalid-tfvars/foo.auto.tfvars | 1 + .../testdata/invalid-tfvars/terraform.tfvars | 2 + .../testdata/invalid-tfvars/variables.tf | 1 + 12 files changed, 240 insertions(+), 16 deletions(-) create mode 100644 internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars create mode 100644 internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars create mode 100644 internal/terraform/module/testdata/invalid-tfvars/variables.tf diff --git a/internal/decoder/decoder.go b/internal/decoder/decoder.go index 318cd488..ed1a20c3 100644 --- a/internal/decoder/decoder.go +++ b/internal/decoder/decoder.go @@ -64,6 +64,7 @@ func varsPathContext(mod *state.Module) (*decoder.PathContext, error) { ReferenceOrigins: make(reference.Origins, 0), ReferenceTargets: make(reference.Targets, 0), Files: make(map[string]*hcl.File), + Validators: varsValidators, } for _, origin := range mod.VarsRefOrigins { diff --git a/internal/decoder/validators.go b/internal/decoder/validators.go index 7bb3bf7b..0b1473ba 100644 --- a/internal/decoder/validators.go +++ b/internal/decoder/validators.go @@ -17,3 +17,8 @@ var moduleValidators = []validator.Validator{ validator.UnexpectedAttribute{}, validator.UnexpectedBlock{}, } + +var varsValidators = []validator.Validator{ + validator.UnexpectedAttribute{}, + validator.UnexpectedBlock{}, +} diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index 2dc243ff..41471796 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -49,6 +49,26 @@ func (idx *Indexer) DocumentChanged(ctx context.Context, modHandle document.DirH } ids = append(ids, parseVarsId) + validationOptions, err := lsctx.ValidationOptions(ctx) + if err != nil { + return ids, err + } + + if validationOptions.EarlyValidation { + _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + return module.SchemaVariablesValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeSchemaVarsValidation.String(), + DependsOn: append(modIds, parseVarsId), + IgnoreState: true, + }) + if err != nil { + return ids, err + } + } + varsRefsId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { @@ -131,9 +151,9 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand _, 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()) + return module.SchemaModuleValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeSchemaValidation.String(), + Type: op.OpTypeSchemaModuleValidation.String(), DependsOn: job.IDs{metaId}, IgnoreState: ignoreState, }) diff --git a/internal/indexer/document_open.go b/internal/indexer/document_open.go index 159c2dde..043840d0 100644 --- a/internal/indexer/document_open.go +++ b/internal/indexer/document_open.go @@ -7,6 +7,7 @@ import ( "context" "github.com/hashicorp/go-multierror" + 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/terraform/exec" @@ -72,6 +73,26 @@ func (idx *Indexer) DocumentOpened(ctx context.Context, modHandle document.DirHa } ids = append(ids, parseVarsId) + validationOptions, err := lsctx.ValidationOptions(ctx) + if err != nil { + return ids, err + } + + if validationOptions.EarlyValidation { + _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + return module.SchemaVariablesValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeSchemaVarsValidation.String(), + DependsOn: append(modIds, parseVarsId), + IgnoreState: true, + }) + if err != nil { + return ids, err + } + } + varsRefsId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { diff --git a/internal/state/module.go b/internal/state/module.go index b06765b3..10838bc0 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -988,7 +988,7 @@ func (s *ModuleStore) SetModuleDiagnosticsState(path string, source ast.Diagnost func (s *ModuleStore) UpdateVarsDiagnostics(path string, source ast.DiagnosticSource, diags ast.VarsDiags) error { txn := s.db.Txn(true) txn.Defer(func() { - s.SetVarsDiagnosticsState(path, ast.HCLParsingSource, op.OpStateLoaded) + s.SetVarsDiagnosticsState(path, source, op.OpStateLoaded) }) defer txn.Abort() diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 3617bc74..778dd421 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -708,7 +708,7 @@ func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, sche return rErr } -func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { +func SchemaModuleValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { mod, err := modStore.ModuleByPath(modPath) if err != nil { return err @@ -728,6 +728,7 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe ModuleReader: modStore, SchemaReader: schemaReader, }) + d.SetContext(idecoder.DecoderContext(ctx)) moduleDecoder, err := d.Path(lang.Path{ @@ -740,8 +741,7 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe var rErr error rpcContext := lsctx.RPCContext(ctx) - isSingleFileChange := rpcContext.Method == "textDocument/didChange" - if isSingleFileChange { + if rpcContext.Method == "textDocument/didChange" && lsctx.IsLanguageId(ctx, ilsp.Terraform.String()) { filename := path.Base(rpcContext.URI) // We only revalidate a single file that changed var fileDiags hcl.Diagnostics @@ -771,6 +771,69 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe return rErr } +func SchemaVariablesValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if mod.VarsDiagnosticsState[ast.SchemaValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetVarsDiagnosticsState(modPath, ast.SchemaValidationSource, op.OpStateLoading) + if err != nil { + return err + } + + d := decoder.NewDecoder(&idecoder.PathReader{ + ModuleReader: modStore, + SchemaReader: schemaReader, + }) + + d.SetContext(idecoder.DecoderContext(ctx)) + + moduleDecoder, err := d.Path(lang.Path{ + Path: modPath, + LanguageID: ilsp.Tfvars.String(), + }) + if err != nil { + return err + } + + var rErr error + rpcContext := lsctx.RPCContext(ctx) + if rpcContext.Method == "textDocument/didChange" && lsctx.IsLanguageId(ctx, ilsp.Tfvars.String()) { + filename := path.Base(rpcContext.URI) + // We only revalidate a single file that changed + var fileDiags hcl.Diagnostics + fileDiags, rErr = moduleDecoder.ValidateFile(ctx, filename) + + varsDiags, ok := mod.VarsDiagnostics[ast.SchemaValidationSource] + if !ok { + varsDiags = make(ast.VarsDiags) + } + varsDiags[ast.VarsFilename(filename)] = fileDiags + + sErr := modStore.UpdateVarsDiagnostics(modPath, ast.SchemaValidationSource, varsDiags) + if sErr != nil { + return sErr + } + } else { + // We validate the whole module, e.g. on open + var diags lang.DiagnosticsMap + diags, rErr = moduleDecoder.Validate(ctx) + + sErr := modStore.UpdateVarsDiagnostics(modPath, ast.SchemaValidationSource, ast.VarsDiagsFromMap(diags)) + if sErr != nil { + return sErr + } + } + + return rErr +} + func ReferenceValidation(ctx context.Context, modStore *state.ModuleStore, schemaReader state.SchemaReader, modPath string) error { mod, err := modStore.ModuleByPath(modPath) if err != nil { diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index 873a8d50..f8b6daa5 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -1124,7 +1124,7 @@ var randomSchemaJSON = `{ } }` -func TestSchemaValidation_FullModule(t *testing.T) { +func TestSchemaModuleValidation_FullModule(t *testing.T) { ctx := context.Background() ss, err := state.NewStateStore() if err != nil { @@ -1151,7 +1151,8 @@ func TestSchemaValidation_FullModule(t *testing.T) { Method: "textDocument/didOpen", URI: "file:///test/variables.tf", }) - err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + ctx = lsctx.WithLanguageId(ctx, ilsp.Terraform.String()) + err = SchemaModuleValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err) } @@ -1168,7 +1169,7 @@ func TestSchemaValidation_FullModule(t *testing.T) { } } -func TestSchemaValidation_SingleFile(t *testing.T) { +func TestSchemaModuleValidation_SingleFile(t *testing.T) { ctx := context.Background() ss, err := state.NewStateStore() if err != nil { @@ -1195,7 +1196,8 @@ func TestSchemaValidation_SingleFile(t *testing.T) { Method: "textDocument/didChange", URI: "file:///test/variables.tf", }) - err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + ctx = lsctx.WithLanguageId(ctx, ilsp.Terraform.String()) + err = SchemaModuleValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err) } @@ -1211,3 +1213,109 @@ func TestSchemaValidation_SingleFile(t *testing.T) { t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) } } + +func TestSchemaVarsValidation_FullModule(t *testing.T) { + ctx := context.Background() + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + testData, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + modPath := filepath.Join(testData, "invalid-tfvars") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = LoadModuleMetadata(ctx, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = ParseVariables(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didOpen", + URI: "file:///test/terraform.tfvars", + }) + ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) + err = SchemaVariablesValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 2 + diagsCount := mod.VarsDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} + +func TestSchemaVarsValidation_SingleFile(t *testing.T) { + ctx := context.Background() + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + testData, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + modPath := filepath.Join(testData, "invalid-tfvars") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = LoadModuleMetadata(ctx, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + err = ParseVariables(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didChange", + URI: "file:///test/terraform.tfvars", + }) + ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) + err = SchemaVariablesValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 1 + diagsCount := mod.VarsDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index 5b4ac874..1c4a10fc 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -21,14 +21,15 @@ func _() { _ = x[OpTypeGetModuleDataFromRegistry-10] _ = x[OpTypeParseProviderVersions-11] _ = x[OpTypePreloadEmbeddedSchema-12] - _ = x[OpTypeSchemaValidation-13] - _ = x[OpTypeReferenceValidation-14] - _ = x[OpTypeTerraformValidate-15] + _ = x[OpTypeSchemaModuleValidation-13] + _ = x[OpTypeSchemaVarsValidation-14] + _ = x[OpTypeReferenceValidation-15] + _ = x[OpTypeTerraformValidate-16] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeSchemaValidationOpTypeReferenceValidationOpTypeTerraformValidate" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeSchemaModuleValidationOpTypeSchemaVarsValidationOpTypeReferenceValidationOpTypeTerraformValidate" -var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 344, 369, 392} +var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 350, 376, 401, 424} func (i OpType) String() string { if i >= OpType(len(_OpType_index)-1) { diff --git a/internal/terraform/module/operation/operation.go b/internal/terraform/module/operation/operation.go index fe408450..3485bc09 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -30,7 +30,8 @@ const ( OpTypeGetModuleDataFromRegistry OpTypeParseProviderVersions OpTypePreloadEmbeddedSchema - OpTypeSchemaValidation + OpTypeSchemaModuleValidation + OpTypeSchemaVarsValidation OpTypeReferenceValidation OpTypeTerraformValidate ) diff --git a/internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars b/internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars new file mode 100644 index 00000000..60a5c57f --- /dev/null +++ b/internal/terraform/module/testdata/invalid-tfvars/foo.auto.tfvars @@ -0,0 +1 @@ +noot = "noot" diff --git a/internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars b/internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars new file mode 100644 index 00000000..79892b8e --- /dev/null +++ b/internal/terraform/module/testdata/invalid-tfvars/terraform.tfvars @@ -0,0 +1,2 @@ +foo = "foo" +bar = "noot" diff --git a/internal/terraform/module/testdata/invalid-tfvars/variables.tf b/internal/terraform/module/testdata/invalid-tfvars/variables.tf new file mode 100644 index 00000000..91084f7a --- /dev/null +++ b/internal/terraform/module/testdata/invalid-tfvars/variables.tf @@ -0,0 +1 @@ +variable "foo" {} From b100516a0d8b4538b7ab53735dcc16de46d2b601 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Tue, 26 Sep 2023 12:52:43 +0200 Subject: [PATCH 14/15] Fix delayed diagnostics / job ordering (#1417) * Move everything into defer * Add comment about lsctx * Have module registry job depend on module metadata --- internal/indexer/document_change.go | 126 +++++++++++++++------------- 1 file changed, 67 insertions(+), 59 deletions(-) diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index 41471796..d921a71a 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -89,6 +89,15 @@ func (idx *Indexer) DocumentChanged(ctx context.Context, modHandle document.DirH func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHandle, dependsOn job.IDs, ignoreState bool) (job.IDs, error) { ids := make(job.IDs, 0) + // Changes to a setting currently requires a LS restart, so the LS + // setting context cannot change during the execution of a job. That's + // why we can extract it here and use it in Defer. + // See https://github.com/hashicorp/terraform-ls/issues/1008 + validationOptions, err := lsctx.ValidationOptions(ctx) + if err != nil { + return ids, err + } + metaId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { @@ -97,10 +106,10 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand Type: op.OpTypeLoadModuleMetadata.String(), DependsOn: dependsOn, IgnoreState: ignoreState, - Defer: func(ctx context.Context, jobErr error) (jobIds job.IDs, err error) { + Defer: func(ctx context.Context, jobErr error) (job.IDs, error) { + ids := make(job.IDs, 0) if jobErr != nil { - err = jobErr - return + return ids, jobErr } modCalls, mcErr := idx.decodeDeclaredModuleCalls(ctx, modHandle, ignoreState) if mcErr != nil { @@ -115,14 +124,42 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand Func: func(ctx context.Context) error { return module.PreloadEmbeddedSchema(ctx, idx.logger, schemas.FS, idx.modStore, idx.schemaStore, modHandle.Path()) }, - DependsOn: modCalls, Type: op.OpTypePreloadEmbeddedSchema.String(), IgnoreState: ignoreState, }) if err != nil { - return + return ids, err + } + ids = append(ids, eSchemaId) + + if validationOptions.EarlyValidation { + _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + return module.SchemaModuleValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeSchemaModuleValidation.String(), + DependsOn: append(modCalls, 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 { + return module.DecodeReferenceTargets(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeDecodeReferenceTargets.String(), + DependsOn: job.IDs{eSchemaId}, + IgnoreState: ignoreState, + }) + if err != nil { + return ids, err } - jobIds = append(jobIds, eSchemaId) + ids = append(ids, refTargetsId) refOriginsId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, @@ -133,63 +170,33 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand DependsOn: append(modCalls, eSchemaId), IgnoreState: ignoreState, }) - jobIds = append(jobIds, refOriginsId) - return - }, - }) - if err != nil { - return ids, err - } - ids = append(ids, metaId) - - validationOptions, err := lsctx.ValidationOptions(ctx) - if err != nil { - return ids, err - } - - if validationOptions.EarlyValidation { - _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ - Dir: modHandle, - Func: func(ctx context.Context) error { - return module.SchemaModuleValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) - }, - Type: op.OpTypeSchemaModuleValidation.String(), - DependsOn: job.IDs{metaId}, - IgnoreState: ignoreState, - }) - if err != nil { - return ids, err - } - } + if err != nil { + return ids, err + } + ids = append(ids, refOriginsId) + + if validationOptions.EarlyValidation { + _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ + Dir: modHandle, + Func: func(ctx context.Context) error { + return module.ReferenceValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + }, + Type: op.OpTypeReferenceValidation.String(), + DependsOn: job.IDs{refOriginsId, refTargetsId}, + IgnoreState: ignoreState, + }) + if err != nil { + return ids, err + } + } - refTargetsId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ - Dir: modHandle, - Func: func(ctx context.Context) error { - return module.DecodeReferenceTargets(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + return ids, nil }, - Type: op.OpTypeDecodeReferenceTargets.String(), - DependsOn: job.IDs{metaId}, - IgnoreState: ignoreState, }) if err != nil { return ids, err } - ids = append(ids, refTargetsId) - - if validationOptions.EarlyValidation { - _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ - Dir: modHandle, - Func: func(ctx context.Context) error { - return module.ReferenceValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) - }, - Type: op.OpTypeReferenceValidation.String(), - DependsOn: job.IDs{metaId, refTargetsId}, - IgnoreState: ignoreState, - }) - if err != nil { - return ids, err - } - } + ids = append(ids, metaId) // This job may make an HTTP request, and we schedule it in // the low-priority queue, so we don't want to wait for it. @@ -199,8 +206,9 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand return module.GetModuleDataFromRegistry(ctx, idx.registryClient, idx.modStore, idx.registryModStore, modHandle.Path()) }, - Priority: job.LowPriority, - Type: op.OpTypeGetModuleDataFromRegistry.String(), + Priority: job.LowPriority, + DependsOn: job.IDs{metaId}, + Type: op.OpTypeGetModuleDataFromRegistry.String(), }) if err != nil { return ids, err From 060b88ef0464d9b3a9791f63bd7efce67390e215 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Wed, 27 Sep 2023 08:36:20 +0200 Subject: [PATCH 15/15] Fix RPCContext related test failures --- internal/terraform/module/module_ops_test.go | 36 ++++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index f8b6daa5..ad136a82 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -1143,15 +1143,15 @@ func TestSchemaModuleValidation_FullModule(t *testing.T) { } fs := filesystem.NewFilesystem(ss.DocumentStore) - err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) - if err != nil { - t.Fatal(err) - } ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ Method: "textDocument/didOpen", URI: "file:///test/variables.tf", }) ctx = lsctx.WithLanguageId(ctx, ilsp.Terraform.String()) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } err = SchemaModuleValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err) @@ -1188,15 +1188,15 @@ func TestSchemaModuleValidation_SingleFile(t *testing.T) { } fs := filesystem.NewFilesystem(ss.DocumentStore) - err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) - if err != nil { - t.Fatal(err) - } ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ Method: "textDocument/didChange", URI: "file:///test/variables.tf", }) ctx = lsctx.WithLanguageId(ctx, ilsp.Terraform.String()) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } err = SchemaModuleValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err) @@ -1233,6 +1233,11 @@ func TestSchemaVarsValidation_FullModule(t *testing.T) { } fs := filesystem.NewFilesystem(ss.DocumentStore) + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didOpen", + URI: "file:///test/terraform.tfvars", + }) + ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) if err != nil { t.Fatal(err) @@ -1245,11 +1250,6 @@ func TestSchemaVarsValidation_FullModule(t *testing.T) { if err != nil { t.Fatal(err) } - ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ - Method: "textDocument/didOpen", - URI: "file:///test/terraform.tfvars", - }) - ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) err = SchemaVariablesValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err) @@ -1286,6 +1286,11 @@ func TestSchemaVarsValidation_SingleFile(t *testing.T) { } fs := filesystem.NewFilesystem(ss.DocumentStore) + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didChange", + URI: "file:///test/terraform.tfvars", + }) + ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) if err != nil { t.Fatal(err) @@ -1298,11 +1303,6 @@ func TestSchemaVarsValidation_SingleFile(t *testing.T) { if err != nil { t.Fatal(err) } - ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ - Method: "textDocument/didChange", - URI: "file:///test/terraform.tfvars", - }) - ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String()) err = SchemaVariablesValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) if err != nil { t.Fatal(err)