From c45ea280eae85757f39f9fedd60301a43a6d1aa3 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Thu, 24 Aug 2023 17:50:01 +0200 Subject: [PATCH 01/10] Introduce different source for diagnostics --- internal/langserver/handlers/hooks_module.go | 9 +- internal/state/module.go | 103 +++++-------------- internal/state/module_changes.go | 14 ++- internal/state/module_test.go | 96 +++++++++-------- internal/terraform/ast/diagnostics.go | 36 +++++++ internal/terraform/ast/module.go | 2 + internal/terraform/ast/variables.go | 2 + internal/terraform/module/module_ops.go | 4 +- 8 files changed, 142 insertions(+), 124 deletions(-) create mode 100644 internal/terraform/ast/diagnostics.go diff --git a/internal/langserver/handlers/hooks_module.go b/internal/langserver/handlers/hooks_module.go index 76ff0b7ff..f0dd387e2 100644 --- a/internal/langserver/handlers/hooks_module.go +++ b/internal/langserver/handlers/hooks_module.go @@ -156,9 +156,12 @@ 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()) + for source, dm := range mod.ModuleDiagnostics { + diags.Append(source.String(), dm.AutoloadedOnly().AsMap()) + } + for source, dm := range mod.VarsDiagnostics { + diags.Append(source.String(), dm.AutoloadedOnly().AsMap()) + } } } return nil diff --git a/internal/state/module.go b/internal/state/module.go index c9f3f986e..520267e10 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" @@ -134,11 +133,8 @@ type Module struct { MetaErr error MetaState op.OpState - ModuleDiagnostics ast.ModDiags - VarsDiagnostics ast.VarsDiags - - ValidationDiagnostics lang.DiagnosticsMap - ValidationDiagnosticsState op.OpState + ModuleDiagnostics ast.SourceModDiags + VarsDiagnostics ast.SourceVarsDiags } func (m *Module) Copy() *Module { @@ -182,8 +178,6 @@ func (m *Module) Copy() *Module { ModuleParsingState: m.ModuleParsingState, VarsParsingState: m.VarsParsingState, - ValidationDiagnosticsState: m.ValidationDiagnosticsState, - Meta: m.Meta.Copy(), MetaErr: m.MetaErr, MetaState: m.MetaState, @@ -214,26 +208,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)) - 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 source, modDiags := range m.ModuleDiagnostics { + newMod.ModuleDiagnostics[source] = make(ast.ModDiags, len(modDiags)) + + 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) + } } } @@ -251,7 +247,6 @@ func newModule(modPath string) *Module { RefTargetsState: op.OpStateUnknown, ModuleParsingState: op.OpStateUnknown, MetaState: op.OpStateUnknown, - ValidationDiagnosticsState: op.OpStateUnknown, } } @@ -971,7 +966,7 @@ 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) defer txn.Abort() @@ -981,55 +976,10 @@ func (s *ModuleStore) UpdateModuleDiagnostics(path string, diags ast.ModDiags) e } mod := oldMod.Copy() - mod.ModuleDiagnostics = 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) 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 + if mod.ModuleDiagnostics == nil { + mod.ModuleDiagnostics = make(ast.SourceModDiags) } - - mod := oldMod.Copy() - mod.ValidationDiagnostics = diags + mod.ModuleDiagnostics[source] = diags err = txn.Insert(s.tableName, mod) if err != nil { @@ -1045,7 +995,7 @@ func (s *ModuleStore) UpdateValidateDiagnostics(path string, diags lang.Diagnost return nil } -func (s *ModuleStore) UpdateVarsDiagnostics(path string, diags ast.VarsDiags) error { +func (s *ModuleStore) UpdateVarsDiagnostics(path string, source ast.DiagnosticSource, diags ast.VarsDiags) error { txn := s.db.Txn(true) defer txn.Abort() @@ -1055,7 +1005,10 @@ func (s *ModuleStore) UpdateVarsDiagnostics(path string, diags ast.VarsDiags) er } mod := oldMod.Copy() - mod.VarsDiagnostics = diags + if mod.VarsDiagnostics == nil { + mod.VarsDiagnostics = make(ast.SourceVarsDiags) + } + mod.VarsDiagnostics[source] = diags err = txn.Insert(s.tableName, mod) if err != nil { diff --git a/internal/state/module_changes.go b/internal/state/module_changes.go index c94e07d60..32a112e78 100644 --- a/internal/state/module_changes.go +++ b/internal/state/module_changes.go @@ -142,10 +142,20 @@ 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() + for _, diags := range oldMod.ModuleDiagnostics { + oldDiags += diags.Count() + } + for _, diags := range oldMod.VarsDiagnostics { + oldDiags += diags.Count() + } } if newMod != nil { - newDiags = newMod.ModuleDiagnostics.Count() + newMod.VarsDiagnostics.Count() + newMod.ValidationDiagnostics.Count() + for _, diags := range newMod.ModuleDiagnostics { + newDiags += diags.Count() + } + for _, diags := range newMod.VarsDiagnostics { + newDiags += diags.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 a45bb1c2b..1769e1131 100644 --- a/internal/state/module_test.go +++ b/internal/state/module_test.go @@ -427,37 +427,42 @@ provider "blah" { region = "london" `), "test.tf") - err = s.Modules.UpdateModuleDiagnostics(tmpDir, ast.ModDiagsFromMap(map[string]hcl.Diagnostics{ + err = s.Modules.UpdateModuleDiagnostics(tmpDir, ast.ModuleParsingSource, 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.ModuleParsingSource: 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 +486,42 @@ dev = { region = "london" `), "test.tfvars") - err = s.Modules.UpdateVarsDiagnostics(tmpDir, ast.VarsDiagsFromMap(map[string]hcl.Diagnostics{ + err = s.Modules.UpdateVarsDiagnostics(tmpDir, ast.VarsParsingSource, 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.VarsParsingSource: 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,7 +742,7 @@ func BenchmarkModuleByPath(b *testing.B) { b.Fatal(err) } mDiags := ast.ModDiagsFromMap(diags) - err = s.Modules.UpdateModuleDiagnostics(modPath, mDiags) + err = s.Modules.UpdateModuleDiagnostics(modPath, ast.ModuleParsingSource, mDiags) if err != nil { b.Fatal(err) } @@ -741,7 +751,9 @@ func BenchmarkModuleByPath(b *testing.B) { Path: modPath, ParsedModuleFiles: mFiles, ModuleParsingState: operation.OpStateLoaded, - ModuleDiagnostics: mDiags, + ModuleDiagnostics: ast.SourceModDiags{ + ast.ModuleParsingSource: mDiags, + }, } 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 000000000..5601daca7 --- /dev/null +++ b/internal/terraform/ast/diagnostics.go @@ -0,0 +1,36 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ast + +import "fmt" + +// DiagnosticSource differentiates different sources of diagnostics. +type DiagnosticSource int + +const ( + ModuleParsingSource DiagnosticSource = iota + VarsParsingSource + SchemaValidationSource + ReferenceValidationSource + TerraformValidateSource +) + +func (d DiagnosticSource) String() string { + switch d { + case ModuleParsingSource: + return "HCL" + case VarsParsingSource: + return "HCL Vars" + case SchemaValidationSource: + return "schema validation" + case ReferenceValidationSource: + return "reference validation" + case TerraformValidateSource: + return "terraform validate" + default: + return fmt.Sprintf("Unknown %d", d) + } +} + +// TODO? combine with langserver/diagnostics diff --git a/internal/terraform/ast/module.go b/internal/terraform/ast/module.go index 61331defd..95263f163 100644 --- a/internal/terraform/ast/module.go +++ b/internal/terraform/ast/module.go @@ -90,3 +90,5 @@ func (md ModDiags) Count() int { } return count } + +type SourceModDiags map[DiagnosticSource]ModDiags diff --git a/internal/terraform/ast/variables.go b/internal/terraform/ast/variables.go index a2f6aaf9e..1a71f5ad1 100644 --- a/internal/terraform/ast/variables.go +++ b/internal/terraform/ast/variables.go @@ -87,3 +87,5 @@ func (vd VarsDiags) Count() int { } return count } + +type SourceVarsDiags map[DiagnosticSource]VarsDiags diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 4f33dfcf0..10a96887d 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -378,7 +378,7 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat return sErr } - sErr = modStore.UpdateModuleDiagnostics(modPath, diags) + sErr = modStore.UpdateModuleDiagnostics(modPath, ast.ModuleParsingSource, diags) if sErr != nil { return sErr } @@ -413,7 +413,7 @@ func ParseVariables(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleSt return sErr } - sErr = modStore.UpdateVarsDiagnostics(modPath, diags) + sErr = modStore.UpdateVarsDiagnostics(modPath, ast.VarsParsingSource, diags) if sErr != nil { return sErr } From 13faff364df190b67e6f5cd66755cb12b91925f5 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Thu, 24 Aug 2023 17:51:16 +0200 Subject: [PATCH 02/10] Add reference validation job --- internal/decoder/decoder.go | 7 -- internal/indexer/document_change.go | 19 +++++- internal/terraform/module/module_ops.go | 68 +++++++++++++++---- .../module/operation/op_type_string.go | 5 +- .../terraform/module/operation/operation.go | 1 + 5 files changed, 73 insertions(+), 27 deletions(-) diff --git a/internal/decoder/decoder.go b/internal/decoder/decoder.go index 74247e4e6..e951e7cf5 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/indexer/document_change.go b/internal/indexer/document_change.go index 284c59a70..e4ad989f9 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -96,6 +96,19 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand } ids = append(ids, eSchemaId) + earlyValidationId, 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{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 { @@ -127,10 +140,10 @@ 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.EarlyValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + return module.ReferenceValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeEarlyValidation.String(), - DependsOn: job.IDs{eSchemaId, refTargetsId, refOriginsId}, + Type: op.OpTypeReferenceValidation.String(), + DependsOn: job.IDs{earlyValidationId, refTargetsId, refOriginsId}, IgnoreState: ignoreState, }) if err != nil { diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 10a96887d..f1f80e3ce 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -19,12 +19,14 @@ import ( "github.com/hashicorp/hcl-lang/lang" tfjson "github.com/hashicorp/terraform-json" 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" ilsp "github.com/hashicorp/terraform-ls/internal/lsp" "github.com/hashicorp/terraform-ls/internal/registry" "github.com/hashicorp/terraform-ls/internal/schemas" "github.com/hashicorp/terraform-ls/internal/state" + "github.com/hashicorp/terraform-ls/internal/terraform/ast" "github.com/hashicorp/terraform-ls/internal/terraform/datadir" op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" "github.com/hashicorp/terraform-ls/internal/terraform/parser" @@ -663,20 +665,21 @@ func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, sche } 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 - } + // 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 + // } + // TODO! track job state d := decoder.NewDecoder(&idecoder.PathReader{ ModuleReader: modStore, @@ -693,7 +696,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 } @@ -701,6 +704,41 @@ 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 known + // 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 + // } + // TODO! track job state + + pathReader := &idecoder.PathReader{ + ModuleReader: modStore, + SchemaReader: schemaReader, + } + pathCtx, err := pathReader.PathContext(lang.Path{ + Path: modPath, + LanguageID: ilsp.Terraform.String(), + }) + if err != nil { + return err + } + + ctx = decoder.WithPathContext(ctx, pathCtx) + + diags := validations.UnreferencedOrigins(ctx) + return modStore.UpdateModuleDiagnostics(modPath, ast.ReferenceValidationSource, ast.ModDiagsFromMap(diags)) +} + 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 a53707460..8e4609989 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -22,11 +22,12 @@ func _() { _ = x[OpTypeParseProviderVersions-11] _ = x[OpTypePreloadEmbeddedSchema-12] _ = x[OpTypeEarlyValidation-13] + _ = x[OpTypeReferenceValidation-14] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeEarlyValidation" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeEarlyValidationOpTypeReferenceValidation" -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, 343, 368} 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 908d04ea0..42f3187c4 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -31,4 +31,5 @@ const ( OpTypeParseProviderVersions OpTypePreloadEmbeddedSchema OpTypeEarlyValidation + OpTypeReferenceValidation ) From 91751245929e71f12eae45a4463a7029d5c8c25c Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 25 Aug 2023 10:49:59 +0200 Subject: [PATCH 03/10] Run terraform validate as a job --- .../langserver/handlers/command/validate.go | 59 ++++--------------- internal/terraform/module/module_ops.go | 25 +++++++- .../module/operation/op_type_string.go | 5 +- .../terraform/module/operation/operation.go | 1 + 4 files changed, 38 insertions(+), 52 deletions(-) diff --git a/internal/langserver/handlers/command/validate.go b/internal/langserver/handlers/command/validate.go index 8ad9d4265..5b61e2237 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/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index f1f80e3ce..f61f51aa5 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -22,6 +22,7 @@ import ( "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" @@ -32,7 +33,7 @@ import ( "github.com/hashicorp/terraform-ls/internal/terraform/parser" 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" @@ -524,7 +525,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 @@ -739,6 +740,26 @@ func ReferenceValidation(ctx context.Context, modStore *state.ModuleStore, schem 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 + } + + 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/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index 8e4609989..215339382 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -23,11 +23,12 @@ func _() { _ = x[OpTypePreloadEmbeddedSchema-12] _ = x[OpTypeEarlyValidation-13] _ = x[OpTypeReferenceValidation-14] + _ = x[OpTypeTerraformValidate-15] } -const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeEarlyValidationOpTypeReferenceValidation" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeEarlyValidationOpTypeReferenceValidationOpTypeTerraformValidate" -var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 343, 368} +var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 343, 368, 391} 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 42f3187c4..757e9510d 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -32,4 +32,5 @@ const ( OpTypePreloadEmbeddedSchema OpTypeEarlyValidation OpTypeReferenceValidation + OpTypeTerraformValidate ) From ebbe0b3674c15b357eef27628e446c8f49241901 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 25 Aug 2023 12:22:51 +0200 Subject: [PATCH 04/10] Track validation job states --- internal/state/module.go | 33 ++++++++++++ internal/terraform/ast/diagnostics.go | 17 ++++++- internal/terraform/module/module_ops.go | 68 ++++++++++++++----------- 3 files changed, 87 insertions(+), 31 deletions(-) diff --git a/internal/state/module.go b/internal/state/module.go index 520267e10..e5e2e47ed 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -135,6 +135,8 @@ type Module struct { ModuleDiagnostics ast.SourceModDiags VarsDiagnostics ast.SourceVarsDiags + + DiagnosticsState ast.DiagnosticSourceState } func (m *Module) Copy() *Module { @@ -181,6 +183,8 @@ func (m *Module) Copy() *Module { Meta: m.Meta.Copy(), MetaErr: m.MetaErr, MetaState: m.MetaState, + + DiagnosticsState: m.DiagnosticsState.Copy(), } if m.InstalledProviders != nil { @@ -247,6 +251,13 @@ func newModule(modPath string) *Module { RefTargetsState: op.OpStateUnknown, ModuleParsingState: op.OpStateUnknown, MetaState: op.OpStateUnknown, + DiagnosticsState: ast.DiagnosticSourceState{ + ast.ModuleParsingSource: op.OpStateUnknown, + ast.VarsParsingSource: op.OpStateUnknown, + ast.SchemaValidationSource: op.OpStateUnknown, + ast.ReferenceValidationSource: op.OpStateUnknown, + ast.TerraformValidateSource: op.OpStateUnknown, + }, } } @@ -968,6 +979,9 @@ func (s *ModuleStore) UpdateMetadata(path string, meta *tfmod.Meta, mErr error) func (s *ModuleStore) UpdateModuleDiagnostics(path string, source ast.DiagnosticSource, diags ast.ModDiags) error { txn := s.db.Txn(true) + txn.Defer(func() { + s.SetDiagnosticsState(path, source, op.OpStateLoaded) + }) defer txn.Abort() oldMod, err := moduleByPath(txn, path) @@ -995,6 +1009,25 @@ func (s *ModuleStore) UpdateModuleDiagnostics(path string, source ast.Diagnostic return nil } +func (s *ModuleStore) SetDiagnosticsState(path string, source ast.DiagnosticSource, state op.OpState) error { + txn := s.db.Txn(true) + defer txn.Abort() + + mod, err := moduleCopyByPath(txn, path) + if err != nil { + return err + } + mod.DiagnosticsState[source] = state + + err = txn.Insert(s.tableName, mod) + if err != nil { + return err + } + + txn.Commit() + return nil +} + func (s *ModuleStore) UpdateVarsDiagnostics(path string, source ast.DiagnosticSource, diags ast.VarsDiags) error { txn := s.db.Txn(true) defer txn.Abort() diff --git a/internal/terraform/ast/diagnostics.go b/internal/terraform/ast/diagnostics.go index 5601daca7..d71e18833 100644 --- a/internal/terraform/ast/diagnostics.go +++ b/internal/terraform/ast/diagnostics.go @@ -3,7 +3,11 @@ package ast -import "fmt" +import ( + "fmt" + + op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" +) // DiagnosticSource differentiates different sources of diagnostics. type DiagnosticSource int @@ -34,3 +38,14 @@ func (d DiagnosticSource) String() string { } // TODO? combine with langserver/diagnostics + +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/module/module_ops.go b/internal/terraform/module/module_ops.go index f61f51aa5..a3fc4809a 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -666,21 +666,20 @@ func DecodeVarsReferences(ctx context.Context, modStore *state.ModuleStore, sche } 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 - // } - // TODO! track job state + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if mod.DiagnosticsState[ast.SchemaValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetDiagnosticsState(modPath, ast.SchemaValidationSource, op.OpStateLoading) + if err != nil { + return err + } d := decoder.NewDecoder(&idecoder.PathReader{ ModuleReader: modStore, @@ -706,21 +705,20 @@ func EarlyValidation(ctx context.Context, modStore *state.ModuleStore, schemaRea } 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 known - // 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 - // } - // TODO! track job state + mod, err := modStore.ModuleByPath(modPath) + if err != nil { + return err + } + + // Avoid validation if it is already in progress or already finished + if mod.DiagnosticsState[ast.ReferenceValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetDiagnosticsState(modPath, ast.ReferenceValidationSource, op.OpStateLoading) + if err != nil { + return err + } pathReader := &idecoder.PathReader{ ModuleReader: modStore, @@ -746,6 +744,16 @@ func TerraformValidate(ctx context.Context, modStore *state.ModuleStore, modPath return err } + // Avoid validation if it is already in progress or already finished + if mod.DiagnosticsState[ast.TerraformValidateSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} + } + + err = modStore.SetDiagnosticsState(modPath, ast.TerraformValidateSource, op.OpStateLoading) + if err != nil { + return err + } + tfExec, err := TerraformExecutorForModule(ctx, mod.Path) if err != nil { return err From aaddba7c29069bc721b8c8ef88b30f032a70a816 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 25 Aug 2023 15:27:58 +0200 Subject: [PATCH 05/10] Review feedback --- internal/indexer/document_change.go | 6 ++++-- internal/langserver/diagnostics/diagnostics.go | 15 +++++++-------- internal/langserver/handlers/hooks_module.go | 16 +++++++--------- internal/state/module.go | 3 +-- internal/state/module_changes.go | 14 ++------------ internal/terraform/ast/diagnostics.go | 13 ++++--------- internal/terraform/ast/module.go | 8 ++++++++ internal/terraform/ast/variables.go | 8 ++++++++ internal/terraform/module/module_ops.go | 4 ++-- 9 files changed, 43 insertions(+), 44 deletions(-) diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index e4ad989f9..704b25df4 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -96,7 +96,8 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand } ids = append(ids, eSchemaId) - earlyValidationId, err := idx.jobStore.EnqueueJob(ctx, job.Job{ + // 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()) @@ -137,13 +138,14 @@ func (idx *Indexer) decodeModule(ctx context.Context, modHandle document.DirHand } ids = append(ids, refOriginsId) + // TODO! check if early validation setting is enabled _, err = idx.jobStore.EnqueueJob(ctx, job.Job{ Dir: modHandle, Func: func(ctx context.Context) error { return module.ReferenceValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, Type: op.OpTypeReferenceValidation.String(), - DependsOn: job.IDs{earlyValidationId, refTargetsId, refOriginsId}, + DependsOn: job.IDs{refTargetsId, refOriginsId}, IgnoreState: ignoreState, }) if err != nil { diff --git a/internal/langserver/diagnostics/diagnostics.go b/internal/langserver/diagnostics/diagnostics.go index 163f62149..b8fb2e8ed 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/handlers/hooks_module.go b/internal/langserver/handlers/hooks_module.go index f0dd387e2..a1bb67506 100644 --- a/internal/langserver/handlers/hooks_module.go +++ b/internal/langserver/handlers/hooks_module.go @@ -153,16 +153,14 @@ func updateDiagnostics(dNotifier *diagnostics.Notifier) notifier.Hook { diags := diagnostics.NewDiagnostics() diags.EmptyRootDiagnostic() - defer dNotifier.PublishHCLDiags(ctx, mod.Path, diags) - - if mod != nil { - for source, dm := range mod.ModuleDiagnostics { - diags.Append(source.String(), dm.AutoloadedOnly().AsMap()) - } - for source, dm := range mod.VarsDiagnostics { - diags.Append(source.String(), dm.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 e5e2e47ed..54feb1dbe 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -252,8 +252,7 @@ func newModule(modPath string) *Module { ModuleParsingState: op.OpStateUnknown, MetaState: op.OpStateUnknown, DiagnosticsState: ast.DiagnosticSourceState{ - ast.ModuleParsingSource: op.OpStateUnknown, - ast.VarsParsingSource: op.OpStateUnknown, + ast.HCLParsingSource: op.OpStateUnknown, ast.SchemaValidationSource: op.OpStateUnknown, ast.ReferenceValidationSource: op.OpStateUnknown, ast.TerraformValidateSource: op.OpStateUnknown, diff --git a/internal/state/module_changes.go b/internal/state/module_changes.go index 32a112e78..7bac6b2ab 100644 --- a/internal/state/module_changes.go +++ b/internal/state/module_changes.go @@ -142,20 +142,10 @@ func (s *ModuleStore) queueModuleChange(txn *memdb.Txn, oldMod, newMod *Module) oldDiags, newDiags := 0, 0 if oldMod != nil { - for _, diags := range oldMod.ModuleDiagnostics { - oldDiags += diags.Count() - } - for _, diags := range oldMod.VarsDiagnostics { - oldDiags += diags.Count() - } + oldDiags = oldMod.ModuleDiagnostics.Count() + oldMod.VarsDiagnostics.Count() } if newMod != nil { - for _, diags := range newMod.ModuleDiagnostics { - newDiags += diags.Count() - } - for _, diags := range newMod.VarsDiagnostics { - newDiags += diags.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/terraform/ast/diagnostics.go b/internal/terraform/ast/diagnostics.go index d71e18833..6d4c8f5c8 100644 --- a/internal/terraform/ast/diagnostics.go +++ b/internal/terraform/ast/diagnostics.go @@ -13,8 +13,7 @@ import ( type DiagnosticSource int const ( - ModuleParsingSource DiagnosticSource = iota - VarsParsingSource + HCLParsingSource DiagnosticSource = iota SchemaValidationSource ReferenceValidationSource TerraformValidateSource @@ -22,14 +21,12 @@ const ( func (d DiagnosticSource) String() string { switch d { - case ModuleParsingSource: + case HCLParsingSource: return "HCL" - case VarsParsingSource: - return "HCL Vars" case SchemaValidationSource: - return "schema validation" + return "early validation" case ReferenceValidationSource: - return "reference validation" + return "early validation" case TerraformValidateSource: return "terraform validate" default: @@ -37,8 +34,6 @@ func (d DiagnosticSource) String() string { } } -// TODO? combine with langserver/diagnostics - type DiagnosticSourceState map[DiagnosticSource]op.OpState func (dss DiagnosticSourceState) Copy() DiagnosticSourceState { diff --git a/internal/terraform/ast/module.go b/internal/terraform/ast/module.go index 95263f163..70ea504a6 100644 --- a/internal/terraform/ast/module.go +++ b/internal/terraform/ast/module.go @@ -92,3 +92,11 @@ func (md ModDiags) Count() int { } 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 1a71f5ad1..3486ba4e1 100644 --- a/internal/terraform/ast/variables.go +++ b/internal/terraform/ast/variables.go @@ -89,3 +89,11 @@ func (vd VarsDiags) Count() int { } 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 a3fc4809a..fee44280c 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -381,7 +381,7 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat return sErr } - sErr = modStore.UpdateModuleDiagnostics(modPath, ast.ModuleParsingSource, diags) + sErr = modStore.UpdateModuleDiagnostics(modPath, ast.HCLParsingSource, diags) if sErr != nil { return sErr } @@ -416,7 +416,7 @@ func ParseVariables(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleSt return sErr } - sErr = modStore.UpdateVarsDiagnostics(modPath, ast.VarsParsingSource, diags) + sErr = modStore.UpdateVarsDiagnostics(modPath, ast.HCLParsingSource, diags) if sErr != nil { return sErr } From 6cb73e83af7b64d44715dd806640a8a48efc990b Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 25 Aug 2023 15:47:28 +0200 Subject: [PATCH 06/10] Pass pathCtx to reference validation funcs --- internal/decoder/validations/unreferenced_origin.go | 7 +------ internal/terraform/module/module_ops.go | 4 +--- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/internal/decoder/validations/unreferenced_origin.go b/internal/decoder/validations/unreferenced_origin.go index e8871153a..ad5f550d0 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/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index fee44280c..45aec70fc 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -732,9 +732,7 @@ func ReferenceValidation(ctx context.Context, modStore *state.ModuleStore, schem return err } - ctx = decoder.WithPathContext(ctx, pathCtx) - - diags := validations.UnreferencedOrigins(ctx) + diags := validations.UnreferencedOrigins(ctx, pathCtx) return modStore.UpdateModuleDiagnostics(modPath, ast.ReferenceValidationSource, ast.ModDiagsFromMap(diags)) } From 09cdb0b25db8a81fb607e7329491ccd0e64f81b0 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 25 Aug 2023 15:47:43 +0200 Subject: [PATCH 07/10] Rename early validation job --- internal/indexer/document_change.go | 4 ++-- internal/terraform/module/module_ops.go | 2 +- internal/terraform/module/operation/op_type_string.go | 6 +++--- internal/terraform/module/operation/operation.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/indexer/document_change.go b/internal/indexer/document_change.go index 704b25df4..7565edde0 100644 --- a/internal/indexer/document_change.go +++ b/internal/indexer/document_change.go @@ -100,9 +100,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.EarlyValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) + return module.SchemaValidation(ctx, idx.modStore, idx.schemaStore, modHandle.Path()) }, - Type: op.OpTypeEarlyValidation.String(), + Type: op.OpTypeSchemaValidation.String(), DependsOn: job.IDs{eSchemaId}, IgnoreState: ignoreState, }) diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 45aec70fc..4f8b8127e 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -665,7 +665,7 @@ 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 diff --git a/internal/terraform/module/operation/op_type_string.go b/internal/terraform/module/operation/op_type_string.go index 215339382..5b4ac8744 100644 --- a/internal/terraform/module/operation/op_type_string.go +++ b/internal/terraform/module/operation/op_type_string.go @@ -21,14 +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 = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeEarlyValidationOpTypeReferenceValidationOpTypeTerraformValidate" +const _OpType_name = "OpTypeUnknownOpTypeGetTerraformVersionOpTypeObtainSchemaOpTypeParseModuleConfigurationOpTypeParseVariablesOpTypeParseModuleManifestOpTypeLoadModuleMetadataOpTypeDecodeReferenceTargetsOpTypeDecodeReferenceOriginsOpTypeDecodeVarsReferencesOpTypeGetModuleDataFromRegistryOpTypeParseProviderVersionsOpTypePreloadEmbeddedSchemaOpTypeSchemaValidationOpTypeReferenceValidationOpTypeTerraformValidate" -var _OpType_index = [...]uint16{0, 13, 38, 56, 86, 106, 131, 155, 183, 211, 237, 268, 295, 322, 343, 368, 391} +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 757e9510d..fe408450d 100644 --- a/internal/terraform/module/operation/operation.go +++ b/internal/terraform/module/operation/operation.go @@ -30,7 +30,7 @@ const ( OpTypeGetModuleDataFromRegistry OpTypeParseProviderVersions OpTypePreloadEmbeddedSchema - OpTypeEarlyValidation + OpTypeSchemaValidation OpTypeReferenceValidation OpTypeTerraformValidate ) From 61f2042d1e769b6221d7f898a30400cbe31e9d99 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Fri, 25 Aug 2023 15:48:52 +0200 Subject: [PATCH 08/10] Bump hcl-lang to `29034e` --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index d6c85b245..5984ab113 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/hc-install v0.5.2 - github.com/hashicorp/hcl-lang v0.0.0-20230823131918-b6a3f8271038 + github.com/hashicorp/hcl-lang v0.0.0-20230825134805-29034e9d534d github.com/hashicorp/hcl/v2 v2.17.0 github.com/hashicorp/terraform-exec v0.18.1 github.com/hashicorp/terraform-json v0.17.1 @@ -27,7 +27,7 @@ require ( github.com/pmezard/go-difflib v1.0.0 github.com/stretchr/testify v1.8.4 github.com/vektra/mockery/v2 v2.32.4 - github.com/zclconf/go-cty v1.13.2 + github.com/zclconf/go-cty v1.13.3 github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b go.bobheadxi.dev/gobenchdata v1.3.1 go.opentelemetry.io/otel/trace v1.16.0 diff --git a/go.sum b/go.sum index 35c6db23f..6024339f0 100644 --- a/go.sum +++ b/go.sum @@ -215,8 +215,8 @@ github.com/hashicorp/hc-install v0.5.2 h1:SfwMFnEXVVirpwkDuSF5kymUOhrUxrTq3udEse github.com/hashicorp/hc-install v0.5.2/go.mod h1:9QISwe6newMWIfEiXpzuu1k9HAGtQYgnSH8H9T8wmoI= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/hashicorp/hcl-lang v0.0.0-20230823131918-b6a3f8271038 h1:fhyfQoAyYFvt+AqIFlTHrstlC4OOT4KB/F/KatqJUfI= -github.com/hashicorp/hcl-lang v0.0.0-20230823131918-b6a3f8271038/go.mod h1:xlYq00R/OYgpAmScjO1zkE9NCjl6zuMex1VVcUU0pjQ= +github.com/hashicorp/hcl-lang v0.0.0-20230825134805-29034e9d534d h1:kr/f+fJGnFGe2al/H55/blLb1Ne5BAZWe4qY/BxA2D0= +github.com/hashicorp/hcl-lang v0.0.0-20230825134805-29034e9d534d/go.mod h1:X9wA1+6Zr0nVspFxRvhVZnDhBMWhvb0uuAjp4bGujuc= github.com/hashicorp/hcl/v2 v2.17.0 h1:z1XvSUyXd1HP10U4lrLg5e0JMVz6CPaJvAgxM0KNZVY= github.com/hashicorp/hcl/v2 v2.17.0/go.mod h1:gJyW2PTShkJqQBKpAmPO3yxMxIuoXkOF2TpqXzrQyx4= github.com/hashicorp/terraform-exec v0.18.1 h1:LAbfDvNQU1l0NOQlTuudjczVhHj061fNX5H8XZxHlH4= @@ -378,8 +378,8 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= -github.com/zclconf/go-cty v1.13.2 h1:4GvrUxe/QUDYuJKAav4EYqdM47/kZa672LwmXFmEKT0= -github.com/zclconf/go-cty v1.13.2/go.mod h1:YKQzy/7pZ7iq2jNFzy5go57xdxdWoLLpaEp4u238AE0= +github.com/zclconf/go-cty v1.13.3 h1:m+b9q3YDbg6Bec5rr+KGy1MzEVzY/jC2X+YX4yqKtHI= +github.com/zclconf/go-cty v1.13.3/go.mod h1:YKQzy/7pZ7iq2jNFzy5go57xdxdWoLLpaEp4u238AE0= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= go.bobheadxi.dev/gobenchdata v1.3.1 h1:3Pts2nPUZdgFSU63nWzvfs2xRbK8WVSNeJ2H9e/Ypew= From ca65c7368ac2b152b1f5ad6af182bf694d061a56 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Mon, 28 Aug 2023 12:19:33 +0200 Subject: [PATCH 09/10] Introduce VarsDiagnosticsState --- internal/state/module.go | 108 ++++++++++-------------- internal/terraform/ast/diagnostics.go | 2 +- internal/terraform/module/module_ops.go | 20 ++--- 3 files changed, 55 insertions(+), 75 deletions(-) diff --git a/internal/state/module.go b/internal/state/module.go index 54feb1dbe..b06765b36 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -122,21 +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.SourceModDiags - VarsDiagnostics ast.SourceVarsDiags - - DiagnosticsState ast.DiagnosticSourceState + ModuleDiagnostics ast.SourceModDiags + ModuleDiagnosticsState ast.DiagnosticSourceState + VarsDiagnostics ast.SourceVarsDiags + VarsDiagnosticsState ast.DiagnosticSourceState } func (m *Module) Copy() *Module { @@ -175,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, + ModuleParsingErr: m.ModuleParsingErr, + VarsParsingErr: m.VarsParsingErr, Meta: m.Meta.Copy(), MetaErr: m.MetaErr, MetaState: m.MetaState, - DiagnosticsState: m.DiagnosticsState.Copy(), + ModuleDiagnosticsState: m.ModuleDiagnosticsState.Copy(), + VarsDiagnosticsState: m.VarsDiagnosticsState.Copy(), } if m.InstalledProviders != nil { @@ -249,9 +246,14 @@ func newModule(modPath string) *Module { PreloadEmbeddedSchemaState: op.OpStateUnknown, InstalledProvidersState: op.OpStateUnknown, RefTargetsState: op.OpStateUnknown, - ModuleParsingState: op.OpStateUnknown, MetaState: op.OpStateUnknown, - DiagnosticsState: ast.DiagnosticSourceState{ + 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, @@ -829,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) @@ -894,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) @@ -979,7 +937,7 @@ func (s *ModuleStore) UpdateMetadata(path string, meta *tfmod.Meta, mErr error) func (s *ModuleStore) UpdateModuleDiagnostics(path string, source ast.DiagnosticSource, diags ast.ModDiags) error { txn := s.db.Txn(true) txn.Defer(func() { - s.SetDiagnosticsState(path, source, op.OpStateLoaded) + s.SetModuleDiagnosticsState(path, source, op.OpStateLoaded) }) defer txn.Abort() @@ -1008,7 +966,7 @@ func (s *ModuleStore) UpdateModuleDiagnostics(path string, source ast.Diagnostic return nil } -func (s *ModuleStore) SetDiagnosticsState(path string, source ast.DiagnosticSource, 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() @@ -1016,7 +974,7 @@ func (s *ModuleStore) SetDiagnosticsState(path string, source ast.DiagnosticSour if err != nil { return err } - mod.DiagnosticsState[source] = state + mod.ModuleDiagnosticsState[source] = state err = txn.Insert(s.tableName, mod) if err != nil { @@ -1029,6 +987,9 @@ func (s *ModuleStore) SetDiagnosticsState(path string, source ast.DiagnosticSour 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) + }) defer txn.Abort() oldMod, err := moduleByPath(txn, path) @@ -1056,6 +1017,25 @@ func (s *ModuleStore) UpdateVarsDiagnostics(path string, source ast.DiagnosticSo return nil } +func (s *ModuleStore) SetVarsDiagnosticsState(path string, source ast.DiagnosticSource, state op.OpState) error { + txn := s.db.Txn(true) + defer txn.Abort() + + mod, err := moduleCopyByPath(txn, path) + if err != nil { + return err + } + mod.VarsDiagnosticsState[source] = state + + err = txn.Insert(s.tableName, mod) + if err != nil { + return err + } + + txn.Commit() + return nil +} + func (s *ModuleStore) SetReferenceTargetsState(path string, state op.OpState) error { txn := s.db.Txn(true) defer txn.Abort() diff --git a/internal/terraform/ast/diagnostics.go b/internal/terraform/ast/diagnostics.go index 6d4c8f5c8..fa8ecb597 100644 --- a/internal/terraform/ast/diagnostics.go +++ b/internal/terraform/ast/diagnostics.go @@ -30,7 +30,7 @@ func (d DiagnosticSource) String() string { case TerraformValidateSource: return "terraform validate" default: - return fmt.Sprintf("Unknown %d", d) + panic(fmt.Sprintf("Unknown diagnostic source %d", d)) } } diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 4f8b8127e..07848c12d 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -365,11 +365,11 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat // 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.ModuleParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) { + if mod.ModuleDiagnosticsState[ast.HCLParsingSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } - err = modStore.SetModuleParsingState(modPath, op.OpStateLoading) + err = modStore.SetModuleDiagnosticsState(modPath, ast.HCLParsingSource, op.OpStateLoading) if err != nil { return err } @@ -400,11 +400,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 } @@ -672,11 +672,11 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe } // Avoid validation if it is already in progress or already finished - if mod.DiagnosticsState[ast.SchemaValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + if mod.ModuleDiagnosticsState[ast.SchemaValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } - err = modStore.SetDiagnosticsState(modPath, ast.SchemaValidationSource, op.OpStateLoading) + err = modStore.SetModuleDiagnosticsState(modPath, ast.SchemaValidationSource, op.OpStateLoading) if err != nil { return err } @@ -711,11 +711,11 @@ func ReferenceValidation(ctx context.Context, modStore *state.ModuleStore, schem } // Avoid validation if it is already in progress or already finished - if mod.DiagnosticsState[ast.ReferenceValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + if mod.ModuleDiagnosticsState[ast.ReferenceValidationSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } - err = modStore.SetDiagnosticsState(modPath, ast.ReferenceValidationSource, op.OpStateLoading) + err = modStore.SetModuleDiagnosticsState(modPath, ast.ReferenceValidationSource, op.OpStateLoading) if err != nil { return err } @@ -743,11 +743,11 @@ func TerraformValidate(ctx context.Context, modStore *state.ModuleStore, modPath } // Avoid validation if it is already in progress or already finished - if mod.DiagnosticsState[ast.TerraformValidateSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { + if mod.ModuleDiagnosticsState[ast.TerraformValidateSource] != op.OpStateUnknown && !job.IgnoreState(ctx) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } - err = modStore.SetDiagnosticsState(modPath, ast.TerraformValidateSource, op.OpStateLoading) + err = modStore.SetModuleDiagnosticsState(modPath, ast.TerraformValidateSource, op.OpStateLoading) if err != nil { return err } From c782567032a30942ec7df04a3e2e223bb50035aa Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Mon, 28 Aug 2023 12:39:23 +0200 Subject: [PATCH 10/10] Fix tests --- .../validations/unreferenced_origin_test.go | 4 +- .../diagnostics/diagnostics_test.go | 23 ++-- internal/state/module_test.go | 128 ++++++++++++++++-- 3 files changed, 129 insertions(+), 26 deletions(-) diff --git a/internal/decoder/validations/unreferenced_origin_test.go b/internal/decoder/validations/unreferenced_origin_test.go index 420d4f243..f36b6122d 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/langserver/diagnostics/diagnostics_test.go b/internal/langserver/diagnostics/diagnostics_test.go index 4a91f5a78..d3c95b680 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/state/module_test.go b/internal/state/module_test.go index 1769e1131..83b9e22bd 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,7 +529,7 @@ provider "blah" { region = "london" `), "test.tf") - err = s.Modules.UpdateModuleDiagnostics(tmpDir, ast.ModuleParsingSource, 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 { @@ -440,7 +542,7 @@ provider "blah" { } expectedDiags := ast.SourceModDiags{ - ast.ModuleParsingSource: ast.ModDiagsFromMap(map[string]hcl.Diagnostics{ + ast.HCLParsingSource: ast.ModDiagsFromMap(map[string]hcl.Diagnostics{ "test.tf": { { Severity: hcl.DiagError, @@ -486,7 +588,7 @@ dev = { region = "london" `), "test.tfvars") - err = s.Modules.UpdateVarsDiagnostics(tmpDir, ast.VarsParsingSource, 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 { @@ -499,7 +601,7 @@ dev = { } expectedDiags := ast.SourceVarsDiags{ - ast.VarsParsingSource: ast.VarsDiagsFromMap(map[string]hcl.Diagnostics{ + ast.HCLParsingSource: ast.VarsDiagsFromMap(map[string]hcl.Diagnostics{ "test.tfvars": { { Severity: hcl.DiagError, @@ -742,17 +844,19 @@ func BenchmarkModuleByPath(b *testing.B) { b.Fatal(err) } mDiags := ast.ModDiagsFromMap(diags) - err = s.Modules.UpdateModuleDiagnostics(modPath, ast.ModuleParsingSource, mDiags) + err = s.Modules.UpdateModuleDiagnostics(modPath, ast.HCLParsingSource, mDiags) if err != nil { b.Fatal(err) } expectedMod := &Module{ - Path: modPath, - ParsedModuleFiles: mFiles, - ModuleParsingState: operation.OpStateLoaded, + Path: modPath, + ParsedModuleFiles: mFiles, ModuleDiagnostics: ast.SourceModDiags{ - ast.ModuleParsingSource: mDiags, + ast.HCLParsingSource: mDiags, + }, + ModuleDiagnosticsState: ast.DiagnosticSourceState{ + ast.HCLParsingSource: operation.OpStateLoaded, }, }