diff --git a/README.md b/README.md index f14eedc..9c4fbe0 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,9 @@ auto-format = true ``` `thrift-ls` must be on `PATH`, or use an absolute path as `command`. The -server logs to `$TMPDIR/thrift-ls.log`; raise verbosity with `-logLevel`. +server logs to `$TMPDIR/thrift-ls.log` and, once the LSP handshake is +done, forwards its records to the client as `window/logMessage` (the +editor's LSP log or output channel); raise verbosity with `-logLevel`. #### neovim @@ -272,13 +274,19 @@ Trailing comments may overflow their line without affecting alignment. Configuration lives in a `thrift-ls.json` file, discovered by walking up from the file being formatted or the workspace root (like Biome). Set the -`THRIFT_LS_CONFIG` env var to point at an explicit config file. - -Layered from lowest to highest precedence: defaults, the config file, LSP -workspace settings, CLI flags. The VS Code extension exposes the formatter +`THRIFT_LS_CONFIG` env var to point at an explicit config file. In the LSP, +discovery happens per workspace folder when the server starts: each folder +formats with the nearest `thrift-ls.json` walking up from it, and a +single-file session discovers from the opened file's directory. An explicit +`--config` flag pins one file for every folder (the first folder's config +also sets the process-wide log level). + +Layered from lowest to highest precedence: defaults, the config file, CLI +flags, LSP workspace settings. The VS Code extension exposes the formatter options as `thrift-ls.*` settings (see `vscode/README.md`), which override -the config file for LSP formatting; `includePaths` and `logLevel` are not -available as settings — use the config file or flags for those. +the config file and CLI flags for LSP formatting; `includePaths` and +`logLevel` are not available as settings — use the config file or flags +for those. ```json { diff --git a/log/log.go b/log/log.go deleted file mode 100644 index a5085a5..0000000 --- a/log/log.go +++ /dev/null @@ -1,41 +0,0 @@ -// Package log configures the process-wide slog logger for thrift-ls. -// -// thrift-ls logs to a file in the temp directory (thrift-ls.log) so that LSP -// traffic on stdio is never polluted with log output. -package log - -import ( - "log/slog" - "os" -) - -// Init configures the default slog logger with the given level and redirects -// it to the temp log file. -// -// level uses the historical thrift-ls scale (1 fatal .. 6 trace), matching -// the old logrus levels so CLI flags keep their meaning. -func Init(level int) { - file := os.TempDir() + "/thrift-ls.log" - - logFile, err := os.OpenFile(file, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0o766) - if err != nil { - panic(err) - } - - slog.SetDefault(slog.New(slog.NewTextHandler(logFile, &slog.HandlerOptions{ - Level: slogLevel(level), - }))) -} - -func slogLevel(level int) slog.Level { - switch { - case level >= 5: // logrus Debug / Trace - return slog.LevelDebug - case level == 4: // logrus Info - return slog.LevelInfo - case level == 3: // logrus Warn - return slog.LevelWarn - default: // logrus Fatal / Error / Panic - return slog.LevelError - } -} diff --git a/lsp/cache/cache.go b/lsp/cache/cache.go index f80662f..3dc8146 100644 --- a/lsp/cache/cache.go +++ b/lsp/cache/cache.go @@ -1,14 +1,35 @@ package cache +import ( + "context" + + "go.lsp.dev/uri" +) + +// Cache is the process-wide file store, backed by a FileSource (the disk +// in production, an in-memory tree in tests). Include paths are not +// global: each view resolves its own from its workspace folder's config +// at creation. type Cache struct { - IncludePaths []string + fs FileSource +} + +// New returns a disk-backed cache. +func New() *Cache { + return NewWithFS(&memoizedFS{filesByID: map[FileID][]*DiskFile{}}) +} + +// NewWithFS returns a cache backed by fs, for tests and embedding. +func NewWithFS(fs FileSource) *Cache { + return &Cache{fs: fs} +} - *memoizedFS +// ReadFile implements FileSource by delegating to the backing source. +func (c *Cache) ReadFile(ctx context.Context, u uri.URI) (FileHandle, error) { + return c.fs.ReadFile(ctx, u) } -func New(includePaths []string) *Cache { - return &Cache{ - IncludePaths: includePaths, - memoizedFS: &memoizedFS{filesByID: map[FileID][]*DiskFile{}}, - } +// WalkFiles implements FileSource by delegating to the backing source. +func (c *Cache) WalkFiles(ctx context.Context, root uri.URI, fn func(uri.URI) error) error { + return c.fs.WalkFiles(ctx, root, fn) } diff --git a/lsp/cache/file.go b/lsp/cache/file.go index 584d13c..d3e8928 100644 --- a/lsp/cache/file.go +++ b/lsp/cache/file.go @@ -48,6 +48,11 @@ 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. ReadFile(ctx context.Context, uri uri.URI) (FileHandle, error) + // WalkFiles calls fn for every file under root, recursively, in + // lexical order. The caller filters by kind (e.g. extension). An + // error returned by fn stops the walk; per-entry failures (missing + // roots, permissions) are the implementation's to skip or report. + WalkFiles(ctx context.Context, root uri.URI, fn func(uri.URI) error) error } type FileChangeType string diff --git a/lsp/cache/fs_mem.go b/lsp/cache/fs_mem.go new file mode 100644 index 0000000..69e43af --- /dev/null +++ b/lsp/cache/fs_mem.go @@ -0,0 +1,65 @@ +package cache + +import ( + "context" + "os" + "path/filepath" + "slices" + "strings" + + "go.lsp.dev/uri" +) + +// A memFS is an in-memory file source seeded with URI → content. It keeps +// tests off the real disk: reads and walks resolve against the map, so a +// test can use file:///tmp/... URIs without touching /tmp. +type memFS struct { + files map[uri.URI][]byte +} + +// NewMemFS returns a FileSource backed by files; nil is an empty tree. +func NewMemFS(files map[uri.URI][]byte) FileSource { + return &memFS{files: files} +} + +func (m *memFS) ReadFile(_ context.Context, u uri.URI) (FileHandle, error) { + content, ok := m.files[u] + if !ok { + return &DiskFile{uri: u, err: os.ErrNotExist}, nil + } + + return &DiskFile{uri: u, content: content}, nil +} + +// 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()) + + uris := make([]uri.URI, 0, len(m.files)) + for u := range m.files { + if underRoot(u, rootPath) { + uris = append(uris, u) + } + } + slices.Sort(uris) + + for _, u := range uris { + if err := fn(u); err != nil { + return err + } + } + + return nil +} + +// underRoot reports whether file is inside rootPath (a directory path): +// same path is out (it is the root itself), and siblings sharing a name +// prefix (e.g. /tmp/ab for root /tmp/a) do not count. +func underRoot(file uri.URI, rootPath string) bool { + p := file.Path() + if p == rootPath { + return false + } + + return strings.HasPrefix(p, rootPath+"/") +} diff --git a/lsp/cache/fs_memoized.go b/lsp/cache/fs_memoized.go index c3c5b7f..7358f25 100644 --- a/lsp/cache/fs_memoized.go +++ b/lsp/cache/fs_memoized.go @@ -2,7 +2,9 @@ package cache import ( "context" + "io/fs" "os" + "path/filepath" "sync" "time" @@ -33,6 +35,23 @@ 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 } +// 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. +func (m *memoizedFS) WalkFiles(ctx context.Context, root uri.URI, fn func(uri.URI) error) error { + return filepath.WalkDir(root.FsPath(), func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil // unreadable entry: keep walking + } + + if d.IsDir() { + return nil + } + + return fn(uri.File(path)) + }) +} + // ReadFile stats and (maybe) reads the file, updates the cache, and returns it. func (fs *memoizedFS) ReadFile(ctx context.Context, uri uri.URI) (FileHandle, error) { id, mtime, err := GetFileID(uri.FsPath()) diff --git a/lsp/cache/fs_overlay.go b/lsp/cache/fs_overlay.go index 3556d9c..3cba221 100644 --- a/lsp/cache/fs_overlay.go +++ b/lsp/cache/fs_overlay.go @@ -37,6 +37,13 @@ func (fs *overlayFS) ReadFile(ctx context.Context, uri uri.URI) (FileHandle, err return fs.delegate.ReadFile(ctx, uri) } +// 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. +func (fs *overlayFS) WalkFiles(ctx context.Context, root uri.URI, fn func(uri.URI) error) error { + return fs.delegate.WalkFiles(ctx, root, fn) +} + // Update applies changes to the overlay set. DidClose changes remove the // overlay; all other types create or replace it. func (fs *overlayFS) Update(_ context.Context, changes []*FileChange) error { diff --git a/lsp/cache/invalidation_test.go b/lsp/cache/invalidation_test.go index 66345b1..b47120a 100644 --- a/lsp/cache/invalidation_test.go +++ b/lsp/cache/invalidation_test.go @@ -1,12 +1,13 @@ package cache import ( - "context" "testing" "time" "github.com/stretchr/testify/assert" "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/options" ) // The gundam test corpus: strike_rouge includes federation.gundam, which @@ -60,20 +61,20 @@ type viewHarness struct { func newViewHarness(t *testing.T, files []*FileChange) *viewHarness { t.Helper() - c := New(nil) + c := New() fs := NewOverlayFS(c) - if err := fs.Update(context.Background(), files); err != nil { + if err := fs.Update(t.Context(), files); err != nil { t.Fatal(err) } - view := NewView("file:///tmp", fs, nil) + view := NewView("file:///tmp", fs, nil, options.Patch{}) ss, release := view.Snapshot() defer release() for _, f := range files { - if _, err := ss.Parse(context.Background(), f.URI); err != nil { + if _, err := ss.Parse(t.Context(), f.URI); err != nil { t.Fatal(err) } } @@ -87,13 +88,13 @@ func newViewHarness(t *testing.T, files []*FileChange) *viewHarness { func (h *viewHarness) change(t *testing.T, change *FileChange) []uri.URI { t.Helper() - if err := h.fs.Update(context.Background(), []*FileChange{change}); err != nil { + if err := h.fs.Update(t.Context(), []*FileChange{change}); err != nil { t.Fatal(err) } done := make(chan []uri.URI, 1) - h.view.FileChange(context.Background(), []*FileChange{change}, func(a []uri.URI) { + h.view.FileChange(t.Context(), []*FileChange{change}, func(a []uri.URI) { done <- a }) diff --git a/lsp/cache/resolver_test.go b/lsp/cache/resolver_test.go index 29fad7a..bae26c5 100644 --- a/lsp/cache/resolver_test.go +++ b/lsp/cache/resolver_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "go.lsp.dev/uri" + "github.com/karitham/thrift-ls/options" "github.com/karitham/thrift-ls/syntax" ) @@ -28,10 +29,10 @@ func TestResolver(t *testing.T) { err = os.WriteFile(sharedThrift, []byte(""), 0o644) assert.NoError(t, err) - c := New(nil) + c := New() fs := NewOverlayFS(c) - view := NewView(uri.File(tmpDir), fs, nil) + view := NewView(uri.File(tmpDir), fs, nil, options.Patch{}) includePaths := []string{sharedDir} ss := NewSnapshot(view, includePaths) diff --git a/lsp/cache/session.go b/lsp/cache/session.go index 6a8c7b5..203d6ed 100644 --- a/lsp/cache/session.go +++ b/lsp/cache/session.go @@ -6,6 +6,8 @@ import ( "sync" "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/options" ) type Session struct { @@ -33,8 +35,10 @@ func NewSession(cache *Cache) *Session { } // AddView registers a view for the workspace folder, returning the -// existing view when the folder is already tracked. -func (s *Session) AddView(folder uri.URI) *View { +// existing view when the folder is already tracked. includePaths and +// config are the folder's resolved configuration; the view fixes them at +// creation. +func (s *Session) AddView(folder uri.URI, includePaths []string, config options.Patch) *View { s.viewMu.Lock() defer s.viewMu.Unlock() @@ -44,7 +48,7 @@ func (s *Session) AddView(folder uri.URI) *View { } } - view := NewView(folder, s.overlayFS, s.cache.IncludePaths) + view := NewView(folder, s.overlayFS, includePaths, config) s.views = append(s.views, view) return view diff --git a/lsp/cache/session_test.go b/lsp/cache/session_test.go index bad499b..41bb1ae 100644 --- a/lsp/cache/session_test.go +++ b/lsp/cache/session_test.go @@ -6,6 +6,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/options" ) func TestSessionViews(t *testing.T) { @@ -25,23 +27,23 @@ func TestSessionViews(t *testing.T) { { name: "one view", setup: func(s *Session) { - s.AddView(folderA) + s.AddView(folderA, nil, options.Patch{}) }, folders: []uri.URI{folderA}, }, { name: "views in registration order", setup: func(s *Session) { - s.AddView(folderB) - s.AddView(folderA) + s.AddView(folderB, nil, options.Patch{}) + s.AddView(folderA, nil, options.Patch{}) }, folders: []uri.URI{folderB, folderA}, }, { name: "removed view disappears", setup: func(s *Session) { - s.AddView(folderA) - s.AddView(folderB) + s.AddView(folderA, nil, options.Patch{}) + s.AddView(folderB, nil, options.Patch{}) s.RemoveView(folderA) }, folders: []uri.URI{folderB}, @@ -49,7 +51,7 @@ func TestSessionViews(t *testing.T) { { name: "removing an untracked folder is a no-op", setup: func(s *Session) { - s.AddView(folderA) + s.AddView(folderA, nil, options.Patch{}) s.RemoveView(folderB) }, folders: []uri.URI{folderA}, @@ -58,7 +60,7 @@ func TestSessionViews(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := NewSession(New(nil)) + s := NewSession(New()) tt.setup(s) views := s.Views() @@ -74,24 +76,24 @@ func TestSessionViews(t *testing.T) { } func TestSessionAddViewDedups(t *testing.T) { - s := NewSession(New(nil)) + s := NewSession(New()) folder := uri.File("/tmp/a") - first := s.AddView(folder) - second := s.AddView(folder) + first := s.AddView(folder, nil, options.Patch{}) + second := s.AddView(folder, nil, options.Patch{}) assert.Same(t, first, second) assert.Len(t, s.Views(), 1) } func TestSessionRemoveViewForgetsMappings(t *testing.T) { - s := NewSession(New(nil)) + s := NewSession(New()) folder := uri.File("/tmp/a") other := uri.File("/tmp/b") - s.AddView(folder) - s.AddView(other) + s.AddView(folder, nil, options.Patch{}) + s.AddView(other, nil, options.Patch{}) fileA := uri.File("/tmp/a/one.thrift") fileB := uri.File("/tmp/b/two.thrift") diff --git a/lsp/cache/snapshot.go b/lsp/cache/snapshot.go index a87e68c..1c695a2 100644 --- a/lsp/cache/snapshot.go +++ b/lsp/cache/snapshot.go @@ -11,6 +11,7 @@ import ( "go.lsp.dev/uri" + "github.com/karitham/thrift-ls/options" "github.com/karitham/thrift-ls/resolver" "github.com/karitham/thrift-ls/syntax" ) @@ -311,11 +312,11 @@ func BuildSnapshotForTest(files []*FileChange) *Snapshot { // BuildSnapshotForTestWithPaths is BuildSnapshotForTest with configured // include paths, for cross-project include resolution tests. func BuildSnapshotForTestWithPaths(includePaths []string, files []*FileChange) *Snapshot { - c := New(includePaths) + c := New() fs := NewOverlayFS(c) _ = fs.Update(context.TODO(), files) - view := NewView("file:///tmp", fs, includePaths) + view := NewView("file:///tmp", fs, includePaths, options.Patch{}) ss := NewSnapshot(view, includePaths) for _, f := range files { diff --git a/lsp/cache/view.go b/lsp/cache/view.go index b9f0768..0a4bc2a 100644 --- a/lsp/cache/view.go +++ b/lsp/cache/view.go @@ -8,11 +8,16 @@ import ( "sync" "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/options" ) +// View is a rooted file tree with the configuration that applies to it. +// It is a god object; splitting the snapshot bookkeeping out remains a +// bigger redesign. type View struct { - // TODO(jpf): view 的设计并不合理 - // workspace folder + // folder is the tree root: a workspace folder, or the opened file's + // directory in single-file mode. folder uri.URI fs FileSource @@ -22,6 +27,10 @@ type View struct { includePaths []string + // config is the folder's base configuration, fixed at creation; + // workspace settings overlay it per request. + config options.Patch + // Track the latest snapshot via the snapshot field, guarded by // snapshotMu. The swap in FileChange releases the previous snapshot's // ref under the same lock. @@ -30,12 +39,13 @@ type View struct { snapshotRelease func() } -func NewView(folder uri.URI, fs FileSource, includePaths []string) *View { +func NewView(folder uri.URI, fs FileSource, includePaths []string, config options.Patch) *View { view := &View{ folder: folder, fs: fs, knownFiles: make(map[uri.URI]bool), includePaths: includePaths, + config: config, } view.snapshot = NewSnapshot(view, includePaths) @@ -66,6 +76,12 @@ func (v *View) Folder() uri.URI { return v.folder } +// Config returns the view's base configuration, without the client's +// workspace settings overlay. +func (v *View) Config() options.Patch { + return v.config +} + func (v *View) MarkFileKnown(fileURI uri.URI) { v.knownFilesMu.Lock() defer v.knownFilesMu.Unlock() diff --git a/lsp/codeaction_test.go b/lsp/codeaction_test.go index 076ba10..3cd0a30 100644 --- a/lsp/codeaction_test.go +++ b/lsp/codeaction_test.go @@ -114,7 +114,7 @@ func Test_CodeAction(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - srv := newTestServer(nil) + srv := newMemServer(nil) err := srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: protocol.TextDocumentItem{ diff --git a/lsp/config_test.go b/lsp/config_test.go new file mode 100644 index 0000000..03d94e1 --- /dev/null +++ b/lsp/config_test.go @@ -0,0 +1,225 @@ +package lsp + +import ( + "os" + "path/filepath" + "testing" + "testing/synctest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" +) + +// Probe: printWidth 80 keeps the long struct on one line, 30 breaks it. +const probe = "struct LongName{1: string fieldNameThatIsQuiteLong}\n" + +const ( + probeOneLine = "struct LongName { 1: string fieldNameThatIsQuiteLong }\n" + probeBroken = "struct LongName {\n 1: string fieldNameThatIsQuiteLong\n}\n" +) + +// openAndFormat opens a thrift document and returns its formatted text. +func openAndFormat(t *testing.T, srv *Server, file string) string { + t.Helper() + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: uri.File(file), + LanguageID: "thrift", + Version: 0, + Text: probe, + }, + })) + + edits, err := srv.Formatting(t.Context(), &protocol.DocumentFormattingParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: uri.File(file)}, + }) + require.NoError(t, err) + require.Len(t, edits, 1) + + return edits[0].NewText +} + +// initWorkspace runs the initialize handshake for folders, waiting for the +// async workspace walk (synctest.Wait) so the views — and their +// per-folder configs — exist before any file opens. Callers must run +// inside a synctest bubble. +func initWorkspace(t *testing.T, srv *Server, folders []uri.URI, initializationOptions []byte) { + t.Helper() + + _, err := srv.Initialize(t.Context(), &protocol.InitializeParams{ + WorkspaceFoldersInitializeParams: protocol.WorkspaceFoldersInitializeParams{ + WorkspaceFolders: protocol.NewNullable(foldersFromURIs(folders)), + }, + InitializationOptions: protocol.LSPAny(initializationOptions), + }) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + + synctest.Wait() +} + +func foldersFromURIs(uris []uri.URI) []protocol.WorkspaceFolder { + folders := make([]protocol.WorkspaceFolder, 0, len(uris)) + for _, u := range uris { + folders = append(folders, protocol.WorkspaceFolder{URI: u}) + } + + return folders +} + +// TestConfigDiscoveryPerWorkspaceFolder verifies that each workspace +// folder formats with its own thrift-ls.json: no single process-global +// config baked in before the workspace was known. +func TestConfigDiscoveryPerWorkspaceFolder(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dirA := t.TempDir() + dirB := t.TempDir() + writeConfig(t, dirA, `{"printWidth": 30}`) + writeConfig(t, dirB, `{"printWidth": 100}`) + + srv := NewServer(cache.New(), nil, Options{}) + initWorkspace(t, srv, []uri.URI{uri.File(dirA), uri.File(dirB)}, nil) + + // One server, two folders: each formats with its own config. + assert.Equal(t, probeBroken, openAndFormat(t, srv, filepath.Join(dirA, "a.thrift")), "folder A config: width 30 breaks") + assert.Equal(t, probeOneLine, openAndFormat(t, srv, filepath.Join(dirB, "b.thrift")), "folder B config: width 100 keeps one line") + }) +} + +// TestConfigDiscoverySingleFileMode verifies that a session without +// workspace folders discovers the config from the opened file's directory +// at the first didOpen, like the CLI's per-file discovery. +func TestConfigDiscoverySingleFileMode(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + writeConfig(t, dir, `{"printWidth": 30}`) + + srv := NewServer(cache.New(), nil, Options{}) + initWorkspace(t, srv, nil, nil) + + assert.Equal(t, probeBroken, openAndFormat(t, srv, filepath.Join(dir, "app.thrift"))) + }) +} + +// TestConfigDiscoveryExplicitPathPins verifies that an explicit --config +// file disables per-folder discovery: every view formats with that file, +// whatever the workspace folder contains. +func TestConfigDiscoveryExplicitPathPins(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + writeConfig(t, dir, `{"printWidth": 30}`) + + srv := NewServer(cache.New(), nil, Options{ + Config: options.Default(), + ConfigPath: "/pinned/thrift-ls.json", + }) + initWorkspace(t, srv, []uri.URI{uri.File(dir)}, nil) + + assert.Equal(t, probeOneLine, openAndFormat(t, srv, filepath.Join(dir, "a.thrift"))) + }) +} + +// TestConfigDiscoveryDefaultsWhenNoConfig verifies that a folder without a +// config file formats with defaults, not with the startup working-directory +// config (the launcher's, which is meaningless to the workspace). +func TestConfigDiscoveryDefaultsWhenNoConfig(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + + // A startup CWD-style config must not leak into the view. + startup := options.Default() + width := 30 + startup.PrintWidth = &width + + srv := NewServer(cache.New(), nil, Options{Config: startup}) + initWorkspace(t, srv, []uri.URI{uri.File(dir)}, nil) + + assert.Equal(t, probeOneLine, openAndFormat(t, srv, filepath.Join(dir, "a.thrift"))) + }) +} + +// TestConfigDiscoveryWorkspaceSettingsOverlay verifies the layering on a +// discovered config: workspace settings (initializationOptions, then +// didChangeConfiguration) sit on top of the folder's config file. +func TestConfigDiscoveryWorkspaceSettingsOverlay(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + writeConfig(t, dir, `{"printWidth": 30}`) + + srv := NewServer(cache.New(), nil, Options{}) + initWorkspace(t, srv, []uri.URI{uri.File(dir)}, []byte(`{"printWidth": 100}`)) + + file := filepath.Join(dir, "a.thrift") + + // The phases share one server: settings evolve sequentially, each + // on top of the previous state. + assert.Equal(t, probeOneLine, openAndFormat(t, srv, file), "initializationOptions width 100 wins over the config's 30") + + require.NoError(t, srv.DidChangeConfiguration(t.Context(), &protocol.DidChangeConfigurationParams{ + Settings: protocol.LSPAny([]byte(`{"printWidth": 30}`)), + })) + assert.Equal(t, probeBroken, openAndFormat(t, srv, file), "didChangeConfiguration width 30 replaces the overlay") + + require.NoError(t, srv.DidChangeConfiguration(t.Context(), &protocol.DidChangeConfigurationParams{ + Settings: protocol.LSPAny([]byte(`{"printWidth": 30, "align": "bogus"}`)), + })) + assert.Equal(t, probeBroken, openAndFormat(t, srv, file), "invalid settings are rejected: the previous overlay stays") + }) +} + +// TestConfigDiscoveryLogLevel verifies that the first view's config sets +// the process log level once the workspace is known. +func TestConfigDiscoveryLogLevel(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + writeConfig(t, dir, `{"logLevel": 5}`) + + srv := NewServer(cache.New(), nil, Options{}) + initWorkspace(t, srv, nil, nil) + + openAndFormat(t, srv, filepath.Join(dir, "app.thrift")) + + srv.logLevelMu.Lock() + defer srv.logLevelMu.Unlock() + require.NotNil(t, srv.logLevel) + assert.Equal(t, 5, *srv.logLevel) + }) +} + +// TestConfigDiscoveryInvalidFileKeepsDefaults verifies that a malformed +// config file is rejected with the defaults in effect, like invalid +// workspace settings: it must not crash the server. +func TestConfigDiscoveryInvalidFileKeepsDefaults(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + writeConfig(t, dir, `{"printWidth": "wide"}`) + + srv := NewServer(cache.New(), nil, Options{}) + initWorkspace(t, srv, []uri.URI{uri.File(dir)}, nil) + + assert.Equal(t, probeOneLine, openAndFormat(t, srv, filepath.Join(dir, "a.thrift"))) + }) +} + +// TestConfigDiscoveryNestedFolder verifies that discovery walks up from +// the workspace folder: a config at the repo root applies to a workspace +// folder nested inside it. +func TestConfigDiscoveryNestedFolder(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + root := t.TempDir() + writeConfig(t, root, `{"printWidth": 30}`) + nested := filepath.Join(root, "packages", "app") + require.NoError(t, os.MkdirAll(nested, 0o755)) + + srv := NewServer(cache.New(), nil, Options{}) + initWorkspace(t, srv, []uri.URI{uri.File(nested)}, nil) + + assert.Equal(t, probeBroken, openAndFormat(t, srv, filepath.Join(nested, "a.thrift"))) + }) +} diff --git a/lsp/diag_new_include_test.go b/lsp/diag_new_include_test.go index 139d6fc..860c9b2 100644 --- a/lsp/diag_new_include_test.go +++ b/lsp/diag_new_include_test.go @@ -49,6 +49,10 @@ func (c *diagClient) RegisterCapability(ctx context.Context, params *protocol.Re return nil } +func (c *diagClient) LogMessage(ctx context.Context, params *protocol.LogMessageParams) error { + return nil // the server forwards its logs via window/logMessage +} + // watchers returns the glob patterns of the registered file watchers. func (c *diagClient) watchers() []string { c.regsMu.Lock() diff --git a/lsp/didchange_test.go b/lsp/didchange_test.go index 33da5c4..4345ba0 100644 --- a/lsp/didchange_test.go +++ b/lsp/didchange_test.go @@ -13,7 +13,6 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/options" ) // recordingClient records PublishDiagnostics calls per URI; every other @@ -38,6 +37,10 @@ func (c *recordingClient) PublishDiagnostics(ctx context.Context, params *protoc return nil } +func (c *recordingClient) LogMessage(ctx context.Context, params *protocol.LogMessageParams) error { + return nil // the server forwards its logs via window/logMessage +} + func (c *recordingClient) reset() { c.mu.Lock() defer c.mu.Unlock() @@ -53,7 +56,14 @@ func (c *recordingClient) count(file uri.URI) int { } func newTestServer(client protocol.Client) *Server { - return NewServer(cache.New(nil), client, options.Patch{}) + return NewServer(cache.New(), client, Options{}) +} + +// newMemServer returns a server backed by an in-memory file source, so +// the workspace walk and file reads never touch the real disk. Files may +// be seeded by URI; opened documents are served from the overlay. +func newMemServer(files map[uri.URI][]byte) *Server { + return NewServer(cache.NewWithFS(cache.NewMemFS(files)), nil, Options{}) } func writeFile(t *testing.T, path, content string) { diff --git a/lsp/format.go b/lsp/format.go index 8fd1c00..d0714a6 100644 --- a/lsp/format.go +++ b/lsp/format.go @@ -11,7 +11,7 @@ import ( func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormattingParams) (result []protocol.TextEdit, err error) { return withFile(ctx, s.session, params.TextDocument.URI, func(ss *cache.Snapshot, fh cache.FileHandle) ([]protocol.TextEdit, error) { - edit, err := source.FormatDocument(ctx, ss, fh, s.formatOptions()) + edit, err := source.FormatDocument(ctx, ss, fh, s.formatOptions(ss.View())) if err != nil { return nil, err } @@ -26,6 +26,6 @@ func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormat func (s *Server) rangeFormatting(ctx context.Context, params *protocol.DocumentRangeFormattingParams) (result []protocol.TextEdit, err error) { return withFile(ctx, s.session, params.TextDocument.URI, func(ss *cache.Snapshot, fh cache.FileHandle) ([]protocol.TextEdit, error) { - return source.FormatRange(ctx, ss, fh, s.formatOptions(), params.Range) + return source.FormatRange(ctx, ss, fh, s.formatOptions(ss.View()), params.Range) }) } diff --git a/lsp/format_range_server_test.go b/lsp/format_range_server_test.go index 49b8976..6e25a50 100644 --- a/lsp/format_range_server_test.go +++ b/lsp/format_range_server_test.go @@ -8,9 +8,7 @@ import ( "go.lsp.dev/protocol" "go.lsp.dev/uri" - "github.com/karitham/thrift-ls/lsp/cache" "github.com/karitham/thrift-ls/lsp/mapper" - "github.com/karitham/thrift-ls/options" ) func TestServerRangeFormatting(t *testing.T) { @@ -27,7 +25,7 @@ struct B { struct C { 3: i64 c } ` - srv := NewServer(cache.New(nil), nil, options.Patch{}) + srv := newMemServer(nil) require.NoError(t, srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: protocol.TextDocumentItem{ diff --git a/lsp/impl.go b/lsp/impl.go index eb78311..415a4fe 100644 --- a/lsp/impl.go +++ b/lsp/impl.go @@ -56,7 +56,7 @@ func (s *Server) openFile(ctx context.Context, change *cache.FileChange) error { view, err := s.session.ViewOf(change.URI) if err != nil { filename := change.URI.Path() - view = s.session.AddView(uri.File(path.Dir(filename))) + view = s.addFolderView(uri.File(path.Dir(filename))) } view.FileChange(ctx, []*cache.FileChange{change}, s.postDiagnostics(ctx, view)) diff --git a/lsp/impl_test.go b/lsp/impl_test.go index d221343..7078a90 100644 --- a/lsp/impl_test.go +++ b/lsp/impl_test.go @@ -12,7 +12,6 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/options" ) func Test_DidOpen(t *testing.T) { @@ -36,8 +35,8 @@ struct Test { }, } - cache := cache.New(nil) - srv := NewServer(cache, nil, options.Patch{}) + srv := newMemServer(nil) + err = srv.DidOpen(ctx, params) assert.NoError(t, err) @@ -94,8 +93,7 @@ struct Test { }, } - cache := cache.New(nil) - srv := NewServer(cache, nil, options.Patch{}) + srv := newMemServer(nil) err = srv.DidOpen(ctx, openParams) assert.NoError(t, err) @@ -156,8 +154,8 @@ struct Test { }, } - cache := cache.New(nil) - srv := NewServer(cache, nil, options.Patch{}) + srv := newMemServer(nil) + err = srv.DidOpen(ctx, openParams) assert.NoError(t, err) @@ -253,8 +251,7 @@ struct Test { }, } - cache := cache.New([]string{"/tmp"}) - srv := NewServer(cache, nil, options.Patch{}) + srv := newMemServer(nil) err = srv.DidOpen(ctx, baseParams) assert.NoError(t, err) @@ -359,8 +356,7 @@ struct Other { }, } - cache := cache.New(nil) - srv := NewServer(cache, nil, options.Patch{}) + srv := newMemServer(nil) err = srv.DidOpen(ctx, file1Params) assert.NoError(t, err) @@ -418,7 +414,7 @@ func Test_DidChangeWorkspaceFolders(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dirA, "a.thrift"), []byte("struct FromA {}"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join(dirB, "b.thrift"), []byte("struct FromB {}"), 0o644)) - srv := NewServer(cache.New(nil), nil, options.Patch{}) + srv := NewServer(cache.New(), nil, Options{}) // Adding folders walks them and registers their thrift files. err := srv.DidChangeWorkspaceFolders(ctx, &protocol.DidChangeWorkspaceFoldersParams{ @@ -483,7 +479,7 @@ func Test_InitializeDefersTheWorkspaceWalk(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "nested"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(dir, "nested", "b.thrift"), []byte("struct FromB {}"), 0o644)) - srv := NewServer(cache.New(nil), nil, options.Patch{}) + srv := NewServer(cache.New(), nil, Options{}) _, err := srv.Initialize(t.Context(), &protocol.InitializeParams{ WorkspaceFoldersInitializeParams: protocol.WorkspaceFoldersInitializeParams{ @@ -549,7 +545,7 @@ struct StrikeRouge { }` testURI := uri.URI("file:///tmp/test.thrift") - srv := NewServer(cache.New([]string{"/tmp"}), nil, options.Patch{}) + srv := newMemServer(nil) require.NoError(t, srv.DidOpen(ctx, baseParams)) require.NoError(t, srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: protocol.TextDocumentItem{ @@ -610,7 +606,7 @@ struct StrikeRouge { }` testURI := uri.URI("file:///tmp/test.thrift") - srv := NewServer(cache.New([]string{"/tmp"}), nil, options.Patch{}) + srv := newMemServer(nil) require.NoError(t, srv.DidOpen(ctx, baseParams)) require.NoError(t, srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: protocol.TextDocumentItem{ diff --git a/lsp/include_paths_test.go b/lsp/include_paths_test.go index f7f8b13..29819c3 100644 --- a/lsp/include_paths_test.go +++ b/lsp/include_paths_test.go @@ -4,40 +4,64 @@ import ( "os" "path/filepath" "testing" + "testing/synctest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.lsp.dev/protocol" "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/options" ) -// TestServerIncludePathsFlow verifies that include paths configured on the -// server's cache flow through the session, view, and snapshot into the -// resolver. -func TestServerIncludePathsFlow(t *testing.T) { - dir := t.TempDir() - includeDir := filepath.Join(dir, "base") - assert.NoError(t, os.MkdirAll(includeDir, 0o755)) - shared := filepath.Join(includeDir, "shared.thrift") - assert.NoError(t, os.WriteFile(shared, []byte("struct Shared {}"), 0o644)) - - c := cache.New([]string{includeDir}) - srv := NewServer(c, nil, options.Default()) - - // Views are created per workspace folder at initialization. - srv.session.AddView(uri.File(dir)) - view, err := srv.session.ViewOf(uri.File(filepath.Join(dir, "app.thrift"))) - assert.NoError(t, err) - - snapshot, release := view.Snapshot() - defer release() - - // Resolving an include that only exists in the configured include path - // finds it there. - resolved := snapshot.Resolver().ResolveInclude(uri.File(filepath.Join(dir, "app.thrift")), "shared.thrift") - assert.Equal(t, uri.File(shared), resolved) - - // The snapshot exposes the configured paths. - assert.Equal(t, []string{includeDir}, snapshot.Resolver().IncludePaths()) +// TestConfigFileIncludePaths verifies that include paths from a workspace +// folder's thrift-ls.json flow through view creation into the snapshot's +// resolver. Paths are resolved relative to the config file, so the +// resolver reaches files that live outside the workspace root. +func TestConfigFileIncludePaths(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ctx := t.Context() + + dir := t.TempDir() + includeDir := filepath.Join(dir, "base") + require.NoError(t, os.MkdirAll(includeDir, 0o755)) + shared := filepath.Join(includeDir, "shared.thrift") + require.NoError(t, os.WriteFile(shared, []byte("struct Shared {}"), 0o644)) + writeConfig(t, dir, `{"includePaths": ["base"]}`) + + srv := NewServer(cache.New(), nil, Options{}) + _, err := srv.Initialize(ctx, &protocol.InitializeParams{ + WorkspaceFoldersInitializeParams: protocol.WorkspaceFoldersInitializeParams{ + WorkspaceFolders: protocol.NewNullable([]protocol.WorkspaceFolder{{URI: uri.File(dir)}}), + }, + }) + require.NoError(t, err) + require.NoError(t, srv.Initialized(ctx, &protocol.InitializedParams{})) + + // The workspace walk runs asynchronously on Initialized; wait for + // it so the view exists before resolving. + synctest.Wait() + + app := uri.File(filepath.Join(dir, "app.thrift")) + view, err := srv.session.ViewOf(app) + require.NoError(t, err) + + snapshot, release := view.Snapshot() + defer release() + + // The config's include path is absolute, resolved against the + // config file's directory, not the process CWD. + assert.Equal(t, []string{includeDir}, snapshot.Resolver().IncludePaths()) + + // Resolving an include that only exists in the configured include + // path finds it there. + resolved := snapshot.Resolver().ResolveInclude(app, "shared.thrift") + assert.Equal(t, uri.File(shared), resolved) + }) +} + +// writeConfig writes a thrift-ls.json config file into dir. +func writeConfig(t *testing.T, dir, content string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "thrift-ls.json"), []byte(content), 0o644)) } diff --git a/lsp/initialize.go b/lsp/initialize.go index 51bb5ac..c466861 100644 --- a/lsp/initialize.go +++ b/lsp/initialize.go @@ -3,9 +3,7 @@ package lsp import ( "context" "encoding/json" - "io/fs" "log/slog" - "path/filepath" "strings" "go.lsp.dev/protocol" @@ -46,8 +44,8 @@ func (s *Server) initialize(params *protocol.InitializeParams) (result *protocol s.folders = folders - // Workspace settings (initializationOptions) overlay the base - // configuration; didChangeConfiguration updates them later. + // Workspace settings (initializationOptions) overlay each view's + // config; didChangeConfiguration updates them later. if len(params.InitializationOptions) > 0 { if patch, err := lspSettings(params.InitializationOptions); err != nil { slog.Error("initializationOptions rejected", "err", err) @@ -111,27 +109,17 @@ func (s *Server) walkFoldersThriftFile(folder uri.URI) { slog.Debug("walk dir", "folder", folder.Path()) // The view is the folder itself, so files in nested directories - // resolve to it via ContainsFile. - s.session.AddView(folder) + // resolve to it via ContainsFile; addFolderView resolves its config. + s.addFolderView(folder) - // WalkDir walk files with lexical order. FsPath, not Path: Path keeps - // the leading slash before a Windows drive letter, which Win32 rejects. - _ = filepath.WalkDir(folder.FsPath(), func(path string, d fs.DirEntry, err error) error { - slog.Debug("walking", "path", path) - - if err != nil { - return nil - } - - if d.IsDir() { - return nil - } - - if !strings.HasSuffix(path, ".thrift") { + // Walk the folder through the cache's file source: the disk in + // production, an in-memory tree in tests. WalkDir walks with lexical + // order; the fs implementations handle their own entry errors. + _ = s.cache.WalkFiles(context.TODO(), folder, func(fileURI uri.URI) error { + if !strings.HasSuffix(fileURI.Path(), ".thrift") { return nil } - fileURI := uri.File(path) slog.Debug("file path", "uri", fileURI) if err := s.openFile(context.TODO(), &cache.FileChange{ diff --git a/lsp/log.go b/lsp/log.go new file mode 100644 index 0000000..0eca034 --- /dev/null +++ b/lsp/log.go @@ -0,0 +1,134 @@ +package lsp + +import ( + "context" + "log/slog" + "os" + "sync" + + "go.lsp.dev/protocol" +) + +// InitLogger configures the process-wide slog logger with the given level +// (1 fatal .. 6 trace) and redirects it to a temp file so LSP traffic on +// stdio is never polluted. Records are also forwarded to the LSP client as +// window/logMessage once the handshake is done (see setLogClient). +// +// Re-calling InitLogger re-levels the existing handler in place: the file +// is opened once and a client wired after the handshake keeps receiving +// records. +func InitLogger(level int) { + if logger == nil { + file := os.TempDir() + "/thrift-ls.log" + + logFile, err := os.OpenFile(file, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0o766) + if err != nil { + panic(err) + } + + logger = &logHandler{file: logFile} + slog.SetDefault(slog.New(logger)) + } + + logger.mu.Lock() + logger.inner = slog.NewTextHandler(logger.file, &slog.HandlerOptions{ + Level: slogLevel(level), + }) + logger.mu.Unlock() +} + +// logger is the handler InitLogger installed, so setLogClient can wire the +// client without type-asserting the process's current default handler +// (which tests and library users may have replaced). +var logger *logHandler + +// logHandler writes every record to the temp file and, once a client is +// set, forwards it to the client as a window/logMessage notification. +type logHandler struct { + file *os.File + + mu sync.RWMutex + inner slog.Handler + client protocol.Client +} + +func (h *logHandler) Enabled(ctx context.Context, level slog.Level) bool { + h.mu.RLock() + defer h.mu.RUnlock() + + return h.inner.Enabled(ctx, level) +} + +func (h *logHandler) Handle(ctx context.Context, r slog.Record) error { + h.mu.RLock() + inner, client := h.inner, h.client + h.mu.RUnlock() + + if err := inner.Handle(ctx, r); err != nil { + return err + } + + if client == nil { + return nil + } + + return client.LogMessage(ctx, &protocol.LogMessageParams{ + Type: logMessageType(r.Level), + Message: r.Message, + }) +} + +func (h *logHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + h.mu.RLock() + defer h.mu.RUnlock() + + return &logHandler{file: h.file, inner: h.inner.WithAttrs(attrs), client: h.client} +} + +func (h *logHandler) WithGroup(name string) slog.Handler { + h.mu.RLock() + defer h.mu.RUnlock() + + return &logHandler{file: h.file, inner: h.inner.WithGroup(name), client: h.client} +} + +// setLogClient forwards subsequent log records to the client. It must only +// run after the initialize handshake is answered: Helix discards (or +// stalls on) notifications from an uninitialized server. A nil client +// unwires forwarding. +func setLogClient(client protocol.Client) { + if logger == nil { + return // InitLogger never ran; nothing to wire + } + + logger.mu.Lock() + logger.client = client + logger.mu.Unlock() +} + +// logMessageType maps a slog level to the LSP message type. +func logMessageType(level slog.Level) protocol.MessageType { + switch { + case level >= slog.LevelError: + return protocol.MessageTypeError + case level >= slog.LevelWarn: + return protocol.MessageTypeWarning + case level >= slog.LevelInfo: + return protocol.MessageTypeInfo + default: + return protocol.MessageTypeLog + } +} + +func slogLevel(level int) slog.Level { + switch { + case level >= 5: // logrus Debug / Trace + return slog.LevelDebug + case level == 4: // logrus Info + return slog.LevelInfo + case level == 3: // logrus Warn + return slog.LevelWarn + default: // logrus Fatal / Error / Panic + return slog.LevelError + } +} diff --git a/lsp/log_test.go b/lsp/log_test.go new file mode 100644 index 0000000..8bc2ab6 --- /dev/null +++ b/lsp/log_test.go @@ -0,0 +1,109 @@ +package lsp + +import ( + "context" + "log/slog" + "sync" + "testing" + "testing/synctest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" +) + +// logClient records window/logMessage notifications; every other client +// method is a no-op. +type logClient struct { + protocol.Client + + mu sync.Mutex + messages []protocol.LogMessageParams +} + +func (c *logClient) LogMessage(ctx context.Context, params *protocol.LogMessageParams) error { + c.mu.Lock() + defer c.mu.Unlock() + + c.messages = append(c.messages, *params) + + return nil +} + +func (c *logClient) RegisterCapability(ctx context.Context, params *protocol.RegistrationParams) error { + return nil // the file watcher registration +} + +func (c *logClient) got() []protocol.LogMessageParams { + c.mu.Lock() + defer c.mu.Unlock() + + return append([]protocol.LogMessageParams(nil), c.messages...) +} + +// TestLoggerForwardsToClientAfterHandshake verifies that log records reach +// the client as window/logMessage notifications only once the initialize +// handshake is answered — before that, Helix discards notifications from +// an uninitialized server. +func TestLoggerForwardsToClientAfterHandshake(t *testing.T) { + InitLogger(5) // debug enabled + defer setLogClient(nil) + + client := &logClient{} + srv := NewServer(cache.New(), client, Options{}) + + slog.Info("pre-handshake") + assert.Empty(t, client.got()) + + _, err := srv.Initialize(t.Context(), &protocol.InitializeParams{}) + require.NoError(t, err) + + // The handshake is answered but not complete: the client sends + // Initialized only after receiving the initialize response. + slog.Info("between handshake and initialized") + assert.Empty(t, client.got()) + + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + + slog.Error("post-handshake boom") + + got := client.got() + // The gate is open at Initialized: the boom record is forwarded, and + // nothing recorded before it (like the pre-handshake info) is. + assert.Contains(t, got, protocol.LogMessageParams{ + Type: protocol.MessageTypeError, + Message: "post-handshake boom", + }) + for _, m := range got { + assert.NotEqual(t, "pre-handshake", m.Message) + assert.NotEqual(t, "between handshake and initialized", m.Message) + } +} + +// TestLoggerForwardingSurvivesConfigRelevel verifies that re-leveling the +// logger for a view config's logLevel keeps the client wiring: the +// handshake wires the client, then the workspace walk re-inits the logger, +// and records must still reach the client. +func TestLoggerForwardingSurvivesConfigRelevel(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + InitLogger(3) + defer setLogClient(nil) + + dir := t.TempDir() + writeConfig(t, dir, `{"logLevel": 5}`) + + client := &logClient{} + srv := NewServer(cache.New(), client, Options{}) + initWorkspace(t, srv, []uri.URI{uri.File(dir)}, nil) + + slog.Error("after config re-level") + + assert.Contains(t, client.got(), protocol.LogMessageParams{ + Type: protocol.MessageTypeError, + Message: "after config re-level", + }) + }) +} diff --git a/lsp/server.go b/lsp/server.go index ebca344..f6e6bcf 100644 --- a/lsp/server.go +++ b/lsp/server.go @@ -22,15 +22,24 @@ type Server struct { client protocol.Client - // base is the process configuration: defaults overlaid with the config - // file and CLI flags. It never changes; workspace settings from - // initializationOptions and didChangeConfiguration are overlaid on it. - base options.Patch + // explicit is the startup configuration (defaults + startup config + + // CLI); every view uses it when configPath pins a file, otherwise each + // view resolves its own config from its folder. + explicit options.Patch + configPath string - // formatOpts is the effective formatter configuration. It is guarded by - // optsMu because settings can change between requests. - optsMu sync.RWMutex - formatOpts formatter.Options + // cli is the CLI-only overlay, applied on top of every view's config. + cli options.Patch + + // workspaceOverlay is the last accepted workspace settings, overlaid + // on every view's config. Guarded by optsMu. + optsMu sync.RWMutex + workspaceOverlay options.Patch + + // logLevel is the first view config's log level, applied once the + // workspace is known. Guarded by logLevelMu. + logLevelMu sync.Mutex + logLevel *int // folders are the workspace folders from the initialize request; the // walk starts on the Initialized notification so the initialize @@ -46,44 +55,111 @@ type Server struct { dirWalkOnce sync.Once } -// NewServer returns a Server formatting with the base options. The base is -// expected to validate; workspace settings overlay it at initialize time. -func NewServer(c *cache.Cache, client protocol.Client, base options.Patch) *Server { - s := &Server{ - cache: c, - session: cache.NewSession(c), - client: client, - base: base, +// NewServer returns a Server resolving configuration per view. The options +// are expected to validate; workspace settings overlay each view's config +// at initialize time and on didChangeConfiguration. +func NewServer(c *cache.Cache, client protocol.Client, opts Options) *Server { + return &Server{ + cache: c, + session: cache.NewSession(c), + client: client, + explicit: opts.Config, + configPath: opts.ConfigPath, + cli: opts.CLI, } - s.formatOpts, _ = base.Formatter() - - return s } -// setWorkspaceSettings overlays workspace settings on the base -// configuration. Invalid settings are rejected: the previous configuration -// stays in effect and the error is logged. +// setWorkspaceSettings stores the workspace settings overlay; invalid +// settings are rejected and the previous document stays in effect. func (s *Server) setWorkspaceSettings(overlay options.Patch) { - merged := overlay.Apply(s.base) - fopts, err := merged.Formatter() - if err != nil { + if _, err := overlay.Formatter(); err != nil { slog.Error("workspace settings rejected", "err", err) + return } s.optsMu.Lock() - s.formatOpts = fopts + s.workspaceOverlay = overlay s.optsMu.Unlock() slog.Debug("workspace settings applied") } -// formatOptions returns the current effective formatter options. -func (s *Server) formatOptions() formatter.Options { +// addFolderView creates the view for a workspace folder, resolving the +// folder's config at creation — when the workspace is finally known. +func (s *Server) addFolderView(folder uri.URI) *cache.View { + cfg := s.viewConfig(folder) + s.applyLogLevel(cfg) + + var includePaths []string + if cfg.IncludePaths != nil { + includePaths = *cfg.IncludePaths + } + + return s.session.AddView(folder, includePaths, cfg) +} + +// viewConfig resolves the config for a view rooted at folder: the pinned +// --config file, or the nearest thrift-ls.json walking up, plus CLI flags. +func (s *Server) viewConfig(folder uri.URI) options.Patch { + if s.configPath != "" { + return s.cli.Apply(s.explicit) + } + + cfgPath, err := options.FindConfig(folder.FsPath()) + if err != nil { + slog.Error("config discovery failed", "dir", folder.FsPath(), "err", err) + + return s.cli.Apply(options.Default()) + } + + if cfgPath == "" { + return s.cli.Apply(options.Default()) + } + + cfg, err := options.Load(cfgPath) + if err != nil { + slog.Error("config file rejected", "path", cfgPath, "err", err) + + return s.cli.Apply(options.Default()) + } + + return s.cli.Apply(options.Effective(cfg)) +} + +// applyLogLevel applies the first view config's log level; the logger is +// process-wide, so later views keep it. +func (s *Server) applyLogLevel(cfg options.Patch) { + if cfg.LogLevel == nil { + return + } + + s.logLevelMu.Lock() + defer s.logLevelMu.Unlock() + + if s.logLevel == nil { + s.logLevel = cfg.LogLevel + InitLogger(*cfg.LogLevel) + } +} + +// formatOptions returns the view's config with the workspace settings +// overlay applied. +func (s *Server) formatOptions(view *cache.View) formatter.Options { s.optsMu.RLock() - defer s.optsMu.RUnlock() + overlay := s.workspaceOverlay + s.optsMu.RUnlock() - return s.formatOpts + fopts, err := overlay.Apply(view.Config()).Formatter() + if err != nil { + // Both layers were validated when stored; this is unreachable + // unless a view config was corrupted. + slog.Error("formatter options rejected", "err", err) + + fopts, _ = view.Config().Formatter() + } + + return fopts } func (s *Server) Initialize(ctx context.Context, params *protocol.InitializeParams) (result *protocol.InitializeResult, err error) { @@ -94,6 +170,11 @@ func (s *Server) Initialize(ctx context.Context, params *protocol.InitializePara } func (s *Server) Initialized(ctx context.Context, params *protocol.InitializedParams) (err error) { + // The client only sends Initialized after receiving the initialize + // response, so from here on window/logMessage notifications are safe + // — Helix discards notifications from an uninitialized server. + setLogClient(s.client) + // The workspace walk and the file watcher registration run here, not // in initialize: the client only sends Initialized after receiving // the initialize response, so nothing the server emits at this @@ -206,7 +287,6 @@ func (s *Server) DidChangeWorkspaceFolders(ctx context.Context, params *protocol } for _, folder := range params.Event.Added { - s.session.AddView(folder.URI) s.walkFoldersThriftFile(folder.URI) } diff --git a/lsp/settings_test.go b/lsp/settings_test.go index 6f0e233..c0785a8 100644 --- a/lsp/settings_test.go +++ b/lsp/settings_test.go @@ -6,9 +6,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.lsp.dev/protocol" - - "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/options" ) func TestLSPSettings(t *testing.T) { @@ -36,7 +33,7 @@ func TestWorkspaceSettings(t *testing.T) { content := "struct LongName{1: string fieldNameThatIsQuiteLong}\n" ctx := t.Context() - srv := NewServer(cache.New(nil), nil, options.Patch{}) + srv := newMemServer(nil) require.NoError(t, srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: protocol.TextDocumentItem{ diff --git a/lsp/source/completion_test.go b/lsp/source/completion_test.go index 1a44c2a..b51d3cf 100644 --- a/lsp/source/completion_test.go +++ b/lsp/source/completion_test.go @@ -9,6 +9,7 @@ import ( "go.lsp.dev/protocol" "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" ) // buildSnapshot builds a snapshot from file contents with optional include @@ -16,10 +17,10 @@ import ( func buildSnapshot(t *testing.T, includePaths []string, files ...*cache.FileChange) *cache.Snapshot { t.Helper() - c := cache.New(nil) + c := cache.New() fs := cache.NewOverlayFS(c) _ = fs.Update(t.Context(), files) - view := cache.NewView(uri.File("/tmp"), fs, includePaths) + view := cache.NewView(uri.File("/tmp"), fs, includePaths, options.Patch{}) return cache.NewSnapshot(view, includePaths) } diff --git a/lsp/source/cycle_detect_test.go b/lsp/source/cycle_detect_test.go index 0162b4f..022829e 100644 --- a/lsp/source/cycle_detect_test.go +++ b/lsp/source/cycle_detect_test.go @@ -10,6 +10,7 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" ) func Test_cycleDetect(t *testing.T) { @@ -286,11 +287,11 @@ include "./test/address.thrift"` } func buildSnapshotForTest(t *testing.T, files []*cache.FileChange) *cache.Snapshot { - c := cache.New(nil) + c := cache.New() fs := cache.NewOverlayFS(c) _ = fs.Update(t.Context(), files) - view := cache.NewView("file:///tmp", fs, nil) + view := cache.NewView("file:///tmp", fs, nil, options.Patch{}) ss := cache.NewSnapshot(view, nil) return ss diff --git a/lsp/source/folding_test.go b/lsp/source/folding_test.go index b905351..b48442d 100644 --- a/lsp/source/folding_test.go +++ b/lsp/source/folding_test.go @@ -12,6 +12,7 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" ) // foldCase is one folding range expectation: the range of lines it covers. @@ -28,7 +29,7 @@ func foldingRanges(t *testing.T, src string) []protocol.FoldingRange { file := uri.File(filepath.Join(dir, "test.thrift")) require.NoError(t, os.WriteFile(filepath.Join(dir, "test.thrift"), []byte(src), 0o644)) - view := cache.NewView(uri.File(dir), cache.NewOverlayFS(cache.New(nil)), nil) + view := cache.NewView(uri.File(dir), cache.NewOverlayFS(cache.New()), nil, options.Patch{}) view.FileChange(t.Context(), []*cache.FileChange{{ URI: file, Version: 0, diff --git a/lsp/source/include_action_test.go b/lsp/source/include_action_test.go index d9343cb..9c2b8cf 100644 --- a/lsp/source/include_action_test.go +++ b/lsp/source/include_action_test.go @@ -12,6 +12,7 @@ import ( "github.com/karitham/thrift-ls/lsp/cache" "github.com/karitham/thrift-ls/lsp/mapper" + "github.com/karitham/thrift-ls/options" ) // buildFolderSnapshotForTest builds a snapshot whose view root is folder, @@ -19,11 +20,11 @@ import ( func buildFolderSnapshotForTest(t *testing.T, folder string, files []*cache.FileChange) *cache.Snapshot { t.Helper() - c := cache.New(nil) + c := cache.New() fs := cache.NewOverlayFS(c) _ = fs.Update(t.Context(), files) - view := cache.NewView(uri.File(folder), fs, nil) + view := cache.NewView(uri.File(folder), fs, nil, options.Patch{}) return cache.NewSnapshot(view, nil) } diff --git a/lsp/source/index_test.go b/lsp/source/index_test.go index 0d4cb9f..68c9036 100644 --- a/lsp/source/index_test.go +++ b/lsp/source/index_test.go @@ -1,7 +1,6 @@ package source import ( - "context" "testing" "github.com/stretchr/testify/assert" @@ -12,9 +11,8 @@ import ( "github.com/karitham/thrift-ls/syntax" ) -var ctx = context.Background() - func TestIndex_ResolveType_SameFile(t *testing.T) { + ctx := t.Context() ss := snap(t, "/t.thrift", "struct Foo {}\ntypedef i32 Age") from := parseOne(t, ss, fu("/t.thrift")) @@ -35,6 +33,7 @@ func TestIndex_ResolveType_SameFile(t *testing.T) { } func TestIndex_ResolveType_IncludeChain(t *testing.T) { + ctx := t.Context() ss := crossSnap(t, "/a.thrift", `include "b.thrift" struct Foo { 1: b.Bar bar, }`, "/b.thrift", "struct Bar {}") a := parseOne(t, ss, fu("/a.thrift")) @@ -48,6 +47,7 @@ struct Foo { 1: b.Bar bar, }`, "/b.thrift", "struct Bar {}") } func TestIndex_ResolveValue(t *testing.T) { + ctx := t.Context() ss := crossSnap(t, "/a.thrift", `include "b.thrift" const i32 C = b.MAX`, "/b.thrift", "const i32 MAX = 10\nenum Color { RED }") a := parseOne(t, ss, fu("/a.thrift")) @@ -71,6 +71,7 @@ const i32 C = b.MAX`, "/b.thrift", "const i32 MAX = 10\nenum Color { RED }") } func TestIndex_ResolveService(t *testing.T) { + ctx := t.Context() ss := snap(t, "/t.thrift", "service Base {}") a := parseOne(t, ss, fu("/t.thrift")) def, err := NewIndex(ss).ResolveService(ctx, a, &syntax.Identifier{Text: "Base"}) @@ -80,6 +81,7 @@ func TestIndex_ResolveService(t *testing.T) { } func TestIndex_References_Type(t *testing.T) { + ctx := t.Context() ss := snap(t, "/t.thrift", "struct User {}\nstruct Foo { 1: User user, 2: list users, }\nservice Svc { User get(1: i32 id); }") _ = parseOne(t, ss, fu("/t.thrift")) @@ -89,6 +91,7 @@ func TestIndex_References_Type(t *testing.T) { } func TestIndex_References_ExceptionRule(t *testing.T) { + ctx := t.Context() ss := snap(t, "/t.thrift", "exception Bad {}\nstruct Foo { 1: Bad bad, }\nservice Svc { void f() throws (1: Bad e); }") _ = parseOne(t, ss, fu("/t.thrift")) @@ -99,6 +102,7 @@ func TestIndex_References_ExceptionRule(t *testing.T) { } func TestIndex_References_ConstValue(t *testing.T) { + ctx := t.Context() ss := snap(t, "/t.thrift", "const i32 MAX = 10\nstruct Foo { 1: i32 id = MAX, }") _ = parseOne(t, ss, fu("/t.thrift")) @@ -108,6 +112,7 @@ func TestIndex_References_ConstValue(t *testing.T) { } func TestIndex_QualifiedValues(t *testing.T) { + ctx := t.Context() ss := snap(t, "/t.thrift", "enum Color { RED = 0, BLUE = 1 }\nstruct Foo { 1: i32 id = Color.RED, }\nconst i32 C = Color.BLUE") _ = parseOne(t, ss, fu("/t.thrift")) @@ -139,6 +144,7 @@ func TestIndex_FindInWorkspace(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := t.Context() ss := crossSnap(t, "/a.thrift", "struct User {}", tt.file, "struct Account {}") _ = parseOne(t, ss, fu("/a.thrift")) _ = parseOne(t, ss, fu(tt.file)) @@ -183,7 +189,7 @@ func fu(p string) uri.URI { u, _ := uri.Parse("file://" + p); return u } func parseOne(t *testing.T, ss *cache.Snapshot, u uri.URI) *cache.ParsedFile { t.Helper() - pf, err := ss.Parse(context.Background(), u) + pf, err := ss.Parse(t.Context(), u) require.NoError(t, err) return pf } diff --git a/lsp/source/workspace_test.go b/lsp/source/workspace_test.go index 97856dd..2f73125 100644 --- a/lsp/source/workspace_test.go +++ b/lsp/source/workspace_test.go @@ -13,6 +13,7 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" ) // writeTree writes the file contents under dir and returns the directory. @@ -36,7 +37,7 @@ func openTree(t *testing.T, session *cache.Session, dir string, only map[string] t.Helper() folder := uri.File(dir) - view := session.AddView(folder) + view := session.AddView(folder, nil, options.Patch{}) require.NoError(t, filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { if err != nil || d.IsDir() || filepath.Ext(path) != ".thrift" { @@ -195,7 +196,7 @@ struct C { 1: string x }`, t.Run(tt.name, func(t *testing.T) { dir := writeTree(t, tt.files) - session := cache.NewSession(cache.New(nil)) + session := cache.NewSession(cache.New()) if tt.nested { // Each top-level directory is a workspace folder. @@ -244,7 +245,7 @@ const i32 DEFAULT_HP = 100, typedef string PilotName`, }) - session := cache.NewSession(cache.New(nil)) + session := cache.NewSession(cache.New()) openTree(t, session, dir, nil) file := uri.File(filepath.Join(dir, "shapes.thrift")) @@ -302,7 +303,7 @@ service Federation { }`, }) - session := cache.NewSession(cache.New(nil)) + session := cache.NewSession(cache.New()) openTree(t, session, dir, nil) tests := []struct { @@ -356,7 +357,7 @@ exception BayFull { }`, }) - session := cache.NewSession(cache.New(nil)) + session := cache.NewSession(cache.New()) openTree(t, session, dir, nil) syms := allWorkspaceSymbols(t.Context(), session, "", 0) diff --git a/lsp/stream.go b/lsp/stream.go index 6233d8b..63bdcf3 100644 --- a/lsp/stream.go +++ b/lsp/stream.go @@ -12,28 +12,33 @@ import ( type StreamServer struct { cache *cache.Cache - config options.Patch + config *Options } -// Options configures the stream server. Config is the base configuration — -// defaults overlaid with the config file and CLI flags — which workspace -// settings from the client overlay at initialize time. +// Options configures the stream server. Config is the startup +// configuration (defaults + config file + CLI). When ConfigPath pins an +// explicit file every view uses Config; otherwise each view resolves its +// own config from its workspace folder at creation, with CLI overlaid. type Options struct { - IncludePaths []string - Config options.Patch + Config options.Patch + // ConfigPath pins an explicit --config file, skipping per-folder + // discovery. + ConfigPath string + // CLI is the CLI-only overlay, applied on top of every view's config. + CLI options.Patch } func NewStreamServer(opts *Options) *StreamServer { return &StreamServer{ - cache: cache.New(opts.IncludePaths), - config: opts.Config, + cache: cache.New(), + config: opts, } } func (s *StreamServer) ServeStream(ctx context.Context, conn jsonrpc2.Conn) error { client := protocol.ClientDispatcher(conn) - server := NewServer(s.cache, client, s.config) + server := NewServer(s.cache, client, *s.config) // Clients may or may not send a shutdown message. Make sure the server is // shut down. defer func() { diff --git a/main.go b/main.go index 7a4168c..7a7c0e4 100644 --- a/main.go +++ b/main.go @@ -15,7 +15,6 @@ import ( "github.com/karitham/thrift-ls/doc" "github.com/karitham/thrift-ls/formatter" - tlog "github.com/karitham/thrift-ls/log" "github.com/karitham/thrift-ls/lsp" "github.com/karitham/thrift-ls/lsp/cache" "github.com/karitham/thrift-ls/lsp/source" @@ -191,17 +190,18 @@ func lspAction(ctx context.Context, cmd *cli.Command) error { logLevelValue = *patch.LogLevel } - tlog.Init(logLevelValue) + lsp.InitLogger(logLevelValue) - // Validate the effective configuration early; the server re-resolves - // it per request, and workspace settings overlay it at initialize time. + // Validate early: a broken --config or working-directory config must + // fail before serving; per-folder configs are re-resolved later. if _, err := patch.Formatter(); err != nil { return err } lspOpts := &lsp.Options{ - IncludePaths: derefStrings(patch.IncludePaths), - Config: patch, + Config: patch, + ConfigPath: cmd.String("config"), + CLI: cli, } ss := lsp.NewStreamServer(lspOpts) @@ -355,9 +355,9 @@ func checkAction(ctx context.Context, cmd *cli.Command) error { // 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) (map[string][]protocol.Diagnostic, error) { - c := cache.New(includePaths) + c := cache.New() sess := cache.NewSession(c) - sess.AddView(uri.File(folder)) + sess.AddView(uri.File(folder), includePaths, options.Patch{}) changes := make([]*cache.FileChange, 0, len(files)) uris := make([]uri.URI, 0, len(files)) diff --git a/vscode/README.md b/vscode/README.md index db1bc73..4d1c5f2 100644 --- a/vscode/README.md +++ b/vscode/README.md @@ -51,11 +51,14 @@ The formatter options (`printWidth`, `indent`, `tabWidth`, `align`, `separators.*`, `break.*`) are exposed as `thrift-ls.*` settings. They are sent to the server on startup and re-sent via `didChangeConfiguration` when they change, so formatting picks up new values without a restart. Settings -override the `thrift-ls.json` config file; CLI flags passed to the server -still win over both. +override the `thrift-ls.json` config file — discovered from the workspace +root, one config per folder — and the CLI flags passed to the server are +applied underneath the settings. `includePaths` and `logLevel` are not exposed as settings — use `thrift-ls.json` or the `-I` / `-logLevel` flags when launching the server. +Server logs go to the "Thrift Language Server" output channel (via +`window/logMessage`) and to `$TMPDIR/thrift-ls.log`. ## Development