From 5160918c290a938d9295f11470910305b944da2c Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Thu, 31 Aug 2023 14:19:27 +0200 Subject: [PATCH] Only validate a single file on `didChange` (#1370) * Introduce new RPCContextData for storing rpc info * Attach RPCContextData to jobs * Add RPCContextData to walker ctx, fix params * Check RPCContext on SchemaValidation * Bump hcl-lang to `485c60` * Review feedback --- internal/context/context.go | 21 +++++ internal/langserver/handlers/initialize.go | 2 + internal/langserver/handlers/service.go | 29 ++++-- internal/state/jobs.go | 6 ++ internal/terraform/module/module_ops.go | 35 +++++++- internal/terraform/module/module_ops_test.go | 90 +++++++++++++++++++ .../module/testdata/invalid-config/main.tf | 9 ++ .../testdata/invalid-config/variables.tf | 11 +++ 8 files changed, 192 insertions(+), 11 deletions(-) create mode 100644 internal/terraform/module/testdata/invalid-config/main.tf create mode 100644 internal/terraform/module/testdata/invalid-config/variables.tf diff --git a/internal/context/context.go b/internal/context/context.go index c3b68c1d7..d26e17041 100644 --- a/internal/context/context.go +++ b/internal/context/context.go @@ -20,6 +20,18 @@ func (k *contextKey) String() string { return k.Name } +type RPCContextData struct { + Method string + URI string +} + +func (rpcc RPCContextData) Copy() RPCContextData { + return RPCContextData{ + Method: rpcc.Method, + URI: rpcc.URI, + } +} + var ( ctxTfExecPath = &contextKey{"terraform executable path"} ctxTfExecLogPath = &contextKey{"terraform executor log path"} @@ -31,6 +43,7 @@ var ( ctxProgressToken = &contextKey{"progress token"} ctxExperimentalFeatures = &contextKey{"experimental features"} ctxValidationOptions = &contextKey{"validation options"} + ctxRPCContext = &contextKey{"rpc context"} ) func missingContextErr(ctxKey *contextKey) *MissingContextErr { @@ -188,3 +201,11 @@ func ValidationOptions(ctx context.Context) (settings.ValidationOptions, error) } return *validationOptions, nil } + +func WithRPCContext(ctx context.Context, rpcc RPCContextData) context.Context { + return context.WithValue(ctx, ctxRPCContext, rpcc) +} + +func RPCContext(ctx context.Context) RPCContextData { + return ctx.Value(ctxRPCContext).(RPCContextData) +} diff --git a/internal/langserver/handlers/initialize.go b/internal/langserver/handlers/initialize.go index 86f6621e1..14e8cf2dc 100644 --- a/internal/langserver/handlers/initialize.go +++ b/internal/langserver/handlers/initialize.go @@ -166,6 +166,8 @@ func (svc *service) Initialize(ctx context.Context, params lsp.InitializeParams) // passing the request context here // Static user-provided paths take precedence over dynamic discovery walkerCtx := context.Background() + walkerCtx = lsctx.WithRPCContext(walkerCtx, lsctx.RPCContext(ctx)) + err = svc.closedDirWalker.StartWalking(walkerCtx) if err != nil { return serverCaps, fmt.Errorf("failed to start closedDirWalker: %w", err) diff --git a/internal/langserver/handlers/service.go b/internal/langserver/handlers/service.go index acc873aec..38240f361 100644 --- a/internal/langserver/handlers/service.go +++ b/internal/langserver/handlers/service.go @@ -613,24 +613,39 @@ func handle(ctx context.Context, req *jrpc2.Request, fn interface{}) (interface{ // We could capture all parameters here but for now we just // opportunistically track the most important ones only. + type t struct { + URI string `json:"uri,omitempty"` + } type p struct { - URI string `json:"uri,omitempty"` - RootURI string `json:"rootUri,omitempty"` + TextDocument t `json:"textDocument,omitempty"` + RootURI string `json:"rootUri,omitempty"` } params := p{} err := req.UnmarshalParams(¶ms) - if err == nil { - attrs = append(attrs, attribute.KeyValue{ - Key: attribute.Key("URI"), - Value: attribute.StringValue(string(params.URI)), - }) + if err != nil { + return nil, err + } + + uri := params.TextDocument.URI + if params.RootURI != "" { + uri = params.RootURI } + attrs = append(attrs, attribute.KeyValue{ + Key: attribute.Key("URI"), + Value: attribute.StringValue(uri), + }) + tracer := otel.Tracer(tracerName) ctx, span := tracer.Start(ctx, "rpc:"+req.Method(), trace.WithAttributes(attrs...)) defer span.End() + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: req.Method(), + URI: uri, + }) + result, err := rpch.New(fn)(ctx, req) if ctx.Err() != nil && errors.Is(ctx.Err(), context.Canceled) { err = fmt.Errorf("%w: %s", requestCancelled.Err(), err) diff --git a/internal/state/jobs.go b/internal/state/jobs.go index 3e3a5c1bd..cb51aa467 100644 --- a/internal/state/jobs.go +++ b/internal/state/jobs.go @@ -13,6 +13,7 @@ import ( "time" "github.com/hashicorp/go-memdb" + lsctx "github.com/hashicorp/terraform-ls/internal/context" "github.com/hashicorp/terraform-ls/internal/document" "github.com/hashicorp/terraform-ls/internal/job" "go.opentelemetry.io/otel" @@ -49,6 +50,8 @@ type ScheduledJob struct { // TraceSpan represents a tracing span for the entire job lifecycle // (from queuing to finishing execution). TraceSpan trace.Span + // RPCContext contains information from when & where the job was scheduled from + RPCContext lsctx.RPCContextData } func (sj *ScheduledJob) Copy() *ScheduledJob { @@ -66,6 +69,7 @@ func (sj *ScheduledJob) Copy() *ScheduledJob { DeferredJobIDs: sj.DeferredJobIDs.Copy(), EnqueueTime: sj.EnqueueTime, TraceSpan: traceSpan, + RPCContext: sj.RPCContext.Copy(), } } @@ -125,6 +129,7 @@ func (js *JobStore) EnqueueJob(ctx context.Context, newJob job.Job) (job.ID, err State: StateQueued, EnqueueTime: time.Now(), TraceSpan: jobSpan, + RPCContext: lsctx.RPCContext(ctx), } err := txn.Insert(js.tableName, sJob) @@ -316,6 +321,7 @@ func (js *JobStore) awaitNextJob(ctx context.Context, priority job.JobPriority) js.logger.Printf("JOBS: Dispatching next job %q (scheduler prio: %d, job prio: %d, isDirOpen: %t): %q for %q", sJob.ID, priority, sJob.Priority, sJob.IsDirOpen, sJob.Type, sJob.Dir) + ctx = lsctx.WithRPCContext(ctx, sJob.RPCContext) ctx = trace.ContextWithSpan(ctx, sJob.TraceSpan) _, span := otel.Tracer(tracerName).Start(ctx, "job-wait", diff --git a/internal/terraform/module/module_ops.go b/internal/terraform/module/module_ops.go index e81bfa5c9..866e1263d 100644 --- a/internal/terraform/module/module_ops.go +++ b/internal/terraform/module/module_ops.go @@ -11,13 +11,16 @@ import ( "io" "io/fs" "log" + "path" "time" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-version" "github.com/hashicorp/hcl-lang/decoder" "github.com/hashicorp/hcl-lang/lang" + "github.com/hashicorp/hcl/v2" 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/decoder/validations" "github.com/hashicorp/terraform-ls/internal/document" @@ -695,10 +698,34 @@ func SchemaValidation(ctx context.Context, modStore *state.ModuleStore, schemaRe return err } - diags, rErr := moduleDecoder.Validate(ctx) - sErr := modStore.UpdateModuleDiagnostics(modPath, ast.SchemaValidationSource, ast.ModDiagsFromMap(diags)) - if sErr != nil { - return sErr + var rErr error + rpcContext := lsctx.RPCContext(ctx) + isSingleFileChange := rpcContext.Method == "textDocument/didChange" + if isSingleFileChange { + filename := path.Base(rpcContext.URI) + // We only revalidate a single file that changed + var fileDiags hcl.Diagnostics + fileDiags, rErr = moduleDecoder.ValidateFile(ctx, filename) + + modDiags, ok := mod.ModuleDiagnostics[ast.SchemaValidationSource] + if !ok { + modDiags = make(ast.ModDiags) + } + modDiags[ast.ModFilename(filename)] = fileDiags + + sErr := modStore.UpdateModuleDiagnostics(modPath, ast.SchemaValidationSource, modDiags) + if sErr != nil { + return sErr + } + } else { + // We validate the whole module, e.g. on open + var diags lang.DiagnosticsMap + diags, rErr = moduleDecoder.Validate(ctx) + + sErr := modStore.UpdateModuleDiagnostics(modPath, ast.SchemaValidationSource, ast.ModDiagsFromMap(diags)) + if sErr != nil { + return sErr + } } return rErr diff --git a/internal/terraform/module/module_ops_test.go b/internal/terraform/module/module_ops_test.go index 23e0f303b..c5b5c43cb 100644 --- a/internal/terraform/module/module_ops_test.go +++ b/internal/terraform/module/module_ops_test.go @@ -23,11 +23,13 @@ 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" "github.com/hashicorp/terraform-ls/internal/registry" "github.com/hashicorp/terraform-ls/internal/state" + "github.com/hashicorp/terraform-ls/internal/terraform/ast" "github.com/hashicorp/terraform-ls/internal/terraform/exec" "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" tfaddr "github.com/hashicorp/terraform-registry-address" @@ -966,3 +968,91 @@ var randomSchemaJSON = `{ } } }` + +func TestSchemaValidation_FullModule(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) + } + modPath := filepath.Join(testData, "invalid-config") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didOpen", + URI: "file:///test/variables.tf", + }) + err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 5 + diagsCount := mod.ModuleDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} + +func TestSchemaValidation_SingleFile(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) + } + modPath := filepath.Join(testData, "invalid-config") + + err = ss.Modules.Add(modPath) + if err != nil { + t.Fatal(err) + } + + fs := filesystem.NewFilesystem(ss.DocumentStore) + err = ParseModuleConfiguration(ctx, fs, ss.Modules, modPath) + if err != nil { + t.Fatal(err) + } + ctx = lsctx.WithRPCContext(ctx, lsctx.RPCContextData{ + Method: "textDocument/didChange", + URI: "file:///test/variables.tf", + }) + err = SchemaValidation(ctx, ss.Modules, ss.ProviderSchemas, modPath) + if err != nil { + t.Fatal(err) + } + + mod, err := ss.Modules.ModuleByPath(modPath) + if err != nil { + t.Fatal(err) + } + + expectedCount := 3 + diagsCount := mod.ModuleDiagnostics[ast.SchemaValidationSource].Count() + if diagsCount != expectedCount { + t.Fatalf("expected %d diagnostics, %d given", expectedCount, diagsCount) + } +} diff --git a/internal/terraform/module/testdata/invalid-config/main.tf b/internal/terraform/module/testdata/invalid-config/main.tf new file mode 100644 index 000000000..ce360b64a --- /dev/null +++ b/internal/terraform/module/testdata/invalid-config/main.tf @@ -0,0 +1,9 @@ +test {} + +variable { + +} + +locals { + test = 1 +} \ No newline at end of file diff --git a/internal/terraform/module/testdata/invalid-config/variables.tf b/internal/terraform/module/testdata/invalid-config/variables.tf new file mode 100644 index 000000000..c94218809 --- /dev/null +++ b/internal/terraform/module/testdata/invalid-config/variables.tf @@ -0,0 +1,11 @@ +variable { + +} + +variable "test" { + +} + +output { + +} \ No newline at end of file