Skip to content

Commit

Permalink
Update ParseModuleConfiguration to only parse changed file (#1404)
Browse files Browse the repository at this point in the history
This modifies ParseModuleConfiguration to only parse changed file if file already parsed and this is a didChange request.

Previously ParseModuleConfiguration would reparse not only the changed file but also the module containing the changed file.

Now ParseModuleConfiguration detects if the file has previously been parsed and the request is from a didChange event. If both are true, then it only parses the changed file and skips re-parsing the entire module. If this is false, it falls back to the previous behavior of re-parsing the entire module including the changed file.
  • Loading branch information
jpogran authored Sep 22, 2023
1 parent 94e47bd commit 10137ee
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 5 deletions.
4 changes: 4 additions & 0 deletions internal/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,7 @@ func IsLanguageId(ctx context.Context, expectedLangId string) bool {
}
return langId == expectedLangId
}

func (ctxData RPCContextData) IsDidChangeRequest() bool {
return ctxData.Method == "textDocument/didChange"
}
2 changes: 2 additions & 0 deletions internal/decoder/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/hashicorp/hcl-lang/decoder"
"github.com/hashicorp/hcl-lang/lang"
lsctx "github.com/hashicorp/terraform-ls/internal/context"
idecoder "github.com/hashicorp/terraform-ls/internal/decoder"
"github.com/hashicorp/terraform-ls/internal/state"
"github.com/hashicorp/terraform-ls/internal/terraform/module"
Expand Down Expand Up @@ -60,6 +61,7 @@ func TestDecoder_CodeLensesForFile_concurrencyBug(t *testing.T) {
if err != nil {
t.Error(err)
}
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = module.ParseModuleConfiguration(ctx, mapFs, ss.Modules, dirName)
if err != nil {
t.Error(err)
Expand Down
16 changes: 16 additions & 0 deletions internal/terraform/ast/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ func (mf ModFiles) AsMap() map[string]*hcl.File {
return m
}

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

type ModDiags map[ModFilename]hcl.Diagnostics

func ModDiagsFromMap(m map[string]hcl.Diagnostics) ModDiags {
Expand Down Expand Up @@ -83,6 +91,14 @@ func (md ModDiags) AsMap() map[string]hcl.Diagnostics {
return m
}

func (md ModDiags) Copy() ModDiags {
m := make(ModDiags, len(md))
for name, diags := range md {
m[name] = diags
}
return m
}

func (md ModDiags) Count() int {
count := 0
for _, diags := range md {
Expand Down
47 changes: 42 additions & 5 deletions internal/terraform/module/module_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,27 @@ import (
"io"
"io/fs"
"log"
"path/filepath"
"time"

"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-version"
"github.com/hashicorp/hcl-lang/decoder"
"github.com/hashicorp/hcl-lang/lang"
tfjson "github.com/hashicorp/terraform-json"
lsctx "github.com/hashicorp/terraform-ls/internal/context"
idecoder "github.com/hashicorp/terraform-ls/internal/decoder"
"github.com/hashicorp/terraform-ls/internal/document"
"github.com/hashicorp/terraform-ls/internal/job"
ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
"github.com/hashicorp/terraform-ls/internal/registry"
"github.com/hashicorp/terraform-ls/internal/schemas"
"github.com/hashicorp/terraform-ls/internal/state"
"github.com/hashicorp/terraform-ls/internal/terraform/ast"
"github.com/hashicorp/terraform-ls/internal/terraform/datadir"
op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation"
"github.com/hashicorp/terraform-ls/internal/terraform/parser"
"github.com/hashicorp/terraform-ls/internal/uri"
tfaddr "github.com/hashicorp/terraform-registry-address"
"github.com/hashicorp/terraform-schema/earlydecoder"
"github.com/hashicorp/terraform-schema/module"
Expand Down Expand Up @@ -359,20 +363,53 @@ func ParseModuleConfiguration(ctx context.Context, fs ReadOnlyFS, modStore *stat

// 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.ModuleParsingState != op.OpStateUnknown && !job.IgnoreState(ctx) {
return job.StateNotChangedErr{Dir: document.DirHandleFromPath(modPath)}
}

err = modStore.SetModuleParsingState(modPath, op.OpStateLoading)
var files ast.ModFiles
var diags ast.ModDiags
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.Terraform.String()) {
// the file has already been parsed, so only examine this file and not the whole module
err = modStore.SetModuleParsingState(modPath, op.OpStateLoading)
if err != nil {
return err
}

filePath, err := uri.PathFromURI(rpcContext.URI)
if err != nil {
return err
}
fileName := filepath.Base(filePath)

f, fDiags, err := parser.ParseModuleFile(fs, filePath)
if err != nil {
return err
}
existingFiles := mod.ParsedModuleFiles.Copy()
existingFiles[ast.ModFilename(fileName)] = f
files = existingFiles

existingDiags := mod.ModuleDiagnostics.Copy()
existingDiags[ast.ModFilename(fileName)] = fDiags
diags = existingDiags
} else {
// this is the first time file is opened so parse the whole module
err = modStore.SetModuleParsingState(modPath, op.OpStateLoading)
if err != nil {
return err
}

files, diags, err = parser.ParseModuleFiles(fs, modPath)
}

if err != nil {
return err
}

files, diags, err := parser.ParseModuleFiles(fs, modPath)

sErr := modStore.UpdateParsedModuleFiles(modPath, files, err)
if sErr != nil {
return sErr
Expand Down
156 changes: 156 additions & 0 deletions internal/terraform/module/module_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@ import (
"github.com/hashicorp/go-version"
"github.com/hashicorp/hcl-lang/lang"
tfjson "github.com/hashicorp/terraform-json"
lsctx "github.com/hashicorp/terraform-ls/internal/context"
"github.com/hashicorp/terraform-ls/internal/document"
"github.com/hashicorp/terraform-ls/internal/filesystem"
"github.com/hashicorp/terraform-ls/internal/job"
ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
"github.com/hashicorp/terraform-ls/internal/registry"
"github.com/hashicorp/terraform-ls/internal/state"
"github.com/hashicorp/terraform-ls/internal/terraform/exec"
"github.com/hashicorp/terraform-ls/internal/terraform/module/operation"
"github.com/hashicorp/terraform-ls/internal/uri"
tfaddr "github.com/hashicorp/terraform-registry-address"
tfregistry "github.com/hashicorp/terraform-schema/registry"
"github.com/stretchr/testify/mock"
Expand All @@ -56,6 +59,7 @@ func TestGetModuleDataFromRegistry_singleModule(t *testing.T) {
}

fs := filesystem.NewFilesystem(ss.DocumentStore)
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -128,6 +132,7 @@ func TestGetModuleDataFromRegistry_moduleNotFound(t *testing.T) {
}

fs := filesystem.NewFilesystem(ss.DocumentStore)
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -228,6 +233,7 @@ func TestGetModuleDataFromRegistry_apiTimeout(t *testing.T) {
}

fs := filesystem.NewFilesystem(ss.DocumentStore)
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -522,6 +528,7 @@ func TestParseProviderVersions_multipleVersions(t *testing.T) {
if err != nil {
t.Fatal(err)
}
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPathFirst)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -691,6 +698,7 @@ func TestPreloadEmbeddedSchema_basic(t *testing.T) {
if err != nil {
t.Fatal(err)
}
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, cfgFS, ss.Modules, modPath)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -761,6 +769,7 @@ func TestPreloadEmbeddedSchema_unknownProviderOnly(t *testing.T) {
if err != nil {
t.Fatal(err)
}
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, cfgFS, ss.Modules, modPath)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -824,6 +833,7 @@ func TestPreloadEmbeddedSchema_idempotency(t *testing.T) {
if err != nil {
t.Fatal(err)
}
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, cfgFS, ss.Modules, modPath)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -903,6 +913,7 @@ func TestPreloadEmbeddedSchema_raceCondition(t *testing.T) {
if err != nil {
t.Fatal(err)
}
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{})
err = ParseModuleConfiguration(ctx, cfgFS, ss.Modules, modPath)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -931,6 +942,151 @@ func TestPreloadEmbeddedSchema_raceCondition(t *testing.T) {
wg.Wait()
}

func TestParseModuleConfiguration(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 = ParseModuleConfiguration(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, "foo.tf"))
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 = ParseModuleConfiguration(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.ParsedModuleFiles["foo.tf"] == after.ParsedModuleFiles["foo.tf"] {
t.Fatal("file should mismatch")
}

// test if main.tf is the same as first seen
if before.ParsedModuleFiles["main.tf"] != after.ParsedModuleFiles["main.tf"] {
t.Fatal("file mismatch")
}

// examine diags should change for foo.tf
if before.ModuleDiagnostics["foo.tf"][0] == after.ModuleDiagnostics["foo.tf"][0] {
t.Fatal("diags should mismatch")
}

// examine diags should change for main.tf
if before.ModuleDiagnostics["main.tf"][0] != after.ModuleDiagnostics["main.tf"][0] {
t.Fatal("diags should match")
}
}
func TestParseModuleConfiguration_ignore_tfvars(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 = ParseModuleConfiguration(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)
}
ctx = lsctx.WithLanguageId(ctx, ilsp.Tfvars.String())
ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{
Method: "textDocument/didChange",
URI: uri.FromPath(fooURI),
})
err = ParseModuleConfiguration(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 be missing
_, beforeExists := before.ParsedModuleFiles["example.tfvars"]
if beforeExists {
t.Fatal("example.tfvars file should be missing")
}
_, afterExists := after.ParsedModuleFiles["example.tfvars"]
if afterExists {
t.Fatal("example.tfvars file should be missing")
}

// diags should be missing for example.tfvars
if _, ok := before.ModuleDiagnostics["example.tfvars"]; ok {
t.Fatal("there should be no diags for tfvars files right now")
}
}

func gzipCompressBytes(t *testing.T, b []byte) []byte {
var compressedBytes bytes.Buffer
gw := gzip.NewWriter(&compressedBytes)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
variable "another" {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
variable "image_id" {
type = string
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
variable "gogo" {

}


variable "awesome" {


Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
variable "wakka" {


Loading

0 comments on commit 10137ee

Please sign in to comment.