diff --git a/lsp/source/batch.go b/lsp/source/batch.go new file mode 100644 index 0000000..6ca15d6 --- /dev/null +++ b/lsp/source/batch.go @@ -0,0 +1,43 @@ +package source + +import ( + "context" + + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" +) + +// Batch is the shared analysis context for one diagnostic run. All checkers +// see the same view and the same cross-file resolver, so a name resolved by +// one check is not resolved again by the next. +type Batch struct { + view *cache.View + ix *Index +} + +// NewBatch starts an analysis run over the view. +func NewBatch(view *cache.View) *Batch { + return &Batch{view: view} +} + +// View returns the store view this run reads from. +func (b *Batch) View() *cache.View { + return b.view +} + +// Tree returns the parsed tree of uri, parsed and memoized by the view. +func (b *Batch) Tree(ctx context.Context, u uri.URI) (*cache.ParsedFile, error) { + return b.view.Parse(ctx, u) +} + +// Index returns the run's shared cross-file resolver. Resolutions are +// memoized per (file, name), so repeated lookups across checkers resolve +// once. +func (b *Batch) Index() *Index { + if b.ix == nil { + b.ix = NewIndex(b.view) + } + + return b.ix +} diff --git a/lsp/source/cycle_detect.go b/lsp/source/cycle_detect.go index 24a74c7..4f242e3 100644 --- a/lsp/source/cycle_detect.go +++ b/lsp/source/cycle_detect.go @@ -18,7 +18,9 @@ import ( // length are caught, including self-includes. type CycleCheck struct{} -func (c *CycleCheck) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { +func (c *CycleCheck) Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) { + view := b.View() + closure := make(map[uri.URI][]Include) for _, file := range changeFiles { _ = getIncludes(ctx, view, file, &closure) diff --git a/lsp/source/cycle_detect_test.go b/lsp/source/cycle_detect_test.go index 10a465b..4dd1ea9 100644 --- a/lsp/source/cycle_detect_test.go +++ b/lsp/source/cycle_detect_test.go @@ -91,7 +91,7 @@ func runCycleCheck(t *testing.T, files map[string]string, root string) []cyclePa view := buildSnapshotForTest(t, changes) - res, err := (&CycleCheck{}).Diagnostic(t.Context(), view, []uri.URI{uri.URI("file:///tmp/" + root)}) + res, err := (&CycleCheck{}).Diagnostic(t.Context(), NewBatch(view), []uri.URI{uri.URI("file:///tmp/" + root)}) require.NoError(t, err) for file, diags := range res { diff --git a/lsp/source/diagnostic.go b/lsp/source/diagnostic.go index 0bee6ff..e8f215e 100644 --- a/lsp/source/diagnostic.go +++ b/lsp/source/diagnostic.go @@ -28,17 +28,21 @@ func init() { } type Checker interface { - Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) + Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) Name() string } type Diagnostic struct{} -func NewDiagnostic() Checker { +func NewDiagnostic() *Diagnostic { return &Diagnostic{} } +// Diagnostic runs every registered checker over changeFiles with a shared +// Batch, joining their results and errors. func (d *Diagnostic) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { + batch := NewBatch(view) + res := make(DiagnosticResult) var errs []error @@ -46,7 +50,7 @@ func (d *Diagnostic) Diagnostic(ctx context.Context, view *cache.View, changeFil for _, impl := range registry { slog.Debug("diagnostic called", "impl", impl.Name()) - diagRes, err := impl.Diagnostic(ctx, view, changeFiles) + diagRes, err := impl.Diagnostic(ctx, batch, changeFiles) if err != nil { errs = append(errs, err) } diff --git a/lsp/source/duplicate_check.go b/lsp/source/duplicate_check.go index fc901ee..0799e8a 100644 --- a/lsp/source/duplicate_check.go +++ b/lsp/source/duplicate_check.go @@ -26,7 +26,9 @@ func (c *DuplicateCheck) Name() string { return "DuplicateCheck" } -func (c *DuplicateCheck) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { +func (c *DuplicateCheck) Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) { + view := b.View() + res := make(DiagnosticResult) for _, file := range changeFiles { diff --git a/lsp/source/duplicate_check_test.go b/lsp/source/duplicate_check_test.go index 213debc..089c7d2 100644 --- a/lsp/source/duplicate_check_test.go +++ b/lsp/source/duplicate_check_test.go @@ -359,7 +359,7 @@ enum User {} }, }) - got, err := (&DuplicateCheck{}).Diagnostic(t.Context(), view, []uri.URI{"file:///tmp/user.thrift"}) + got, err := (&DuplicateCheck{}).Diagnostic(t.Context(), NewBatch(view), []uri.URI{"file:///tmp/user.thrift"}) assert.NoError(t, err) assert.Equal(t, DiagnosticResult{"file:///tmp/user.thrift": tt.want}, got) diff --git a/lsp/source/enum_value_check.go b/lsp/source/enum_value_check.go index 4fc3ec1..4f60cee 100644 --- a/lsp/source/enum_value_check.go +++ b/lsp/source/enum_value_check.go @@ -22,7 +22,9 @@ type EnumValueCheck struct{} // one greater than the preceding member's value otherwise. Their on-wire // value therefore follows their position; inserting, removing, or // reordering members silently changes serialized data. -func (c *EnumValueCheck) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { +func (c *EnumValueCheck) Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) { + view := b.View() + res := make(DiagnosticResult) for _, file := range changeFiles { diff --git a/lsp/source/enum_value_check_test.go b/lsp/source/enum_value_check_test.go index 7024b86..d3407fc 100644 --- a/lsp/source/enum_value_check_test.go +++ b/lsp/source/enum_value_check_test.go @@ -101,7 +101,7 @@ func Test_EnumValueCheck_Diagnostic(t *testing.T) { }, }) - got, err := (&EnumValueCheck{}).Diagnostic(t.Context(), view, []uri.URI{"file:///tmp/user.thrift"}) + got, err := (&EnumValueCheck{}).Diagnostic(t.Context(), NewBatch(view), []uri.URI{"file:///tmp/user.thrift"}) assert.NoError(t, err) assert.Equal(t, DiagnosticResult{"file:///tmp/user.thrift": tt.want}, got) diff --git a/lsp/source/fieldid_check.go b/lsp/source/fieldid_check.go index 42a4319..69c4977 100644 --- a/lsp/source/fieldid_check.go +++ b/lsp/source/fieldid_check.go @@ -17,7 +17,9 @@ type FieldIDCheck struct{} // FieldIDCheck checks struct, union, exception, function parameter, and // throws field ids: they must be unique positive integers in [1, 32767]. -func (c *FieldIDCheck) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { +func (c *FieldIDCheck) Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) { + view := b.View() + res := make(DiagnosticResult) for _, file := range changeFiles { diff --git a/lsp/source/fieldid_check_test.go b/lsp/source/fieldid_check_test.go index b36222c..c220a99 100644 --- a/lsp/source/fieldid_check_test.go +++ b/lsp/source/fieldid_check_test.go @@ -411,7 +411,7 @@ service Demo { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &FieldIDCheck{} - got, err := c.Diagnostic(tt.args.ctx, tt.args.view, tt.args.changeFiles) + got, err := c.Diagnostic(tt.args.ctx, NewBatch(tt.args.view), tt.args.changeFiles) for key := range got { sort.SliceStable(got[key], func(i, j int) bool { diff --git a/lsp/source/include_action_test.go b/lsp/source/include_action_test.go index cdacdaa..22dfd2f 100644 --- a/lsp/source/include_action_test.go +++ b/lsp/source/include_action_test.go @@ -60,7 +60,7 @@ func Test_MakeRemoveUnusedIncludeAction(t *testing.T) { require.NoError(t, err) // The diagnostic the check produces, as the server would pass it. - diags, err := (&UnusedIncludeCheck{}).diagnostic(t.Context(), view, uri.File(filePath)) + diags, err := (&UnusedIncludeCheck{}).diagnostic(t.Context(), NewBatch(view), uri.File(filePath)) require.NoError(t, err) require.Len(t, diags, 1) diff --git a/lsp/source/index_memo_test.go b/lsp/source/index_memo_test.go index 52b697f..be05b9e 100644 --- a/lsp/source/index_memo_test.go +++ b/lsp/source/index_memo_test.go @@ -197,7 +197,7 @@ func TestSemanticAnalysisSkipsBrokenFile(t *testing.T) { }, }) - got, err := (&SemanticAnalysis{}).Diagnostic(t.Context(), view, []uri.URI{"file:///f.thrift"}) + got, err := (&SemanticAnalysis{}).Diagnostic(t.Context(), NewBatch(view), []uri.URI{"file:///f.thrift"}) require.NoError(t, err, "a broken file must not fail the diagnostics run") assert.NotNil(t, got) }) diff --git a/lsp/source/parse.go b/lsp/source/parse.go index cec1d99..04e824b 100644 --- a/lsp/source/parse.go +++ b/lsp/source/parse.go @@ -14,7 +14,9 @@ import ( type Parse struct{} -func (p *Parse) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { +func (p *Parse) Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) { + view := b.View() + var errs []error res := make(DiagnosticResult) diff --git a/lsp/source/semantic_analysis.go b/lsp/source/semantic_analysis.go index dc84014..22d9071 100644 --- a/lsp/source/semantic_analysis.go +++ b/lsp/source/semantic_analysis.go @@ -14,11 +14,12 @@ import ( type SemanticAnalysis struct{} -func (s *SemanticAnalysis) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { +func (s *SemanticAnalysis) Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) { + res := make(DiagnosticResult) for _, file := range changeFiles { - items, err := s.diagnostic(ctx, view, file) + items, err := s.diagnostic(ctx, b, file) if err != nil { return nil, err } @@ -33,8 +34,8 @@ func (s *SemanticAnalysis) Name() string { return "SemanticAnalysis" } -func (s *SemanticAnalysis) diagnostic(ctx context.Context, view *cache.View, changeFile uri.URI) ([]protocol.Diagnostic, error) { - pf, err := view.Parse(ctx, changeFile) +func (s *SemanticAnalysis) diagnostic(ctx context.Context, b *Batch, changeFile uri.URI) ([]protocol.Diagnostic, error) { + pf, err := b.Tree(ctx, changeFile) if err != nil { return nil, err } @@ -50,9 +51,9 @@ func (s *SemanticAnalysis) diagnostic(ctx context.Context, view *cache.View, cha slog.Debug("parse failed", "err", err) } - // One index per file: resolutions are memoized per (file, name), so - // repeated references resolve once. - res := s.checkDefinitionExist(ctx, view, NewIndex(view), pf) + // The run's shared index: resolutions are memoized per (file, name), + // across every checker in the batch. + res := s.checkDefinitionExist(ctx, b.View(), b.Index(), pf) return res, nil } diff --git a/lsp/source/semantic_analysis_test.go b/lsp/source/semantic_analysis_test.go index 23220de..cf94e8a 100644 --- a/lsp/source/semantic_analysis_test.go +++ b/lsp/source/semantic_analysis_test.go @@ -255,7 +255,7 @@ struct TestUUID { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &SemanticAnalysis{} - got, err := c.Diagnostic(tt.args.ctx, tt.args.view, tt.args.changeFiles) + got, err := c.Diagnostic(tt.args.ctx, NewBatch(tt.args.view), tt.args.changeFiles) for key := range got { sort.SliceStable(got[key], func(i, j int) bool { @@ -332,7 +332,7 @@ func Test_SemanticAnalysis_MapKeyScalar(t *testing.T) { }, }) - got, err := (&SemanticAnalysis{}).diagnostic(t.Context(), view, "file:///tmp/user.thrift") + got, err := (&SemanticAnalysis{}).diagnostic(t.Context(), NewBatch(view), "file:///tmp/user.thrift") require.NoError(t, err) var msgs []string diff --git a/lsp/source/unused_include_check.go b/lsp/source/unused_include_check.go index 8134dbf..dca22b6 100644 --- a/lsp/source/unused_include_check.go +++ b/lsp/source/unused_include_check.go @@ -22,11 +22,11 @@ func (c *UnusedIncludeCheck) Name() string { return "UnusedIncludeCheck" } -func (c *UnusedIncludeCheck) Diagnostic(ctx context.Context, view *cache.View, changeFiles []uri.URI) (DiagnosticResult, error) { +func (c *UnusedIncludeCheck) Diagnostic(ctx context.Context, b *Batch, changeFiles []uri.URI) (DiagnosticResult, error) { res := make(DiagnosticResult) for _, file := range changeFiles { - items, err := c.diagnostic(ctx, view, file) + items, err := c.diagnostic(ctx, b, file) if err != nil { return nil, err } @@ -37,8 +37,8 @@ func (c *UnusedIncludeCheck) Diagnostic(ctx context.Context, view *cache.View, c return res, nil } -func (c *UnusedIncludeCheck) diagnostic(ctx context.Context, view *cache.View, file uri.URI) ([]protocol.Diagnostic, error) { - pf, err := view.Parse(ctx, file) +func (c *UnusedIncludeCheck) diagnostic(ctx context.Context, b *Batch, file uri.URI) ([]protocol.Diagnostic, error) { + pf, err := b.Tree(ctx, file) if err != nil { return nil, err } @@ -51,7 +51,7 @@ func (c *UnusedIncludeCheck) diagnostic(ctx context.Context, view *cache.View, f slog.Debug("parse failed", "err", err) } - return unusedIncludeDiagnostics(ctx, view, file, pf), nil + return unusedIncludeDiagnostics(ctx, b, file, pf), nil } // unusedIncludeDiagnostics warns on every include whose target file never @@ -59,13 +59,13 @@ func (c *UnusedIncludeCheck) diagnostic(ctx context.Context, view *cache.View, f // name, a constant value identifier, or a service extends clause, used // qualified ("base.Type") or unqualified (resolving through the include // chain). -func unusedIncludeDiagnostics(ctx context.Context, view *cache.View, file uri.URI, pf *cache.ParsedFile) []protocol.Diagnostic { +func unusedIncludeDiagnostics(ctx context.Context, b *Batch, file uri.URI, pf *cache.ParsedFile) []protocol.Diagnostic { includes := pf.AST().Includes() if len(includes) == 0 { return nil } - used := usedIncludes(ctx, view, file, pf) + used := usedIncludes(ctx, b, file, pf) var ret []protocol.Diagnostic @@ -87,11 +87,11 @@ func unusedIncludeDiagnostics(ctx context.Context, view *cache.View, file uri.UR } // usedIncludes marks every include that at least one reference in the -// document resolves into. Resolution goes through the per-file reference -// index, which handles both qualified ("base.Type") and unqualified names -// that resolve through the include chain. -func usedIncludes(ctx context.Context, view *cache.View, file uri.URI, pf *cache.ParsedFile) map[*syntax.Include]bool { - resolver := view.Resolver() +// document resolves into. Resolution goes through the run's shared +// cross-file index, which handles both qualified ("base.Type") and +// unqualified names that resolve through the include chain. +func usedIncludes(ctx context.Context, b *Batch, file uri.URI, pf *cache.ParsedFile) map[*syntax.Include]bool { + resolver := b.View().Resolver() includeByFile := make(map[uri.URI]*syntax.Include) for _, inc := range pf.AST().Includes() { @@ -102,7 +102,7 @@ func usedIncludes(ctx context.Context, view *cache.View, file uri.URI, pf *cache used := make(map[*syntax.Include]bool) seen := make(map[string]bool) - ix := NewIndex(view) + ix := b.Index() for _, ref := range pf.Index().References() { if seen[ref.Name] { diff --git a/lsp/source/unused_include_check_test.go b/lsp/source/unused_include_check_test.go index f29d283..b4a002c 100644 --- a/lsp/source/unused_include_check_test.go +++ b/lsp/source/unused_include_check_test.go @@ -82,7 +82,7 @@ func Test_UnusedIncludeCheck(t *testing.T) { }, }) - got, err := (&UnusedIncludeCheck{}).diagnostic(t.Context(), view, uri.File(filePath)) + got, err := (&UnusedIncludeCheck{}).diagnostic(t.Context(), NewBatch(view), uri.File(filePath)) require.NoError(t, err) var msgs []string diff --git a/lsp/source/utf16_test.go b/lsp/source/utf16_test.go index d12e03a..5d0cb7f 100644 --- a/lsp/source/utf16_test.go +++ b/lsp/source/utf16_test.go @@ -87,7 +87,7 @@ func TestParseErrorDiagnosticUTF16(t *testing.T) { view := utf16Snapshot(t, src) - res, err := (&Parse{}).Diagnostic(t.Context(), view, []uri.URI{"file:///tmp/htt.thrift"}) + res, err := (&Parse{}).Diagnostic(t.Context(), NewBatch(view), []uri.URI{"file:///tmp/htt.thrift"}) require.NoError(t, err) diags := res["file:///tmp/htt.thrift"]