From 334433ff3755406bb1556191b12a762429670cb9 Mon Sep 17 00:00:00 2001 From: James Pogran Date: Tue, 26 Sep 2023 14:35:58 -0400 Subject: [PATCH] Update ParseVariables for single files Modifies ParsedVarsFiles to only re-parse the single tfvars file which is being changed, if the job was scheduled as part of `textDocument/didChange` request. This is a follow up to https://github.com/hashicorp/terraform-ls/pull/1404 which updated the parsing job for terraform files. --- internal/terraform/ast/variables.go | 16 +++++ internal/terraform/module/module_ops.go | 44 ++++++++++-- internal/terraform/module/module_ops_test.go | 72 ++++++++++++++++++++ internal/terraform/parser/variables.go | 15 ++++ 4 files changed, 142 insertions(+), 5 deletions(-) diff --git a/internal/terraform/ast/variables.go b/internal/terraform/ast/variables.go index a2f6aaf9e..07ea52ac1 100644 --- a/internal/terraform/ast/variables.go +++ b/internal/terraform/ast/variables.go @@ -52,6 +52,14 @@ func VarsFilesFromMap(m map[string]*hcl.File) VarsFiles { return mf } +func (mf VarsFiles) Copy() VarsFiles { + m := make(VarsFiles, len(mf)) + for name, file := range mf { + m[name] = file + } + return m +} + type VarsDiags map[VarsFilename]hcl.Diagnostics func VarsDiagsFromMap(m map[string]hcl.Diagnostics) VarsDiags { @@ -62,6 +70,14 @@ func VarsDiagsFromMap(m map[string]hcl.Diagnostics) VarsDiags { return mf } +func (mf VarsDiags) Copy() VarsDiags { + m := make(VarsDiags, len(mf)) + for name, file := range mf { + m[name] = file + } + return m +} + func (vd VarsDiags) AutoloadedOnly() VarsDiags { diags := make(VarsDiags) for name, f := range vd { diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index 42d9e1cd9..5e2bba7d2 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -431,20 +431,54 @@ func ParseVariables(ctx context.Context, fs ReadOnlyFS, modStore *state.ModuleSt // TODO: Avoid parsing if the content matches existing AST - // 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) { return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)} } - err = modStore.SetVarsParsingState(modPath, op.OpStateLoading) + var files ast.VarsFiles + var diags ast.VarsDiags + rpcContext := lsctx.RPCContext(ctx) + // Only parse the file that's being changed/opened, unless this is 1st-time parsing + if mod.ModuleParsingState == op.OpStateLoaded && rpcContext.IsDidChangeRequest() && lsctx.IsLanguageId(ctx, ilsp.Tfvars.String()) { + // the file has already been parsed, so only examine this file and not the whole module + err = modStore.SetVarsParsingState(modPath, op.OpStateLoading) + if err != nil { + return err + } + filePath, err := uri.PathFromURI(rpcContext.URI) + if err != nil { + return err + } + fileName := filepath.Base(filePath) + + f, vdiags, err := parser.ParseVariableFile(fs, filePath) + if err != nil { + return err + } + + existingFiles := mod.ParsedVarsFiles.Copy() + existingFiles[ast.VarsFilename(fileName)] = f + files = existingFiles + + existingDiags := mod.VarsDiagnostics.Copy() + existingDiags[ast.VarsFilename(fileName)] = vdiags + diags = existingDiags + + } else { + // this is the first time file is opened so parse the whole module + err = modStore.SetVarsParsingState(modPath, op.OpStateLoading) + if err != nil { + return err + } + + files, diags, err = parser.ParseVariableFiles(fs, modPath) + } + if err != nil { return err } - files, diags, err := parser.ParseVariableFiles(fs, modPath) - sErr := modStore.UpdateParsedVarsFiles(modPath, files, err) if sErr != nil { return sErr diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index 3bb0486fb..2a4ef9fcb 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -1017,6 +1017,7 @@ func TestParseModuleConfiguration(t *testing.T) { t.Fatal("diags should match") } } + func TestParseModuleConfiguration_ignore_tfvars(t *testing.T) { ctx := context.Background() ss, err := state.NewStateStore() @@ -1087,6 +1088,77 @@ func TestParseModuleConfiguration_ignore_tfvars(t *testing.T) { } } +func TestParseVariables(t *testing.T) { + ctx := context.Background() + ss, err := state.NewStateStore() + if err != nil { + t.Fatal(err) + } + + testData, err := filepath.Abs("testdata") + if err != nil { + t.Fatal(err) + } + testFs := filesystem.NewFilesystem(ss.DocumentStore) + + singleFileModulePath := filepath.Join(testData, "single-file-change-module") + + err = ss.Modules.Add(singleFileModulePath) + if err != nil { + t.Fatal(err) + } + + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{}) + err = ParseVariables(ctx, testFs, ss.Modules, singleFileModulePath) + if err != nil { + t.Fatal(err) + } + + before, err := ss.Modules.ModuleByPath(singleFileModulePath) + if err != nil { + t.Fatal(err) + } + + // ignore job state + ctx = job.WithIgnoreState(ctx, true) + + // say we're coming from did_change request + fooURI, err := filepath.Abs(filepath.Join(singleFileModulePath, "example.tfvars")) + if err != nil { + t.Fatal(err) + } + x := lsctx.RPCContextData{ + Method: "textDocument/didChange", + URI: uri.FromPath(fooURI), + } + ctx = lsctx.WithLanguageId(ctx, ilsp.Terraform.String()) + ctx = lsctx.WithRPCContext(ctx, x) + err = ParseVariables(ctx, testFs, ss.Modules, singleFileModulePath) + if err != nil { + t.Fatal(err) + } + + after, err := ss.Modules.ModuleByPath(singleFileModulePath) + if err != nil { + t.Fatal(err) + } + + // test if foo.tf is not the same as first seen + if before.ParsedVarsFiles["example.tfvars"] == after.ParsedVarsFiles["example.tfvars"] { + t.Fatal("file should mismatch") + } + + // test if main.tf is the same as first seen + if before.ParsedVarsFiles["main.tf"] != after.ParsedVarsFiles["main.tf"] { + t.Fatal("file should not mismatch") + } + + // // examine diags should change for foo.tf + // if before.VarsDiagnostics["example.tfvars"] == after.VarsDiagnostics["example.tfvars"] { + // t.Fatal("diags should mismatch") + // } +} + func gzipCompressBytes(t *testing.T, b []byte) []byte { var compressedBytes bytes.Buffer gw := gzip.NewWriter(&compressedBytes) diff --git a/internal/terraform/parser/variables.go b/internal/terraform/parser/variables.go index eec4579d2..b40efa887 100644 --- a/internal/terraform/parser/variables.go +++ b/internal/terraform/parser/variables.go @@ -6,6 +6,7 @@ package parser import ( "path/filepath" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform-ls/internal/terraform/ast" ) @@ -48,3 +49,17 @@ func ParseVariableFiles(fs FS, modPath string) (ast.VarsFiles, ast.VarsDiags, er return files, diags, nil } + +func ParseVariableFile(fs FS, filePath string) (*hcl.File, hcl.Diagnostics, error) { + src, err := fs.ReadFile(filePath) + if err != nil { + return nil, nil, err + } + + name := filepath.Base(filePath) + filename := ast.VarsFilename(name) + + f, pDiags := parseFile(src, filename) + + return f, pDiags, nil +}