Skip to content

Commit

Permalink
Update ParseVariables for single files (#1422)
Browse files Browse the repository at this point in the history
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 #1404 which updated the parsing job for terraform files.
  • Loading branch information
jpogran committed Oct 3, 2023
1 parent 3f7092b commit 449d442
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 6 deletions.
16 changes: 16 additions & 0 deletions internal/terraform/ast/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ func VarsFilesFromMap(m map[string]*hcl.File) VarsFiles {
return mf
}

func (vf VarsFiles) Copy() VarsFiles {
m := make(VarsFiles, len(vf))
for name, file := range vf {
m[name] = file
}
return m
}

type VarsDiags map[VarsFilename]hcl.Diagnostics

func VarsDiagsFromMap(m map[string]hcl.Diagnostics) VarsDiags {
Expand All @@ -62,6 +70,14 @@ func VarsDiagsFromMap(m map[string]hcl.Diagnostics) VarsDiags {
return mf
}

func (vd VarsDiags) Copy() VarsDiags {
m := make(VarsDiags, len(vd))
for name, file := range vd {
m[name] = file
}
return m
}

func (vd VarsDiags) AutoloadedOnly() VarsDiags {
diags := make(VarsDiags)
for name, f := range vd {
Expand Down
48 changes: 43 additions & 5 deletions internal/terraform/module/module_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,20 +456,58 @@ 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.VarsDiagnosticsState[ast.HCLParsingSource] != op.OpStateUnknown && !job.IgnoreState(ctx) {
return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)}
}

err = modStore.SetVarsDiagnosticsState(modPath, ast.HCLParsingSource, op.OpStateLoading)
var files ast.VarsFiles
var diags ast.VarsDiags
rpcContext := lsctx.DocumentContext(ctx)
// Only parse the file that's being changed/opened, unless this is 1st-time parsing
if mod.ModuleDiagnosticsState[ast.HCLParsingSource] == op.OpStateLoaded && rpcContext.IsDidChangeRequest() && rpcContext.LanguageID == ilsp.Tfvars.String() {
// the file has already been parsed, so only examine this file and not the whole module
err = modStore.SetVarsDiagnosticsState(modPath, ast.HCLParsingSource, 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, ok := mod.VarsDiagnostics[ast.HCLParsingSource]
if !ok {
existingDiags = make(ast.VarsDiags)
} else {
existingDiags = existingDiags.Copy()
}
existingDiags[ast.VarsFilename(fileName)] = vDiags
diags = existingDiags
} else {
// this is the first time file is opened so parse the whole module
err = modStore.SetVarsDiagnosticsState(modPath, ast.HCLParsingSource, 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
Expand Down
87 changes: 86 additions & 1 deletion internal/terraform/module/module_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,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()
Expand Down Expand Up @@ -1089,6 +1090,86 @@ 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.WithDocumentContext(ctx, lsctx.Document{})
err = ParseModuleConfiguration(ctx, testFs, ss.Modules, singleFileModulePath)
if err != nil {
t.Fatal(err)
}
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
filePath, err := filepath.Abs(filepath.Join(singleFileModulePath, "example.tfvars"))
if err != nil {
t.Fatal(err)
}
ctx = lsctx.WithDocumentContext(ctx, lsctx.Document{
Method: "textDocument/didChange",
LanguageID: ilsp.Tfvars.String(),
URI: uri.FromPath(filePath),
})
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)
}

// example.tfvars should not be the same as first seen
if before.ParsedVarsFiles["example.tfvars"] == after.ParsedVarsFiles["example.tfvars"] {
t.Fatal("file should mismatch")
}

beforeDiags := before.VarsDiagnostics[ast.HCLParsingSource]
afterDiags := after.VarsDiagnostics[ast.HCLParsingSource]

// diags should change for example.tfvars
if beforeDiags[ast.VarsFilename("example.tfvars")][0] == afterDiags[ast.VarsFilename("example.tfvars")][0] {
t.Fatal("diags should mismatch")
}

if before.ParsedVarsFiles["nochange.tfvars"] != after.ParsedVarsFiles["nochange.tfvars"] {
t.Fatal("unchanged file should match")
}

if beforeDiags[ast.VarsFilename("nochange.tfvars")][0] != afterDiags[ast.VarsFilename("nochange.tfvars")][0] {
t.Fatal("diags should match for unchanged file")
}
}

func gzipCompressBytes(t *testing.T, b []byte) []byte {
var compressedBytes bytes.Buffer
gw := gzip.NewWriter(&compressedBytes)
Expand Down Expand Up @@ -1287,10 +1368,14 @@ func TestSchemaVarsValidation_SingleFile(t *testing.T) {
}

fs := filesystem.NewFilesystem(ss.DocumentStore)
filePath, err := filepath.Abs(filepath.Join(modPath, "terraform.tfvars"))
if err != nil {
t.Fatal(err)
}
ctx = lsctx.WithDocumentContext(ctx, lsctx.Document{
Method: "textDocument/didChange",
LanguageID: ilsp.Tfvars.String(),
URI: "file:///test/terraform.tfvars",
URI: uri.FromPath(filePath),
})
err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
variable "image_id" {
type = string
}

# this is supposed to generate a diagnostic
lalalalal "goo"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
variable "no_change_id" {
type = string

15 changes: 15 additions & 0 deletions internal/terraform/parser/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package parser
import (
"path/filepath"

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform-ls/internal/terraform/ast"
)

Expand Down Expand Up @@ -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
}

0 comments on commit 449d442

Please sign in to comment.