diff --git a/check/check.go b/check/check.go new file mode 100644 index 0000000..2bbf218 --- /dev/null +++ b/check/check.go @@ -0,0 +1,319 @@ +// Package check runs the language server's diagnostic pipeline over files +// without an editor: the high-level boundary for non-LSP frontends. +// Config discovery belongs to the caller — Run takes resolved inputs — +// and so does presentation: results are data, printing is the CLI's job. +package check + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" + "github.com/karitham/thrift-ls/sema" + "github.com/karitham/thrift-ls/syntax" +) + +// Request is one check run over materialized files. +type Request struct { + // Files are the absolute thrift paths to check. + Files []string + // Folder is the absolute resolution root. Empty derives from the + // first file's directory. + Folder string + // IncludePaths are the compiler-equivalent include roots, authoritative + // for the run. + IncludePaths []string + // Lint tunes the pipeline. Nil means defaults. + Lint *options.LintConfig + // Fix rewrites files in place until nothing applies, preserving file + // permissions. + Fix bool +} + +// Severity is a diagnostic's display weight. +type Severity string + +const ( + SeverityError Severity = "error" + SeverityWarning Severity = "warning" + SeverityInfo Severity = "info" + SeverityHint Severity = "hint" +) + +// Diagnostic is one finding in 1-based file coordinates with UTF-16 +// columns: ready to print, with no editor-protocol vocabulary. +type Diagnostic struct { + Line, Col int + EndLine, EndCol int + Severity Severity + Code string + Message string +} + +// SkippedFix is a fix that could not apply. +type SkippedFix struct { + // File is the absolute path of the file carrying the fix. + File string + // Title names the fix. + Title string + // Reason explains why it was skipped. + Reason string +} + +// FixSummary reports a fix run. +type FixSummary struct { + // Applied is the number of fixes applied across all passes. + Applied int + // Files lists the fixed files' absolute paths. + Files []string + // Passes is the number of pipeline runs. + Passes int + // Skipped lists the fixes that could not apply. + Skipped []SkippedFix +} + +// Result is one check run. Diagnostics are keyed by absolute path. +// Without Fix they hold everything found; with Fix they hold what remains +// unfixed. Fix is nil unless Request.Fix. +type Result struct { + Diagnostics map[string][]Diagnostic + Fix *FixSummary +} + +// Run checks req.Files through the same cache and checker pipeline the LSP +// uses, and returns the diagnostics per file. It exits no process and +// prints nothing; severity gating (failing CI on errors) belongs to the +// caller. +func Run(ctx context.Context, req Request) (Result, error) { + folder := req.Folder + if folder == "" { + if len(req.Files) == 0 { + return Result{Diagnostics: map[string][]Diagnostic{}}, nil + } + + folder = filepath.Dir(req.Files[0]) + } + + lint := sema.Config{} + if req.Lint != nil { + var disabled []string + if req.Lint.Disabled != nil { + disabled = *req.Lint.Disabled + } + + var severity map[string]string + if req.Lint.Severity != nil { + severity = *req.Lint.Severity + } + + lint = sema.ConfigFromLint(disabled, severity) + } + + if req.Fix { + return runFix(ctx, req.Files, folder, req.IncludePaths, lint) + } + + diags, err := runDiagnostics(ctx, req.Files, folder, req.IncludePaths, lint) + if err != nil { + return Result{}, err + } + + return Result{Diagnostics: diags}, nil +} + +// runDiagnostics runs the language server's diagnostic pipeline — parse, +// semantic analysis, and lints — over files opened in a session rooted at +// folder, and returns the diagnostics per file, keyed by absolute path. +func runDiagnostics(ctx context.Context, files []string, folder string, includePaths []string, lint sema.Config) (map[string][]Diagnostic, error) { + _, view, uris, err := openSession(ctx, files, folder, includePaths) + if err != nil { + return nil, err + } + + out := make(map[string][]Diagnostic, len(files)) + + // One pipeline run over the whole corpus: the shared index memoizes + // resolutions across files, so each name resolves once. + report, err := sema.DefaultPipeline(lint).Run(ctx, view, uris) + if err != nil { + return nil, err + } + + for i := range files { + diags, err := toDiagnostics(ctx, view, uris[i], report[uris[i]]) + if err != nil { + return nil, err + } + + out[files[i]] = diags + } + + return out, nil +} + +// runFix applies the diagnostics' fixes to the checked files and reports +// what remains. Only the requested files are fixed — one file, or one +// folder — while resolution reads the whole view, so fixing a greenfield +// module resolves its types against the tree without touching the tree. +func runFix(ctx context.Context, files []string, folder string, includePaths []string, lint sema.Config) (Result, error) { + sess, view, uris, err := openSession(ctx, files, folder, includePaths) + if err != nil { + return Result{}, err + } + + // Fix passes land within the same mtime tick the memoized disk source + // may have cached, so the fixed content flows back through the + // session overlay: the next pass always re-parses what was written. + version := 0 + + persist := func(ctx context.Context, u uri.URI, content []byte) error { + if err := ctx.Err(); err != nil { + return fmt.Errorf("check canceled: %w", err) + } + + version++ + + if err := sess.UpdateOverlayFS(ctx, []*cache.FileChange{ + {URI: u, Version: version, Content: content, From: cache.FileChangeTypeDidChange}, + }); err != nil { + return err + } + + perms := os.FileMode(0o644) + if info, statErr := os.Stat(u.FsPath()); statErr == nil { + perms = info.Mode() + } + + return os.WriteFile(u.FsPath(), content, perms) + } + + res, err := sema.DefaultPipeline(lint).FixAll(ctx, view, uris, persist) + + summary := &FixSummary{ + Applied: res.Applied, + Passes: res.Passes, + Files: make([]string, 0, len(res.FixedFiles)), + Skipped: make([]SkippedFix, 0, len(res.Skipped)), + } + + for _, u := range res.FixedFiles { + summary.Files = append(summary.Files, u.FsPath()) + } + + for _, s := range res.Skipped { + summary.Skipped = append(summary.Skipped, SkippedFix{ + File: s.File.FsPath(), + Title: s.Fix.Title, + Reason: s.Reason, + }) + } + + if err != nil { + if res.Applied == 0 && len(res.FixedFiles) == 0 { + return Result{}, err + } + + return Result{Fix: summary}, err + } + + remaining := make(map[string][]Diagnostic, len(files)) + + for i := range files { + diags, err := toDiagnostics(ctx, view, uris[i], res.Remaining[uris[i]]) + if err != nil { + return Result{}, err + } + + remaining[files[i]] = diags + } + + return Result{Diagnostics: remaining, Fix: summary}, nil +} + +// toDiagnostics translates one file's pipeline findings into printable +// diagnostics: spans map through the file's mapper to 1-based UTF-16 +// columns. +func toDiagnostics(ctx context.Context, view *cache.View, file uri.URI, diags []sema.Diagnostic) ([]Diagnostic, error) { + pf, err := view.Parse(ctx, file) + if err != nil { + return nil, err + } + + out := make([]Diagnostic, len(diags)) + for i, d := range diags { + line, col := position(pf, d.Span.Start) + endLine, endCol := position(pf, d.Span.End) + out[i] = Diagnostic{ + Line: line, + Col: col, + EndLine: endLine, + EndCol: endCol, + Severity: toSeverity(d.Severity), + Code: d.Code, + Message: d.Message, + } + } + + return out, nil +} + +// position maps a parser offset to 1-based file coordinates with UTF-16 +// columns. When the offset does not map, the parser's own 1-based +// coordinates pass through. +func position(pf *cache.ParsedFile, pos syntax.Position) (line, col int) { + p, err := pf.Mapper().OffsetToLSPPosition(pos.Offset) + if err != nil { + return pos.Line, pos.Col + } + + return int(p.Line) + 1, int(p.Character) + 1 +} + +// toSeverity maps the pipeline scale onto the display scale. +func toSeverity(s sema.Severity) Severity { + switch s { + case sema.SeverityError: + return SeverityError + case sema.SeverityWarning: + return SeverityWarning + case sema.SeverityInfo: + return SeverityInfo + case sema.SeverityHint: + return SeverityHint + default: + return SeverityWarning + } +} + +// openSession opens a session with the files open in the overlay of +// a view rooted at folder, and returns the session (needed to push new +// content into the overlay later), its view, and the files' URIs. +func openSession(ctx context.Context, files []string, folder string, includePaths []string) (*cache.Session, *cache.View, []uri.URI, error) { + sess := cache.NewSession(cache.NewMemoizedFS()) + view := sess.AddView(uri.File(folder), includePaths) + + changes := make([]*cache.FileChange, 0, len(files)) + uris := make([]uri.URI, 0, len(files)) + + for _, file := range files { + content, err := os.ReadFile(file) + if err != nil { + return nil, nil, nil, err + } + + u := uri.File(file) + uris = append(uris, u) + changes = append(changes, &cache.FileChange{URI: u, Version: 0, Content: content, From: cache.FileChangeTypeDidOpen}) + } + + if err := sess.UpdateOverlayFS(ctx, changes); err != nil { + return nil, nil, nil, err + } + + return sess, view, uris, nil +} diff --git a/check/check_test.go b/check/check_test.go new file mode 100644 index 0000000..ceb390e --- /dev/null +++ b/check/check_test.go @@ -0,0 +1,183 @@ +package check + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/karitham/thrift-ls/options" +) + +// corpusDir is the shared lint corpus, one intentional mistake per section +// in lints.thrift, an include cycle on the cycle_a/b pair, and no +// diagnostics on the clean files. +func corpusDir(t *testing.T) string { + t.Helper() + + abs, err := filepath.Abs(filepath.Join("..", "tests", "made-in-abyss")) + require.NoError(t, err) + + return abs +} + +// corpusFiles returns the absolute paths of the corpus files in lexical +// order. +func corpusFiles(t *testing.T) []string { + t.Helper() + + entries, err := os.ReadDir(corpusDir(t)) + require.NoError(t, err) + + var files []string + + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".thrift") { + files = append(files, filepath.Join(corpusDir(t), e.Name())) + } + } + + return files +} + +// Test_CheckMadeInAbyss pins the lint corpus through the public boundary: +// the mistake showcase, the include cycle, and the clean files. +func Test_CheckMadeInAbyss(t *testing.T) { + ctx := t.Context() + root := corpusDir(t) + files := corpusFiles(t) + + result, err := Run(ctx, Request{Files: files, Folder: root}) + require.NoError(t, err) + + diags := result.Diagnostics + assert.Nil(t, result.Fix) + + // The clean corpus files report nothing. + for _, name := range []string{"abyss.thrift", "delvers.thrift", "orth.thrift"} { + assert.Empty(t, diags[filepath.Join(root, name)], "%s must be clean", name) + } + + // The mutual include cycle: one warning on cycle_a (its include of + // cycle_b closes the cycle), two on cycle_b. + cycleA := diags[filepath.Join(root, "cycle_a.thrift")] + require.Len(t, cycleA, 1) + assert.Contains(t, cycleA[0].Message, "cycle dependency") + + cycleB := diags[filepath.Join(root, "cycle_b.thrift")] + require.Len(t, cycleB, 2) + assert.Contains(t, cycleB[0].Message, "cycle dependency") + assert.Contains(t, cycleB[1].Message, "cycle dependency") + + // The mistake showcase: 18 errors and 7 warnings. + lints := diags[filepath.Join(root, "lints.thrift")] + require.Len(t, lints, 25) + + errs, warns := 0, 0 + for _, d := range lints { + switch d.Severity { + case SeverityError: + errs++ + case SeverityWarning: + warns++ + } + } + assert.Equal(t, 18, errs) + assert.Equal(t, 7, warns) + for _, d := range lints { + if strings.Contains(d.Message, "map key must be a scalar type") { + assert.Equal(t, SeverityWarning, d.Severity) + } + } + + // Every intentional mistake is reported. + for _, msg := range []string{ + `unused include "unused.thrift"`, + "field id conflict", + "field id should be a positive integer in [1, 32767]", + "duplicate enum Reg", + "duplicate field same_name", + "duplicate field repeat", + "duplicate member RIKO", + "enum value 1 duplicates OZEN", + "duplicate function descend", + "duplicate argument depth", + `duplicate map key "zone1"`, + "duplicate set value 4", + "field type doesn't exist", + "map key must be a scalar type, found struct", + "STAR_COMPASS has no explicit value (implicitly 0)", + "UNHEARD_BELL has no explicit value (implicitly 3)", + "CROSSED_STILLS has no explicit value (implicitly 5)", + } { + assert.True(t, hasMessage(lints, msg), "missing diagnostic %q", msg) + } + + // Two checks on one line: the second `repeat` field conflicts on both + // its id (FieldIDCheck) and its name (DuplicateCheck). + var repeatLine int + for _, d := range lints { + if strings.Contains(d.Message, "duplicate field repeat") { + repeatLine = d.Line + } + } + + onLine := diagsOnLine(lints, repeatLine) + require.Len(t, onLine, 2, "line %d must carry both diagnostics", repeatLine) + assert.Contains(t, onLine[0].Message, "field id conflict") + assert.Contains(t, onLine[1].Message, "duplicate field repeat") +} + +// hasMessage reports whether any diagnostic carries msg. +func hasMessage(diags []Diagnostic, msg string) bool { + for _, d := range diags { + if strings.Contains(d.Message, msg) { + return true + } + } + + return false +} + +// diagsOnLine returns the diagnostics starting on the 1-based line. +func diagsOnLine(diags []Diagnostic, line int) []Diagnostic { + var out []Diagnostic + + for _, d := range diags { + if d.Line == line { + out = append(out, d) + } + } + + return out +} + +// Test_Check_LintConfig pins that lint settings reach the pipeline: a +// disabled analyzer produces no diagnostics, while the default +// configuration reports the unused include. +func Test_Check_LintConfig(t *testing.T) { + folder := t.TempDir() + file := filepath.Join(folder, "user.thrift") + content := "include \"shared.thrift\"\nstruct S { 1: i32 a }\n" + require.NoError(t, os.WriteFile(file, []byte(content), 0o644)) + t.Setenv("THRIFT_LS_CONFIG", "") + + config := `{"lint": {"disabled": ["UnusedIncludeCheck"]}}` + configPath := filepath.Join(folder, "thrift-ls.json") + require.NoError(t, os.WriteFile(configPath, []byte(config), 0o644)) + + patch, err := options.Load(configPath) + require.NoError(t, err) + + result, err := Run(t.Context(), Request{Files: []string{file}, Folder: folder, Lint: options.Effective(patch).Lint}) + require.NoError(t, err) + assert.Empty(t, result.Diagnostics[file]) + + // Without the config the warning fires. + result, err = Run(t.Context(), Request{Files: []string{file}, Folder: folder}) + require.NoError(t, err) + assert.True(t, hasMessage(result.Diagnostics[file], "unused include")) +} diff --git a/check_test.go b/check_test.go index 9ad4256..0ea2c6b 100644 --- a/check_test.go +++ b/check_test.go @@ -1,110 +1,13 @@ package main import ( - "os" "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.lsp.dev/protocol" - - "github.com/karitham/thrift-ls/options" - "github.com/karitham/thrift-ls/sema" ) -// Test_CheckMadeInAbyss pins the lint corpus: tests/made-in-abyss is the -// living spec of the diagnostics, one intentional mistake per section in -// lints.thrift, an include cycle on the cycle_a/b pair, and no -// diagnostics on the clean files. -func Test_CheckMadeInAbyss(t *testing.T) { - ctx := t.Context() - - files, err := collectThriftFiles("tests/made-in-abyss") - require.NoError(t, err) - - rootAbs, err := filepath.Abs("tests/made-in-abyss") - require.NoError(t, err) - - diags, err := checkFiles(ctx, files, rootAbs, nil, sema.Config{}) - require.NoError(t, err) - - // The clean corpus files report nothing. - for _, name := range []string{"abyss.thrift", "delvers.thrift", "orth.thrift"} { - assert.Empty(t, diags[corpusAbs(t, name)], "%s must be clean", name) - } - - // The mutual include cycle: one warning on cycle_a (its include of - // cycle_b closes the cycle), two on cycle_b. - cycleA := diags[corpusAbs(t, "cycle_a.thrift")] - require.Len(t, cycleA, 1) - assert.Contains(t, cycleA[0].Message, "cycle dependency") - - cycleB := diags[corpusAbs(t, "cycle_b.thrift")] - require.Len(t, cycleB, 2) - assert.Contains(t, cycleB[0].Message, "cycle dependency") - assert.Contains(t, cycleB[1].Message, "cycle dependency") - - // The mistake showcase: 18 errors and 7 warnings. - lints := diags[corpusAbs(t, "lints.thrift")] - require.Len(t, lints, 25) - - errs, warns := 0, 0 - for _, d := range lints { - switch d.Severity { - case protocol.DiagnosticSeverityError: - errs++ - case protocol.DiagnosticSeverityWarning: - warns++ - } - } - assert.Equal(t, 18, errs) - assert.Equal(t, 7, warns) - for _, d := range lints { - if strings.Contains(message(d), "map key must be a scalar type") { - assert.Equal(t, protocol.DiagnosticSeverityWarning, d.Severity) - } - } - - // Every intentional mistake is reported. - for _, msg := range []string{ - `unused include "unused.thrift"`, - "field id conflict", - "field id should be a positive integer in [1, 32767]", - "duplicate enum Reg", - "duplicate field same_name", - "duplicate field repeat", - "duplicate member RIKO", - "enum value 1 duplicates OZEN", - "duplicate function descend", - "duplicate argument depth", - `duplicate map key "zone1"`, - "duplicate set value 4", - "field type doesn't exist", - "map key must be a scalar type, found struct", - "STAR_COMPASS has no explicit value (implicitly 0)", - "UNHEARD_BELL has no explicit value (implicitly 3)", - "CROSSED_STILLS has no explicit value (implicitly 5)", - } { - assert.True(t, hasMessage(lints, msg), "missing diagnostic %q", msg) - } - - // Two checks on one line: the second `repeat` field conflicts on both - // its id (FieldIDCheck) and its name (DuplicateCheck). - var repeatLine uint32 - for _, d := range lints { - if strings.Contains(message(d), "duplicate field repeat") { - repeatLine = d.Range.Start.Line - } - } - - onLine := diagsOnLine(lints, repeatLine) - require.Len(t, onLine, 2, "line %d must carry both diagnostics", repeatLine+1) - assert.Contains(t, onLine[0].Message, "field id conflict") - assert.Contains(t, onLine[1].Message, "duplicate field repeat") -} - func Test_CollectThriftFiles(t *testing.T) { // A single file. files, err := collectThriftFiles("tests/made-in-abyss/lints.thrift") @@ -129,69 +32,3 @@ func Test_CollectThriftFiles(t *testing.T) { _, err = collectThriftFiles("tests/does-not-exist") require.Error(t, err) } - -// corpusAbs is the absolute path of a made-in-abyss corpus file. -func corpusAbs(t *testing.T, name string) string { - t.Helper() - - p, err := filepath.Abs(filepath.Join("tests", "made-in-abyss", name)) - require.NoError(t, err) - - return p -} - -// message is the diagnostic message as plain text. -func message(d protocol.Diagnostic) string { - return string(d.Message.(protocol.String)) -} - -// hasMessage reports whether any diagnostic carries msg. -func hasMessage(diags []protocol.Diagnostic, msg string) bool { - for _, d := range diags { - if strings.Contains(message(d), msg) { - return true - } - } - - return false -} - -// diagsOnLine returns the diagnostics starting on the 0-based line. -func diagsOnLine(diags []protocol.Diagnostic, line uint32) []protocol.Diagnostic { - var out []protocol.Diagnostic - - for _, d := range diags { - if d.Range.Start.Line == line { - out = append(out, d) - } - } - - return out -} - -// Test_Check_LintConfig pins that thrift-ls.json lint settings reach the -// check pipeline: a disabled analyzer produces no diagnostics, while the -// default configuration reports the unused include. -func Test_Check_LintConfig(t *testing.T) { - folder := t.TempDir() - file := filepath.Join(folder, "user.thrift") - content := "include \"shared.thrift\"\nstruct S { 1: i32 a }\n" - require.NoError(t, os.WriteFile(file, []byte(content), 0o644)) - t.Setenv("THRIFT_LS_CONFIG", "") - - config := `{"lint": {"disabled": ["UnusedIncludeCheck"]}}` - configPath := filepath.Join(folder, "thrift-ls.json") - require.NoError(t, os.WriteFile(configPath, []byte(config), 0o644)) - - patch, err := loadConfig(configPath, folder) - require.NoError(t, err) - - diags, err := checkFiles(t.Context(), []string{file}, folder, nil, lintConfigOf(options.Effective(patch).Lint)) - require.NoError(t, err) - assert.Empty(t, diags[file]) - - // Without the config the warning fires. - diags, err = checkFiles(t.Context(), []string{file}, folder, nil, sema.Config{}) - require.NoError(t, err) - assert.True(t, hasMessage(diags[file], "unused include")) -} diff --git a/lsp/cache/file.go b/lsp/cache/file.go index cd7416f..8638f5c 100644 --- a/lsp/cache/file.go +++ b/lsp/cache/file.go @@ -5,6 +5,8 @@ import ( "time" "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/resolver" ) // A FileID uniquely identifies a file in the file system. @@ -42,7 +44,18 @@ type FileHandle interface { Content() ([]byte, error) } -// A FileSource maps URIs to FileHandles. +// Checker tests file existence by OS path, without reading content. +// It is the existence half of the resolving system: the include resolver +// probes candidates through it, and frontends with a custom file layout +// (build systems, virtual trees) plug in by serving it from their graph. +// A FileSource that also implements Checker gets cheap probes for free. +type Checker = resolver.Checker + +// A FileSource maps URIs to FileHandles. It is the only filesystem seam: +// disk in production, in-memory in tests, build-system backed internally. +// A source may optionally implement Checker (Exists by OS path) to answer +// include probes without reading file bodies; sources that don't get a +// ReadFile fallback. type FileSource interface { // ReadFile returns the FileHandle for a given URI, either by // reading the content of the file or by obtaining it from a cache. diff --git a/lsp/cache/fs_mem.go b/lsp/cache/fs_mem.go index 69e43af..63dc7f5 100644 --- a/lsp/cache/fs_mem.go +++ b/lsp/cache/fs_mem.go @@ -31,6 +31,17 @@ func (m *memFS) ReadFile(_ context.Context, u uri.URI) (FileHandle, error) { return &DiskFile{uri: u, content: content}, nil } +// Exists reports whether path is a seeded file, without reading content. +func (m *memFS) Exists(ctx context.Context, path string) bool { + if err := ctx.Err(); err != nil { + return false + } + + _, ok := m.files[uri.File(path)] + + return ok +} + // WalkFiles calls fn for every seeded file under root, in lexical order. func (m *memFS) WalkFiles(_ context.Context, root uri.URI, fn func(uri.URI) error) error { rootPath := filepath.Clean(root.Path()) diff --git a/lsp/cache/fs_memoized.go b/lsp/cache/fs_memoized.go index 365c70d..779c39b 100644 --- a/lsp/cache/fs_memoized.go +++ b/lsp/cache/fs_memoized.go @@ -41,6 +41,22 @@ func (h *DiskFile) URI() uri.URI { return h.uri } func (h *DiskFile) Version() int32 { return 0 } func (h *DiskFile) Content() ([]byte, error) { return h.content, h.err } +// Exists reports whether path names an existing regular file, without +// reading content. It stats only, so include probing never pulls file +// bodies into memory. +func (m *memoizedFS) Exists(ctx context.Context, path string) bool { + if err := ctx.Err(); err != nil { + return false + } + + st, err := os.Stat(path) + if err != nil { + return false + } + + return !st.IsDir() +} + // WalkFiles calls fn for every file under root in lexical order. Entries // that fail to stat are skipped: one unreadable or deleted file must not // kill the walk. diff --git a/lsp/cache/fs_overlay.go b/lsp/cache/fs_overlay.go index 3cba221..1fc8b6e 100644 --- a/lsp/cache/fs_overlay.go +++ b/lsp/cache/fs_overlay.go @@ -37,6 +37,38 @@ func (fs *overlayFS) ReadFile(ctx context.Context, uri uri.URI) (FileHandle, err return fs.delegate.ReadFile(ctx, uri) } +// Exists reports whether path names an open overlay or an existing delegate +// file, without reading content. Open files count even when the disk copy +// is missing, so includes resolve for unsaved buffers. +func (fs *overlayFS) Exists(ctx context.Context, path string) bool { + if err := ctx.Err(); err != nil { + return false + } + + u := uri.File(path) + + fs.mu.Lock() + _, ok := fs.overlays[u] + fs.mu.Unlock() + + if ok { + return true + } + + if ex, ok := fs.delegate.(Checker); ok { + return ex.Exists(ctx, path) + } + + fh, err := fs.delegate.ReadFile(ctx, u) + if err != nil { + return false + } + + _, err = fh.Content() + + return err == nil +} + // WalkFiles enumerates the delegate's tree, not the overlay: the walk // discovers files on the underlying source, while open files are already // known to the session via their didOpen. diff --git a/lsp/cache/resolver_test.go b/lsp/cache/resolver_test.go index fe767e5..2e90844 100644 --- a/lsp/cache/resolver_test.go +++ b/lsp/cache/resolver_test.go @@ -1,37 +1,28 @@ package cache import ( - "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" "go.lsp.dev/uri" + "github.com/karitham/thrift-ls/resolver/resolvertest" "github.com/karitham/thrift-ls/syntax" ) func TestResolver(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "resolver-test") - assert.NoError(t, err) - - defer func() { _ = os.RemoveAll(tmpDir) }() - - baseDir := filepath.Join(tmpDir, "base") - sharedDir := filepath.Join(tmpDir, "shared") - err = os.MkdirAll(baseDir, 0o755) - assert.NoError(t, err) - err = os.MkdirAll(sharedDir, 0o755) - assert.NoError(t, err) - + root := filepath.Join(string(filepath.Separator), "mem") + baseDir := filepath.Join(root, "base") + sharedDir := filepath.Join(root, "shared") sharedThrift := filepath.Join(sharedDir, "shared.thrift") - err = os.WriteFile(sharedThrift, []byte(""), 0o644) - assert.NoError(t, err) - c := NewMemoizedFS() - fs := NewOverlayFS(c) + tree := resolvertest.Map{ + sharedThrift: []byte("struct Shared {}"), + } + fs := NewOverlayFS(NewMemFS(tree.URIs())) - view := NewView(uri.File(tmpDir), fs, []string{sharedDir}) + view := NewView(uri.File(root), fs, []string{sharedDir}) resolver := view.Resolver() @@ -43,11 +34,9 @@ func TestResolver(t *testing.T) { name: "ResolveInclude/relative_to_current_file", fn: func(t *testing.T) { currentFile := filepath.Join(baseDir, "current.thrift") - err := os.WriteFile(currentFile, []byte(""), 0o644) - assert.NoError(t, err) currentURI := uri.File(currentFile) - result := resolver.ResolveInclude(currentURI, "local.thrift") + result := resolver.ResolveInclude(t.Context(), currentURI, "local.thrift") expected := uri.File(filepath.Join(baseDir, "local.thrift")) assert.Equal(t, expected, result) @@ -57,25 +46,37 @@ func TestResolver(t *testing.T) { name: "ResolveInclude/using_include_paths", fn: func(t *testing.T) { currentFile := filepath.Join(baseDir, "current.thrift") - err := os.WriteFile(currentFile, []byte(""), 0o644) - assert.NoError(t, err) currentURI := uri.File(currentFile) - result := resolver.ResolveInclude(currentURI, "shared.thrift") + result := resolver.ResolveInclude(t.Context(), currentURI, "shared.thrift") expected := uri.File(sharedThrift) assert.Equal(t, expected, result) }, }, + { + name: "ResolveInclude/finds_an_open_overlay_missing_from_disk", + fn: func(t *testing.T) { + // The include lives only in the editor overlay: an unsaved + // buffer resolves even with no disk copy. + overlayURI := uri.File(filepath.Join(sharedDir, "overlay.thrift")) + assert.NoError(t, fs.Update(t.Context(), []*FileChange{ + {URI: overlayURI, Version: 1, Content: []byte("struct Overlay {}"), From: FileChangeTypeDidOpen}, + })) + t.Cleanup(func() { fs.Forget(overlayURI) }) + + currentURI := uri.File(filepath.Join(baseDir, "current.thrift")) + assert.Equal(t, overlayURI, resolver.ResolveInclude(t.Context(), currentURI, "overlay.thrift")) + assert.Contains(t, resolver.ResolveIncludeCandidates(t.Context(), currentURI, "overlay.thrift"), overlayURI) + }, + }, { name: "ResolveInclude/fallback_uri_when_not_found", fn: func(t *testing.T) { currentFile := filepath.Join(baseDir, "current.thrift") - err := os.WriteFile(currentFile, []byte(""), 0o644) - assert.NoError(t, err) currentURI := uri.File(currentFile) - result := resolver.ResolveInclude(currentURI, "nonexistent.thrift") + result := resolver.ResolveInclude(t.Context(), currentURI, "nonexistent.thrift") expected := uri.File(filepath.Join(baseDir, "nonexistent.thrift")) assert.Equal(t, expected, result) @@ -124,8 +125,6 @@ func TestResolver(t *testing.T) { name: "GetIncludeURI/returns_correct_uri", fn: func(t *testing.T) { currentFile := filepath.Join(baseDir, "current.thrift") - err := os.WriteFile(currentFile, []byte(""), 0o644) - assert.NoError(t, err) currentURI := uri.File(currentFile) doc := &syntax.Document{ @@ -134,7 +133,7 @@ func TestResolver(t *testing.T) { }, } - result := resolver.GetIncludeURI(currentURI, doc, "shared") + result := resolver.GetIncludeURI(t.Context(), currentURI, doc, "shared") expected := uri.File(sharedThrift) assert.Equal(t, expected, result) @@ -144,8 +143,6 @@ func TestResolver(t *testing.T) { name: "GetIncludeURI/not_found_returns_empty", fn: func(t *testing.T) { currentFile := filepath.Join(baseDir, "current.thrift") - err := os.WriteFile(currentFile, []byte(""), 0o644) - assert.NoError(t, err) currentURI := uri.File(currentFile) doc := &syntax.Document{ @@ -154,7 +151,7 @@ func TestResolver(t *testing.T) { }, } - result := resolver.GetIncludeURI(currentURI, doc, "shared") + result := resolver.GetIncludeURI(t.Context(), currentURI, doc, "shared") assert.Equal(t, uri.URI(""), result) }, diff --git a/lsp/cache/snapshot.go b/lsp/cache/snapshot.go index 7c045aa..82ce1ef 100644 --- a/lsp/cache/snapshot.go +++ b/lsp/cache/snapshot.go @@ -1,13 +1,10 @@ package cache import ( - "bytes" "context" - "io/fs" "log/slog" "slices" "strings" - "time" "go.lsp.dev/uri" @@ -22,14 +19,40 @@ type Resolver struct { } // newResolver builds a Resolver resolving against src: open files by their -// overlay content, the rest through the memoized disk source. +// overlay presence, the rest through cheap existence checks. No content is +// read to test existence. func newResolver(includePaths []string, src FileSource) *Resolver { return &Resolver{ includePaths: includePaths, - central: resolver.NewWithFS(includePaths, viewFS{fs: src}), + central: resolver.New(includePaths, resolver.WithChecker(fileChecker{src: src})), } } +// fileChecker translates the resolver's string paths to URIs at the cache +// boundary. Known sources answer cheaply; unknown ones fall back to a read. +type fileChecker struct { + src FileSource +} + +func (c fileChecker) Exists(ctx context.Context, path string) bool { + if err := ctx.Err(); err != nil { + return false + } + + if ex, ok := c.src.(Checker); ok { + return ex.Exists(ctx, path) + } + + fh, err := c.src.ReadFile(ctx, uri.File(path)) + if err != nil { + return false + } + + _, err = fh.Content() + + return err == nil +} + // IncludePaths returns the include paths configured for this resolver. func (r *Resolver) IncludePaths() []string { return slices.Clone(r.includePaths) @@ -37,11 +60,11 @@ func (r *Resolver) IncludePaths() []string { // ResolveInclude resolves an include path to a file URI. // It first tries relative to the current file, then tries each include path. -func (r *Resolver) ResolveInclude(cur uri.URI, includePath string) uri.URI { +func (r *Resolver) ResolveInclude(ctx context.Context, cur uri.URI, includePath string) uri.URI { // FsPath, not Path: Path keeps the leading slash before a Windows drive // letter ("/c:/dir/x.thrift"), which breaks the resolver's filepath ops. filePath := cur.FsPath() - resolvedPath := r.central.Resolve(filePath, includePath) + resolvedPath := r.central.Resolve(ctx, filePath, includePath) slog.Debug("include resolved", "file", filePath, "include", includePath, "resolved", resolvedPath) @@ -51,15 +74,15 @@ func (r *Resolver) ResolveInclude(cur uri.URI, includePath string) uri.URI { // ResolveIncludeCandidates returns the existing locations of includePath // for cur, nearest first. More than one location means the include path is // shadowed by another include path. -func (r *Resolver) ResolveIncludeCandidates(cur uri.URI, includePath string) []uri.URI { +func (r *Resolver) ResolveIncludeCandidates(ctx context.Context, cur uri.URI, includePath string) []uri.URI { filePath := cur.FsPath() - paths := r.central.Candidates(filePath, includePath) + paths := r.central.Candidates(ctx, filePath, includePath) slog.Debug("include candidates", "file", filePath, "include", includePath, "paths", paths) - uris := make([]uri.URI, 0, len(paths)) - for _, p := range paths { - uris = append(uris, uri.File(p)) + uris := make([]uri.URI, len(paths)) + for i, p := range paths { + uris[i] = uri.File(p) } return uris @@ -86,13 +109,13 @@ func (r *Resolver) GetIncludePath(ast *syntax.Document, includeName string) stri // GetIncludeURI returns the URI for an included file by include name. // Returns empty URI if not found. -func (r *Resolver) GetIncludeURI(cur uri.URI, ast *syntax.Document, includeName string) uri.URI { +func (r *Resolver) GetIncludeURI(ctx context.Context, cur uri.URI, ast *syntax.Document, includeName string) uri.URI { path := r.GetIncludePath(ast, includeName) if path == "" { return "" } - return r.ResolveInclude(cur, path) + return r.ResolveInclude(ctx, cur, path) } // getIncludeNameFromPath extracts the include name from a path like "base.thrift" @@ -103,65 +126,6 @@ func getIncludeNameFromPath(path string) string { return strings.TrimSuffix(name, ".thrift") } -// viewFS adapts a FileSource to fs.FS for include resolution: files open in -// the editor resolve by their overlay content, everything else falls -// through to the memoized disk source. This lets includes resolve for files -// that are open but not yet saved. -type viewFS struct { - fs FileSource -} - -func (f viewFS) Stat(name string) (fs.FileInfo, error) { - content, err := readThrough(name, f.fs) - if err != nil { - return nil, err - } - - return viewFileInfo{name: name, size: int64(len(content))}, nil -} - -func (f viewFS) Open(name string) (fs.File, error) { - content, err := readThrough(name, f.fs) - if err != nil { - return nil, err - } - - info := viewFileInfo{name: name, size: int64(len(content))} - - return &viewFile{Reader: bytes.NewReader(content), info: info}, nil -} - -// readThrough reads name as an absolute OS path through src, surfacing -// read failures (missing files report their error via Content). -func readThrough(name string, src FileSource) ([]byte, error) { - fh, err := src.ReadFile(context.Background(), uri.File(name)) - if err != nil { - return nil, err - } - - return fh.Content() -} - -type viewFileInfo struct { - name string - size int64 -} - -func (i viewFileInfo) Name() string { return i.name } -func (i viewFileInfo) Size() int64 { return i.size } -func (i viewFileInfo) Mode() fs.FileMode { return 0o644 } -func (i viewFileInfo) ModTime() time.Time { return time.Time{} } -func (i viewFileInfo) IsDir() bool { return false } -func (i viewFileInfo) Sys() any { return nil } - -type viewFile struct { - *bytes.Reader - info fs.FileInfo -} - -func (f *viewFile) Stat() (fs.FileInfo, error) { return f.info, nil } -func (f *viewFile) Close() error { return nil } - func BuildViewForTest(files []*FileChange) *View { return BuildViewForTestWithPaths(nil, files) } diff --git a/lsp/cache/view.go b/lsp/cache/view.go index e568429..dc3b67b 100644 --- a/lsp/cache/view.go +++ b/lsp/cache/view.go @@ -131,7 +131,7 @@ func (v *View) Parse(ctx context.Context, u uri.URI) (*ParsedFile, error) { var includes []uri.URI if pf.AST() != nil { - includes = resolveIncludes(u, pf.AST().Includes(), v.Resolver().ResolveInclude) + includes = resolveIncludes(ctx, u, pf.AST().Includes(), v.Resolver().ResolveInclude) } v.mu.Lock() @@ -439,7 +439,7 @@ func (v *View) affected(uris []uri.URI) []uri.URI { // resolveIncludes dedupes include statements by path text (first wins) and // resolves each to a URI, sorted ascending. -func resolveIncludes(file uri.URI, includes []*syntax.Include, resolve func(uri.URI, string) uri.URI) []uri.URI { +func resolveIncludes(ctx context.Context, file uri.URI, includes []*syntax.Include, resolve func(context.Context, uri.URI, string) uri.URI) []uri.URI { seen := make(map[string]struct{}, len(includes)) uris := make([]uri.URI, 0, len(includes)) @@ -455,7 +455,7 @@ func resolveIncludes(file uri.URI, includes []*syntax.Include, resolve func(uri. seen[text] = struct{}{} - uris = append(uris, resolve(file, text)) + uris = append(uris, resolve(ctx, file, text)) } slices.Sort(uris) diff --git a/lsp/cache/view_test.go b/lsp/cache/view_test.go index 850b5ee..238c448 100644 --- a/lsp/cache/view_test.go +++ b/lsp/cache/view_test.go @@ -1,6 +1,7 @@ package cache import ( + "context" "path/filepath" "testing" @@ -21,7 +22,7 @@ const ( // resolveTestInclude resolves an include path relative to the including // file's directory, mirroring the snapshot resolver's relative fallback. -func resolveTestInclude(cur uri.URI, includePath string) uri.URI { +func resolveTestInclude(_ context.Context, cur uri.URI, includePath string) uri.URI { return uri.File(filepath.Join(filepath.Dir(cur.Path()), includePath)) } @@ -36,7 +37,7 @@ func seedEdges(t *testing.T, v *View, edges map[string][]string) { inc = append(inc, &syntax.Include{Path: &syntax.Token{Text: includePath}}) } - v.setEntry(uri.URI(file), &viewEntry{}, resolveIncludes(uri.URI(file), inc, resolveTestInclude)) + v.setEntry(uri.URI(file), &viewEntry{}, resolveIncludes(t.Context(), uri.URI(file), inc, resolveTestInclude)) } } diff --git a/lsp/config_test.go b/lsp/config_test.go index 09ed23e..f8d4c1d 100644 --- a/lsp/config_test.go +++ b/lsp/config_test.go @@ -12,6 +12,7 @@ import ( "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/options" + "github.com/karitham/thrift-ls/resolver/resolvertest" ) // Probe: printWidth 80 keeps the long struct on one line, 30 breaks it. @@ -177,27 +178,25 @@ func TestConfigDiscoveryAppliesConfiguredDefaults(t *testing.T) { } } -// TestProjectConfigLayering pins the loader-project precedence: the -// project's format settings beat the file document, CLI flags beat the -// project, and project include paths stay authoritative throughout. +// TestProjectConfigLayering pins the loader-project precedence: the loader +// carries include paths only — formatting comes from the file document and +// CLI. Project include paths stay authoritative over both. func TestProjectConfigLayering(t *testing.T) { for _, tt := range []struct { name string file int - project int cli int wantWidth int wantText string }{ - {name: "project beats file", file: 100, project: 30, wantWidth: 30, wantText: probeBroken}, - {name: "cli beats project", file: 100, project: 30, cli: 100, wantWidth: 100, wantText: probeOneLine}, + {name: "file sets format, project sets includes", file: 30, wantWidth: 30, wantText: probeBroken}, + {name: "cli beats file", file: 30, cli: 100, wantWidth: 100, wantText: probeOneLine}, } { t.Run(tt.name, func(t *testing.T) { root := uri.File("/workspace/proj") target := uri.File("/workspace/proj/api.thrift") - fileWidth, projectWidth := tt.file, tt.project + fileWidth := tt.file fileDoc := &options.Patch{FormatPatch: formatter.FormatPatch{PrintWidth: &fileWidth}} - projectCfg := options.Patch{FormatPatch: formatter.FormatPatch{PrintWidth: &projectWidth}} var cliPatch options.Patch if tt.cli > 0 { cliWidth := tt.cli @@ -206,7 +205,6 @@ func TestProjectConfigLayering(t *testing.T) { projectIncludes := []string{"/build/includes"} fileDoc.IncludePaths = &[]string{"/file/includes"} cliPatch.IncludePaths = &[]string{"/cli/includes"} - projectCfg.IncludePaths = &projectIncludes srv := newSyncServerWithOptions(nil, seedFiles(map[string]string{"/workspace/proj/api.thrift": "struct API {}"}), @@ -216,10 +214,10 @@ func TestProjectConfigLayering(t *testing.T) { }) initCustomFolders(t, srv, []uri.URI{uri.File("/workspace")}) installSnapshot(t, srv, uri.File("/workspace"), WorkspaceSnapshot{Projects: []Project{{ - ConfigURI: uri.File("/workspace/proj.json"), - RootURI: root, - TargetFiles: []uri.URI{target}, - Config: projectCfg, + ConfigURI: uri.File("/workspace/proj.json"), + RootURI: root, + TargetFiles: []uri.URI{target}, + IncludePaths: projectIncludes, }}}) cfg := srv.folderConfig(root) @@ -246,7 +244,7 @@ func TestWorkspaceLoaderUsesConfigSourcePerProjectRoot(t *testing.T) { patches[root] = &options.Patch{FormatPatch: formatter.FormatPatch{PrintWidth: &width}} entries[root+"/api.thrift"] = "struct API {}" projects[i] = Project{ - ConfigURI: uri.File(root + "/tbuild.yaml"), + ConfigURI: uri.File(root + "/project.json"), RootURI: uri.File(root), TargetFiles: []uri.URI{uri.File(root + "/api.thrift")}, } @@ -284,7 +282,7 @@ func TestPinnedSourceBypassesDiscovery(t *testing.T) { Options{ConfigSource: options.PinnedSource(&pinned)}) initCustomFolders(t, srv, []uri.URI{uri.File("/workspace")}) installSnapshot(t, srv, uri.File("/workspace"), WorkspaceSnapshot{Projects: []Project{{ - ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + ConfigURI: uri.File("/workspace/project/project.json"), RootURI: root, TargetFiles: []uri.URI{target}, }}}) @@ -300,10 +298,10 @@ func TestPinnedSourceBypassesDiscovery(t *testing.T) { func TestConfigFileIncludePaths(t *testing.T) { t.Setenv("THRIFT_LS_CONFIG", "") - files := seedFiles(map[string]string{ - "/ws/proj/thrift-ls.json": `{"includePaths": ["base"]}`, - "/ws/proj/base/shared.thrift": "struct Shared {}", - }) + files := resolvertest.Map{ + "/ws/proj/thrift-ls.json": []byte(`{"includePaths": ["base"]}`), + "/ws/proj/base/shared.thrift": []byte("struct Shared {}"), + }.URIs() srv := newSyncServerWithOptions(nil, files, Options{}) initWorkspace(t, srv, []uri.URI{uri.File("/ws/proj")}, nil) @@ -314,7 +312,7 @@ func TestConfigFileIncludePaths(t *testing.T) { assert.Equal(t, []string{"/ws/proj/base"}, view.Resolver().IncludePaths()) - resolved := view.Resolver().ResolveInclude(app, "shared.thrift") + resolved := view.Resolver().ResolveInclude(t.Context(), app, "shared.thrift") assert.Equal(t, uri.File("/ws/proj/base/shared.thrift"), resolved) } @@ -344,16 +342,16 @@ func TestCustomProjectIncludePathsAreAuthoritative(t *testing.T) { require.NoError(t, err) installSnapshot(t, srv, uri.File("/ws"), WorkspaceSnapshot{Projects: []Project{{ - ConfigURI: uri.File("/ws/project/tbuild.yaml"), - RootURI: root, - TargetFiles: []uri.URI{target}, - Config: options.Patch{IncludePaths: &[]string{projectIncludes}}, + ConfigURI: uri.File("/ws/project/project.json"), + RootURI: root, + TargetFiles: []uri.URI{target}, + IncludePaths: []string{projectIncludes}, }}}) view, err := srv.session.ViewOf(target) require.NoError(t, err) assert.Equal(t, []string{projectIncludes}, view.Resolver().IncludePaths()) - assert.Equal(t, uri.File("/ws/project-includes/shared.thrift"), view.Resolver().ResolveInclude(target, "shared.thrift")) + assert.Equal(t, uri.File("/ws/project-includes/shared.thrift"), view.Resolver().ResolveInclude(t.Context(), target, "shared.thrift")) _ = context.Background } diff --git a/lsp/frontend.go b/lsp/frontend.go index ec41dd9..03fc2c0 100644 --- a/lsp/frontend.go +++ b/lsp/frontend.go @@ -10,14 +10,16 @@ import ( // // defaults (options.Default) // + Frontend.Defaults (per-installation or build-system defaults) -// + Project.Config, or the ConfigSource document for the project root +// + the ConfigSource document for the project root // (thrift-ls.json via lsp.FileConfigSource, or a build-system patch) // + CLI overlay (--config pinning is a ConfigSource, -I/--printWidth are CLI) // + LSP workspace settings (initializationOptions, didChangeConfiguration) // -// Exception: include paths from a loader Project.Config are authoritative +// Exception: a non-empty Project.IncludePaths from a loader is authoritative // over the file document and CLI, because the build system owns resolution // and a stray -I must not break it. All other keys follow the order above. +// The loader never carries formatting or lint: those come from the document +// and CLI layers only. // ConfigSource resolves the config document for a project root directory. // It returns the patch plus its origin, so build-system frontends can serve diff --git a/lsp/protocol_test.go b/lsp/protocol_test.go index bde1643..3e2dd63 100644 --- a/lsp/protocol_test.go +++ b/lsp/protocol_test.go @@ -15,7 +15,7 @@ func TestInitializeReportsConfiguredVersion(t *testing.T) { version string want string }{ - {name: "configured", version: "tbuild-test-version", want: "tbuild-test-version"}, + {name: "configured", version: "custom-test-version", want: "custom-test-version"}, {name: "fallback", want: ServerVersion}, } { t.Run(tt.name, func(t *testing.T) { diff --git a/lsp/server.go b/lsp/server.go index 8535139..d791f26 100644 --- a/lsp/server.go +++ b/lsp/server.go @@ -213,24 +213,22 @@ func (s *Server) viewConfig(folder uri.URI) options.Patch { return s.cli.Apply(patch.Apply(s.defaults)) } -// projectViewConfig resolves the config for a loader-discovered project. -// Format, lint and friends layer as defaults + document + Project.Config + -// CLI, so flags keep working over build-system settings. Include paths are -// the exception: a Project.Config that sets them is authoritative over the -// file document and CLI, because the build system owns resolution and a -// stray -I must not break it. A project without opinions (empty Config) -// falls back to the file document, preserving thrift-ls.json behavior. +// projectViewConfig resolves the config for a loader-discovered project: +// defaults + ConfigSource document + CLI. Format and lint never come from +// the loader; include paths do. A non-empty Project.IncludePaths is +// authoritative over the file document and CLI, because the build system +// owns resolution and a stray -I must not break it. An empty list falls +// back to the file document, preserving thrift-ls.json behavior. func (s *Server) projectViewConfig(project Project) options.Patch { filePatch := s.loadFilePatch(project.RootURI) merged := s.defaults if filePatch != nil { merged = filePatch.Apply(merged) } - merged = project.Config.Apply(merged) merged = s.cli.Apply(merged) - if project.Config.IncludePaths != nil { - merged.IncludePaths = project.Config.IncludePaths + if len(project.IncludePaths) > 0 { + merged.IncludePaths = &project.IncludePaths } return merged diff --git a/lsp/source/filerename.go b/lsp/source/filerename.go index f341533..8d8cf84 100644 --- a/lsp/source/filerename.go +++ b/lsp/source/filerename.go @@ -38,7 +38,7 @@ func RenameFileEdits(ctx context.Context, view *cache.View, oldURI, newURI uri.U continue } - if resolver.ResolveInclude(f, inc.PathText()) != oldURI { + if resolver.ResolveInclude(ctx, f, inc.PathText()) != oldURI { continue } diff --git a/lsp/source/links.go b/lsp/source/links.go index 3a2b317..c04f07b 100644 --- a/lsp/source/links.go +++ b/lsp/source/links.go @@ -37,7 +37,7 @@ func Links(ctx context.Context, view *cache.View, file uri.URI) []protocol.Docum return } - target := resolver.ResolveInclude(file, text) + target := resolver.ResolveInclude(ctx, file, text) out = append(out, protocol.DocumentLink{ Range: tokenRange(pf, path), Target: &target, diff --git a/lsp/source/reference.go b/lsp/source/reference.go index 5f77d9d..c887467 100644 --- a/lsp/source/reference.go +++ b/lsp/source/reference.go @@ -238,7 +238,7 @@ func searchDefRefs(ctx context.Context, ix *sema.Index, view *cache.View, file u resolver := view.Resolver() if path := resolver.GetIncludePath(pf.AST(), include); path != "" { - file = resolver.ResolveInclude(file, path) + file = resolver.ResolveInclude(ctx, file, path) } } else { svcName = fmt.Sprintf("%s.%s", sema.IncludeNameOf(file), svcName) diff --git a/lsp/source/rename.go b/lsp/source/rename.go index 4b3ef74..bf15ded 100644 --- a/lsp/source/rename.go +++ b/lsp/source/rename.go @@ -79,7 +79,7 @@ func Rename(ctx context.Context, view *cache.View, file uri.URI, pos protocol.Po resolver := view.Resolver() if path := resolver.GetIncludePath(pf.AST(), include); path != "" { - file = resolver.ResolveInclude(file, path) + file = resolver.ResolveInclude(ctx, file, path) } } diff --git a/lsp/source/semantic_completion.go b/lsp/source/semantic_completion.go index da028bf..0de3b52 100644 --- a/lsp/source/semantic_completion.go +++ b/lsp/source/semantic_completion.go @@ -21,7 +21,7 @@ func typeCandidates(ctx context.Context, view *cache.View, file uri.URI, c Conte if i := strings.LastIndexByte(c.Prefix, '.'); i >= 0 { includeName := c.Prefix[:i] - incURI := view.Resolver().GetIncludeURI(file, c.Doc, includeName) + incURI := view.Resolver().GetIncludeURI(ctx, file, c.Doc, includeName) if incURI == "" { return nil } diff --git a/lsp/workspace.go b/lsp/workspace.go index f3600be..d193708 100644 --- a/lsp/workspace.go +++ b/lsp/workspace.go @@ -3,7 +3,6 @@ package lsp import ( "context" "fmt" - "reflect" "slices" "strings" "sync" @@ -12,7 +11,6 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/options" ) // WorkspaceLoader discovers the projects in one LSP workspace folder. A @@ -28,7 +26,10 @@ type WorkspaceSnapshot struct { Issues []WorkspaceIssue } -// Project describes one independently configured Thrift project. +// Project describes one independently configured Thrift project. It is +// the build-system seam: discovery hands thrift-ls roots, files, and +// include paths, and nothing else. Formatting and lint come from the +// config document and CLI layers, never from here. type Project struct { // ConfigURI is the stable identity and source location of the project // configuration. @@ -37,10 +38,11 @@ type Project struct { RootURI uri.URI // TargetFiles are the Thrift files to index for the project. TargetFiles []uri.URI - // Config is the project's full configuration patch (format, lint, - // include paths, ...). Empty means resolve via the server ConfigSource - // for the root, preserving the thrift-ls.json behavior. - Config options.Patch + // IncludePaths are the compiler-equivalent include roots for the + // project. A non-empty list is authoritative over the config document + // and CLI, because the build system owns resolution. Empty means + // resolve includes via the server ConfigSource for the root. + IncludePaths []string } // WorkspaceIssue is a non-fatal discovery problem publishable at URI. @@ -106,28 +108,17 @@ func cloneWorkspaceSnapshot(snapshot WorkspaceSnapshot) WorkspaceSnapshot { for i, project := range snapshot.Projects { project.TargetFiles = slices.Clone(project.TargetFiles) - if project.Config.IncludePaths != nil { - ips := slices.Clone(*project.Config.IncludePaths) - project.Config.IncludePaths = &ips - } + project.IncludePaths = slices.Clone(project.IncludePaths) out.Projects[i] = project } return out } -// projectConfigEqual reports whether two project configs are identical. -// A view is reused only when its project config is unchanged. -func projectConfigEqual(a, b options.Patch) bool { - return reflect.DeepEqual(a, b) -} - -// projectIncludePaths returns the include paths in effect for project. -func projectIncludePaths(project Project) []string { - if project.Config.IncludePaths == nil { - return nil - } - return *project.Config.IncludePaths +// projectIncludesEqual reports whether two projects resolve includes +// identically. A view is reused only when its include paths are unchanged. +func projectIncludesEqual(a, b []string) bool { + return slices.Equal(a, b) } func validateWorkspaceSnapshot(folder uri.URI, snapshot WorkspaceSnapshot) WorkspaceSnapshot { @@ -170,10 +161,6 @@ func validateProject(project Project) error { } } - if err := project.Config.Validate(); err != nil { - return fmt.Errorf("config: %w", err) - } - return nil } @@ -221,11 +208,11 @@ func workspaceModelOf(snapshots map[uri.URI]WorkspaceSnapshot) workspaceModel { snapshot := snapshots[folder] for _, project := range snapshot.Projects { previous, exists := model.roots[project.RootURI] - if exists && !projectConfigEqual(previous.Config, project.Config) { + if exists && !projectIncludesEqual(previous.IncludePaths, project.IncludePaths) { model.issues[project.ConfigURI] = append(model.issues[project.ConfigURI], WorkspaceIssue{ URI: project.ConfigURI, Message: fmt.Sprintf( - "project conflicts with %s: root %s has different configuration", + "project conflicts with %s: root %s has different include paths", previous.ConfigURI, project.RootURI, ), }) @@ -475,7 +462,7 @@ func (w *customWorkspace) reconcileLocked(ctx context.Context) { for root := range w.views { project, exists := next.roots[root] previous := w.model.roots[root] - if exists && projectConfigEqual(previous.Config, project.Config) { + if exists && projectIncludesEqual(previous.IncludePaths, project.IncludePaths) { var lost []uri.URI for _, file := range w.model.ownedFiles(root, w.documents) { owner, owned := next.ownerOf(file, w.documents) diff --git a/lsp/workspace_test.go b/lsp/workspace_test.go index 7c1d49f..630c9ba 100644 --- a/lsp/workspace_test.go +++ b/lsp/workspace_test.go @@ -168,16 +168,16 @@ func TestWorkspaceLoaderDefersViewsAndIndexesOverlayInMostSpecificProject(t *tes installSnapshot(t, srv, workspace, WorkspaceSnapshot{ Projects: []Project{ { - ConfigURI: uri.File("/workspace/project.json"), - RootURI: workspace, - TargetFiles: []uri.URI{outerFile, innerFile}, - Config: options.Patch{IncludePaths: &[]string{"/outer/includes"}}, + ConfigURI: uri.File("/workspace/project.json"), + RootURI: workspace, + TargetFiles: []uri.URI{outerFile, innerFile}, + IncludePaths: []string{"/outer/includes"}, }, { - ConfigURI: uri.File("/workspace/service/project.json"), - RootURI: innerRoot, - TargetFiles: []uri.URI{externalFile}, - Config: options.Patch{IncludePaths: &[]string{"/inner/includes"}}, + ConfigURI: uri.File("/workspace/service/project.json"), + RootURI: innerRoot, + TargetFiles: []uri.URI{externalFile}, + IncludePaths: []string{"/inner/includes"}, }, }, }) @@ -381,16 +381,16 @@ func TestWorkspaceLoaderRejectsInvalidProjectsAndConflictingRoots(t *testing.T) TargetFiles: []uri.URI{invalidConfigTarget}, }, { - ConfigURI: uri.File("/workspace/service.json"), - RootURI: validRoot, - TargetFiles: []uri.URI{validTarget}, - Config: options.Patch{IncludePaths: &[]string{"/first/includes"}}, + ConfigURI: uri.File("/workspace/service.json"), + RootURI: validRoot, + TargetFiles: []uri.URI{validTarget}, + IncludePaths: []string{"/first/includes"}, }, { - ConfigURI: uri.File("/workspace/conflicting.json"), - RootURI: validRoot, - TargetFiles: []uri.URI{conflictingTarget}, - Config: options.Patch{IncludePaths: &[]string{"/second/includes"}}, + ConfigURI: uri.File("/workspace/conflicting.json"), + RootURI: validRoot, + TargetFiles: []uri.URI{conflictingTarget}, + IncludePaths: []string{"/second/includes"}, }, }}) @@ -537,7 +537,7 @@ func TestCustomNonTargetHandling(t *testing.T) { Options{ConfigSource: options.PinnedSource(nil)}) initCustomFolders(t, srv, []uri.URI{root}) installSnapshot(t, srv, root, WorkspaceSnapshot{Projects: []Project{{ - ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + ConfigURI: uri.File("/workspace/project/project.json"), RootURI: root, TargetFiles: []uri.URI{target}, }}}) @@ -596,7 +596,7 @@ func TestCustomNonTargetHandling(t *testing.T) { Options{ConfigSource: options.PinnedSource(nil)}) initCustomFolders(t, srv, []uri.URI{root}) installSnapshot(t, srv, root, WorkspaceSnapshot{Projects: []Project{{ - ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + ConfigURI: uri.File("/workspace/project/project.json"), RootURI: root, TargetFiles: []uri.URI{target}, }}}) @@ -637,7 +637,7 @@ func TestCustomWatchedFilesDoNotReadUnownedEvents(t *testing.T) { _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: root}})) require.NoError(t, err) installSnapshot(t, srv, root, WorkspaceSnapshot{Projects: []Project{{ - ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + ConfigURI: uri.File("/workspace/project/project.json"), RootURI: root, TargetFiles: []uri.URI{target}, }}}) diff --git a/main.go b/main.go index 931ba60..5100bd6 100644 --- a/main.go +++ b/main.go @@ -13,16 +13,14 @@ import ( "github.com/urfave/cli/v3" + "github.com/karitham/thrift-ls/check" "github.com/karitham/thrift-ls/doc" "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/lsp" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/lsp/source" "github.com/karitham/thrift-ls/options" - "github.com/karitham/thrift-ls/sema" "github.com/karitham/thrift-ls/syntax" - "go.lsp.dev/protocol" "go.lsp.dev/uri" ) @@ -405,7 +403,7 @@ func dumpIncludes(ctx context.Context, file string, cmd *cli.Command) error { continue } - candidates := resolver.ResolveIncludeCandidates(u, path) + candidates := resolver.ResolveIncludeCandidates(ctx, u, path) fmt.Fprintf(w, "%s\n", path) if len(candidates) == 0 { @@ -480,167 +478,53 @@ func checkAction(ctx context.Context, cmd *cli.Command) error { return err } - if cmd.Bool("fix") { - return checkFix(ctx, cmd, files, rootAbs, derefStrings(patch.IncludePaths), lintConfigOf(patch.Lint)) - } - - diags, err := checkFiles(ctx, files, rootAbs, derefStrings(patch.IncludePaths), lintConfigOf(patch.Lint)) + result, err := check.Run(ctx, check.Request{ + Files: files, + Folder: rootAbs, + IncludePaths: derefStrings(patch.IncludePaths), + Lint: patch.Lint, + Fix: cmd.Bool("fix"), + }) if err != nil { return err } w := cmd.Writer - errCount, warnCount := 0, 0 - for _, file := range files { - for _, d := range diags[file] { - sev := "warning" - if d.Severity == protocol.DiagnosticSeverityError { - sev = "error" - errCount++ - } else { - warnCount++ - } + if result.Fix != nil { + fmt.Fprintf(w, "applied %d fix(es) in %d file(s) over %d pass(es)\n", + result.Fix.Applied, len(result.Fix.Files), result.Fix.Passes) - fmt.Fprintf(w, "%s:%d:%d %s %s\n", relPath(file), d.Range.Start.Line+1, d.Range.Start.Character+1, sev, d.Message) + for _, s := range result.Fix.Skipped { + fmt.Fprintf(w, "skipped %s %s (%s)\n", relPath(s.File), s.Title, s.Reason) } } - if errCount > 0 { - return fmt.Errorf("found %d error(s), %d warning(s)", errCount, warnCount) - } - - return nil -} - -// checkFiles runs the language server's diagnostic pipeline — parse, -// semantic analysis, and lints — over files opened in a session rooted at -// folder, and returns the diagnostics per file, keyed by absolute path. -func checkFiles(ctx context.Context, files []string, folder string, includePaths []string, lint sema.Config) (map[string][]protocol.Diagnostic, error) { - _, view, uris, err := openCheckSession(ctx, files, folder, includePaths) - if err != nil { - return nil, err - } - - out := make(map[string][]protocol.Diagnostic, len(files)) - - // One pipeline run over the whole corpus: the shared index memoizes - // resolutions across files, so each name resolves once. - report, err := sema.DefaultPipeline(lint).Run(ctx, view, uris) - if err != nil { - return nil, err - } - - for i := range files { - diags, err := source.ToProtocolDiagnostics(ctx, view, uris[i], report[uris[i]]) - if err != nil { - return nil, err - } - - out[files[i]] = diags - } - - return out, nil -} - -// checkFix applies the diagnostics' fixes to the checked files and reports -// what remains. Only the requested files are fixed — one file, or one -// folder — while resolution reads the whole view, so fixing a greenfield -// module resolves its types against the tree without touching the tree. -func checkFix(ctx context.Context, cmd *cli.Command, files []string, folder string, includePaths []string, lint sema.Config) error { - sess, view, uris, err := openCheckSession(ctx, files, folder, includePaths) - if err != nil { - return err - } - - // Fix passes land within the same mtime tick the memoized disk source - // may have cached, so the fixed content flows back through the - // session overlay: the next pass always re-parses what was written. - version := 0 - - persist := func(ctx context.Context, u uri.URI, content []byte) error { - if err := ctx.Err(); err != nil { - return fmt.Errorf("check canceled: %w", err) - } - - version++ - - if err := sess.UpdateOverlayFS(ctx, []*cache.FileChange{ - {URI: u, Version: version, Content: content, From: cache.FileChangeTypeDidChange}, - }); err != nil { - return err - } - - perms := os.FileMode(0o644) - if info, statErr := os.Stat(u.FsPath()); statErr == nil { - perms = info.Mode() - } - - return os.WriteFile(u.FsPath(), content, perms) - } - - res, err := sema.DefaultPipeline(lint).FixAll(ctx, view, uris, persist) - if err != nil { - return err - } - - w := cmd.Writer - - fmt.Fprintf(w, "applied %d fix(es) in %d file(s) over %d pass(es)\n", res.Applied, len(res.FixedFiles), res.Passes) - - for _, s := range res.Skipped { - fmt.Fprintf(w, "skipped %s %s (%s)\n", relPath(s.File.FsPath()), s.Fix.Title, s.Reason) - } - errCount, warnCount := 0, 0 - for i := range files { - for _, d := range res.Remaining[uris[i]] { + for _, file := range files { + for _, d := range result.Diagnostics[file] { sev := "warning" - if d.Severity == sema.SeverityError { + if d.Severity == check.SeverityError { sev = "error" errCount++ } else { warnCount++ } - fmt.Fprintf(w, "%s:%d:%d %s %s\n", relPath(uris[i].FsPath()), d.Span.Start.Line, d.Span.Start.Col, sev, d.Message) + fmt.Fprintf(w, "%s:%d:%d %s %s\n", relPath(file), d.Line, d.Col, sev, d.Message) } } if errCount > 0 { - return fmt.Errorf("%d error(s), %d warning(s) remain unfixed", errCount, warnCount) - } - - return nil -} - -// openCheckSession opens a session with the files open in the overlay of -// a view rooted at folder, and returns the session (needed to push new -// content into the overlay later), its view, and the files' URIs. -func openCheckSession(ctx context.Context, files []string, folder string, includePaths []string) (*cache.Session, *cache.View, []uri.URI, error) { - sess := cache.NewSession(cache.NewMemoizedFS()) - view := sess.AddView(uri.File(folder), includePaths) - - changes := make([]*cache.FileChange, 0, len(files)) - uris := make([]uri.URI, 0, len(files)) - - for _, file := range files { - content, err := os.ReadFile(file) - if err != nil { - return nil, nil, nil, err + if result.Fix != nil { + return fmt.Errorf("%d error(s), %d warning(s) remain unfixed", errCount, warnCount) } - u := uri.File(file) - uris = append(uris, u) - changes = append(changes, &cache.FileChange{URI: u, Version: 0, Content: content, From: cache.FileChangeTypeDidOpen}) - } - - if err := sess.UpdateOverlayFS(ctx, changes); err != nil { - return nil, nil, nil, err + return fmt.Errorf("found %d error(s), %d warning(s)", errCount, warnCount) } - return sess, view, uris, nil + return nil } // collectThriftFiles returns the absolute paths of the thrift files under @@ -821,27 +705,6 @@ func derefStrings(p *[]string) []string { return *p } -// lintConfigOf converts the config layer's lint settings into the -// pipeline's config. The options package stays plain data, so each -// frontend owns this translation; sema.ConfigFromLint does the work. -func lintConfigOf(l *options.LintConfig) sema.Config { - if l == nil { - return sema.Config{} - } - - var disabled []string - if l.Disabled != nil { - disabled = *l.Disabled - } - - var severity map[string]string - if l.Severity != nil { - severity = *l.Severity - } - - return sema.ConfigFromLint(disabled, severity) -} - func main() { if err := rootCommand().Run(context.Background(), os.Args); err != nil { fmt.Fprintln(os.Stderr, "thrift-ls:", err) diff --git a/resolver/integration_test.go b/resolver/integration_test.go index 3458663..bea66df 100644 --- a/resolver/integration_test.go +++ b/resolver/integration_test.go @@ -64,7 +64,7 @@ struct User { r := New([]string{includeDir}) // Resolve middle.thrift from main.thrift's perspective - filename := r.Resolve(mainFile, "middle.thrift") + filename := r.Resolve(t.Context(), mainFile, "middle.thrift") if filename != middleFile { t.Errorf("expected %q, got %q", middleFile, filename) } @@ -92,7 +92,7 @@ struct User { } // Resolve base.thrift from middle.thrift's perspective - filename2 := r.Resolve(middleFile, strings.Trim(includePath, "\"'")) + filename2 := r.Resolve(t.Context(), middleFile, strings.Trim(includePath, "\"'")) if filename2 != baseFile { t.Errorf("expected %q, got %q", baseFile, filename2) } @@ -207,7 +207,7 @@ struct A { t.Fatalf("%s: expected one include, got %v", chain[i], includes) } - next := r.Resolve(chain[i], includes[0]) + next := r.Resolve(t.Context(), chain[i], includes[0]) if next != chain[i+1] { t.Errorf("expected %q, got %q", chain[i+1], next) } diff --git a/resolver/proximity_test.go b/resolver/proximity_test.go index 9c2889a..d18effc 100644 --- a/resolver/proximity_test.go +++ b/resolver/proximity_test.go @@ -2,30 +2,21 @@ package resolver import ( "testing" + + "github.com/karitham/thrift-ls/resolver/resolvertest" ) -// The fixtures give two dungeon kitchens the same relative layout: the +// dungeonFiles gives two dungeon kitchens the same relative layout: the // same include path exists under several include paths, so search order // decides which file wins. The far kitchen (laios) comes first in the // include paths. -func dungeonFS(t *testing.T) map[string][]byte { - t.Helper() - - files := []string{ - "/dungeon/senshi/kitchen/recipes/hotpot.thrift", - "/dungeon/senshi/kitchen/dishes.thrift", - "/dungeon/senshi/pantry/hotpot.thrift", - "/dungeon/laios/kitchen/recipes/hotpot.thrift", - "/dungeon/laios/kitchen/dishes.thrift", - "/dungeon/laios/pantry/hotpot.thrift", - } - - m := make(map[string][]byte, len(files)) - for _, f := range files { - m[f] = []byte("struct Monster {}") - } - - return m +var dungeonFiles = []string{ + "/dungeon/senshi/kitchen/recipes/hotpot.thrift", + "/dungeon/senshi/kitchen/dishes.thrift", + "/dungeon/senshi/pantry/hotpot.thrift", + "/dungeon/laios/kitchen/recipes/hotpot.thrift", + "/dungeon/laios/kitchen/dishes.thrift", + "/dungeon/laios/pantry/hotpot.thrift", } func dungeonIncludePaths() []string { @@ -38,9 +29,9 @@ func dungeonIncludePaths() []string { } func TestCandidatesNearestIncludePathWins(t *testing.T) { - r := NewWithFS(dungeonIncludePaths(), absMapFS(dungeonFS(t))) + r := New(dungeonIncludePaths(), WithFS(resolvertest.Seed(dungeonFiles...))) - got := r.Candidates("/dungeon/senshi/kitchen/recipes/stew.thrift", "recipes/hotpot.thrift") + got := r.Candidates(t.Context(), "/dungeon/senshi/kitchen/recipes/stew.thrift", "recipes/hotpot.thrift") want := []string{ "/dungeon/senshi/kitchen/recipes/hotpot.thrift", "/dungeon/laios/kitchen/recipes/hotpot.thrift", @@ -58,9 +49,9 @@ func TestCandidatesNearestIncludePathWins(t *testing.T) { } func TestCandidatesOrderedNearestFirst(t *testing.T) { - r := NewWithFS(dungeonIncludePaths(), absMapFS(dungeonFS(t))) + r := New(dungeonIncludePaths(), WithFS(resolvertest.Seed(dungeonFiles...))) - got := r.Candidates("/dungeon/senshi/kitchen/recipes/stew.thrift", "dishes.thrift") + got := r.Candidates(t.Context(), "/dungeon/senshi/kitchen/recipes/stew.thrift", "dishes.thrift") want := []string{ "/dungeon/senshi/kitchen/dishes.thrift", "/dungeon/laios/kitchen/dishes.thrift", @@ -80,9 +71,9 @@ func TestCandidatesOrderedNearestFirst(t *testing.T) { func TestCandidatesTiesKeepConfigOrder(t *testing.T) { // From a sibling floor, every root ties on shared prefix, so the // include path order decides. - r := NewWithFS(dungeonIncludePaths(), absMapFS(dungeonFS(t))) + r := New(dungeonIncludePaths(), WithFS(resolvertest.Seed(dungeonFiles...))) - got := r.Candidates("/dungeon/floor4/main.thrift", "hotpot.thrift") + got := r.Candidates(t.Context(), "/dungeon/floor4/main.thrift", "hotpot.thrift") want := []string{ "/dungeon/laios/pantry/hotpot.thrift", "/dungeon/senshi/pantry/hotpot.thrift", @@ -100,19 +91,19 @@ func TestCandidatesTiesKeepConfigOrder(t *testing.T) { } func TestResolveUsesNearestCandidate(t *testing.T) { - r := NewWithFS(dungeonIncludePaths(), absMapFS(dungeonFS(t))) + r := New(dungeonIncludePaths(), WithFS(resolvertest.Seed(dungeonFiles...))) - got := r.Resolve("/dungeon/senshi/kitchen/recipes/stew.thrift", "dishes.thrift") + got := r.Resolve(t.Context(), "/dungeon/senshi/kitchen/recipes/stew.thrift", "dishes.thrift") if want := "/dungeon/senshi/kitchen/dishes.thrift"; got != want { t.Errorf("Resolve = %q, want %q", got, want) } } func TestResolveFallsBackToFileDir(t *testing.T) { - r := NewWithFS(dungeonIncludePaths(), absMapFS(dungeonFS(t))) + r := New(dungeonIncludePaths(), WithFS(resolvertest.Seed(dungeonFiles...))) cur := "/dungeon/senshi/kitchen/recipes/stew.thrift" - got := r.Resolve(cur, "missing.thrift") + got := r.Resolve(t.Context(), cur, "missing.thrift") if want := "/dungeon/senshi/kitchen/recipes/missing.thrift"; got != want { t.Errorf("Resolve = %q, want %q", got, want) } @@ -139,15 +130,15 @@ func TestSortByProximityKeepsInputOrder(t *testing.T) { // depth. The senshi root shares more components with the file's directory // and wins, though the config lists laios first. func TestCandidatesParentRootOutranksFarRoot(t *testing.T) { - r := NewWithFS([]string{ + r := New([]string{ "/dungeon/laios/recipes", // farther "/dungeon/senshi/recipes", - }, absMapFS(map[string][]byte{ - "/dungeon/laios/recipes/stew.thrift": []byte("struct Monster {}"), - "/dungeon/senshi/recipes/stew.thrift": []byte("struct Monster {}"), - })) + }, WithFS(resolvertest.Seed( + "/dungeon/laios/recipes/stew.thrift", + "/dungeon/senshi/recipes/stew.thrift", + ))) - got := r.Resolve("/dungeon/senshi/recipes/chapter2/stew.thrift", "stew.thrift") + got := r.Resolve(t.Context(), "/dungeon/senshi/recipes/chapter2/stew.thrift", "stew.thrift") if want := "/dungeon/senshi/recipes/stew.thrift"; got != want { t.Errorf("Resolve = %q, want %q", got, want) } @@ -157,15 +148,15 @@ func TestCandidatesParentRootOutranksFarRoot(t *testing.T) { // first (3 shared components vs 2), filepath.Rel depth ranks the laios // root first. Shared prefix wins. func TestCandidatesSharedPrefixNotRelDepth(t *testing.T) { - r := NewWithFS([]string{ + r := New([]string{ "/dungeon/laios", // shallower by filepath.Rel "/dungeon/senshi/kitchen2/monsters/beasts", - }, absMapFS(map[string][]byte{ - "/dungeon/laios/stew.thrift": []byte("struct Monster {}"), - "/dungeon/senshi/kitchen2/monsters/beasts/stew.thrift": []byte("struct Monster {}"), - })) + }, WithFS(resolvertest.Seed( + "/dungeon/laios/stew.thrift", + "/dungeon/senshi/kitchen2/monsters/beasts/stew.thrift", + ))) - got := r.Resolve("/dungeon/senshi/kitchen/stew.thrift", "stew.thrift") + got := r.Resolve(t.Context(), "/dungeon/senshi/kitchen/stew.thrift", "stew.thrift") if want := "/dungeon/senshi/kitchen2/monsters/beasts/stew.thrift"; got != want { t.Errorf("Resolve = %q, want %q", got, want) } diff --git a/resolver/resolver.go b/resolver/resolver.go index 7dae882..98183b4 100644 --- a/resolver/resolver.go +++ b/resolver/resolver.go @@ -1,6 +1,7 @@ package resolver import ( + "context" "io/fs" "os" "path/filepath" @@ -9,6 +10,13 @@ import ( "strings" ) +// Checker tests file existence by absolute or relative path. It keeps the +// resolver pure string paths: the cache boundary translates uri.URI to OS +// paths before calling Exists. +type Checker interface { + Exists(ctx context.Context, path string) bool +} + // absFS adapts an fs.FS so absolute names work: os.DirFS rejects paths // starting with "/" (fs.ValidPath), but the resolver works with absolute // paths. @@ -47,29 +55,83 @@ func FS() fs.FS { return absFS{os.DirFS("/")} } // by both CLI and LSP components. type Resolver struct { includePaths []string - fsys fs.FS + exists func(ctx context.Context, path string) bool +} + +// Option configures a Resolver. +type Option func(*Resolver) + +// WithFS checks file existence through fsys. Paths are looked up as given; +// when an absolute path misses, it is retried without the leading slash, +// so os.DirFS("/") and other relative-keyed trees behave like the real +// filesystem. resolvertest.Map and fstest.MapFS make tests hermetic. +func WithFS(fsys fs.FS) Option { + return WithExistsFunc(func(ctx context.Context, path string) bool { + if err := ctx.Err(); err != nil { + return false + } + + if info, err := fs.Stat(fsys, path); err == nil { + return !info.IsDir() + } + + trimmed := strings.TrimPrefix(path, "/") + if trimmed == path { + return false + } + + info, err := fs.Stat(fsys, trimmed) + if err != nil { + return false + } + + return !info.IsDir() + }) +} + +// WithChecker checks existence through checker. +func WithChecker(checker Checker) Option { + return WithExistsFunc(checker.Exists) } -// New creates a Resolver over the real filesystem. -func New(includePaths []string) *Resolver { - return NewWithFS(includePaths, FS()) +// WithExistsFunc checks existence through exists. +func WithExistsFunc(exists func(ctx context.Context, path string) bool) Option { + return func(r *Resolver) { + r.exists = exists + } } -// NewWithFS creates a Resolver that checks file existence through fsys. -// Absolute paths are looked up as-is; os.DirFS("/") reproduces the default -// filesystem behavior, and fstest.MapFS makes tests hermetic. -func NewWithFS(includePaths []string, fsys fs.FS) *Resolver { - return &Resolver{ +// New creates a Resolver over the real filesystem, unless an Option +// overrides existence checks. +func New(includePaths []string, opts ...Option) *Resolver { + r := &Resolver{ includePaths: includePaths, - fsys: fsys, + exists: func(ctx context.Context, path string) bool { + if err := ctx.Err(); err != nil { + return false + } + + st, err := os.Stat(path) + if err != nil { + return false + } + + return !st.IsDir() + }, } + + for _, opt := range opts { + opt(r) + } + + return r } // Resolve returns the nearest location of includePath for currentFile: the // file's directory first, then include paths by proximity (see Candidates). // When the include resolves nowhere, it returns the file-relative candidate. -func (r *Resolver) Resolve(currentFile, includePath string) string { - if candidates := r.Candidates(currentFile, includePath); len(candidates) > 0 { +func (r *Resolver) Resolve(ctx context.Context, currentFile, includePath string) string { + if candidates := r.Candidates(ctx, currentFile, includePath); len(candidates) > 0 { return candidates[0] } @@ -81,13 +143,17 @@ func (r *Resolver) Resolve(currentFile, includePath string) string { // shared directory prefix with the file's directory, config order breaking // ties. More than one location means the include path is shadowed by // another include path. -func (r *Resolver) Candidates(currentFile, includePath string) []string { +func (r *Resolver) Candidates(ctx context.Context, currentFile, includePath string) []string { basePath := filepath.Dir(currentFile) var candidates []string add := func(c string) { - if !r.exists(c) || containsPath(candidates, c) { + if ctx.Err() != nil { + return + } + + if !r.exists(ctx, c) || containsPath(candidates, c) { return } @@ -183,10 +249,3 @@ func containsPath(candidates []string, p string) bool { return false } - -// exists reports whether the file exists on the resolver's filesystem. -func (r *Resolver) exists(path string) bool { - _, err := fs.Stat(r.fsys, path) - - return err == nil -} diff --git a/resolver/resolver_test.go b/resolver/resolver_test.go index f901569..a5c9d96 100644 --- a/resolver/resolver_test.go +++ b/resolver/resolver_test.go @@ -1,15 +1,12 @@ package resolver import ( - "bytes" - "io/fs" "os" "path/filepath" "testing" - "testing/fstest" - "time" "github.com/karitham/thrift-ls/options" + "github.com/karitham/thrift-ls/resolver/resolvertest" ) // TestConfigRelativeIncludePaths verifies that include paths in a config are @@ -37,57 +34,28 @@ func TestConfigRelativeIncludePaths(t *testing.T) { t.Errorf("includePaths[0] = %q, want %q", got, want) } - // Resolution works from any CWD, against a hermetic in-memory fs. The - // config-relative include path is absolute, so the map keys are too - // (fstest.MapFS only supports relative keys). + // Resolution works from any CWD against an in-memory tree. The + // config-relative include path is absolute, so the seed keys are too. baseFile := filepath.Join(dir, "project", "base", "types.thrift") - fsys := absMapFS{baseFile: []byte("struct T {}")} - r := NewWithFS(*cfg.IncludePaths, fsys) + r := New(*cfg.IncludePaths, WithFS(resolvertest.Seed(baseFile))) cur := filepath.Join(dir, "project", "app.thrift") - if got := r.Resolve(cur, "types.thrift"); got != baseFile { + if got := r.Resolve(t.Context(), cur, "types.thrift"); got != baseFile { t.Errorf("Resolve = %q, want %q", got, baseFile) } } -// absMapFS is an in-memory fs.FS keyed by absolute paths, for hermetic tests -// of absolute include-path resolution. -type absMapFS map[string][]byte +// TestWithFSStripsAbsolutePrefix verifies relative-keyed trees (os.DirFS, +// fstest.MapFS) resolve absolute candidates: the lookup retries without +// the leading slash. +func TestWithFSStripsAbsolutePrefix(t *testing.T) { + r := New([]string{"/base"}, WithFS(resolvertest.Seed("base/shared.thrift"))) -func (m absMapFS) Stat(name string) (fs.FileInfo, error) { - if _, ok := m[name]; !ok { - return nil, &fs.PathError{Op: "stat", Path: name, Err: fs.ErrNotExist} + if got := r.Resolve(t.Context(), "/work/app.thrift", "shared.thrift"); got != "/base/shared.thrift" { + t.Errorf("Resolve = %q, want %q", got, "/base/shared.thrift") } - - return absMapFileInfo{name: name}, nil -} - -func (m absMapFS) Open(name string) (fs.File, error) { - data, ok := m[name] - if !ok { - return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} - } - - return &absMapFile{Reader: bytes.NewReader(data), info: absMapFileInfo{name: name}}, nil } -type absMapFileInfo struct{ name string } - -func (absMapFileInfo) Name() string { return "" } -func (absMapFileInfo) Size() int64 { return 0 } -func (absMapFileInfo) Mode() fs.FileMode { return 0 } -func (absMapFileInfo) ModTime() time.Time { return time.Time{} } -func (absMapFileInfo) IsDir() bool { return false } -func (absMapFileInfo) Sys() any { return nil } - -type absMapFile struct { - *bytes.Reader - info fs.FileInfo -} - -func (f *absMapFile) Stat() (fs.FileInfo, error) { return f.info, nil } -func (f *absMapFile) Close() error { return nil } - // TestConfigAbsoluteIncludePaths keeps absolute paths as-is. func TestConfigAbsoluteIncludePaths(t *testing.T) { dir := t.TempDir() @@ -108,17 +76,16 @@ func TestConfigAbsoluteIncludePaths(t *testing.T) { } } -// TestResolveOrder verifies resolution order against a hermetic fs: +// TestResolveOrder verifies resolution order against an in-memory tree: // relative to the current file first, then each include path. func TestResolveOrder(t *testing.T) { - fsys := fstest.MapFS{ - "proj/service/types.thrift": &fstest.MapFile{Data: []byte("local")}, - "proj/base/types.thrift": &fstest.MapFile{Data: []byte("base")}, - "proj/base/other.thrift": &fstest.MapFile{Data: []byte("base")}, - "proj/vendor/types.thrift": &fstest.MapFile{Data: []byte("vendor")}, - "proj/vendor/deep/nested.thrift": &fstest.MapFile{Data: []byte("nested")}, - } - r := NewWithFS([]string{"proj/base", "proj/vendor"}, fsys) + r := New([]string{"proj/base", "proj/vendor"}, WithFS(resolvertest.Seed( + "proj/service/types.thrift", + "proj/base/types.thrift", + "proj/base/other.thrift", + "proj/vendor/types.thrift", + "proj/vendor/deep/nested.thrift", + ))) cur := "proj/service/order.thrift" tests := []struct { @@ -133,7 +100,7 @@ func TestResolveOrder(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := r.Resolve(cur, tt.includePath); got != tt.want { + if got := r.Resolve(t.Context(), cur, tt.includePath); got != tt.want { t.Errorf("Resolve(%q) = %q, want %q", tt.includePath, got, tt.want) } }) diff --git a/resolver/resolvertest/resolvertest.go b/resolver/resolvertest/resolvertest.go new file mode 100644 index 0000000..79f7bbc --- /dev/null +++ b/resolver/resolvertest/resolvertest.go @@ -0,0 +1,114 @@ +// Package resolvertest provides an in-memory file tree for include +// resolution tests, so tests never touch disk. One seed serves every +// suite: +// +// resolvertest.Seed("/base/shared.thrift") // existence only +// resolver.New(paths, resolver.WithFS(tree)) // fs.FS shape +// resolver.New(paths, resolver.WithChecker(tree)) // Checker shape +// cache.NewMemFS(tree.URIs()) // FileSource shape +// +// The production path stays string paths end to end; URIs converts keys +// for FileSource-based suites only. +package resolvertest + +import ( + "bytes" + "context" + "io/fs" + "path" + "time" + + "go.lsp.dev/uri" +) + +// Map is an in-memory file tree keyed by path. Lookups are exact matches, +// so seed the same form the resolver produces (absolute paths joined from +// include roots). The zero value is an empty tree, ready for existence +// checks. +type Map map[string][]byte + +var ( + _ fs.FS = Map{} + _ fs.StatFS = Map{} +) + +// Seed builds a Map from paths with no content. Include resolution only +// probes existence, so content is irrelevant; tests asserting on content +// seed Map literals directly. +func Seed(paths ...string) Map { + m := make(Map, len(paths)) + for _, p := range paths { + m[p] = nil + } + + return m +} + +// Exists reports whether path is a seeded file, without reading content. It +// satisfies the resolver's existence checker contract. +func (m Map) Exists(ctx context.Context, path string) bool { + if err := ctx.Err(); err != nil { + return false + } + + _, ok := m[path] + + return ok +} + +// URIs converts the tree to a FileSource seed: each path becomes its file +// URI with the same content. The content slices are shared with the Map, +// so tests must not mutate them. +func (m Map) URIs() map[uri.URI][]byte { + out := make(map[uri.URI][]byte, len(m)) + for p, content := range m { + out[uri.File(p)] = content + } + + return out +} + +// Open implements fs.FS by exact path match. +func (m Map) Open(name string) (fs.File, error) { + data, ok := m[name] + if !ok { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + + return &file{Reader: bytes.NewReader(data), info: stat(name, data)}, nil +} + +// Stat implements fs.StatFS by exact path match, so fs.Stat prefers it over +// opening the file. +func (m Map) Stat(name string) (fs.FileInfo, error) { + data, ok := m[name] + if !ok { + return nil, &fs.PathError{Op: "stat", Path: name, Err: fs.ErrNotExist} + } + + return stat(name, data), nil +} + +func stat(name string, data []byte) fs.FileInfo { + return fileInfo{name: path.Base(name), size: int64(len(data))} +} + +type fileInfo struct { + name string + size int64 +} + +func (i fileInfo) Name() string { return i.name } +func (i fileInfo) Size() int64 { return i.size } +func (i fileInfo) Mode() fs.FileMode { return 0 } +func (i fileInfo) ModTime() time.Time { return time.Time{} } +func (i fileInfo) IsDir() bool { return false } +func (i fileInfo) Sys() any { return nil } + +type file struct { + *bytes.Reader + info fs.FileInfo +} + +func (f *file) Stat() (fs.FileInfo, error) { return f.info, nil } +func (f *file) Close() error { return nil } diff --git a/resolver/resolvertest/resolvertest_test.go b/resolver/resolvertest/resolvertest_test.go new file mode 100644 index 0000000..16de703 --- /dev/null +++ b/resolver/resolvertest/resolvertest_test.go @@ -0,0 +1,116 @@ +package resolvertest + +import ( + "context" + "io/fs" + "testing" + + "github.com/karitham/thrift-ls/resolver" +) + +// Map stays a valid existence checker for the resolver: if the Checker +// contract drifts, this fails to compile. +var _ resolver.Checker = Map{} + +func TestMap(t *testing.T) { + seed := Map{"/base/shared.thrift": []byte("struct Shared {}")} + + t.Run("exists", func(t *testing.T) { + tests := []struct { + name string + path string + want bool + }{ + {name: "seeded file", path: "/base/shared.thrift", want: true}, + {name: "missing file", path: "/base/missing.thrift", want: false}, + {name: "directory prefix is not a file", path: "/base", want: false}, + {name: "relative form misses an absolute seed", path: "base/shared.thrift", want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := seed.Exists(t.Context(), tt.path); got != tt.want { + t.Errorf("Exists(%q) = %v, want %v", tt.path, got, tt.want) + } + }) + } + }) + + t.Run("cancelled context finds nothing", func(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + if seed.Exists(ctx, "/base/shared.thrift") { + t.Error("Exists with a cancelled context = true, want false") + } + }) + + t.Run("stat", func(t *testing.T) { + info, err := fs.Stat(seed, "/base/shared.thrift") + if err != nil { + t.Fatalf("Stat: %v", err) + } + + if info.Name() != "shared.thrift" { + t.Errorf("Name() = %q, want %q", info.Name(), "shared.thrift") + } + + if info.IsDir() { + t.Error("IsDir() = true, want false") + } + + if _, err := fs.Stat(seed, "/base/missing.thrift"); err == nil { + t.Error("Stat(missing) = nil, want an error") + } + }) + + t.Run("open reads seeded content", func(t *testing.T) { + f, err := seed.Open("/base/shared.thrift") + if err != nil { + t.Fatalf("Open: %v", err) + } + + defer func() { + if err := f.Close(); err != nil { + t.Errorf("Close: %v", err) + } + }() + + content := make([]byte, 16) + n, err := f.Read(content) + + if n != len("struct Shared {}") || string(content) != "struct Shared {}" { + t.Errorf("Read = %q (n=%d, err=%v), want the seeded content", content, n, err) + } + + if _, err := seed.Open("/base/missing.thrift"); err == nil { + t.Error("Open(missing) = nil, want an error") + } + }) + + t.Run("uris converts keys for FileSource seeds", func(t *testing.T) { + uris := seed.URIs() + + content, ok := uris["file:///base/shared.thrift"] + if !ok { + t.Fatal("URIs() misses the seeded path") + } + + if string(content) != "struct Shared {}" { + t.Errorf("URIs() content = %q, want the seeded content", content) + } + }) + + t.Run("seed builds a presence-only tree", func(t *testing.T) { + m := Seed("/a.thrift", "/b.thrift") + + for _, p := range []string{"/a.thrift", "/b.thrift"} { + if !m.Exists(t.Context(), p) { + t.Errorf("Exists(%q) = false, want true", p) + } + } + + if m.Exists(t.Context(), "/c.thrift") { + t.Error("Exists(/c.thrift) = true, want false") + } + }) +} diff --git a/sema/cycle_detect.go b/sema/cycle_detect.go index f453004..0715480 100644 --- a/sema/cycle_detect.go +++ b/sema/cycle_detect.go @@ -90,7 +90,7 @@ func getIncludes(ctx context.Context, view *cache.View, file uri.URI, includesMa continue } - includeURI := resolver.ResolveInclude(file, includes[i].PathText()) + includeURI := resolver.ResolveInclude(ctx, file, includes[i].PathText()) (*includesMap)[file] = append((*includesMap)[file], Include{ file: includeURI, include: includes[i], diff --git a/sema/include_shadow_check.go b/sema/include_shadow_check.go index 271ba72..0d6430a 100644 --- a/sema/include_shadow_check.go +++ b/sema/include_shadow_check.go @@ -28,7 +28,7 @@ func (c *IncludeShadowCheck) AnalyzeFile(ctx context.Context, f File) ([]Diagnos continue } - candidates := resolver.ResolveIncludeCandidates(f.URI, path) + candidates := resolver.ResolveIncludeCandidates(ctx, f.URI, path) if len(candidates) < 2 { continue } diff --git a/sema/unused_include_check.go b/sema/unused_include_check.go index 65cc55e..d9d6326 100644 --- a/sema/unused_include_check.go +++ b/sema/unused_include_check.go @@ -100,7 +100,7 @@ func usedIncludes(ctx context.Context, f File, pf *cache.ParsedFile) map[*syntax for _, inc := range pf.AST().Includes() { if p := inc.PathText(); p != "" { - includeByFile[resolver.ResolveInclude(f.URI, p)] = inc + includeByFile[resolver.ResolveInclude(ctx, f.URI, p)] = inc } } diff --git a/sema/utils.go b/sema/utils.go index 048c362..b614fd5 100644 --- a/sema/utils.go +++ b/sema/utils.go @@ -41,7 +41,7 @@ func definitionFiles(ctx context.Context, view *cache.View, file uri.URI, ast *s return []uri.URI{file} } - return []uri.URI{resolver.ResolveInclude(file, path)} + return []uri.URI{resolver.ResolveInclude(ctx, file, path)} } files := []uri.URI{file} @@ -64,7 +64,7 @@ func definitionFiles(ctx context.Context, view *cache.View, file uri.URI, ast *s for _, inc := range doc.Includes() { if path := inc.PathText(); path != "" { - incFile := resolver.ResolveInclude(f, path) + incFile := resolver.ResolveInclude(ctx, f, path) if seen[incFile] { continue }