WIP

.github/workflows/ci.yml

@@ -99,4 +99,4 @@ jobs:
         run: make fmt
       -
         name: Run tests
-        run: go test -cover -covermode=atomic -race ./...
+        run: go test -cover -covermode=atomic -v -timeout=7m -race ./...

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",

internal/langserver/handlers/did_open.go

@@ -48,14 +48,20 @@ 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)
+	lh.logger.Printf("DIDOPEN_HANDLER: configuration parsed OK")
+	modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeParseVariables)
+	lh.logger.Printf("DIDOPEN_HANDLER: vars parsed OK")
+	modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeLoadModuleMetadata)
+	lh.logger.Printf("DIDOPEN_HANDLER: meta parsed OK")
+	modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeDecodeReferenceTargets)
+	lh.logger.Printf("DIDOPEN_HANDLER: targets parsed OK")
+	modMgr.EnqueueModuleOpWait(mod.Path, op.OpTypeDecodeReferenceOrigins)
+	lh.logger.Printf("DIDOPEN_HANDLER: origins parsed OK")
 
 	if mod.TerraformVersionState == op.OpStateUnknown {
 		modMgr.EnqueueModuleOp(mod.Path, op.OpTypeGetTerraformVersion, nil)
+		lh.logger.Printf("DIDOPEN_HANDLER: version parsed OK")
 	}
 
 	watcher, err := lsctx.Watcher(ctx)
@@ -64,17 +70,21 @@ func (lh *logHandler) TextDocumentDidOpen(ctx context.Context, params lsp.DidOpe
 	}
 
 	if !watcher.IsModuleWatched(mod.Path) {
+		lh.logger.Printf("DIDOPEN_HANDLER: adding module to watcher")
 		err := watcher.AddModule(mod.Path)
 		if err != nil {
 			return err
 		}
+		lh.logger.Printf("DIDOPEN_HANDLER: added module to watcher")
 	}
 
 	notifier, err := lsctx.DiagnosticsNotifier(ctx)
 	if err != nil {
 		return err
 	}
 
+	lh.logger.Printf("DIDOPEN_HANDLER: assembling all available diagnostics")
+
 	diags := diagnostics.NewDiagnostics()
 	diags.EmptyRootDiagnostic()
 	diags.Append("HCL", mod.ModuleDiagnostics.AsMap())
@@ -83,7 +93,9 @@ func (lh *logHandler) TextDocumentDidOpen(ctx context.Context, params lsp.DidOpe
 		diags.Append("HCL", mod.VarsDiagnostics.ForFile(vf).AsMap())
 	}
 
+	lh.logger.Printf("DIDOPEN_HANDLER: publishing diagnostics")
 	notifier.PublishHCLDiags(ctx, mod.Path, diags)
+	lh.logger.Printf("DIDOPEN_HANDLER: diagnostics published")
 
 	return nil
 }

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(`{

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:

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 {

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)

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{} {

internal/terraform/module/walker.go

@@ -260,27 +260,36 @@ 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
 			}
 
 			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 != "" {

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)
 	}
 }