From be0d532959693c64c2d4c9b5bd749cfd32dd5cbd Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 15 Nov 2021 21:21:37 +0100 Subject: [PATCH] WIP --- internal/langserver/handlers/complete_test.go | 2 +- internal/langserver/handlers/did_open.go | 10 ++-- .../langserver/handlers/references_test.go | 8 +++ internal/terraform/module/module_loader.go | 6 +- .../terraform/module/module_loader_test.go | 19 +++--- internal/terraform/module/module_manager.go | 60 +++++++++++++++++++ internal/terraform/module/module_ops.go | 1 + internal/terraform/module/walker.go | 27 ++++++--- internal/terraform/module/watcher.go | 7 ++- 9 files changed, 108 insertions(+), 32 deletions(-) diff --git a/internal/langserver/handlers/complete_test.go b/internal/langserver/handlers/complete_test.go index 142e7f455..443458685 100644 --- a/internal/langserver/handlers/complete_test.go +++ b/internal/langserver/handlers/complete_test.go @@ -721,7 +721,7 @@ output "test" { // asynchronously and we currently have no way of waiting // for them to complete. // TODO remove once we support synchronous dependent tasks - time.Sleep(1500 * time.Millisecond) + time.Sleep(2500 * time.Millisecond) ls.CallAndExpectResponse(t, &langserver.CallRequest{ Method: "textDocument/completion", diff --git a/internal/langserver/handlers/did_open.go b/internal/langserver/handlers/did_open.go index 40dd052a8..b51465335 100644 --- a/internal/langserver/handlers/did_open.go +++ b/internal/langserver/handlers/did_open.go @@ -48,11 +48,11 @@ func (lh *logHandler) TextDocumentDidOpen(ctx context.Context, params lsp.DidOpe // We reparse because the file being opened may not match // (originally parsed) content on the disk // TODO: Do this only if we can verify the file differs? - modMgr.EnqueueModuleOp(mod.Path, op.OpTypeParseModuleConfiguration, nil) - modMgr.EnqueueModuleOp(mod.Path, op.OpTypeParseVariables, nil) - modMgr.EnqueueModuleOp(mod.Path, op.OpTypeLoadModuleMetadata, nil) - modMgr.EnqueueModuleOp(mod.Path, op.OpTypeDecodeReferenceTargets, nil) - modMgr.EnqueueModuleOp(mod.Path, op.OpTypeDecodeReferenceOrigins, nil) + modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeParseModuleConfiguration) + modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeParseVariables) + modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeLoadModuleMetadata) + modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeDecodeReferenceTargets) + modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeDecodeReferenceOrigins) if mod.TerraformVersionState == op.OpStateUnknown { modMgr.EnqueueModuleOp(mod.Path, op.OpTypeGetTerraformVersion, nil) diff --git a/internal/langserver/handlers/references_test.go b/internal/langserver/handlers/references_test.go index 8dc187fff..dafbc3f42 100644 --- a/internal/langserver/handlers/references_test.go +++ b/internal/langserver/handlers/references_test.go @@ -4,6 +4,7 @@ import ( "fmt" "path/filepath" "testing" + "time" "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-ls/internal/langserver" @@ -178,6 +179,13 @@ variable "instances" { "uri": "%s/main.tf" } }`, submodUri.URI())}) + + // module manifest-dependent tasks are scheduled & executed + // asynchronously and we currently have no way of waiting + // for them to complete. + // TODO remove once we support synchronous dependent tasks + time.Sleep(3500 * time.Millisecond) + ls.CallAndExpectResponse(t, &langserver.CallRequest{ Method: "textDocument/references", ReqParams: fmt.Sprintf(`{ diff --git a/internal/terraform/module/module_loader.go b/internal/terraform/module/module_loader.go index 36816dd7c..31dfd8a91 100644 --- a/internal/terraform/module/module_loader.go +++ b/internal/terraform/module/module_loader.go @@ -144,7 +144,7 @@ func (ml *moduleLoader) nonPrioCapacity() int64 { } func (ml *moduleLoader) executeModuleOp(ctx context.Context, modOp ModuleOperation) { - ml.logger.Printf("executing %q for %s", modOp.Type, modOp.ModulePath) + ml.logger.Printf("ML: executing %q for %q", modOp.Type, modOp.ModulePath) // TODO: Report progress in % for each op based on queue length defer modOp.markAsDone() @@ -196,7 +196,7 @@ func (ml *moduleLoader) executeModuleOp(ctx context.Context, modOp ModuleOperati modOp.ModulePath, modOp.Type) return } - ml.logger.Printf("finished %q for %s", modOp.Type, modOp.ModulePath) + ml.logger.Printf("ML: finished %q for %q", modOp.Type, modOp.ModulePath) if modOp.Defer != nil { go modOp.Defer(opErr) @@ -209,7 +209,7 @@ func (ml *moduleLoader) EnqueueModuleOp(modOp ModuleOperation) error { return err } - ml.logger.Printf("ML: enqueing %q module operation: %s", modOp.Type, modOp.ModulePath) + ml.logger.Printf("ML: enqueing %q module operation: %q", modOp.Type, modOp.ModulePath) switch modOp.Type { case op.OpTypeGetTerraformVersion: diff --git a/internal/terraform/module/module_loader_test.go b/internal/terraform/module/module_loader_test.go index d78403cf6..6788a17ab 100644 --- a/internal/terraform/module/module_loader_test.go +++ b/internal/terraform/module/module_loader_test.go @@ -46,7 +46,7 @@ func TestModuleLoader_referenceCollection(t *testing.T) { if err != nil { t.Fatal(err) } - <-modOp.doneCh + <-modOp.Done() manifestOp := NewModuleOperation(modPath, op.OpTypeLoadModuleMetadata) err = ml.EnqueueModuleOp(manifestOp) @@ -66,17 +66,12 @@ func TestModuleLoader_referenceCollection(t *testing.T) { } t.Log("waiting for all operations to finish") - for i := 0; i <= 2; i++ { - select { - case <-manifestOp.doneCh: - t.Log("manifest parsed") - case <-originsOp.doneCh: - t.Log("origins collected") - case <-targetsOp.doneCh: - t.Log("targets collected") - case <-ctx.Done(): - } - } + <-manifestOp.Done() + t.Log("manifest parsed") + <-originsOp.Done() + t.Log("origins collected") + <-targetsOp.Done() + t.Log("targets collected") mod, err := ss.Modules.ModuleByPath(modPath) if err != nil { diff --git a/internal/terraform/module/module_manager.go b/internal/terraform/module/module_manager.go index f3645fe20..f68db083d 100644 --- a/internal/terraform/module/module_manager.go +++ b/internal/terraform/module/module_manager.go @@ -82,6 +82,14 @@ func (mm *moduleManager) RemoveModule(modPath string) error { } func (mm *moduleManager) EnqueueModuleOpWait(modPath string, opType op.OpType) error { + isQueued, err := mm.isAlreadyQueued(modPath, opType) + if err != nil { + return err + } + if isQueued { + return nil + } + modOp := NewModuleOperation(modPath, opType) mm.loader.EnqueueModuleOp(modOp) @@ -91,6 +99,14 @@ func (mm *moduleManager) EnqueueModuleOpWait(modPath string, opType op.OpType) e } func (mm *moduleManager) EnqueueModuleOp(modPath string, opType op.OpType, deferFunc DeferFunc) error { + isQueued, err := mm.isAlreadyQueued(modPath, opType) + if err != nil { + return err + } + if isQueued { + return nil + } + modOp := NewModuleOperation(modPath, opType) modOp.Defer = deferFunc mm.loader.EnqueueModuleOp(modOp) @@ -100,6 +116,50 @@ func (mm *moduleManager) EnqueueModuleOp(modPath string, opType op.OpType, defer return nil } +func (mm *moduleManager) isAlreadyQueued(modPath string, opType op.OpType) (bool, error) { + mod, err := mm.moduleStore.ModuleByPath(modPath) + if err != nil { + return false, err + } + + switch opType { + case op.OpTypeGetTerraformVersion: + if mod.TerraformVersionState == op.OpStateQueued { + return true, nil + } + case op.OpTypeObtainSchema: + if mod.ProviderSchemaState == op.OpStateQueued { + return true, nil + } + case op.OpTypeParseModuleConfiguration: + if mod.ModuleParsingState == op.OpStateQueued { + return true, nil + } + case op.OpTypeParseVariables: + if mod.VarsParsingState == op.OpStateQueued { + return true, nil + } + case op.OpTypeParseModuleManifest: + if mod.ModManifestState == op.OpStateQueued { + return true, nil + } + case op.OpTypeLoadModuleMetadata: + if mod.MetaState == op.OpStateQueued { + return true, nil + } + case op.OpTypeDecodeReferenceTargets: + if mod.RefTargetsState == op.OpStateQueued { + return true, nil + } + case op.OpTypeDecodeReferenceOrigins: + if mod.RefOriginsState == op.OpStateQueued { + return true, nil + } + } + + return false, nil +} + func (mm *moduleManager) CallersOfModule(modPath string) ([]Module, error) { modules := make([]Module, 0) callers, err := mm.moduleStore.CallersOfModule(modPath) diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 83d0bd749..2a925f3df 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -39,6 +39,7 @@ func NewModuleOperation(modPath string, typ op.OpType) ModuleOperation { func (mo ModuleOperation) markAsDone() { mo.doneCh <- struct{}{} + close(mo.doneCh) } func (mo ModuleOperation) Done() <-chan struct{} { diff --git a/internal/terraform/module/walker.go b/internal/terraform/module/walker.go index 5b6bb633b..e32895bba 100644 --- a/internal/terraform/module/walker.go +++ b/internal/terraform/module/walker.go @@ -260,6 +260,11 @@ func (w *Walker) walk(ctx context.Context, rootPath string) error { } } + err = w.modMgr.EnqueueModuleOp(dir, op.OpTypeParseModuleConfiguration, nil) + if err != nil { + return err + } + err = w.modMgr.EnqueueModuleOp(dir, op.OpTypeGetTerraformVersion, nil) if err != nil { return err @@ -267,20 +272,24 @@ func (w *Walker) walk(ctx context.Context, rootPath string) error { dataDir := datadir.WalkDataDirOfModule(w.fs, dir) if dataDir.ModuleManifestPath != "" { + // References are collected *after* manifest parsing + // so that we reflect any references to submodules. err = w.modMgr.EnqueueModuleOp(dir, op.OpTypeParseModuleManifest, decodeCalledModulesFunc(w.modMgr, w.watcher, dir)) if err != nil { return err } - } - - err = w.modMgr.EnqueueModuleOp(dir, op.OpTypeDecodeReferenceTargets, nil) - if err != nil { - return err - } - err = w.modMgr.EnqueueModuleOp(dir, op.OpTypeDecodeReferenceOrigins, nil) - if err != nil { - return err + } else { + // If there is no module manifest we still collect references + // as this module may also be called by other modules. + err = w.modMgr.EnqueueModuleOp(dir, op.OpTypeDecodeReferenceTargets, nil) + if err != nil { + return err + } + err = w.modMgr.EnqueueModuleOp(dir, op.OpTypeDecodeReferenceOrigins, nil) + if err != nil { + return err + } } if dataDir.PluginLockFilePath != "" { diff --git a/internal/terraform/module/watcher.go b/internal/terraform/module/watcher.go index c78a9b086..979069b86 100644 --- a/internal/terraform/module/watcher.go +++ b/internal/terraform/module/watcher.go @@ -245,8 +245,8 @@ func decodeCalledModulesFunc(modMgr ModuleManager, w Watcher, modPath string) De } modMgr.AddModule(mc.Path) - modMgr.EnqueueModuleOpWait(mc.Path, op.OpTypeParseModuleConfiguration) - modMgr.EnqueueModuleOpWait(mc.Path, op.OpTypeLoadModuleMetadata) + modMgr.EnqueueModuleOp(mc.Path, op.OpTypeParseModuleConfiguration, nil) + modMgr.EnqueueModuleOp(mc.Path, op.OpTypeLoadModuleMetadata, nil) modMgr.EnqueueModuleOp(mc.Path, op.OpTypeParseVariables, nil) modMgr.EnqueueModuleOp(mc.Path, op.OpTypeDecodeReferenceTargets, nil) @@ -256,6 +256,9 @@ func decodeCalledModulesFunc(modMgr ModuleManager, w Watcher, modPath string) De w.AddModule(mc.Path) } } + + modMgr.EnqueueModuleOp(modPath, op.OpTypeDecodeReferenceTargets, nil) + modMgr.EnqueueModuleOp(modPath, op.OpTypeDecodeReferenceOrigins, nil) } }