Skip to content

Commit

Permalink
Only validate a single file on didChange (#1370)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dbanck committed Sep 7, 2023
1 parent dc9e631 commit 5160918
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 11 deletions.
21 changes: 21 additions & 0 deletions internal/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
2 changes: 2 additions & 0 deletions internal/langserver/handlers/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
29 changes: 22 additions & 7 deletions internal/langserver/handlers/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(&params)
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)
Expand Down
6 changes: 6 additions & 0 deletions internal/state/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -66,6 +69,7 @@ func (sj *ScheduledJob) Copy() *ScheduledJob {
DeferredJobIDs: sj.DeferredJobIDs.Copy(),
EnqueueTime: sj.EnqueueTime,
TraceSpan: traceSpan,
RPCContext: sj.RPCContext.Copy(),
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down
35 changes: 31 additions & 4 deletions internal/terraform/module/module_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
90 changes: 90 additions & 0 deletions internal/terraform/module/module_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
9 changes: 9 additions & 0 deletions internal/terraform/module/testdata/invalid-config/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
test {}

variable {

}

locals {
test = 1
}
11 changes: 11 additions & 0 deletions internal/terraform/module/testdata/invalid-config/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
variable {

}

variable "test" {

}

output {

}

0 comments on commit 5160918

Please sign in to comment.