Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ParseVariables for single files #1422

Merged
merged 10 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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")
}
radeksimko marked this conversation as resolved.
Show resolved Hide resolved

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
}