diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 744ee47..24e36a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,8 +40,9 @@ jobs: name: thrift-ls-${{ matrix.goos }}-${{ matrix.goarch }} path: dist/ - # Test every package: unit, golden (cli_test.go + tests/e2e fixtures), - # corpus (check_test.go + tests/made-in-abyss), and fuzz regression. + # Test every package: unit, golden (formatter/file_test.go + + # tests/e2e fixtures), corpus (check_test.go + tests/made-in-abyss), + # and fuzz regression. test: runs-on: ubuntu-latest steps: diff --git a/README.md b/README.md index ade806f..ed68e78 100644 --- a/README.md +++ b/README.md @@ -455,9 +455,10 @@ git tag v0.1.0 && git push origin v0.1.0 go test ./... # unit, golden, and fuzz regression tests ``` -The golden CLI tests under `cli_test.go` pin the formatter flag combinations -(and the `check` lint corpus under `tests/made-in-abyss`) against committed -outputs in `tests/e2e`. The formatter is fuzz-tested end to end: +The formatter golden tests in `formatter/file_test.go` pin the formatter +options against committed outputs in `tests/e2e`. The `check` lint corpus +test in `check_test.go` covers `tests/made-in-abyss`. The formatter is +fuzz-tested end to end: `FuzzFormat` checks that any clean document formats without errors, keeps every comment, is idempotent and deterministic across the whole option space. The lexer, parser, doc printer, LSP offset mapper, and range diff --git a/check_test.go b/check_test.go index 40ec695..2c28dc4 100644 --- a/check_test.go +++ b/check_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "go.lsp.dev/protocol" + "github.com/karitham/thrift-ls/options" "github.com/karitham/thrift-ls/sema" ) @@ -164,24 +165,28 @@ func diagsOnLine(diags []protocol.Diagnostic, line uint32) []protocol.Diagnostic } // Test_Check_LintConfig pins that thrift-ls.json lint settings reach the -// check pipeline: a disabled analyzer produces no diagnostics and exits 0, -// where it would otherwise warn and exit 1. +// 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(filepath.Join(folder, "user.thrift"), []byte(content), 0o644)) + require.NoError(t, os.WriteFile(file, []byte(content), 0o644)) + t.Setenv("THRIFT_LS_CONFIG", "") - // The unused include is a warning: the check fails on it only via - // the config downgrade. Disable the analyzer entirely. config := `{"lint": {"disabled": ["UnusedIncludeCheck"]}}` - require.NoError(t, os.WriteFile(filepath.Join(folder, "thrift-ls.json"), []byte(config), 0o644)) + configPath := filepath.Join(folder, "thrift-ls.json") + require.NoError(t, os.WriteFile(configPath, []byte(config), 0o644)) - stdout, _, err := runCLI(t, "check", "--config", filepath.Join(folder, "thrift-ls.json"), filepath.Join(folder, "user.thrift")) + patch, err := loadConfig(configPath, folder) require.NoError(t, err) - assert.Empty(t, strings.TrimSpace(stdout)) + + 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. - stdout, _, err = runCLI(t, "check", filepath.Join(folder, "user.thrift")) + diags, err = checkFiles(t.Context(), []string{file}, folder, nil, sema.Config{}) require.NoError(t, err) - assert.Contains(t, stdout, "unused include") + assert.True(t, hasMessage(diags[file], "unused include")) } diff --git a/cli_test.go b/cli_test.go deleted file mode 100644 index d69bf66..0000000 --- a/cli_test.go +++ /dev/null @@ -1,178 +0,0 @@ -package main - -import ( - "bytes" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// runCLI executes the root command with args, capturing its writers. -func runCLI(t *testing.T, args ...string) (string, string, error) { - t.Helper() - - cmd := rootCommand() - var out, errOut bytes.Buffer - cmd.Writer = &out - cmd.ErrWriter = &errOut - - err := cmd.Run(t.Context(), append([]string{"thrift-ls"}, args...)) - - return out.String(), errOut.String(), err -} - -// Test_FormatCli_GoldenFields pins the indent and align combinations on a -// struct body against the golden outputs in tests/e2e/fields. -func Test_FormatCli_GoldenFields(t *testing.T) { - cases := []struct { - name, indent, align string - }{ - {"2spaces.assign", " ", "assign"}, - {"2spaces.disable", " ", "disable"}, - {"4spaces.field", " ", "field"}, - {"tab.assign", "\t", "assign"}, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - stdout, stderr, err := runCLI(t, "format", "-indent", c.indent, "-align", c.align, "tests/e2e/fields/fields.thrift") - require.NoError(t, err) - assert.Empty(t, stderr) - assert.Equal(t, readGolden(t, "tests/e2e/fields/"+c.name+".expect"), stdout) - }) - } -} - -// Test_FormatCli_GoldenAnnotations runs the formatter over the structured -// annotation fixture (tests/e2e/annotations), which exercises @Name -// annotations on definitions, fields, functions, and throws -// entries across value forms (map, list, parenthesized scalar). -func Test_FormatCli_GoldenAnnotations(t *testing.T) { - cases := []struct { - name string - args []string - }{ - {"annotations", nil}, - {"annotations.comma", []string{"-struct-separator", "comma"}}, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - args := append([]string{"format"}, c.args...) - args = append(args, "tests/e2e/annotations/annotations.thrift") - - stdout, stderr, err := runCLI(t, args...) - require.NoError(t, err) - assert.Empty(t, stderr) - assert.Equal(t, readGolden(t, "tests/e2e/annotations/"+c.name+".expect"), stdout) - }) - } -} - -// Test_FormatCli_GoldenFieldSeparators runs the struct separator modes on -// the comma-less fields fixture. The golden names keep the flag values of -// the CLI they were recorded against: add, remove, disable. -func Test_FormatCli_GoldenFieldSeparators(t *testing.T) { - cases := []struct { - name, sep string - }{ - {"add", "comma"}, - {"remove", "none"}, - {"disable", "preserve"}, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - stdout, stderr, err := runCLI(t, "format", "-struct-separator", c.sep, "tests/e2e/field_line_comma/fields.thrift") - require.NoError(t, err) - assert.Empty(t, stderr) - assert.Equal(t, readGolden(t, "tests/e2e/field_line_comma/"+c.name+".expect"), stdout) - }) - } -} - -// Test_FormatCli_GoldenEnums covers indent, align, and enum separator -// combinations on tests/e2e/enums. -func Test_FormatCli_GoldenEnums(t *testing.T) { - cases := []struct { - name, indent, align, sep string - }{ - {"2spaces.assign.add", " ", "assign", "comma"}, - {"2spaces.disable.remove", " ", "disable", "none"}, - {"4spaces.field.disable", " ", "field", "preserve"}, - {"tab.assign.disable", "\t", "assign", "preserve"}, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - stdout, stderr, err := runCLI(t, "format", - "-indent", c.indent, "-align", c.align, "-enum-separator", c.sep, - "tests/e2e/enums/enums.thrift") - require.NoError(t, err) - assert.Empty(t, stderr) - assert.Equal(t, readGolden(t, "tests/e2e/enums/"+c.name+".expect"), stdout) - }) - } -} - -// Test_CheckCLI_MadeInAbyss runs the check command over the corpus and -// pins the diagnostic counts, including the per-file breakdown and the -// exit code gating on error-severity diagnostics. -func Test_CheckCLI_MadeInAbyss(t *testing.T) { - stdout, stderr, err := runCLI(t, "check", "tests/made-in-abyss") - require.Error(t, err) - assert.Empty(t, stderr) - - assert.Contains(t, stdout, "lints.thrift:") - assert.Contains(t, stdout, "cycle_a.thrift:") - assert.Contains(t, stdout, "cycle_b.thrift:") - - errCount := strings.Count(stdout, " error ") - warnCount := strings.Count(stdout, " warning ") - assert.Equal(t, 19, errCount, "error diagnostics") - assert.Equal(t, 9, warnCount, "warning diagnostics (6 lints + 3 cycles)") - - assert.Contains(t, err.Error(), "19 error(s)") - assert.Contains(t, err.Error(), "9 warning(s)") -} - -// Test_DumpIncludesCLI runs dump --includes over a file whose include -// matches two sibling include roots, and checks the report names both. -func Test_DumpIncludesCLI(t *testing.T) { - folder := t.TempDir() - - content := "include \"recipes/stew.thrift\"\nstruct Party { 1: i32 members }\n" - stew := "struct Monster {}\n" - - require.NoError(t, os.MkdirAll(filepath.Join(folder, "laios", "kitchen", "recipes"), 0o755)) - require.NoError(t, os.MkdirAll(filepath.Join(folder, "senshi", "kitchen", "recipes"), 0o755)) - require.NoError(t, os.MkdirAll(filepath.Join(folder, "camp"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(folder, "laios", "kitchen", "recipes", "stew.thrift"), []byte(stew), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(folder, "senshi", "kitchen", "recipes", "stew.thrift"), []byte(stew), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(folder, "camp", "main.thrift"), []byte(content), 0o644)) - - configPath := filepath.Join(folder, "thrift-ls.json") - require.NoError(t, os.WriteFile(configPath, []byte(`{"includePaths": ["laios/kitchen", "senshi/kitchen"]}`), 0o644)) - - stdout, stderr, err := runCLI(t, "dump", "--includes", "--config", configPath, filepath.Join(folder, "camp", "main.thrift")) - require.NoError(t, err) - assert.Empty(t, stderr) - - assert.Contains(t, stdout, "recipes/stew.thrift") - assert.Contains(t, stdout, filepath.Join(folder, "senshi", "kitchen", "recipes", "stew.thrift")) - assert.Contains(t, stdout, filepath.Join(folder, "laios", "kitchen", "recipes", "stew.thrift")) -} - -// readGolden returns the recorded output of a CLI test case. -func readGolden(t *testing.T, path string) string { - t.Helper() - - b, err := os.ReadFile(filepath.FromSlash(path)) - require.NoError(t, err) - - return string(b) -} diff --git a/flake.lock b/flake.lock index 17ab962..a7863d4 100644 --- a/flake.lock +++ b/flake.lock @@ -2,15 +2,16 @@ "nodes": { "nixpkgs": { "locked": { - "lastModified": 1787437750, - "narHash": "sha256-8qnR53JToWl9xm6MWgbmmOzh4rsQiQCwH4KukHytNkk=", + "lastModified": 1787964612, + "narHash": "sha256-0N9nghg3nwzX6b6qc77EzjR9cu/Z+UR66FlfsCqiURs=", "owner": "nixos", "repo": "nixpkgs", - "rev": "e990d4ae42fb93cf7919c654225117ddc6bef1b4", + "rev": "e8be7818e19ada32105a8af937a6a473b38167ca", "type": "github" }, "original": { "owner": "nixos", + "ref": "nixpkgs-unstable", "repo": "nixpkgs", "type": "github" } diff --git a/flake.nix b/flake.nix index b4d7d44..3ceac66 100644 --- a/flake.nix +++ b/flake.nix @@ -1,7 +1,7 @@ { description = "thrift-ls: a Thrift language server and formatter"; inputs = { - nixpkgs.url = "github:nixos/nixpkgs"; + nixpkgs.url = "github:nixos/nixpkgs/nixpkgs-unstable"; }; outputs = { self, nixpkgs }: @@ -14,7 +14,7 @@ let version = "0.1.8"; in - pkgs.buildGo127Module { + pkgs.buildGoModule { pname = "thrift-ls"; inherit version; src = nixpkgs.lib.cleanSource ./.; @@ -62,10 +62,10 @@ packages = with pkgs; [ - go_1_27 + go treefmt golangci-lint - nodejs_22 + nodejs ] ++ formatterTools pkgs; }; diff --git a/diff.go b/formatter/diff.go similarity index 98% rename from diff.go rename to formatter/diff.go index 44ba4bf..2be4b4c 100644 --- a/diff.go +++ b/formatter/diff.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package main +package formatter import ( "bytes" @@ -15,7 +15,7 @@ import ( // It is typically a pair of line indexes. type pair struct{ x, y int } -// Diff returns an anchored diff of the two texts old and new +// diff returns an anchored diff of the two texts old and new // in the “unified diff” format. If old and new are identical, // Diff returns a nil slice (no output). // @@ -43,7 +43,7 @@ type pair struct{ x, y int } // Second, the name is frequently interpreted as meaning that you have // to wait longer (to be patient) for the diff, meaning that it is a slower algorithm, // when in fact the algorithm is faster than the standard one. -func Diff(oldName string, old []byte, newName string, new []byte) []byte { +func diff(oldName string, old []byte, newName string, new []byte) []byte { if bytes.Equal(old, new) { return nil } diff --git a/formatter/file.go b/formatter/file.go new file mode 100644 index 0000000..9c1e50f --- /dev/null +++ b/formatter/file.go @@ -0,0 +1,117 @@ +package formatter + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/karitham/thrift-ls/syntax" +) + +// FileOptions controls file-level formatting. Output is required unless Write +// is true. ResolveConfig receives ConfigPath and the file's absolute directory; +// a non-empty ConfigPath requires a resolver. When ConfigPath is empty, a +// resolver may discover a config. Patch overlays the resolved config, or the +// default formatting config when ResolveConfig is nil. +type FileOptions struct { + Output io.Writer + Write bool + Diff bool + ConfigPath string + Patch FormatPatch + ResolveConfig func(path, dir string) (FormatPatch, error) +} + +// FormatFile reads and formats file. It writes formatted output to Output by +// default, overwrites file when Write is true, and writes a unified diff when +// Diff is true. Write takes precedence over Diff. File permissions are +// preserved when overwriting an existing file. +func FormatFile(file string, opts FileOptions) error { + if file == "" { + return errors.New("must specify a thrift file to format, e.g. thrift-ls format file.thrift") + } + if !opts.Write && opts.Output == nil { + return errors.New("formatter output is required when Write is false") + } + if opts.ConfigPath != "" && opts.ResolveConfig == nil { + return errors.New("formatter ConfigPath requires ResolveConfig") + } + + src, err := os.ReadFile(file) + if err != nil { + return err + } + + absFile, err := filepath.Abs(file) + if err != nil { + return err + } + + patch := FormatPatch{} + if opts.ResolveConfig != nil { + patch, err = opts.ResolveConfig(opts.ConfigPath, filepath.Dir(absFile)) + if err != nil { + return err + } + } + + formatOptions, err := opts.Patch.Apply(patch).Options() + if err != nil { + return err + } + + parsed, errs := syntax.Parse(src) + if hasFileParseErrors(errs) { + return fmt.Errorf("%s: file does not parse:\n%s", file, formatErrors(errs)) + } + + formatted, err := Format(parsed, formatOptions) + if err != nil { + return fmt.Errorf("%s: %w", file, err) + } + + if _, errs := syntax.Parse([]byte(formatted)); hasFileParseErrors(errs) { + return fmt.Errorf("%s: formatting produced invalid output", file) + } + + switch { + case opts.Write: + permissions := os.FileMode(0o644) + if info, statErr := os.Stat(file); statErr == nil { + permissions = info.Mode() + } + + return os.WriteFile(file, []byte(formatted), permissions) + case opts.Diff: + _, err = opts.Output.Write(diff("old", src, "new", []byte(formatted))) + default: + _, err = io.WriteString(opts.Output, formatted) + } + + return err +} + +func hasFileParseErrors(errs []syntax.Error) bool { + for _, err := range errs { + if err.Severity == syntax.SeverityError { + return true + } + } + + return false +} + +func formatErrors(errs []syntax.Error) string { + var output strings.Builder + + for _, err := range errs { + if err.Severity == syntax.SeverityError { + fmt.Fprintf(&output, " %s\n", err) + } + } + + return output.String() +} diff --git a/formatter/file_test.go b/formatter/file_test.go new file mode 100644 index 0000000..b196561 --- /dev/null +++ b/formatter/file_test.go @@ -0,0 +1,347 @@ +package formatter_test + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/karitham/thrift-ls/formatter" + "github.com/karitham/thrift-ls/options" +) + +func resolveConfig(path, dir string) (formatter.FormatPatch, error) { + if path == "" { + var err error + + path, err = options.FindConfig(dir) + if err != nil { + return formatter.FormatPatch{}, err + } + } + + if path == "" { + return formatter.DefaultFormatPatch(), nil + } + + cfg, err := options.Load(path) + if err != nil { + return formatter.FormatPatch{}, err + } + + return options.Effective(cfg).FormatPatch, nil +} + +func TestFormatOutputModes(t *testing.T) { + tests := []struct { + name string + write bool + diff bool + wantOutput string + wantFile string + }{ + { + name: "formatted output", + wantOutput: "struct API { 1: i32 id }\n", + wantFile: "struct API{1:i32 id}", + }, + { + name: "write in place", + write: true, + wantFile: "struct API { 1: i32 id }\n", + }, + { + name: "diff output", + diff: true, + wantOutput: "diff old new\n--- old\n+++ new\n@@ -1,1 +1,1 @@\n-struct API{1:i32 id}\n\\ No newline at end of file\n+struct API { 1: i32 id }\n", + wantFile: "struct API{1:i32 id}", + }, + { + name: "write takes precedence over diff", + write: true, + diff: true, + wantFile: "struct API { 1: i32 id }\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + file := filepath.Join(t.TempDir(), "api.thrift") + require.NoError(t, os.WriteFile(file, []byte("struct API{1:i32 id}"), 0o640)) + + var output bytes.Buffer + err := formatter.FormatFile(file, formatter.FileOptions{ + Output: &output, + Write: tt.write, + Diff: tt.diff, + ResolveConfig: resolveConfig, + }) + require.NoError(t, err) + assert.Equal(t, tt.wantOutput, output.String()) + + content, err := os.ReadFile(file) + require.NoError(t, err) + assert.Equal(t, tt.wantFile, string(content)) + + info, err := os.Stat(file) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o640), info.Mode().Perm()) + }) + } +} + +func TestFormatFileRequiresOutputUnlessWriting(t *testing.T) { + tests := []struct { + name string + diff bool + }{ + {name: "formatted output"}, + {name: "diff output", diff: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + file := filepath.Join(t.TempDir(), "api.thrift") + source := "struct API{1:i32 id}" + require.NoError(t, os.WriteFile(file, []byte(source), 0o640)) + + err := formatter.FormatFile(file, formatter.FileOptions{Diff: tt.diff}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "output") + content, readErr := os.ReadFile(file) + require.NoError(t, readErr) + assert.Equal(t, source, string(content)) + }) + } +} + +func TestFormatFileRequiresResolverForConfigPath(t *testing.T) { + file := filepath.Join(t.TempDir(), "api.thrift") + source := "struct API{1:i32 id}" + require.NoError(t, os.WriteFile(file, []byte(source), 0o640)) + + var output bytes.Buffer + err := formatter.FormatFile(file, formatter.FileOptions{ + Output: &output, + ConfigPath: "thrift-ls.json", + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "ConfigPath") + assert.Contains(t, err.Error(), "ResolveConfig") + assert.Empty(t, output.String()) + content, readErr := os.ReadFile(file) + require.NoError(t, readErr) + assert.Equal(t, source, string(content)) +} + +func TestFormatFileReturnsParseErrorsWithoutOutput(t *testing.T) { + file := filepath.Join(t.TempDir(), "api.thrift") + source := "struct API {" + require.NoError(t, os.WriteFile(file, []byte(source), 0o644)) + + var output bytes.Buffer + err := formatter.FormatFile(file, formatter.FileOptions{Output: &output, Write: true}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "file does not parse") + assert.Empty(t, output.String()) + + content, readErr := os.ReadFile(file) + require.NoError(t, readErr) + assert.Equal(t, source, string(content)) +} + +func TestFormatAppliesConfigAndPatch(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "api.thrift") + config := filepath.Join(dir, options.ConfigFileName) + require.NoError(t, os.WriteFile(file, []byte("struct API { 1: i32 id }"), 0o644)) + require.NoError(t, os.WriteFile(config, []byte(`{"printWidth": 10}`), 0o644)) + + width := 80 + tests := []struct { + name string + patch formatter.FormatPatch + want string + }{ + { + name: "discovered config", + want: "struct API {\n 1: i32 id\n}\n", + }, + { + name: "patch overrides config", + patch: formatter.FormatPatch{PrintWidth: &width}, + want: "struct API { 1: i32 id }\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var output bytes.Buffer + err := formatter.FormatFile(file, formatter.FileOptions{ + Output: &output, + Patch: tt.patch, + ResolveConfig: resolveConfig, + }) + require.NoError(t, err) + assert.Equal(t, tt.want, output.String()) + }) + } +} + +func TestFormatFileReturnsConfigErrors(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "api.thrift") + require.NoError(t, os.WriteFile(file, []byte("struct API {}\n"), 0o644)) + + tests := []struct { + name string + configPath string + config string + want string + }{ + { + name: "missing config", + configPath: filepath.Join(dir, "missing.json"), + want: "missing.json", + }, + { + name: "malformed config", + configPath: filepath.Join(dir, "malformed.json"), + config: `{ "printWidth": "wide" }`, + want: "malformed.json", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.config != "" { + require.NoError(t, os.WriteFile(tt.configPath, []byte(tt.config), 0o644)) + } + + var output bytes.Buffer + err := formatter.FormatFile(file, formatter.FileOptions{ + Output: &output, + ConfigPath: tt.configPath, + ResolveConfig: resolveConfig, + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), tt.want) + assert.Empty(t, output.String()) + }) + } +} + +func TestFormatFileGoldenFields(t *testing.T) { + tests := []struct { + name string + indent string + align string + }{ + {name: "2spaces.assign", indent: " ", align: "assign"}, + {name: "2spaces.disable", indent: " ", align: "disable"}, + {name: "4spaces.field", indent: " ", align: "field"}, + {name: "tab.assign", indent: "\t", align: "assign"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + indent, err := formatter.ParseIndentValue(tt.indent) + require.NoError(t, err) + + assertGoldenFile(t, "fields", "fields.thrift", tt.name, formatter.FormatPatch{ + Indent: &indent, + Align: &tt.align, + }) + }) + } +} + +func TestFormatFileGoldenAnnotations(t *testing.T) { + comma := "comma" + tests := []struct { + name string + patch formatter.FormatPatch + }{ + {name: "annotations"}, + { + name: "annotations.comma", + patch: formatter.FormatPatch{ + Separators: &formatter.Separators{Structs: &comma}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertGoldenFile(t, "annotations", "annotations.thrift", tt.name, tt.patch) + }) + } +} + +func TestFormatFileGoldenFieldSeparators(t *testing.T) { + tests := []struct { + name string + separator string + }{ + {name: "add", separator: "comma"}, + {name: "remove", separator: "none"}, + {name: "disable", separator: "preserve"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertGoldenFile(t, "field_line_comma", "fields.thrift", tt.name, formatter.FormatPatch{ + Separators: &formatter.Separators{Structs: &tt.separator}, + }) + }) + } +} + +func TestFormatFileGoldenEnums(t *testing.T) { + tests := []struct { + name string + indent string + align string + separator string + }{ + {name: "2spaces.assign.add", indent: " ", align: "assign", separator: "comma"}, + {name: "2spaces.disable.remove", indent: " ", align: "disable", separator: "none"}, + {name: "4spaces.field.disable", indent: " ", align: "field", separator: "preserve"}, + {name: "tab.assign.disable", indent: "\t", align: "assign", separator: "preserve"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + indent, err := formatter.ParseIndentValue(tt.indent) + require.NoError(t, err) + + assertGoldenFile(t, "enums", "enums.thrift", tt.name, formatter.FormatPatch{ + Indent: &indent, + Align: &tt.align, + Separators: &formatter.Separators{Enums: &tt.separator}, + }) + }) + } +} + +func assertGoldenFile(t *testing.T, fixture, source, golden string, patch formatter.FormatPatch) { + t.Helper() + + var output bytes.Buffer + err := formatter.FormatFile(filepath.Join("..", "tests", "e2e", fixture, source), formatter.FileOptions{ + Output: &output, + Patch: patch, + }) + require.NoError(t, err) + + want, err := os.ReadFile(filepath.Join("..", "tests", "e2e", fixture, golden+".expect")) + require.NoError(t, err) + assert.Equal(t, string(want), output.String()) +} diff --git a/formatter/format.go b/formatter/format.go index 8a07223..3bcdeb7 100644 --- a/formatter/format.go +++ b/formatter/format.go @@ -1,7 +1,6 @@ -// Package formatter turns a parsed thrift document into a doc IR document -// and renders it. It is the pure core of the formatting pipeline: parsing -// and file I/O happen in the caller (CLI, LSP), and the formatter never -// touches the filesystem. +// Package formatter formats parsed Thrift documents and files. Format is the +// pure formatting core used by callers such as the CLI and LSP; FormatFile +// owns the filesystem boundary for standalone file formatting. // // Layout decisions are width-driven: every construct is a group that stays // on one line when it fits and breaks otherwise, with nested groups diff --git a/go.mod b/go.mod index 57a4121..172f840 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/karitham/thrift-ls -go 1.27 +go 1.26 require ( github.com/stretchr/testify v1.11.1 diff --git a/lsp/analyzer_options_test.go b/lsp/analyzer_options_test.go new file mode 100644 index 0000000..12907a3 --- /dev/null +++ b/lsp/analyzer_options_test.go @@ -0,0 +1,211 @@ +package lsp + +import ( + "context" + "fmt" + "sync/atomic" + "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/sema" + "github.com/karitham/thrift-ls/syntax" +) + +type optionAnalyzer struct{} + +func (optionAnalyzer) Name() string { return "option-analyzer" } + +func (optionAnalyzer) Analyze(ctx context.Context, run *sema.Run) error { + pos := syntax.Position{Line: 1, Col: 1, Offset: 0} + + for _, file := range run.Files() { + run.Add(file, sema.Diagnostic{ + Code: "option-diagnostic", + Severity: sema.SeverityError, + Message: "reported by option analyzer", + Span: sema.Span{Start: pos, End: pos}, + Fixes: []sema.Fix{{ + Title: "Apply option analyzer fix", + Edits: []sema.Edit{{Span: sema.Span{Start: pos, End: pos}, NewText: "// fixed\n"}}, + }}, + }) + } + + return nil +} + +func TestOptionsAnalyzersExtendDiagnosticsAndCodeActions(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + file := uri.File("/workspace/api.thrift") + srv := NewServer(cache.NewMemFS(nil), nil, Options{ + Analyzers: []sema.Analyzer{optionAnalyzer{}}, + }) + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: file, + LanguageID: LanguageIDThrift, + Text: "enum E { A, B = 1 }\n", + }, + })) + synctest.Wait() + + report := srv.reportFor(file) + require.NotNil(t, report) + + var codes []string + for _, diagnostic := range report[file] { + codes = append(codes, diagnostic.Code) + } + assert.Contains(t, codes, "option-diagnostic") + assert.Contains(t, codes, sema.CodeImplicitEnumValue, "built-in analyzers must remain enabled") + + actions, err := srv.codeAction(t.Context(), &protocol.CodeActionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: file}, + Range: protocol.Range{ + Start: protocol.Position{Line: 0, Character: 0}, + End: protocol.Position{Line: 0, Character: 0}, + }, + }) + require.NoError(t, err) + + titles := make([]string, 0, len(actions)) + for _, action := range actions { + codeAction, ok := action.(*protocol.CodeAction) + require.True(t, ok) + titles = append(titles, codeAction.Title) + } + assert.Contains(t, titles, "Apply option analyzer fix") + assert.Contains(t, titles, "Make enum values explicit", "built-in providers must remain enabled") + }) +} + +type statefulAnalyzer struct { + calls atomic.Int32 + active atomic.Int32 + max atomic.Int32 + started chan struct{} + release chan struct{} +} + +func (a *statefulAnalyzer) Name() string { return "stateful-analyzer" } + +func (a *statefulAnalyzer) Analyze(ctx context.Context, run *sema.Run) error { + call := a.calls.Add(1) + active := a.active.Add(1) + for { + max := a.max.Load() + if active <= max || a.max.CompareAndSwap(max, active) { + break + } + } + defer a.active.Add(-1) + + if call == 1 { + close(a.started) + <-a.release + } + + pos := syntax.Position{Line: 1, Col: 1, Offset: 0} + for _, file := range run.Files() { + run.Add(file, sema.Diagnostic{ + Code: "stateful-diagnostic", + Severity: sema.SeverityError, + Message: fmt.Sprintf("analysis run %d", call), + Span: sema.Span{Start: pos, End: pos}, + }) + } + + return nil +} + +func TestAnalyzerRunsAreSerializedAndStaleResultsAreDropped(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + file := uri.File("/workspace/api.thrift") + client := &diagClient{} + analyzer := &statefulAnalyzer{ + started: make(chan struct{}), + release: make(chan struct{}), + } + srv := NewServer(cache.NewMemFS(nil), client, Options{ + Analyzers: []sema.Analyzer{analyzer}, + }) + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: file, + LanguageID: LanguageIDThrift, + Text: "struct VersionOne {}", + }, + })) + <-analyzer.started + + require.NoError(t, srv.DidChange(t.Context(), &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{URI: file}, + Version: 1, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + &protocol.TextDocumentContentChangeWholeDocument{Text: "struct VersionTwo {}"}, + }, + })) + + close(analyzer.release) + synctest.Wait() + + assert.Equal(t, int32(1), analyzer.max.Load(), "the injected analyzer instance must not run concurrently") + messages := diagMessages(client.last(file)) + assert.Contains(t, messages, "analysis run 2") + assert.NotContains(t, messages, "analysis run 1", "a superseded analysis must not be published") + }) +} + +func TestRetiredViewDropsInFlightAnalysis(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folder := uri.File("/workspace") + root := uri.File("/workspace/project") + file := uri.File("/workspace/project/api.thrift") + client := &diagClient{} + analyzer := &statefulAnalyzer{ + started: make(chan struct{}), + release: make(chan struct{}), + } + loader := WorkspaceLoader(func(ctx context.Context, got uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project.json"), + RootURI: root, + TargetFiles: []uri.URI{file}, + }}}, nil + }) + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + file: []byte("struct VersionOne {}"), + }), client, Options{ + WorkspaceLoader: loader, + Analyzers: []sema.Analyzer{analyzer}, + ConfigPath: "pinned", + }) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: folder}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + <-analyzer.started + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folder}}, + }, + })) + close(analyzer.release) + synctest.Wait() + + assert.Empty(t, srv.session.Views()) + assert.Empty(t, client.last(file)) + assert.Nil(t, srv.reportFor(file)) + }) +} diff --git a/lsp/cache/session.go b/lsp/cache/session.go index 6afe5bf..46cae7e 100644 --- a/lsp/cache/session.go +++ b/lsp/cache/session.go @@ -33,8 +33,6 @@ func NewSession(fs FileSource) *Session { return sess } -// AddView registers a view for the workspace folder, returning the -// existing view when the folder is already tracked. includePaths and // AddView registers a view for the workspace folder, returning the // existing view when the folder is already tracked. includePaths is the // folder's resolved include configuration; the view fixes it at creation. @@ -50,13 +48,14 @@ func (s *Session) AddView(folder uri.URI, includePaths []string) *View { view := NewView(folder, s.overlayFS, includePaths) s.views = append(s.views, view) + clear(s.viewMap) return view } -// RemoveView drops the view for the workspace folder and forgets every -// cached file-to-view mapping that pointed at it, so ViewOf re-resolves -// against the remaining folders. +// RemoveView drops the view for the workspace folder, invalidates its +// asynchronous work, and forgets every cached file-to-view mapping that +// pointed at it, so ViewOf re-resolves against the remaining folders. func (s *Session) RemoveView(folder uri.URI) { s.viewMu.Lock() defer s.viewMu.Unlock() @@ -66,6 +65,7 @@ func (s *Session) RemoveView(folder uri.URI) { continue } + v.Evict(v.KnownFiles()...) s.views = append(s.views[:i], s.views[i+1:]...) for file, view := range s.viewMap { if view == v { @@ -97,13 +97,20 @@ func (s *Session) ViewOf(fileURI uri.URI) (*View, error) { return nil, fmt.Errorf("views is nil") } - for i := range s.views { - if s.views[i].ContainsFile(fileURI) { - s.viewMap[fileURI] = s.views[i] - - return s.views[i], nil + var best *View + for _, view := range s.views { + if !view.ContainsFile(fileURI) { + continue + } + if best == nil || len(view.folder.Path()) > len(best.folder.Path()) { + best = view } } + if best != nil { + s.viewMap[fileURI] = best + + return best, nil + } for i := range s.views { if s.views[i].FileKnown(fileURI) { diff --git a/lsp/cache/session_test.go b/lsp/cache/session_test.go index 21375ff..9873d78 100644 --- a/lsp/cache/session_test.go +++ b/lsp/cache/session_test.go @@ -118,3 +118,82 @@ func TestSessionRemoveViewForgetsMappings(t *testing.T) { require.NoError(t, err) assert.Equal(t, other, view.Folder()) } + +func TestSessionViewOf(t *testing.T) { + outer := uri.File("/workspace") + inner := uri.File("/workspace/service") + nestedFile := uri.File("/workspace/service/api.thrift") + externalFile := uri.File("/dependencies/shared.thrift") + + tests := []struct { + name string + setup func(*Session) + file uri.URI + want uri.URI + }{ + { + name: "most specific view added last", + setup: func(s *Session) { + s.AddView(outer, nil) + s.AddView(inner, nil) + }, + file: nestedFile, + want: inner, + }, + { + name: "most specific view added first", + setup: func(s *Session) { + s.AddView(inner, nil) + s.AddView(outer, nil) + }, + file: nestedFile, + want: inner, + }, + { + name: "adding a specific view invalidates cached routing", + setup: func(s *Session) { + s.AddView(outer, nil) + + view, err := s.ViewOf(nestedFile) + require.NoError(t, err) + require.Equal(t, outer, view.Folder()) + + s.AddView(inner, nil) + }, + file: nestedFile, + want: inner, + }, + { + name: "known file outside roots", + setup: func(s *Session) { + s.AddView(outer, nil) + view := s.AddView(inner, nil) + view.Update(t.Context(), &FileChange{URI: externalFile, From: FileChangeTypeInitialize}) + }, + file: externalFile, + want: inner, + }, + { + name: "unknown file falls back to first view", + setup: func(s *Session) { + s.AddView(outer, nil) + s.AddView(inner, nil) + }, + file: uri.File("/elsewhere/unknown.thrift"), + want: outer, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := NewSession(NewMemFS(map[uri.URI][]byte{ + externalFile: []byte("struct Shared {}"), + })) + tt.setup(s) + + view, err := s.ViewOf(tt.file) + require.NoError(t, err) + assert.Equal(t, tt.want, view.Folder()) + }) + } +} diff --git a/lsp/cache/snapshot.go b/lsp/cache/snapshot.go index 8b07411..7c045aa 100644 --- a/lsp/cache/snapshot.go +++ b/lsp/cache/snapshot.go @@ -5,6 +5,7 @@ import ( "context" "io/fs" "log/slog" + "slices" "strings" "time" @@ -31,7 +32,7 @@ func newResolver(includePaths []string, src FileSource) *Resolver { // IncludePaths returns the include paths configured for this resolver. func (r *Resolver) IncludePaths() []string { - return r.includePaths + return slices.Clone(r.includePaths) } // ResolveInclude resolves an include path to a file URI. diff --git a/lsp/cache/view.go b/lsp/cache/view.go index cc48c90..8320e15 100644 --- a/lsp/cache/view.go +++ b/lsp/cache/view.go @@ -25,9 +25,9 @@ type viewEntry struct { // graph between them, and the include configuration that applies to them. // // Concurrency: entries and edges are guarded by mu; reads share immutable -// values, writes replace entries wholesale. gen bumps on every Update; -// asynchronous work compares its captured generation against View.IsCurrent -// to drop superseded results. +// values, writes replace entries wholesale. gen bumps on every Update or +// Evict; asynchronous work compares its captured generation against +// View.IsCurrent to drop superseded results. type View struct { // folder is the tree root: a workspace folder, or the opened file's // directory in single-file mode. @@ -45,11 +45,25 @@ type View struct { gen atomic.Uint64 } +type generationContextKey struct{} + +// WithGeneration marks ctx as analysis work for generation. Parses performed +// with the context are not cached after the view advances past that generation. +func WithGeneration(ctx context.Context, generation uint64) context.Context { + return context.WithValue(ctx, generationContextKey{}, generation) +} + +func generationOf(ctx context.Context) (uint64, bool) { + generation, ok := ctx.Value(generationContextKey{}).(uint64) + + return generation, ok +} + func NewView(folder uri.URI, fs FileSource, includePaths []string) *View { return &View{ folder: folder, fs: fs, - includePaths: includePaths, + includePaths: slices.Clone(includePaths), entries: make(map[uri.URI]*viewEntry), includes: make(map[uri.URI][]uri.URI), includers: make(map[uri.URI][]uri.URI), @@ -100,6 +114,9 @@ func (v *View) Parse(ctx context.Context, u uri.URI) (*ParsedFile, error) { return pf, nil } + generation, guarded := generationOf(ctx) + parseGeneration := v.gen.Load() + fh, err := v.ReadFile(ctx, u) if err != nil { return nil, err @@ -117,7 +134,17 @@ func (v *View) Parse(ctx context.Context, u uri.URI) (*ParsedFile, error) { includes = resolveIncludes(u, pf.AST().Includes(), v.Resolver().ResolveInclude) } - v.setEntry(u, &viewEntry{fh: fh, parsed: pf}, includes) + v.mu.Lock() + if (!guarded || generation == parseGeneration) && v.gen.Load() == parseGeneration { + v.removeEdgesLocked(u) + v.entries[u] = &viewEntry{fh: fh, parsed: pf} + + for _, inc := range includes { + v.includes[u] = append(v.includes[u], inc) + v.includers[inc] = append(v.includers[inc], u) + } + } + v.mu.Unlock() return pf, nil } @@ -294,6 +321,43 @@ func (v *View) Generation() uint64 { return v.gen.Load() } +// Evict removes files from the view and advances its generation. Advancing the +// generation also invalidates asynchronous work that was started for the +// evicted entries. +func (v *View) Evict(files ...uri.URI) { + uris := make([]uri.URI, 0, len(files)) + for _, file := range files { + uris = append(uris, file) + } + + slices.Sort(uris) + uris = slices.Compact(uris) + + v.mu.Lock() + for _, file := range uris { + v.removeEdgesLocked(file) + for _, includer := range v.includers[file] { + includes := v.includes[includer] + for i, include := range slices.Backward(includes) { + if include != file { + continue + } + + includes = append(includes[:i], includes[i+1:]...) + } + if len(includes) == 0 { + delete(v.includes, includer) + } else { + v.includes[includer] = includes + } + } + delete(v.includers, file) + delete(v.entries, file) + } + v.gen.Add(1) + v.mu.Unlock() +} + // IsCurrent reports whether gen is still the view's latest generation. // Used by asynchronous work to drop results that a newer change superseded. func (v *View) IsCurrent(gen uint64) bool { @@ -334,6 +398,7 @@ func (v *View) Update(ctx context.Context, changes ...*FileChange) ChangeResult // if the parse below fails, so routing and KnownFiles see it. v.entries[u] = &viewEntry{} } + generation := v.gen.Add(1) v.mu.Unlock() // Parse is lazy and cached, so requests racing ahead of this loop @@ -346,7 +411,7 @@ func (v *View) Update(ctx context.Context, changes ...*FileChange) ChangeResult return ChangeResult{ Affected: v.affected(uris), - Gen: v.gen.Add(1), + Gen: generation, } } diff --git a/lsp/codeaction.go b/lsp/codeaction.go index 32a8160..f08788b 100644 --- a/lsp/codeaction.go +++ b/lsp/codeaction.go @@ -17,7 +17,7 @@ import ( // fixes a reported diagnostic is also offered as a quickfix. Actions are // filtered to the kinds the client requested. func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CommandOrCodeAction, error) { - return withFile(ctx, s.session, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) ([]protocol.CommandOrCodeAction, error) { + return withFile(ctx, s.viewOf, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) ([]protocol.CommandOrCodeAction, error) { pf, err := view.Parse(ctx, params.TextDocument.URI) if err != nil { return nil, err @@ -30,7 +30,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara report := s.reportFor(params.TextDocument.URI) - actions := sema.DefaultPipeline(s.lintConfig(view)). + actions := s.pipeline(view). CodeActions(ctx, view, params.TextDocument.URI, span, report) proto := make([]protocol.CodeAction, 0, len(actions)) diff --git a/lsp/codeaction_test.go b/lsp/codeaction_test.go index c5fef00..b6d8892 100644 --- a/lsp/codeaction_test.go +++ b/lsp/codeaction_test.go @@ -110,7 +110,7 @@ func Test_CodeAction(t *testing.T) { // Produce the report the server would publish, so code // actions pair with the server's own diagnostics. - _, err = withFile(ctx, srv.session, fileURI, func(view *cache.View, _ cache.FileHandle) (struct{}, error) { + _, err = withFile(ctx, srv.session.ViewOf, fileURI, func(view *cache.View, _ cache.FileHandle) (struct{}, error) { srv.diagnose(ctx, view, []uri.URI{fileURI}) return struct{}{}, nil }) @@ -184,7 +184,7 @@ func TestCodeActionAddMissingInclude(t *testing.T) { openDocument(t, srv, fileURI, content) // Produce the report the server would publish. - _, err := withFile(t.Context(), srv.session, fileURI, func(view *cache.View, _ cache.FileHandle) (struct{}, error) { + _, err := withFile(t.Context(), srv.session.ViewOf, fileURI, func(view *cache.View, _ cache.FileHandle) (struct{}, error) { srv.diagnose(t.Context(), view, []uri.URI{fileURI}) return struct{}{}, nil }) diff --git a/lsp/codejump.go b/lsp/codejump.go index d5db071..9d4e4e9 100644 --- a/lsp/codejump.go +++ b/lsp/codejump.go @@ -10,19 +10,19 @@ import ( ) func (s *Server) definition(ctx context.Context, params *protocol.DefinitionParams) (result []protocol.Location, err error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) ([]protocol.Location, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) ([]protocol.Location, error) { return source.Definition(ctx, view, params.TextDocument.URI, params.Position) }) } func (s *Server) references(ctx context.Context, params *protocol.ReferenceParams) (result []protocol.Location, err error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) ([]protocol.Location, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) ([]protocol.Location, error) { return source.Reference(ctx, view, params.TextDocument.URI, params.Position) }) } func (s *Server) typeDefinition(ctx context.Context, params *protocol.TypeDefinitionParams) (result []protocol.Location, err error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) ([]protocol.Location, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) ([]protocol.Location, error) { return source.TypeDefinition(ctx, view, params.TextDocument.URI, params.Position) }) } diff --git a/lsp/config_test.go b/lsp/config_test.go index 177f859..30c960d 100644 --- a/lsp/config_test.go +++ b/lsp/config_test.go @@ -52,10 +52,9 @@ func openAndFormat(t *testing.T, srv *Server, file string) string { func initWorkspace(t *testing.T, srv *Server, folders []uri.URI, initializationOptions []byte) { t.Helper() - _, err := srv.Initialize(t.Context(), &protocol.InitializeParams{ - WorkspaceFolders: protocol.NewNullable(foldersFromURIs(folders)), - InitializationOptions: protocol.LSPAny(initializationOptions), - }) + params := testInitializeParams(foldersFromURIs(folders)) + params.InitializationOptions = protocol.LSPAny(initializationOptions) + _, err := srv.Initialize(t.Context(), params) require.NoError(t, err) require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) diff --git a/lsp/diagnostic.go b/lsp/diagnostic.go index 638c8c2..5cf9516 100644 --- a/lsp/diagnostic.go +++ b/lsp/diagnostic.go @@ -4,6 +4,7 @@ import ( "context" "errors" "log/slog" + "slices" "go.lsp.dev/protocol" "go.lsp.dev/uri" @@ -48,23 +49,48 @@ func lintConfigOf(l options.LintConfig) sema.Config { return sema.ConfigFromLint(disabled, severity) } +func (s *Server) pipeline(view *cache.View) *sema.Pipeline { + return sema.DefaultPipeline(s.lintConfig(view)).WithAnalyzers(s.analyzers...) +} + // diagnose runs the analysis pipeline once over every affected file — one // run, one shared cross-file index — and publishes the findings per file. // The per-file findings are cached for code actions. func (s *Server) diagnose(ctx context.Context, view *cache.View, affected []uri.URI) { + s.diagnoseAt(ctx, view, affected, view.Generation()) +} + +func (s *Server) diagnoseAt(ctx context.Context, view *cache.View, affected []uri.URI, generation uint64) { + s.analysisMu.Lock() + defer s.analysisMu.Unlock() + ctx = cache.WithGeneration(ctx, generation) + slog.Debug("diagnose called", "files", len(affected)) defer slog.Debug("diagnose finished") - report, err := sema.DefaultPipeline(s.lintConfig(view)).Run(ctx, view, affected) + if !view.IsCurrent(generation) { + return + } + + report, err := s.pipeline(view).Run(ctx, view, affected) if err != nil { logError("diagnostic failed", err) } + if !view.IsCurrent(generation) { + return + } + // Cache the findings whether or not a client is attached: code // actions pair fixes with the server's own diagnostics. A file that // no longer parses (deleted or unreadable) gets its cached report // dropped instead, so the cache never pins stale findings. s.reportMu.Lock() + defer s.reportMu.Unlock() + if !view.IsCurrent(generation) { + return + } + for _, file := range affected { if _, err := view.Parse(ctx, file); err != nil { delete(s.reports, file) @@ -74,15 +100,22 @@ func (s *Server) diagnose(ctx context.Context, view *cache.View, affected []uri. s.reports[file] = report } - s.reportMu.Unlock() if s.client == nil { return } + if !view.IsCurrent(generation) { + return + } + var errs []error for _, file := range affected { + if !view.IsCurrent(generation) { + return + } + if _, err := view.Parse(ctx, file); err != nil { continue } @@ -101,6 +134,10 @@ func (s *Server) diagnose(ctx context.Context, view *cache.View, affected []uri. slog.Debug("publish diagnostics", "file", file, "count", len(res)) + if !view.IsCurrent(generation) { + return + } + err = s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{ URI: file, Diagnostics: res, @@ -115,6 +152,29 @@ func (s *Server) diagnose(ctx context.Context, view *cache.View, affected []uri. } } +func (s *Server) clearDiagnostics(ctx context.Context, files ...uri.URI) { + slices.Sort(files) + files = slices.Compact(files) + ctx = context.WithoutCancel(ctx) + s.reportMu.Lock() + defer s.reportMu.Unlock() + + for _, file := range files { + delete(s.reports, file) + + if s.client == nil { + continue + } + + if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{ + URI: file, + Diagnostics: []protocol.Diagnostic{}, + }); err != nil { + logError("clear diagnostics failed", err, "uri", file) + } + } +} + // reportFor returns the diagnostics last published for file. func (s *Server) reportFor(file uri.URI) sema.Report { s.reportMu.RLock() diff --git a/lsp/filerename.go b/lsp/filerename.go index aacb8d4..cecfa90 100644 --- a/lsp/filerename.go +++ b/lsp/filerename.go @@ -29,7 +29,7 @@ func (s *Server) willRenameFiles(ctx context.Context, params *protocol.RenameFil continue } - view, err := s.session.ViewOf(oldURI) + view, err := s.viewOf(oldURI) if err != nil { continue } @@ -62,7 +62,7 @@ func (s *Server) didRenameFiles(ctx context.Context, params *protocol.RenameFile continue } - view, err := s.session.ViewOf(oldURI) + view, err := s.viewOf(oldURI) if err != nil { continue } diff --git a/lsp/folding.go b/lsp/folding.go index 729d01c..25a1200 100644 --- a/lsp/folding.go +++ b/lsp/folding.go @@ -10,7 +10,7 @@ import ( ) func (s *Server) foldingRanges(ctx context.Context, params *protocol.FoldingRangeParams) ([]protocol.FoldingRange, error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) ([]protocol.FoldingRange, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) ([]protocol.FoldingRange, error) { return source.Ranges(ctx, view, params.TextDocument.URI), nil }) } diff --git a/lsp/format.go b/lsp/format.go index 13fc18c..8054be3 100644 --- a/lsp/format.go +++ b/lsp/format.go @@ -10,7 +10,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(view *cache.View, fh cache.FileHandle) ([]protocol.TextEdit, error) { + return withFile(ctx, s.viewOf, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) ([]protocol.TextEdit, error) { edit, err := source.FormatDocument(ctx, view, fh, s.formatOptions(view)) if err != nil { return nil, err @@ -25,7 +25,7 @@ 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(view *cache.View, fh cache.FileHandle) ([]protocol.TextEdit, error) { + return withFile(ctx, s.viewOf, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) ([]protocol.TextEdit, error) { return source.FormatRange(ctx, view, fh, s.formatOptions(view), params.Range) }) } diff --git a/lsp/highlight.go b/lsp/highlight.go index 350b0e5..577db63 100644 --- a/lsp/highlight.go +++ b/lsp/highlight.go @@ -10,7 +10,7 @@ import ( ) func (s *Server) documentHighlight(ctx context.Context, params *protocol.DocumentHighlightParams) ([]protocol.DocumentHighlight, error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) ([]protocol.DocumentHighlight, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) ([]protocol.DocumentHighlight, error) { return source.Highlight(ctx, view, params.TextDocument.URI, params.Position) }) } diff --git a/lsp/hover.go b/lsp/hover.go index 216a2b0..a51df5f 100644 --- a/lsp/hover.go +++ b/lsp/hover.go @@ -11,7 +11,7 @@ import ( ) func (s *Server) hover(ctx context.Context, params *protocol.HoverParams) (*protocol.Hover, error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) (*protocol.Hover, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) (*protocol.Hover, error) { content, err := source.Hover(ctx, view, params.TextDocument.URI, params.Position) if err != nil { return nil, err diff --git a/lsp/impl.go b/lsp/impl.go index b3b5deb..3173f06 100644 --- a/lsp/impl.go +++ b/lsp/impl.go @@ -30,85 +30,87 @@ func (s *Server) didOpen(ctx context.Context, params *protocol.DidOpenTextDocume From: cache.FileChangeTypeDidOpen, } - s.dirWalkOnce.Do(func() { - file := change.URI + if s.workspace == nil { + s.dirWalkOnce.Do(func() { + file := change.URI - dirPos := strings.LastIndexByte(string(file), '/') - if dirPos == -1 { - return - } + dirPos := strings.LastIndexByte(string(file), '/') + if dirPos == -1 { + return + } - dir := file[0:dirPos] - s.walkFoldersThriftFile(dir) - }) + dir := file[0:dirPos] + s.walkFoldersThriftFile(dir) + }) + } - return s.openFile(ctx, change) + return s.applyChanges(ctx, []*cache.FileChange{change}, true) } -func (s *Server) openFile(ctx context.Context, change *cache.FileChange) error { - if change.From != cache.FileChangeTypeInitialize { - if err := s.session.UpdateOverlayFS(ctx, []*cache.FileChange{change}); err != nil { - return err - } - } - - // The file's directory becomes the view when no workspace folder - // covers it (single-file mode); AddView dedups and is concurrency-safe. - view, err := s.session.ViewOf(change.URI) - if err != nil { - filename := change.URI.Path() - view = s.addFolderView(uri.File(path.Dir(filename))) - } +func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error { + return s.applyChanges(ctx, FileChangeFromLSPDidChange(params), true) +} - s.postDiagnostics(ctx, view, view.Update(ctx, change)) +func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { + fileURI := params.TextDocument.URI + change := &cache.FileChange{URI: fileURI, From: cache.FileChangeTypeDidClose} - return nil + return s.applyChanges(ctx, []*cache.FileChange{change}, true) } -func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDocumentParams) error { - changes := FileChangeFromLSPDidChange(params) - if err := s.session.UpdateOverlayFS(ctx, changes); err != nil { - return err +// applyChanges is the single path from file events to overlays and views. +// Custom workspaces route only through snapshot ownership; stock sessions keep +// their historical first-view fallback and create a view for a lone open file. +func (s *Server) applyChanges(ctx context.Context, changes []*cache.FileChange, overlay bool) error { + if len(changes) == 0 { + return nil } - document := params.TextDocument - fileURI := document.URI + for _, change := range changes { + if change.From == cache.FileChangeTypeDidClose { + s.forgetReport(change.URI) + } + } - view, err := s.session.ViewOf(fileURI) - if err != nil { - return err + if s.workspace != nil { + return s.workspace.applyChanges(ctx, changes, overlay) } - s.postDiagnostics(ctx, view, view.Update(ctx, changes...)) + if overlay { + if err := s.session.UpdateOverlayFS(ctx, changes); err != nil { + return err + } + } - return nil -} + byView := make(map[*cache.View][]*cache.FileChange) + for _, change := range changes { + view, err := s.session.ViewOf(change.URI) + if err != nil { + if change.From != cache.FileChangeTypeDidOpen { + return err + } -func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { - fileURI := params.TextDocument.URI + view = s.addFolderView(uri.File(path.Dir(change.URI.Path()))) + } - view, err := s.session.ViewOf(fileURI) - if err != nil { - return err + byView[view] = append(byView[view], change) } - change := &cache.FileChange{URI: fileURI, From: cache.FileChangeTypeDidClose} - - if err := s.session.UpdateOverlayFS(ctx, []*cache.FileChange{change}); err != nil { - return err + for view, viewChanges := range byView { + s.postDiagnostics(ctx, view, view.Update(ctx, viewChanges...)) } - s.forgetReport(fileURI) - - s.postDiagnostics(ctx, view, view.Update(ctx, change)) - return nil } func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error { - byView := make(map[*cache.View][]*cache.FileChange) + var changes []*cache.FileChange for _, event := range params.Changes { + if s.workspace != nil && !s.workspace.owns(event.URI) { + continue + } + if s.session.HasOverlay(event.URI) { // The editor overlay is authoritative for open documents; disk // events for them are ignored. @@ -120,24 +122,10 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did return err } - // A watched file reported as closed is a deletion from disk. - if change.From == cache.FileChangeTypeDidClose { - s.forgetReport(event.URI) - } - - view, err := s.session.ViewOf(event.URI) - if err != nil { - continue - } - - byView[view] = append(byView[view], change) + changes = append(changes, change) } - for view, changes := range byView { - s.postDiagnostics(ctx, view, view.Update(ctx, changes...)) - } - - return nil + return s.applyChanges(ctx, changes, false) } // watchedFileChange builds a FileChange from a disk event, reading the @@ -191,12 +179,12 @@ func (s *Server) postDiagnostics(ctx context.Context, view *cache.View, res cach return } - s.diagnose(ctx, view, res.Affected) + s.diagnoseAt(ctx, view, res.Affected, res.Gen) }() } func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) { - return withFile(ctx, s.session, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) (*protocol.CompletionList, error) { + return withFile(ctx, s.viewOf, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) (*protocol.CompletionList, error) { items, rng, truncated, err := source.DefaultTokenCompletion.Completion(ctx, view, &source.CompletionRequest{ Pos: protocol.Position{ Line: params.Position.Line, diff --git a/lsp/impl_test.go b/lsp/impl_test.go index ffe8235..fa6d4f2 100644 --- a/lsp/impl_test.go +++ b/lsp/impl_test.go @@ -159,20 +159,7 @@ struct Test { err = srv.DidOpen(ctx, openParams) assert.NoError(t, err) - completionParams := &protocol.CompletionParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: fileURI, - }, - Position: protocol.Position{ - Line: tt.line, - Character: tt.character, - }, - WorkDoneToken: protocol.String(""), - PartialResultToken: protocol.String(""), - Context: protocol.CompletionContext{ - TriggerKind: protocol.CompletionTriggerKindInvoked, - }, - } + completionParams := testCompletionParams(fileURI, tt.line, tt.character) completionResult, err := srv.Completion(ctx, completionParams) assert.NoError(t, err) @@ -252,20 +239,7 @@ struct Test { err = srv.DidOpen(ctx, testParams) assert.NoError(t, err) - completionParams := &protocol.CompletionParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: testURI, - }, - Position: protocol.Position{ - Line: 5, - Character: 28, - }, - WorkDoneToken: protocol.String(""), - PartialResultToken: protocol.String(""), - Context: protocol.CompletionContext{ - TriggerKind: protocol.CompletionTriggerKindInvoked, - }, - } + completionParams := testCompletionParams(testURI, 5, 28) completionResult, err := srv.Completion(ctx, completionParams) assert.NoError(t, err) @@ -354,20 +328,7 @@ struct Other { completionURI, err := uri.Parse(tt.completionURI) assert.NoError(t, err) - completionParams := &protocol.CompletionParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: completionURI, - }, - Position: protocol.Position{ - Line: 5, - Character: 28, - }, - WorkDoneToken: protocol.String(""), - PartialResultToken: protocol.String(""), - Context: protocol.CompletionContext{ - TriggerKind: protocol.CompletionTriggerKindInvoked, - }, - } + completionParams := testCompletionParams(completionURI, 5, 28) completionResult, err := srv.Completion(ctx, completionParams) assert.NoError(t, err) @@ -463,9 +424,7 @@ func Test_InitializeDefersTheWorkspaceWalk(t *testing.T) { srv := NewServer(cache.NewMemoizedFS(), nil, Options{}) - _, err := srv.Initialize(t.Context(), &protocol.InitializeParams{ - WorkspaceFolders: protocol.NewNullable([]protocol.WorkspaceFolder{{URI: uri.File(dir)}}), - }) + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: uri.File(dir)}})) require.NoError(t, err) // Nothing runs during the handshake: no views until the client @@ -496,6 +455,79 @@ func Test_InitializeDefersTheWorkspaceWalk(t *testing.T) { }) } +func Test_InitializeNestedWorkspaceFoldersDoesNotDuplicateFiles(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + outer := uri.File("/workspace") + inner := uri.File("/workspace/service") + nested := uri.File("/workspace/service/api.thrift") + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + nested: []byte("struct Nested {}"), + }), nil, Options{}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: outer}, {URI: inner}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + outerView, err := srv.session.ViewOf(uri.File("/workspace/root.thrift")) + require.NoError(t, err) + innerView, err := srv.session.ViewOf(nested) + require.NoError(t, err) + assert.Equal(t, outer, outerView.Folder()) + assert.Equal(t, inner, innerView.Folder()) + assert.False(t, outerView.FileKnown(nested), "the outer view must not retain a nested project's file") + assert.True(t, innerView.FileKnown(nested)) + + result, err := srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok := result.(protocol.SymbolInformationSlice) + require.True(t, ok) + assert.Equal(t, []string{"Nested"}, symbolNames(symbols)) + }) +} + +func Test_AddingNestedWorkspaceFolderEvictsOuterFiles(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + outer := uri.File("/workspace") + inner := uri.File("/workspace/service") + nested := uri.File("/workspace/service/api.thrift") + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + nested: []byte("struct Nested {}"), + }), nil, Options{}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: outer}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + outerView, err := srv.session.ViewOf(nested) + require.NoError(t, err) + assert.Equal(t, outer, outerView.Folder()) + assert.True(t, outerView.FileKnown(nested)) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Added: []protocol.WorkspaceFolder{{URI: inner}}, + }, + })) + synctest.Wait() + + innerView, err := srv.session.ViewOf(nested) + require.NoError(t, err) + assert.Equal(t, inner, innerView.Folder()) + assert.False(t, outerView.FileKnown(nested), "adding a specific folder must evict the old owner") + assert.True(t, innerView.FileKnown(nested)) + + result, err := srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok := result.(protocol.SymbolInformationSlice) + require.True(t, ok) + assert.Equal(t, []string{"Nested"}, symbolNames(symbols)) + }) +} + // Test_CompletionQualifiedType pins qualified type completion: in a type // position, typing an include name followed by a dot suggests the // include's types, qualified. @@ -537,11 +569,7 @@ struct StrikeRouge { })) completion := func(line, character uint32) []string { - result, err := srv.Completion(ctx, &protocol.CompletionParams{ - TextDocument: protocol.TextDocumentIdentifier{URI: testURI}, - Position: protocol.Position{Line: line, Character: character}, - Context: protocol.CompletionContext{TriggerKind: protocol.CompletionTriggerKindInvoked}, - }) + result, err := srv.Completion(ctx, testCompletionParams(testURI, line, character)) require.NoError(t, err) list, ok := result.(*protocol.CompletionList) @@ -595,11 +623,7 @@ struct StrikeRouge { }, })) - result, err := srv.Completion(ctx, &protocol.CompletionParams{ - TextDocument: protocol.TextDocumentIdentifier{URI: testURI}, - Position: protocol.Position{Line: 3, Character: 15}, - Context: protocol.CompletionContext{TriggerKind: protocol.CompletionTriggerKindInvoked}, - }) + result, err := srv.Completion(ctx, testCompletionParams(testURI, 3, 15)) require.NoError(t, err) list, ok := result.(*protocol.CompletionList) diff --git a/lsp/include_paths_test.go b/lsp/include_paths_test.go index 7c3dfc2..c5ebf53 100644 --- a/lsp/include_paths_test.go +++ b/lsp/include_paths_test.go @@ -1,6 +1,7 @@ package lsp import ( + "context" "os" "path/filepath" "testing" @@ -12,6 +13,7 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" ) // TestConfigFileIncludePaths verifies that include paths from a workspace @@ -30,9 +32,7 @@ func TestConfigFileIncludePaths(t *testing.T) { writeConfig(t, dir, `{"includePaths": ["base"]}`) srv := NewServer(cache.NewMemoizedFS(), nil, Options{}) - _, err := srv.Initialize(ctx, &protocol.InitializeParams{ - WorkspaceFolders: protocol.NewNullable([]protocol.WorkspaceFolder{{URI: uri.File(dir)}}), - }) + _, err := srv.Initialize(ctx, testInitializeParams([]protocol.WorkspaceFolder{{URI: uri.File(dir)}})) require.NoError(t, err) require.NoError(t, srv.Initialized(ctx, &protocol.InitializedParams{})) @@ -55,6 +55,53 @@ func TestConfigFileIncludePaths(t *testing.T) { }) } +func TestCustomProjectIncludePathsAreAuthoritative(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + root := filepath.Join(dir, "project") + projectIncludes := filepath.Join(dir, "project-includes") + configIncludes := filepath.Join(dir, "config-includes") + cliIncludes := filepath.Join(dir, "cli-includes") + settingsIncludes := filepath.Join(dir, "settings-includes") + for _, path := range []string{root, projectIncludes, configIncludes, cliIncludes, settingsIncludes} { + require.NoError(t, os.MkdirAll(path, 0o755)) + } + + configPath := filepath.Join(root, "thrift-ls.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"includePaths":["`+configIncludes+`"]}`), 0o644)) + target := uri.File(filepath.Join(root, "api.thrift")) + projectDependency := uri.File(filepath.Join(projectIncludes, "shared.thrift")) + loader := func(context.Context, uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File(filepath.Join(root, "tbuild.yaml")), + RootURI: uri.File(root), + TargetFiles: []uri.URI{target}, + IncludePaths: []string{projectIncludes}, + }}}, nil + } + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + target: []byte(`include "shared.thrift"`), + projectDependency: []byte("struct Shared {}"), + }), nil, Options{ + CLI: options.Patch{IncludePaths: &[]string{cliIncludes}}, + ConfigFinder: func(string) (string, error) { return configPath, nil }, + WorkspaceLoader: loader, + }) + params := testInitializeParams([]protocol.WorkspaceFolder{{URI: uri.File(dir)}}) + params.InitializationOptions = protocol.LSPAny([]byte(`{"includePaths":["` + settingsIncludes + `"]}`)) + + _, err := srv.Initialize(t.Context(), params) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + view, err := srv.session.ViewOf(target) + require.NoError(t, err) + assert.Equal(t, []string{projectIncludes}, view.Resolver().IncludePaths()) + assert.Equal(t, projectDependency, view.Resolver().ResolveInclude(target, "shared.thrift")) + }) +} + // writeConfig writes a thrift-ls.json config file into dir. func writeConfig(t *testing.T, dir, content string) { t.Helper() diff --git a/lsp/initialize.go b/lsp/initialize.go index 85d2f8e..9c03d84 100644 --- a/lsp/initialize.go +++ b/lsp/initialize.go @@ -42,7 +42,11 @@ func (s *Server) initialize(params *protocol.InitializeParams) (result *protocol slog.Debug("initialized folders", "folders", folders) - s.folders = folders + if s.workspace != nil { + s.workspace.initialize(folders) + } else { + s.folders = folders + } // Workspace settings (initializationOptions) overlay each view's // config; didChangeConfiguration updates them later. @@ -60,7 +64,7 @@ func (s *Server) initialize(params *protocol.InitializeParams) (result *protocol // initialize. Helix deadlocks on the registerCapability request during // the handshake and discards (or stalls on) notifications from an // uninitialized server. - return initializeResult(), nil + return initializeResult(s.version), nil } // registerFileWatcher subscribes the client to disk events for thrift files, @@ -110,30 +114,186 @@ func (s *Server) walkFoldersThriftFile(folder uri.URI) { // The view is the folder itself, so files in nested directories // resolve to it via ContainsFile; addFolderView resolves its config. - s.addFolderView(folder) + view := s.addFolderView(folder) + migrated := s.reconcileStockAddition(context.TODO(), view) // Walk the folder through the session'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.session.WalkFiles(context.TODO(), folder, func(fileURI uri.URI) error { - if !strings.HasSuffix(fileURI.Path(), ".thrift") { + changes := s.workspaceFileChanges([]uri.URI{folder}) + seen := make(map[uri.URI]struct{}, len(changes)) + for _, change := range changes { + seen[change.URI] = struct{}{} + } + + for _, fileURI := range migrated { + if _, ok := seen[fileURI]; ok { + continue + } + + changes = append(changes, &cache.FileChange{ + URI: fileURI, + From: cache.FileChangeTypeInitialize, + }) + } + + if err := s.applyChanges(context.TODO(), changes, false); err != nil { + slog.Warn("workspace files failed", "err", err) + } +} + +func (s *Server) walkWorkspaceFolders(folders []uri.URI) { + var migrated []uri.URI + for _, folder := range folders { + view := s.addFolderView(folder) + migrated = append(migrated, s.reconcileStockAddition(context.TODO(), view)...) + } + + changes := s.workspaceFileChanges(folders) + seen := make(map[uri.URI]struct{}, len(changes)) + for _, change := range changes { + seen[change.URI] = struct{}{} + } + + for _, fileURI := range migrated { + if _, ok := seen[fileURI]; ok { + continue + } + + changes = append(changes, &cache.FileChange{ + URI: fileURI, + From: cache.FileChangeTypeInitialize, + }) + } + + if err := s.applyChanges(context.TODO(), changes, false); err != nil { + slog.Warn("workspace files failed", "err", err) + } +} + +func (s *Server) workspaceFileChanges(folders []uri.URI) []*cache.FileChange { + var changes []*cache.FileChange + seen := make(map[uri.URI]struct{}) + + for _, folder := range folders { + _ = s.session.WalkFiles(context.TODO(), folder, func(fileURI uri.URI) error { + if !strings.HasSuffix(fileURI.Path(), ".thrift") { + return nil + } + + if _, ok := seen[fileURI]; ok { + return nil + } + seen[fileURI] = struct{}{} + + slog.Debug("file path", "uri", fileURI) + changes = append(changes, &cache.FileChange{ + URI: fileURI, + Version: 0, + Content: []byte{}, + From: cache.FileChangeTypeInitialize, + }) + return nil + }) + } + + return changes +} + +func (s *Server) reconcileStockAddition(ctx context.Context, view *cache.View) []uri.URI { + var migrated []uri.URI + seen := make(map[uri.URI]struct{}) + + for _, other := range s.session.Views() { + if other == view || !other.ContainsFile(view.Folder()) { + continue } - slog.Debug("file path", "uri", fileURI) + var evicted []uri.URI + for _, fileURI := range other.KnownFiles() { + if !view.ContainsFile(fileURI) { + continue + } - if err := s.openFile(context.TODO(), &cache.FileChange{ - URI: fileURI, - Version: 0, - Content: []byte{}, - From: cache.FileChangeTypeInitialize, - }); err != nil { - slog.Warn("openFile failed", "err", err) + evicted = append(evicted, fileURI) + if _, ok := seen[fileURI]; !ok { + seen[fileURI] = struct{}{} + migrated = append(migrated, fileURI) + } } - // always return nil to continue parse - return nil - }) + if len(evicted) == 0 { + continue + } + + other.Evict(evicted...) + s.clearDiagnostics(ctx, evicted...) + } + + return migrated +} + +func (s *Server) removeStockView(ctx context.Context, folder uri.URI) { + var removed *cache.View + for _, view := range s.session.Views() { + if view.Folder() == folder { + removed = view + + break + } + } + if removed == nil { + s.removeView(folder) + + return + } + + files := removed.KnownFiles() + s.removeView(folder) + s.clearDiagnostics(ctx, files...) + + byView := make(map[*cache.View][]uri.URI) + for _, fileURI := range files { + view := s.stockViewOf(fileURI) + if view == nil { + continue + } + + byView[view] = append(byView[view], fileURI) + } + + for view, files := range byView { + updates := make([]*cache.FileChange, len(files)) + for i, fileURI := range files { + updates[i] = &cache.FileChange{URI: fileURI, From: cache.FileChangeTypeInitialize} + } + + s.postDiagnostics(ctx, view, view.Update(ctx, updates...)) + } +} + +func (s *Server) stockViewOf(fileURI uri.URI) *cache.View { + var best *cache.View + for _, view := range s.session.Views() { + if !view.ContainsFile(fileURI) { + continue + } + if best == nil || len(view.Folder().Path()) > len(best.Folder().Path()) { + best = view + } + } + if best != nil { + return best + } + + for _, view := range s.session.Views() { + if view.FileKnown(fileURI) { + return view + } + } + + return nil } // thriftFileOperationFilters is the registration for one file operation: @@ -152,11 +312,12 @@ func thriftFileOperationFilters() protocol.FileOperationRegistrationOptions { } } -func initializeResult() *protocol.InitializeResult { +func initializeResult(version string) *protocol.InitializeResult { thriftSelector := &protocol.DocumentSelector{ - &protocol.TextDocumentFilterLanguage{Language: "thrift"}, + &protocol.TextDocumentFilterLanguage{Language: LanguageIDThrift}, } - res := &protocol.InitializeResult{ + + return &protocol.InitializeResult{ Capabilities: protocol.ServerCapabilities{ TextDocumentSync: &protocol.TextDocumentSyncOptions{ OpenClose: new(true), @@ -183,28 +344,52 @@ func initializeResult() *protocol.InitializeResult { TriggerCharacters: []string{".", "\"", "("}, }, HoverProvider: &protocol.HoverOptions{ - WorkDoneProgress: new(true), + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, }, DeclarationProvider: &protocol.DeclarationRegistrationOptions{ - WorkDoneProgress: new(true), - DocumentSelector: thriftSelector, - ID: new("thrift-ls"), + DeclarationOptions: protocol.DeclarationOptions{ + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, + }, + TextDocumentRegistrationOptions: protocol.TextDocumentRegistrationOptions{ + DocumentSelector: thriftSelector, + }, + StaticRegistrationOptions: protocol.StaticRegistrationOptions{ + ID: new("thrift-ls"), + }, }, DefinitionProvider: &protocol.DefinitionOptions{ - WorkDoneProgress: new(true), + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, }, TypeDefinitionProvider: &protocol.TypeDefinitionRegistrationOptions{ - DocumentSelector: thriftSelector, - WorkDoneProgress: new(true), - ID: new("thrift-ls"), + TextDocumentRegistrationOptions: protocol.TextDocumentRegistrationOptions{ + DocumentSelector: thriftSelector, + }, + TypeDefinitionOptions: protocol.TypeDefinitionOptions{ + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, + }, + StaticRegistrationOptions: protocol.StaticRegistrationOptions{ + ID: new("thrift-ls"), + }, }, ReferencesProvider: &protocol.ReferenceOptions{ - WorkDoneProgress: new(true), + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, }, DocumentHighlightProvider: protocol.Boolean(true), DocumentSymbolProvider: &protocol.DocumentSymbolOptions{ - WorkDoneProgress: new(true), - Label: new("thrift-ls"), + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, + Label: new("thrift-ls"), }, CodeActionProvider: &protocol.CodeActionOptions{ // Keep in sync with the kinds codeAction returns: @@ -222,13 +407,19 @@ func initializeResult() *protocol.InitializeResult { ColorProvider: protocol.Boolean(false), FoldingRangeProvider: protocol.Boolean(true), WorkspaceSymbolProvider: &protocol.WorkspaceSymbolOptions{ - WorkDoneProgress: new(true), + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, }, DocumentFormattingProvider: &protocol.DocumentFormattingOptions{ - WorkDoneProgress: new(true), + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, }, DocumentRangeFormattingProvider: &protocol.DocumentRangeFormattingOptions{ - WorkDoneProgress: new(true), + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, }, DocumentOnTypeFormattingProvider: protocol.DocumentOnTypeFormattingOptions{ FirstTriggerCharacter: "}", @@ -240,17 +431,25 @@ func initializeResult() *protocol.InitializeResult { CallHierarchyProvider: protocol.Boolean(false), LinkedEditingRangeProvider: protocol.Boolean(false), SemanticTokensProvider: &protocol.SemanticTokensRegistrationOptions{ - DocumentSelector: thriftSelector, - WorkDoneProgress: new(true), - Legend: protocol.SemanticTokensLegend{ - TokenTypes: source.Legend(), - TokenModifiers: []string{}, + TextDocumentRegistrationOptions: protocol.TextDocumentRegistrationOptions{ + DocumentSelector: thriftSelector, }, - Full: &protocol.SemanticTokensFullDelta{ - Delta: new(true), + SemanticTokensOptions: protocol.SemanticTokensOptions{ + WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ + WorkDoneProgress: new(true), + }, + Legend: protocol.SemanticTokensLegend{ + TokenTypes: source.Legend(), + TokenModifiers: []string{}, + }, + Full: &protocol.SemanticTokensFullDelta{ + Delta: new(true), + }, + Range: protocol.Boolean(false), + }, + StaticRegistrationOptions: protocol.StaticRegistrationOptions{ + ID: new("thrift-ls"), }, - Range: protocol.Boolean(false), - ID: new("thrift-ls"), }, Workspace: &protocol.WorkspaceOptions{ WorkspaceFolders: &protocol.WorkspaceFoldersServerCapabilities{ @@ -270,9 +469,7 @@ func initializeResult() *protocol.InitializeResult { }, ServerInfo: protocol.ServerInfo{ Name: ServerName, - Version: protocol.NewOptional(ServerVersion), + Version: protocol.NewOptional(version), }, } - - return res } diff --git a/lsp/links.go b/lsp/links.go index 032598c..5afc024 100644 --- a/lsp/links.go +++ b/lsp/links.go @@ -10,7 +10,7 @@ import ( ) func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLinkParams) ([]protocol.DocumentLink, error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) ([]protocol.DocumentLink, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) ([]protocol.DocumentLink, error) { return source.Links(ctx, view, params.TextDocument.URI), nil }) } diff --git a/lsp/lsptest/server.go b/lsp/lsptest/server.go index fbf8ac4..7745d98 100644 --- a/lsp/lsptest/server.go +++ b/lsp/lsptest/server.go @@ -133,10 +133,10 @@ func New(command []string, dir string, opts Options) (*Server, error) { initParams := &protocol.InitializeParams{ RootURI: &root, Capabilities: protocol.ClientCapabilities{}, - WorkspaceFolders: protocol.NewNullable([]protocol.WorkspaceFolder{ - {URI: root, Name: filepath.Base(dir)}, - }), } + initParams.WorkspaceFolders = protocol.NewNullable([]protocol.WorkspaceFolder{ + {URI: root, Name: filepath.Base(dir)}, + }) if _, ierr := s.disp.Initialize(initCtx, initParams); ierr != nil { _ = s.Close() diff --git a/lsp/protocol_test.go b/lsp/protocol_test.go new file mode 100644 index 0000000..d67856f --- /dev/null +++ b/lsp/protocol_test.go @@ -0,0 +1,55 @@ +package lsp + +import ( + "testing" + + "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" +) + +func TestInitializeReportsConfiguredVersion(t *testing.T) { + tests := []struct { + name string + version string + want string + }{ + {name: "configured", version: "tbuild-test-version", want: "tbuild-test-version"}, + {name: "fallback", want: ServerVersion}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := NewServer(cache.NewMemFS(nil), nil, Options{Version: tt.version}) + result, err := srv.Initialize(t.Context(), testInitializeParams(nil)) + require.NoError(t, err) + version, ok := result.ServerInfo.Version.Get() + require.True(t, ok) + assert.Equal(t, tt.want, version) + }) + } +} + +func testInitializeParams(folders []protocol.WorkspaceFolder) *protocol.InitializeParams { + params := &protocol.InitializeParams{} + params.WorkspaceFolders = protocol.NewNullable(folders) + + return params +} + +func testCompletionParams(file uri.URI, line, character uint32) *protocol.CompletionParams { + params := &protocol.CompletionParams{ + Context: protocol.CompletionContext{ + TriggerKind: protocol.CompletionTriggerKindInvoked, + }, + } + params.TextDocument = protocol.TextDocumentIdentifier{URI: file} + params.Position = protocol.Position{Line: line, Character: character} + params.WorkDoneToken = protocol.String("") + params.PartialResultToken = protocol.String("") + + return params +} diff --git a/lsp/rename.go b/lsp/rename.go index 9d62507..5de869f 100644 --- a/lsp/rename.go +++ b/lsp/rename.go @@ -10,13 +10,13 @@ import ( ) func (s *Server) prepareRename(ctx context.Context, params *protocol.PrepareRenameParams) (*protocol.Range, error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) (*protocol.Range, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) (*protocol.Range, error) { return source.PrepareRename(ctx, view, params.TextDocument.URI, params.Position) }) } func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*protocol.WorkspaceEdit, error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) (*protocol.WorkspaceEdit, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) (*protocol.WorkspaceEdit, error) { return source.Rename(ctx, view, params.TextDocument.URI, params.Position, params.NewName) }) } diff --git a/lsp/semantic.go b/lsp/semantic.go index 6449cdf..99ccaa6 100644 --- a/lsp/semantic.go +++ b/lsp/semantic.go @@ -10,7 +10,7 @@ import ( ) func (s *Server) semanticTokensFull(ctx context.Context, params *protocol.SemanticTokensParams) (*protocol.SemanticTokens, error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) (*protocol.SemanticTokens, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) (*protocol.SemanticTokens, error) { data, err := source.Tokens(ctx, view, params.TextDocument.URI) if err != nil { return nil, err diff --git a/lsp/server.go b/lsp/server.go index e0e7cc2..4194cf7 100644 --- a/lsp/server.go +++ b/lsp/server.go @@ -27,8 +27,10 @@ type Server struct { // 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 + explicit options.Patch + configPath string + configFinder func(string) (string, error) + version string // cli is the CLI-only overlay, applied on top of every view's config. cli options.Patch @@ -48,6 +50,9 @@ type Server struct { // handshake never blocks on parsing the workspace. folders []uri.URI + workspace *customWorkspace + analyzers []sema.Analyzer + // configs holds each view folder's resolved configuration. Views only // carry what the store needs (include paths); formatting settings and // log level stay here, where the workspace overlay applies. @@ -67,6 +72,9 @@ type Server struct { workspaceWalkOnce sync.Once dirWalkOnce sync.Once + // analysisMu serializes analyzer instances shared by diagnostic workers. + analysisMu sync.Mutex + // lastReport remembers the diagnostics the server last published per // file, so code actions can pair fixes with the diagnostics without a // round trip through the client. Guarded by reportMu. @@ -78,16 +86,33 @@ type Server struct { // are expected to validate; workspace settings overlay each view's config // at initialize time and on didChangeConfiguration. func NewServer(fs cache.FileSource, client protocol.Client, opts Options) *Server { - return &Server{ + configFinder := opts.ConfigFinder + if configFinder == nil { + configFinder = options.FindConfig + } + version := opts.Version + if version == "" { + version = ServerVersion + } + + server := &Server{ session: cache.NewSession(fs), client: client, explicit: opts.Config, configPath: opts.ConfigPath, + configFinder: configFinder, + version: version, cli: opts.CLI, configs: make(map[uri.URI]options.Patch), configIssues: make(map[uri.URI]configIssue), reports: make(map[uri.URI]sema.Report), + analyzers: slices.Clone(opts.Analyzers), + } + if opts.WorkspaceLoader != nil { + server.workspace = newCustomWorkspace(server, opts.WorkspaceLoader) } + + return server } // setWorkspaceSettings stores the workspace settings overlay; invalid @@ -112,20 +137,38 @@ func (s *Server) setWorkspaceSettings(overlay options.Patch) { // what the store needs. func (s *Server) addFolderView(folder uri.URI) *cache.View { cfg := s.viewConfig(folder) - s.applyLogLevel(cfg) - - s.cfgMu.Lock() - s.configs[folder] = cfg - s.cfgMu.Unlock() var includePaths []string if cfg.IncludePaths != nil { includePaths = *cfg.IncludePaths } + return s.addView(folder, cfg, includePaths) +} + +func (s *Server) addProjectView(project Project) *cache.View { + return s.addView(project.RootURI, s.viewConfig(project.RootURI), project.IncludePaths) +} + +func (s *Server) addView(folder uri.URI, cfg options.Patch, includePaths []string) *cache.View { + s.applyLogLevel(cfg) + + s.cfgMu.Lock() + s.configs[folder] = cfg + s.cfgMu.Unlock() + return s.session.AddView(folder, includePaths) } +func (s *Server) removeView(folder uri.URI) { + s.session.RemoveView(folder) + + s.cfgMu.Lock() + delete(s.configs, folder) + delete(s.configIssues, folder) + s.cfgMu.Unlock() +} + // folderConfig returns the resolved configuration of a view's folder. func (s *Server) folderConfig(folder uri.URI) options.Patch { s.cfgMu.RLock() @@ -151,7 +194,7 @@ func (s *Server) viewConfig(folder uri.URI) options.Patch { return s.cli.Apply(s.explicit) } - cfgPath, err := options.FindConfig(folder.FsPath()) + cfgPath, err := s.configFinder(folder.FsPath()) if err != nil { logError("config discovery failed", Expected(err), "dir", folder.FsPath()) @@ -319,10 +362,15 @@ func (s *Server) Initialized(ctx context.Context, params *protocol.InitializedPa // or diagnostics any earlier violates the spec — Helix deadlocks on // a client request that arrives before initialize is answered. s.workspaceWalkOnce.Do(func() { + if s.workspace != nil { + s.workspace.start() + + return + } + + folders := slices.Clone(s.folders) go func() { - for _, folder := range s.folders { - s.walkFoldersThriftFile(folder) - } + s.walkWorkspaceFolders(folders) }() }) @@ -332,6 +380,10 @@ func (s *Server) Initialized(ctx context.Context, params *protocol.InitializedPa } func (s *Server) Shutdown(ctx context.Context) (err error) { + if s.workspace != nil { + s.workspace.shutdown() + } + return nil } @@ -428,13 +480,23 @@ func (s *Server) DidChangeWatchedFiles(ctx context.Context, params *protocol.Did } func (s *Server) DidChangeWorkspaceFolders(ctx context.Context, params *protocol.DidChangeWorkspaceFoldersParams) (err error) { - for _, folder := range params.Event.Removed { - s.session.RemoveView(folder.URI) + if s.workspace != nil { + added := make([]uri.URI, len(params.Event.Added)) + for i, folder := range params.Event.Added { + added[i] = folder.URI + } + removed := make([]uri.URI, len(params.Event.Removed)) + for i, folder := range params.Event.Removed { + removed[i] = folder.URI + } + + s.workspace.changeFolders(added, removed) + + return nil + } - s.cfgMu.Lock() - delete(s.configs, folder.URI) - delete(s.configIssues, folder.URI) - s.cfgMu.Unlock() + for _, folder := range params.Event.Removed { + s.removeStockView(ctx, folder.URI) } for _, folder := range params.Event.Added { @@ -509,7 +571,7 @@ func (s *Server) Implementation(ctx context.Context, params *protocol.Implementa } func (s *Server) OnTypeFormatting(ctx context.Context, params *protocol.DocumentOnTypeFormattingParams) (result []protocol.TextEdit, err error) { - return withFile(ctx, s.session, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) ([]protocol.TextEdit, error) { + return withFile(ctx, s.viewOf, params.TextDocument.URI, func(view *cache.View, fh cache.FileHandle) ([]protocol.TextEdit, error) { return source.OnTypeFormat(ctx, view, fh, s.formatOptions(view), params.Position) }) } @@ -554,7 +616,12 @@ func (s *Server) Symbols(ctx context.Context, params *protocol.WorkspaceSymbolPa var res []protocol.SymbolInformation for _, view := range views { - syms := source.WorkspaceSymbols(ctx, view, view.KnownFiles(), params.Query, maxResults-len(res)) + files := view.KnownFiles() + if s.workspace != nil { + files = s.workspace.files(view) + } + + syms := source.WorkspaceSymbols(ctx, view, files, params.Query, maxResults-len(res)) res = append(res, syms...) if len(res) >= maxResults { diff --git a/lsp/snapshot.go b/lsp/snapshot.go index f3f9898..abc6c0c 100644 --- a/lsp/snapshot.go +++ b/lsp/snapshot.go @@ -8,10 +8,12 @@ import ( "github.com/karitham/thrift-ls/lsp/cache" ) +type viewResolver func(uri.URI) (*cache.View, error) + // withView resolves file's view and runs fn with it. Every request handler // funnels through this helper so view routing lives in one place. -func withView[T any](session *cache.Session, file uri.URI, fn func(*cache.View) (T, error)) (T, error) { - view, err := session.ViewOf(file) +func withView[T any](resolve viewResolver, file uri.URI, fn func(*cache.View) (T, error)) (T, error) { + view, err := resolve(file) if err != nil { var zero T @@ -22,8 +24,8 @@ func withView[T any](session *cache.Session, file uri.URI, fn func(*cache.View) } // withFile is withView plus the file handle for file. -func withFile[T any](ctx context.Context, session *cache.Session, file uri.URI, fn func(*cache.View, cache.FileHandle) (T, error)) (T, error) { - return withView(session, file, func(view *cache.View) (T, error) { +func withFile[T any](ctx context.Context, resolve viewResolver, file uri.URI, fn func(*cache.View, cache.FileHandle) (T, error)) (T, error) { + return withView(resolve, file, func(view *cache.View) (T, error) { fh, err := view.ReadFile(ctx, file) if err != nil { var zero T @@ -34,3 +36,11 @@ func withFile[T any](ctx context.Context, session *cache.Session, file uri.URI, return fn(view, fh) }) } + +func (s *Server) viewOf(file uri.URI) (*cache.View, error) { + if s.workspace != nil { + return s.workspace.viewOf(file) + } + + return s.session.ViewOf(file) +} diff --git a/lsp/source/workspace.go b/lsp/source/workspace.go index 0bfa690..566180c 100644 --- a/lsp/source/workspace.go +++ b/lsp/source/workspace.go @@ -50,8 +50,10 @@ func documentSymbolsFlat(ctx context.Context, view *cache.View, file uri.URI) [] func flattenSymbol(sym *protocol.DocumentSymbol, file uri.URI, container string, out *[]protocol.SymbolInformation) { info := protocol.SymbolInformation{ - Name: sym.Name, - Kind: sym.Kind, + BaseSymbolInformation: protocol.BaseSymbolInformation{ + Name: sym.Name, + Kind: sym.Kind, + }, Location: protocol.Location{URI: file, Range: sym.SelectionRange}, } if container != "" { diff --git a/lsp/stream.go b/lsp/stream.go index e4c0434..d64c30d 100644 --- a/lsp/stream.go +++ b/lsp/stream.go @@ -2,12 +2,16 @@ package lsp import ( "context" + "errors" + "io" "go.lsp.dev/jsonrpc2" + "go.lsp.dev/pkg/fakenet" "go.lsp.dev/protocol" "github.com/karitham/thrift-ls/lsp/cache" "github.com/karitham/thrift-ls/options" + "github.com/karitham/thrift-ls/sema" ) type StreamServer struct { @@ -26,8 +30,42 @@ type Options struct { ConfigPath string // CLI is the CLI-only overlay, applied on top of every view's config. CLI options.Patch + // ConfigFinder resolves an implicit config for each view root. A nil + // finder uses options.FindConfig. + ConfigFinder func(string) (string, error) + // WorkspaceLoader replaces the default recursive workspace scan when set. + WorkspaceLoader WorkspaceLoader + // Analyzers are appended to thrift-ls's built-in semantic analyzers. + Analyzers []sema.Analyzer + // Version is reported in the initialize result. An empty value uses + // ServerVersion. + Version string } +// ServeStdio serves the language server over input and output until the +// context is canceled or the input closes. End-of-file is a clean shutdown. +// A nil opts uses the server defaults. +func ServeStdio(ctx context.Context, opts *Options, input io.Reader, output io.Writer) error { + if opts == nil { + opts = &Options{} + } + + server := NewStreamServer(opts) + stream := jsonrpc2.NewStream(fakenet.NewConn("stdio", io.NopCloser(input), nopWriteCloser{output})) + err := server.ServeStream(ctx, jsonrpc2.NewConn(stream)) + if errors.Is(err, io.EOF) { + return nil + } + + return err +} + +type nopWriteCloser struct { + io.Writer +} + +func (nopWriteCloser) Close() error { return nil } + func NewStreamServer(opts *Options) *StreamServer { return &StreamServer{ fs: cache.NewMemoizedFS(), diff --git a/lsp/stream_test.go b/lsp/stream_test.go index 0cca0a7..1706f74 100644 --- a/lsp/stream_test.go +++ b/lsp/stream_test.go @@ -315,3 +315,10 @@ func TestServeStream(t *testing.T) { t.Fatal("ServeStream did not return after exit") } } + +func TestServeStdioTreatsEOFAsCleanShutdown(t *testing.T) { + var output bytes.Buffer + + err := ServeStdio(t.Context(), &Options{}, bytes.NewReader(nil), &output) + require.NoError(t, err) +} diff --git a/lsp/symbols.go b/lsp/symbols.go index 4a858fa..0549a97 100644 --- a/lsp/symbols.go +++ b/lsp/symbols.go @@ -10,7 +10,7 @@ import ( ) func (s *Server) documentSymbol(ctx context.Context, params *protocol.DocumentSymbolParams) (result protocol.DocumentSymbolSlice, err error) { - return withView(s.session, params.TextDocument.URI, func(view *cache.View) (protocol.DocumentSymbolSlice, error) { + return withView(s.viewOf, params.TextDocument.URI, func(view *cache.View) (protocol.DocumentSymbolSlice, error) { syms := source.DocumentSymbols(ctx, view, params.TextDocument.URI) result := make(protocol.DocumentSymbolSlice, 0, len(syms)) diff --git a/lsp/workspace.go b/lsp/workspace.go new file mode 100644 index 0000000..ec6e9bb --- /dev/null +++ b/lsp/workspace.go @@ -0,0 +1,644 @@ +package lsp + +import ( + "context" + "fmt" + "slices" + "strings" + "sync" + + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" +) + +// WorkspaceLoader discovers the projects in one LSP workspace folder. A +// returned error means discovery failed for the folder. Non-fatal project +// failures belong in WorkspaceSnapshot.Issues so valid projects can still be +// indexed. +type WorkspaceLoader func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) + +// WorkspaceSnapshot is one immutable result of workspace discovery. The +// loader must not mutate it after returning. +type WorkspaceSnapshot struct { + Projects []Project + Issues []WorkspaceIssue +} + +// Project describes one independently configured Thrift project. +type Project struct { + // ConfigURI is the stable identity and source location of the project + // configuration. + ConfigURI uri.URI + // RootURI is the directory whose files use this project's view. + RootURI uri.URI + // TargetFiles are the Thrift files to index for the project. + TargetFiles []uri.URI + // IncludePaths are the project-specific include search paths. + IncludePaths []string +} + +// WorkspaceIssue is a non-fatal discovery problem publishable at URI. +type WorkspaceIssue struct { + URI uri.URI + Message string +} + +type customWorkspace struct { + server *Server + loader WorkspaceLoader + + mu sync.Mutex + folders map[uri.URI]*workspaceFolder + model workspaceModel + views map[uri.URI]*cache.View + documents map[uri.URI]struct{} +} + +type workspaceFolder struct { + cancel context.CancelFunc + snapshot WorkspaceSnapshot +} + +type workspaceLoadResult struct { + folder uri.URI + state *workspaceFolder + cancel context.CancelFunc + snapshot WorkspaceSnapshot + err error +} + +type workspaceModel struct { + roots map[uri.URI]Project + targets map[uri.URI]uri.URI + issues map[uri.URI][]WorkspaceIssue +} + +func newCustomWorkspace(server *Server, loader WorkspaceLoader) *customWorkspace { + return &customWorkspace{ + server: server, + loader: loader, + folders: make(map[uri.URI]*workspaceFolder), + model: emptyWorkspaceModel(), + views: make(map[uri.URI]*cache.View), + documents: make(map[uri.URI]struct{}), + } +} + +func emptyWorkspaceModel() workspaceModel { + return workspaceModel{ + roots: make(map[uri.URI]Project), + targets: make(map[uri.URI]uri.URI), + issues: make(map[uri.URI][]WorkspaceIssue), + } +} + +func cloneWorkspaceSnapshot(snapshot WorkspaceSnapshot) WorkspaceSnapshot { + out := WorkspaceSnapshot{ + Projects: make([]Project, len(snapshot.Projects)), + Issues: slices.Clone(snapshot.Issues), + } + + for i, project := range snapshot.Projects { + project.TargetFiles = slices.Clone(project.TargetFiles) + project.IncludePaths = slices.Clone(project.IncludePaths) + out.Projects[i] = project + } + + return out +} + +func validateWorkspaceSnapshot(folder uri.URI, snapshot WorkspaceSnapshot) WorkspaceSnapshot { + projects := make([]Project, 0, len(snapshot.Projects)) + + for _, project := range snapshot.Projects { + if err := validateProject(project); err != nil { + logError("workspace project rejected", Expected(fmt.Errorf("workspace %s: %w", folder, err))) + + if issueURI := projectIssueURI(folder, project); issueURI != "" { + snapshot.Issues = append(snapshot.Issues, WorkspaceIssue{ + URI: issueURI, + Message: fmt.Sprintf("project rejected: %v", err), + }) + } + + continue + } + + projects = append(projects, project) + } + + snapshot.Projects = projects + + return snapshot +} + +func validateProject(project Project) error { + if err := validateProjectURI("root URI", project.RootURI); err != nil { + return err + } + + if err := validateProjectURI("config URI", project.ConfigURI); err != nil { + return err + } + + for i, target := range project.TargetFiles { + if err := validateProjectURI(fmt.Sprintf("target file %d", i), target); err != nil { + return err + } + } + + return nil +} + +func validateProjectURI(name string, value uri.URI) error { + if value == "" { + return fmt.Errorf("%s is empty", name) + } + + parsed, err := uri.ParseStrict(string(value)) + if err != nil { + return fmt.Errorf("%s is invalid: %w", name, err) + } + + if !parsed.IsFile() { + return fmt.Errorf("%s must use the file URI scheme", name) + } + + if parsed.Path() == "" { + return fmt.Errorf("%s has an empty path", name) + } + + return nil +} + +func projectIssueURI(folder uri.URI, project Project) uri.URI { + if validateProjectURI("config URI", project.ConfigURI) == nil { + return project.ConfigURI + } + + if validateProjectURI("workspace folder", folder) == nil { + return folder + } + + return "" +} + +// workspaceModelOf derives all routing and diagnostics ownership from the +// accepted folder snapshots. Folder order makes shared roots deterministic. +func workspaceModelOf(snapshots map[uri.URI]WorkspaceSnapshot) workspaceModel { + model := emptyWorkspaceModel() + folders := sortedURIs(snapshots) + var projects []Project + + for _, folder := range folders { + snapshot := snapshots[folder] + for _, project := range snapshot.Projects { + previous, exists := model.roots[project.RootURI] + if exists && !slices.Equal(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 include paths", + previous.ConfigURI, project.RootURI, + ), + }) + + continue + } + + if !exists { + model.roots[project.RootURI] = project + } + + projects = append(projects, project) + } + + for _, issue := range snapshot.Issues { + model.issues[issue.URI] = append(model.issues[issue.URI], issue) + } + } + + for _, project := range projects { + for _, target := range project.TargetFiles { + if _, exists := model.targets[target]; exists { + continue + } + + root, ok := model.rootFor(target) + if !ok { + root = project.RootURI + } + model.targets[target] = root + } + } + + return model +} + +func (m workspaceModel) rootFor(file uri.URI) (uri.URI, bool) { + var best uri.URI + + for root := range m.roots { + if !containsURI(root, file) { + continue + } + if best == "" || len(root.Path()) > len(best.Path()) { + best = root + } + } + + if best != "" { + return best, true + } + + root, ok := m.targets[file] + + return root, ok +} + +func (m workspaceModel) ownerOf(file uri.URI, documents map[uri.URI]struct{}) (uri.URI, bool) { + if root, ok := m.targets[file]; ok { + return root, true + } + + if _, ok := documents[file]; ok { + return m.rootFor(file) + } + + return "", false +} + +func (m workspaceModel) ownedFiles(root uri.URI, documents map[uri.URI]struct{}) []uri.URI { + files := make([]uri.URI, 0) + for target, owner := range m.targets { + if owner == root { + files = append(files, target) + } + } + for document := range documents { + owner, ok := m.ownerOf(document, documents) + if ok && owner == root { + files = append(files, document) + } + } + + slices.Sort(files) + + return slices.Compact(files) +} + +func containsURI(root, file uri.URI) bool { + folder := strings.TrimSuffix(root.Path(), "/") + + return strings.HasPrefix(file.Path(), folder+"/") +} + +func (w *customWorkspace) initialize(folders []uri.URI) { + w.mu.Lock() + defer w.mu.Unlock() + + for _, folder := range folders { + if _, exists := w.folders[folder]; !exists { + w.folders[folder] = &workspaceFolder{} + } + } +} + +func (w *customWorkspace) start() { + w.mu.Lock() + defer w.mu.Unlock() + + w.loadLocked(sortedURIs(w.folders)) +} + +func (w *customWorkspace) shutdown() { + w.mu.Lock() + defer w.mu.Unlock() + + w.loader = nil + for _, folder := range w.folders { + if folder.cancel != nil { + folder.cancel() + folder.cancel = nil + } + } +} + +func (w *customWorkspace) changeFolders(added, removed []uri.URI) { + w.mu.Lock() + defer w.mu.Unlock() + + removedAny := false + for _, folder := range removed { + state, exists := w.folders[folder] + if !exists { + continue + } + if state.cancel != nil { + state.cancel() + } + + delete(w.folders, folder) + removedAny = true + } + + if removedAny { + w.reconcileLocked(context.Background()) + } + + toLoad := make([]uri.URI, 0, len(added)) + for _, folder := range added { + if _, exists := w.folders[folder]; exists { + continue + } + + w.folders[folder] = &workspaceFolder{} + toLoad = append(toLoad, folder) + } + + w.loadLocked(toLoad) +} + +// loadLocked starts one asynchronous batch. The batch is committed only after +// every loader returns, so every discovered view exists before target routing. +func (w *customWorkspace) loadLocked(folders []uri.URI) { + if w.loader == nil || len(folders) == 0 { + return + } + + loader := w.loader + results := make([]workspaceLoadResult, 0, len(folders)) + contexts := make([]context.Context, 0, len(folders)) + + for _, folder := range folders { + state, active := w.folders[folder] + if !active || state.cancel != nil { + continue + } + + ctx, cancel := context.WithCancel(context.Background()) + state.cancel = cancel + results = append(results, workspaceLoadResult{folder: folder, state: state, cancel: cancel}) + contexts = append(contexts, ctx) + } + + if len(results) == 0 { + return + } + + go func() { + for i := range results { + snapshot, err := loader(contexts[i], results[i].folder) + results[i].cancel() + if err == nil { + snapshot = cloneWorkspaceSnapshot(snapshot) + } + results[i].snapshot = snapshot + results[i].err = err + } + + w.commit(results) + }() +} + +func (w *customWorkspace) commit(results []workspaceLoadResult) { + w.mu.Lock() + defer w.mu.Unlock() + + if w.loader == nil { + return + } + + accepted := false + for _, result := range results { + folder, active := w.folders[result.folder] + if !active || folder != result.state { + continue + } + + folder.cancel = nil + accepted = true + if result.err != nil { + logError("workspace loading failed", fmt.Errorf("load workspace %s: %w", result.folder, result.err)) + + continue + } + + folder.snapshot = validateWorkspaceSnapshot(result.folder, result.snapshot) + } + + if accepted { + w.reconcileLocked(context.Background()) + } +} + +func (w *customWorkspace) reconcileLocked(ctx context.Context) { + snapshots := make(map[uri.URI]WorkspaceSnapshot) + for folder, state := range w.folders { + snapshots[folder] = state.snapshot + } + + next := workspaceModelOf(snapshots) + beforeIssues := w.model.issues + known := make(map[uri.URI][]uri.URI, len(w.views)) + for root, view := range w.views { + known[root] = view.KnownFiles() + } + + for root := range w.views { + project, exists := next.roots[root] + previous := w.model.roots[root] + if exists && slices.Equal(previous.IncludePaths, project.IncludePaths) { + var lost []uri.URI + for _, file := range w.model.ownedFiles(root, w.documents) { + owner, owned := next.ownerOf(file, w.documents) + if !owned || owner != root { + lost = append(lost, file) + } + } + + if len(lost) > 0 { + w.views[root].Evict(lost...) + w.server.clearDiagnostics(ctx, lost...) + } + + continue + } + + w.views[root].Evict(known[root]...) + w.server.clearDiagnostics(ctx, known[root]...) + w.server.removeView(root) + delete(w.views, root) + } + + for _, root := range sortedURIs(next.roots) { + if w.views[root] == nil { + w.views[root] = w.server.addProjectView(next.roots[root]) + } + } + + w.model = next + + changes := make(map[uri.URI][]uri.URI, len(w.views)) + for target, root := range next.targets { + changes[root] = append(changes[root], target) + } + for document := range w.documents { + if root, ok := next.ownerOf(document, w.documents); ok { + changes[root] = append(changes[root], document) + } + } + + w.updateViewsLocked(ctx, changes) + w.publishIssueChangesLocked(ctx, beforeIssues, next.issues) +} + +func (w *customWorkspace) updateViewsLocked(ctx context.Context, changes map[uri.URI][]uri.URI) { + for _, root := range sortedURIs(changes) { + view := w.views[root] + if view == nil { + continue + } + + files := changes[root] + slices.Sort(files) + files = slices.Compact(files) + updates := make([]*cache.FileChange, len(files)) + for i, file := range files { + updates[i] = &cache.FileChange{URI: file, From: cache.FileChangeTypeInitialize} + } + + w.server.postDiagnostics(ctx, view, view.Update(ctx, updates...)) + } +} + +func (w *customWorkspace) applyChanges(ctx context.Context, changes []*cache.FileChange, overlay bool) error { + w.mu.Lock() + defer w.mu.Unlock() + + if overlay { + if err := w.server.session.UpdateOverlayFS(ctx, changes); err != nil { + return err + } + } + + byRoot := make(map[uri.URI][]uri.URI) + var evicted []uri.URI + for _, change := range changes { + previousRoot, previouslyOwned := w.model.ownerOf(change.URI, w.documents) + if overlay { + if change.From == cache.FileChangeTypeDidClose { + delete(w.documents, change.URI) + } else { + w.documents[change.URI] = struct{}{} + } + } + + if root, ok := w.model.ownerOf(change.URI, w.documents); ok { + byRoot[root] = append(byRoot[root], change.URI) + continue + } + + if change.From == cache.FileChangeTypeDidClose && previouslyOwned { + if view := w.views[previousRoot]; view != nil { + view.Evict(change.URI) + evicted = append(evicted, change.URI) + } + } + } + + if len(evicted) > 0 { + w.server.clearDiagnostics(ctx, evicted...) + } + w.updateViewsLocked(ctx, byRoot) + + return nil +} + +func (w *customWorkspace) viewOf(file uri.URI) (*cache.View, error) { + w.mu.Lock() + defer w.mu.Unlock() + + root, ok := w.model.ownerOf(file, w.documents) + if !ok || w.views[root] == nil { + return nil, fmt.Errorf("no workspace project owns %s", file) + } + + return w.views[root], nil +} + +func (w *customWorkspace) owns(file uri.URI) bool { + w.mu.Lock() + defer w.mu.Unlock() + + _, ok := w.model.ownerOf(file, w.documents) + + return ok +} + +func (w *customWorkspace) files(view *cache.View) []uri.URI { + w.mu.Lock() + defer w.mu.Unlock() + + if w.views[view.Folder()] != view { + return nil + } + + return w.model.ownedFiles(view.Folder(), w.documents) +} + +func (w *customWorkspace) publishIssueChangesLocked(ctx context.Context, before, after map[uri.URI][]WorkspaceIssue) { + changed := make(map[uri.URI]struct{}, len(before)+len(after)) + for issueURI := range before { + changed[issueURI] = struct{}{} + } + for issueURI := range after { + changed[issueURI] = struct{}{} + } + + for _, issueURI := range sortedURIs(changed) { + if slices.Equal(before[issueURI], after[issueURI]) { + continue + } + + w.server.publishWorkspaceIssues(ctx, issueURI, after[issueURI]) + } +} + +func (s *Server) publishWorkspaceIssues(ctx context.Context, issueURI uri.URI, issues []WorkspaceIssue) { + if s.client == nil { + return + } + + diagnostics := make([]protocol.Diagnostic, len(issues)) + for i, issue := range issues { + diagnostics[i] = protocol.Diagnostic{ + Range: protocol.Range{ + Start: protocol.Position{}, + End: protocol.Position{}, + }, + Severity: protocol.DiagnosticSeverityError, + Source: protocol.NewOptional("thrift-ls"), + Message: protocol.String(issue.Message), + } + } + + if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{ + URI: issueURI, + Diagnostics: diagnostics, + }); err != nil { + logError("workspace issue diagnostic failed", err, "uri", issueURI) + } +} + +func sortedURIs[V any](values map[uri.URI]V) []uri.URI { + result := make([]uri.URI, 0, len(values)) + for value := range values { + result = append(result, value) + } + slices.Sort(result) + + return result +} diff --git a/lsp/workspace_lifecycle_test.go b/lsp/workspace_lifecycle_test.go new file mode 100644 index 0000000..ca0d3ab --- /dev/null +++ b/lsp/workspace_lifecycle_test.go @@ -0,0 +1,727 @@ +package lsp + +import ( + "context" + "errors" + "fmt" + "sync/atomic" + "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" +) + +func TestCustomDocumentChangesDuringWorkspaceLoad(t *testing.T) { + tests := []struct { + name string + afterOpen func(context.Context, *Server, uri.URI) error + wantDef string + wantOverlay bool + }{ + { + name: "open then change", + afterOpen: func(ctx context.Context, srv *Server, file uri.URI) error { + return srv.DidChange(ctx, &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{URI: file}, + Version: 1, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + &protocol.TextDocumentContentChangeWholeDocument{Text: "struct ChangedVersion {}"}, + }, + }) + }, + wantDef: "ChangedVersion", + wantOverlay: true, + }, + { + name: "open then close", + afterOpen: func(ctx context.Context, srv *Server, file uri.URI) error { + return srv.DidClose(ctx, &protocol.DidCloseTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: file}, + }) + }, + wantDef: "DiskVersion", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + file := uri.File("/workspace/api.thrift") + started := make(chan struct{}) + release := make(chan struct{}) + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + close(started) + <-release + + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project.json"), + RootURI: workspace, + TargetFiles: []uri.URI{file}, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + file: []byte("struct DiskVersion {}"), + }), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + <-started + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: file, + LanguageID: LanguageIDThrift, + Text: "struct OpenVersion {}", + }, + })) + changeErr := tt.afterOpen(t.Context(), srv, file) + + close(release) + synctest.Wait() + require.NoError(t, changeErr) + + view, err := srv.session.ViewOf(file) + require.NoError(t, err) + parsed, err := view.Parse(t.Context(), file) + require.NoError(t, err) + assert.Contains(t, parsed.Definitions(), tt.wantDef) + assert.NotContains(t, parsed.Definitions(), "OpenVersion") + assert.Equal(t, tt.wantOverlay, srv.session.HasOverlay(file)) + }) + }) + } +} + +func TestCustomChangesNeverUseAnotherWorkspaceFallback(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + badFolder := uri.File("/bad") + badFile := uri.File("/bad/api.thrift") + goodFolder := uri.File("/good") + goodFile := uri.File("/good/api.thrift") + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + if folder == badFolder { + return WorkspaceSnapshot{}, errors.New("bad workspace") + } + + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/good/project.json"), + RootURI: goodFolder, + TargetFiles: []uri.URI{goodFile}, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + goodFile: []byte("struct Good {}"), + }), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: badFolder}, {URI: goodFolder}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + goodView, err := srv.session.ViewOf(goodFile) + require.NoError(t, err) + _, err = srv.viewOf(badFile) + require.Error(t, err, "a file without snapshot ownership must not use another workspace's view") + + openDocument(t, srv, badFile, "struct BadOpen {}") + require.NoError(t, srv.DidChange(t.Context(), &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{URI: badFile}, + Version: 1, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + &protocol.TextDocumentContentChangeWholeDocument{Text: "struct BadChanged {}"}, + }, + })) + assert.False(t, goodView.FileKnown(badFile)) + + require.NoError(t, srv.DidClose(t.Context(), &protocol.DidCloseTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: badFile}, + })) + assert.False(t, goodView.FileKnown(badFile)) + assert.False(t, srv.session.HasOverlay(badFile)) + }) +} + +func TestCustomWatchedNonTargetIsNotIndexed(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + root := uri.File("/workspace/project") + target := uri.File("/workspace/project/api.thrift") + nonTarget := uri.File("/workspace/project/stray.thrift") + client := &diagClient{} + loader := WorkspaceLoader(func(context.Context, uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + RootURI: root, + TargetFiles: []uri.URI{target}, + }}}, nil + }) + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + target: []byte("struct API {}"), + nonTarget: []byte("struct Stray { 1: Missing value }"), + }), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: root}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + client.reset() + + require.NoError(t, srv.DidChangeWatchedFiles(t.Context(), &protocol.DidChangeWatchedFilesParams{ + Changes: []protocol.FileEvent{{URI: nonTarget, Type: protocol.FileChangeTypeChanged}}, + })) + synctest.Wait() + + view, err := srv.session.ViewOf(target) + require.NoError(t, err) + assert.False(t, view.FileKnown(nonTarget)) + assert.Empty(t, client.last(nonTarget), "watched non-target must not publish diagnostics") + assert.Nil(t, srv.reportFor(nonTarget), "watched non-target must not cache diagnostics") + result, err := srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok := result.(protocol.SymbolInformationSlice) + require.True(t, ok) + names := symbolNames(symbols) + assert.Contains(t, names, "API") + assert.NotContains(t, names, "Stray") + }) +} + +func TestCustomWorkspaceSymbolsExcludeIncludedNonTarget(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + root := uri.File("/workspace/project") + target := uri.File("/workspace/project/api.thrift") + included := uri.File("/workspace/project/stray.thrift") + loader := WorkspaceLoader(func(context.Context, uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + RootURI: root, + TargetFiles: []uri.URI{target}, + }}}, nil + }) + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + target: []byte("include \"stray.thrift\"\nstruct API { 1: stray.Stray value }"), + included: []byte("struct Stray {}"), + }), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: root}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + view, err := srv.viewOf(target) + require.NoError(t, err) + require.True(t, view.FileKnown(included), "recursive analysis must retain included files in the view cache") + + result, err := srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok := result.(protocol.SymbolInformationSlice) + require.True(t, ok) + names := symbolNames(symbols) + assert.Contains(t, names, "API") + assert.NotContains(t, names, "Stray") + + locations, err := srv.definition(t.Context(), &protocol.DefinitionParams{ + TextDocumentPositionParams: protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: target}, + Position: protocol.Position{Line: 1, Character: 24}, + }, + }) + require.NoError(t, err) + require.Len(t, locations, 1) + assert.Equal(t, included, locations[0].URI) + }) +} + +func TestCustomWatchedFilesDoNotReadUnownedEvents(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + root := uri.File("/workspace/project") + target := uri.File("/workspace/project/api.thrift") + unowned := uri.File("/workspace/project/stray.thrift") + fs := &watchSpyFS{ + FileSource: cache.NewMemFS(map[uri.URI][]byte{target: []byte("struct Before {}")}), + forbidden: unowned, + } + loader := WorkspaceLoader(func(context.Context, uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + RootURI: root, + TargetFiles: []uri.URI{target}, + }}}, nil + }) + srv := NewServer(fs, nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: root}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + fs.reads.Store(0) + fs.contents.Store(0) + + for _, eventType := range []protocol.FileChangeType{ + protocol.FileChangeTypeChanged, + protocol.FileChangeTypeCreated, + protocol.FileChangeTypeDeleted, + } { + err = srv.DidChangeWatchedFiles(t.Context(), &protocol.DidChangeWatchedFilesParams{Changes: []protocol.FileEvent{ + {URI: unowned, Type: eventType}, + }}) + require.NoError(t, err) + } + assert.Zero(t, fs.reads.Load()) + assert.Zero(t, fs.contents.Load()) + + err = srv.DidChangeWatchedFiles(t.Context(), &protocol.DidChangeWatchedFilesParams{Changes: []protocol.FileEvent{ + {URI: target, Type: protocol.FileChangeTypeChanged}, + }}) + require.NoError(t, err) + synctest.Wait() + assert.Positive(t, fs.reads.Load()) + assert.Positive(t, fs.contents.Load()) + }) +} + +type watchSpyFS struct { + cache.FileSource + forbidden uri.URI + reads atomic.Int32 + contents atomic.Int32 +} + +func (fs *watchSpyFS) ReadFile(ctx context.Context, file uri.URI) (cache.FileHandle, error) { + fs.reads.Add(1) + if file == fs.forbidden { + return nil, errors.New("unowned file was read") + } + + handle, err := fs.FileSource.ReadFile(ctx, file) + if err != nil { + return nil, err + } + + return &watchSpyHandle{FileHandle: handle, contents: &fs.contents}, nil +} + +type watchSpyHandle struct { + cache.FileHandle + contents *atomic.Int32 +} + +func (h *watchSpyHandle) Content() ([]byte, error) { + h.contents.Add(1) + + return h.FileHandle.Content() +} + +func TestCustomClosingOpenNonTargetEvictsIt(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + root := uri.File("/workspace/project") + target := uri.File("/workspace/project/api.thrift") + nonTarget := uri.File("/workspace/project/stray.thrift") + client := &diagClient{} + loader := WorkspaceLoader(func(context.Context, uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + RootURI: root, + TargetFiles: []uri.URI{target}, + }}}, nil + }) + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + target: []byte("struct API {}"), + nonTarget: []byte("struct DiskStray { 1: Missing value }"), + }), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: root}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + openDocument(t, srv, nonTarget, "struct OpenStray { 1: Missing value }") + synctest.Wait() + view, err := srv.session.ViewOf(target) + require.NoError(t, err) + require.True(t, view.FileKnown(nonTarget)) + require.NotEmpty(t, client.last(nonTarget)) + result, err := srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok := result.(protocol.SymbolInformationSlice) + require.True(t, ok) + assert.Contains(t, symbolNames(symbols), "OpenStray") + + require.NoError(t, srv.DidClose(t.Context(), &protocol.DidCloseTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: nonTarget}, + })) + synctest.Wait() + + assert.False(t, srv.session.HasOverlay(nonTarget)) + assert.False(t, view.FileKnown(nonTarget)) + assert.Empty(t, client.last(nonTarget), "closing the final non-target owner must clear diagnostics") + result, err = srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok = result.(protocol.SymbolInformationSlice) + require.True(t, ok) + assert.Equal(t, []string{"API"}, symbolNames(symbols)) + }) +} + +func TestWorkspaceFolderAdditionIsCanceledOnShutdown(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folder := uri.File("/added") + started := make(chan struct{}) + canceled := make(chan error, 1) + release := make(chan struct{}) + + loader := WorkspaceLoader(func(ctx context.Context, got uri.URI) (WorkspaceSnapshot, error) { + close(started) + + select { + case <-ctx.Done(): + canceled <- ctx.Err() + + return WorkspaceSnapshot{}, ctx.Err() + case <-release: + return WorkspaceSnapshot{}, nil + } + }) + + srv := NewServer(cache.NewMemFS(nil), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + _, err := srv.Initialize(t.Context(), &protocol.InitializeParams{}) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + + result := make(chan error, 1) + go func() { + result <- srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Added: []protocol.WorkspaceFolder{{URI: folder}}, + }, + }) + }() + <-started + + require.NoError(t, srv.Shutdown(t.Context())) + synctest.Wait() + + select { + case err := <-canceled: + require.ErrorIs(t, err, context.Canceled) + require.NoError(t, <-result, "workspace-folder notifications must not return loader errors") + default: + close(release) + synctest.Wait() + t.Fatal("workspace-folder loader did not observe shutdown cancellation") + } + }) +} + +func TestRemovedFolderIsNotResurrectedByInFlightLoad(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folder := uri.File("/workspace") + projectRoot := uri.File("/workspace/project") + started := make(chan struct{}) + release := make(chan struct{}) + + loader := WorkspaceLoader(func(ctx context.Context, got uri.URI) (WorkspaceSnapshot, error) { + close(started) + <-release // Deliberately ignore cancellation to exercise generation invalidation. + + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project.json"), + RootURI: projectRoot, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(nil), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + _, err := srv.Initialize(t.Context(), &protocol.InitializeParams{}) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + + result := make(chan error, 1) + go func() { + result <- srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Added: []protocol.WorkspaceFolder{{URI: folder}}, + }, + }) + }() + <-started + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folder}}, + }, + })) + close(release) + synctest.Wait() + + require.NoError(t, <-result) + assertViewMissing(t, srv, projectRoot) + }) +} + +func TestWorkspaceTargetsAreIndexedOncePerView(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + first := uri.File("/workspace/first.thrift") + second := uri.File("/workspace/second.thrift") + client := &diagClient{} + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project.json"), + RootURI: workspace, + TargetFiles: []uri.URI{first, second}, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + first: []byte("struct First {"), + second: []byte("struct Second {"), + }), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + view, err := srv.session.ViewOf(first) + require.NoError(t, err) + assert.Equal(t, uint64(1), view.Generation()) + assert.NotEmpty(t, client.last(first)) + assert.NotEmpty(t, client.last(second)) + }) +} + +func TestWorkspaceIssuesPersistAcrossFolders(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folderA := uri.File("/workspace-a") + folderB := uri.File("/workspace-b") + issueURI := uri.File("/shared/project.json") + client := &diagClient{} + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Issues: []WorkspaceIssue{{ + URI: issueURI, + Message: fmt.Sprintf("issue from %s", folder), + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(nil), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: folderA}, {URI: folderB}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + assert.ElementsMatch(t, []string{ + "issue from file:///workspace-a", + "issue from file:///workspace-b", + }, diagMessages(client.last(issueURI))) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folderB}}, + }, + })) + assert.Equal(t, []string{"issue from file:///workspace-a"}, diagMessages(client.last(issueURI))) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folderA}}, + }, + })) + assert.Empty(t, client.last(issueURI), "removing the final owner must clear stale diagnostics") + }) +} + +func TestSharedProjectViewSurvivesWorkspaceFolderRemoval(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folderA := uri.File("/workspace-a") + folderB := uri.File("/workspace-b") + projectRoot := uri.File("/shared/project") + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.URI(fmt.Sprintf("%s/project.json", folder)), + RootURI: projectRoot, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(nil), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: folderA}, {URI: folderB}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + assertViewPresent(t, srv, projectRoot) + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folderA}}, + }, + })) + assertViewPresent(t, srv, projectRoot) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folderB}}, + }, + })) + assertViewMissing(t, srv, projectRoot) + }) +} + +func TestSharedRootEvictsTargetsWhenFolderLosesOwnership(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folderA := uri.File("/workspace-a") + folderB := uri.File("/workspace-b") + root := uri.File("/shared/project") + targetA := uri.File("/shared/project/a.thrift") + targetB := uri.File("/shared/project/b.thrift") + client := &diagClient{} + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + target := targetA + if folder == folderB { + target = targetB + } + + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File(fmt.Sprintf("%s/project.json", folder)), + RootURI: root, + TargetFiles: []uri.URI{target}, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + targetA: []byte("struct A {}"), + targetB: []byte("struct B { 1: Missing value }"), + }), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: folderA}, {URI: folderB}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + view, err := srv.session.ViewOf(targetA) + require.NoError(t, err) + assert.True(t, view.FileKnown(targetA)) + assert.True(t, view.FileKnown(targetB)) + assert.NotEmpty(t, client.last(targetB)) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folderB}}, + }, + })) + synctest.Wait() + + assert.False(t, view.FileKnown(targetB), "a target removed from the snapshot must leave the retained view") + assert.Empty(t, client.last(targetB), "removing the final target owner must clear its diagnostics") + result, err := srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok := result.(protocol.SymbolInformationSlice) + require.True(t, ok) + assert.Equal(t, []string{"A"}, symbolNames(symbols)) + }) +} + +func TestRemovingFinalCustomViewClearsDiagnostics(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folder := uri.File("/workspace") + root := uri.File("/workspace/project") + target := uri.File("/workspace/project/api.thrift") + client := &diagClient{} + + loader := WorkspaceLoader(func(ctx context.Context, got uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project.json"), + RootURI: root, + TargetFiles: []uri.URI{target}, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + target: []byte("struct A { 1: Missing value }"), + }), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: folder}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + require.NotEmpty(t, client.last(target)) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folder}}, + }, + })) + + assert.Empty(t, srv.session.Views()) + assert.Empty(t, client.last(target), "removing the final view must publish a clear diagnostic set") + }) +} + +func TestCustomOpenOverlayMovesToNewOwner(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + outerFolder := uri.File("/workspace") + innerFolder := uri.File("/workspace/service") + file := uri.File("/workspace/service/api.thrift") + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + if folder == innerFolder { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/service/project.json"), + RootURI: innerFolder, + }}}, nil + } + + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project.json"), + RootURI: outerFolder, + TargetFiles: []uri.URI{file}, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + file: []byte("struct DiskVersion {}"), + }), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: outerFolder}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + openDocument(t, srv, file, "struct OverlayVersion {}") + outerView, err := srv.session.ViewOf(file) + require.NoError(t, err) + assert.Equal(t, outerFolder, outerView.Folder()) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Added: []protocol.WorkspaceFolder{{URI: innerFolder}}, + }, + })) + synctest.Wait() + + innerView, err := srv.session.ViewOf(file) + require.NoError(t, err) + assert.Equal(t, innerFolder, innerView.Folder()) + assert.False(t, outerView.FileKnown(file)) + parsed, err := innerView.Parse(t.Context(), file) + require.NoError(t, err) + assert.Contains(t, parsed.Definitions(), "OverlayVersion") + assert.True(t, srv.session.HasOverlay(file)) + }) +} diff --git a/lsp/workspace_loader_test.go b/lsp/workspace_loader_test.go new file mode 100644 index 0000000..2c1d242 --- /dev/null +++ b/lsp/workspace_loader_test.go @@ -0,0 +1,527 @@ +package lsp + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "slices" + "sync/atomic" + "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/formatter" + "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/options" +) + +func TestWorkspaceLoaderUsesConfigFinderPerProjectRoot(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + roots := []string{filepath.Join(dir, "one"), filepath.Join(dir, "two")} + widths := []int{91, 92} + configs := make(map[string]string, len(roots)) + projects := make([]Project, len(roots)) + files := make(map[uri.URI][]byte, len(roots)) + for i, root := range roots { + require.NoError(t, os.MkdirAll(root, 0o755)) + config := filepath.Join(root, options.ConfigFileName) + require.NoError(t, os.WriteFile(config, fmt.Appendf(nil, `{"printWidth":%d}`, widths[i]), 0o644)) + configs[root] = config + target := uri.File(filepath.Join(root, "api.thrift")) + files[target] = []byte("struct API {}") + projects[i] = Project{ + ConfigURI: uri.File(filepath.Join(root, "tbuild.yaml")), + RootURI: uri.File(root), + TargetFiles: []uri.URI{target}, + } + } + + var calls []string + finder := func(root string) (string, error) { + calls = append(calls, root) + + return configs[root], nil + } + loader := func(context.Context, uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: projects}, nil + } + srv := NewServer(cache.NewMemFS(files), nil, Options{ + ConfigFinder: finder, + WorkspaceLoader: loader, + }) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: uri.File(dir)}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + slices.Sort(calls) + assert.Equal(t, roots, calls) + for i, root := range roots { + cfg := srv.folderConfig(uri.File(root)) + require.NotNil(t, cfg.PrintWidth) + assert.Equal(t, widths[i], *cfg.PrintWidth) + } + }) +} + +func TestExplicitConfigPathBypassesConfigFinder(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + root := uri.File("/workspace/project") + target := uri.File("/workspace/project/api.thrift") + width := 97 + var calls atomic.Int32 + finder := func(string) (string, error) { + calls.Add(1) + + return "", nil + } + loader := func(context.Context, uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File("/workspace/project/tbuild.yaml"), + RootURI: root, + TargetFiles: []uri.URI{target}, + }}}, nil + } + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{target: []byte("struct API {}")}), nil, Options{ + Config: options.Patch{FormatPatch: formatter.FormatPatch{PrintWidth: &width}}, + ConfigPath: "/pinned/thrift-ls.json", + ConfigFinder: finder, + WorkspaceLoader: loader, + }) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: uri.File("/workspace")}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + assert.Zero(t, calls.Load()) + cfg := srv.folderConfig(root) + require.NotNil(t, cfg.PrintWidth) + assert.Equal(t, width, *cfg.PrintWidth) + }) +} + +func TestWorkspaceLoaderDefersViewsAndIndexesOverlayInMostSpecificProject(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + outerFile := uri.File("/workspace/root.thrift") + innerRoot := uri.File("/workspace/service") + innerFile := uri.File("/workspace/service/api.thrift") + externalFile := uri.File("/dependencies/shared.thrift") + + started := make(chan struct{}) + release := make(chan struct{}) + var calls atomic.Int32 + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + calls.Add(1) + close(started) + <-release + + return WorkspaceSnapshot{ + Projects: []Project{ + { + 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}, + IncludePaths: []string{"/inner/includes"}, + }, + }, + }, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + outerFile: []byte("struct Root {}"), + innerFile: []byte("struct DiskVersion {}"), + externalFile: []byte("struct Shared {}"), + }), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + assert.Zero(t, calls.Load(), "loader must not run during initialize") + assert.Empty(t, srv.session.Views()) + + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + <-started + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: innerFile, + LanguageID: LanguageIDThrift, + Version: 1, + Text: "struct OverlayVersion {}", + }, + })) + assert.Empty(t, srv.session.Views(), "didOpen must not create a fallback view while loading") + + close(release) + synctest.Wait() + + require.Equal(t, int32(1), calls.Load()) + require.Len(t, srv.session.Views(), 2) + + outerView, err := srv.session.ViewOf(outerFile) + require.NoError(t, err) + assert.Equal(t, workspace, outerView.Folder()) + assert.True(t, outerView.FileKnown(outerFile)) + + innerView, err := srv.session.ViewOf(innerFile) + require.NoError(t, err) + assert.Equal(t, innerRoot, innerView.Folder()) + assert.True(t, innerView.FileKnown(innerFile), "all views must exist before targets are routed") + assert.Equal(t, []string{"/inner/includes"}, innerView.Resolver().IncludePaths()) + + parsed, err := innerView.Parse(t.Context(), innerFile) + require.NoError(t, err) + assert.Contains(t, parsed.Definitions(), "OverlayVersion") + assert.NotContains(t, parsed.Definitions(), "DiskVersion") + + externalView, err := srv.session.ViewOf(externalFile) + require.NoError(t, err) + assert.Equal(t, innerRoot, externalView.Folder(), "an external target stays in its declared project") + + result, err := srv.Symbols(t.Context(), &protocol.WorkspaceSymbolParams{Query: ""}) + require.NoError(t, err) + symbols, ok := result.(protocol.SymbolInformationSlice) + require.True(t, ok) + assert.Contains(t, symbolNames(symbols), "Shared", "an external target remains a workspace symbol") + }) +} + +func TestWorkspaceLoaderIsCanceledOnShutdown(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + started := make(chan struct{}) + canceled := make(chan error, 1) + release := make(chan struct{}) + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + close(started) + + select { + case <-ctx.Done(): + canceled <- ctx.Err() + + return WorkspaceSnapshot{}, ctx.Err() + case <-release: + return WorkspaceSnapshot{}, nil + } + }) + + srv := NewServer(cache.NewMemFS(nil), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + + requestCtx, cancelRequest := context.WithCancel(t.Context()) + require.NoError(t, srv.Initialized(requestCtx, &protocol.InitializedParams{})) + <-started + + cancelRequest() + synctest.Wait() + + select { + case err := <-canceled: + close(release) + t.Fatalf("loader was canceled with Initialized request: %v", err) + default: + } + + require.NoError(t, srv.Shutdown(t.Context())) + synctest.Wait() + + select { + case err := <-canceled: + require.ErrorIs(t, err, context.Canceled) + default: + close(release) + synctest.Wait() + t.Fatal("loader did not observe cancellation on shutdown") + } + }) +} + +func TestWorkspaceLoaderFailureDoesNotCreateFallbackView(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + file := uri.File("/workspace/api.thrift") + started := make(chan struct{}) + release := make(chan struct{}) + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + close(started) + <-release + + return WorkspaceSnapshot{}, errors.New("discovery failed") + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + file: []byte("struct DiskVersion {}"), + }), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + <-started + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: file, + LanguageID: LanguageIDThrift, + Text: "struct OverlayVersion {}", + }, + })) + close(release) + synctest.Wait() + + assert.Empty(t, srv.session.Views(), "a deferred open must not create a stock view after loader failure") + assert.True(t, srv.session.HasOverlay(file), "the editor overlay remains available for a later snapshot") + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: file, + LanguageID: LanguageIDThrift, + Version: 1, + Text: "struct LaterOverlayVersion {}", + }, + })) + assert.Empty(t, srv.session.Views(), "later opens remain owned by the custom loader") + }) +} + +func TestWorkspaceLoaderPublishesIssuesWithoutDroppingProjects(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + target := uri.File("/workspace/api.thrift") + issueURI := uri.File("/workspace/project.json") + client := &diagClient{} + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{ + Projects: []Project{{ + ConfigURI: issueURI, + RootURI: workspace, + TargetFiles: []uri.URI{target}, + }}, + Issues: []WorkspaceIssue{{ + URI: issueURI, + Message: "dependency project could not be loaded", + }, { + URI: issueURI, + Message: "target could not be resolved", + }}, + }, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + target: []byte("struct API {}"), + }), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + view, err := srv.session.ViewOf(target) + require.NoError(t, err) + assert.True(t, view.FileKnown(target)) + + diagnostics := client.last(issueURI) + require.Len(t, diagnostics, 2) + assert.Equal(t, protocol.DiagnosticSeverityError, diagnostics[0].Severity) + assert.Equal(t, protocol.String("dependency project could not be loaded"), diagnostics[0].Message) + assert.Equal(t, protocol.String("target could not be resolved"), diagnostics[1].Message) + }) +} + +func TestWorkspaceLoaderIssuesOnlyDoesNotCreateFallbackView(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + file := uri.File("/workspace/api.thrift") + issueURI := uri.File("/workspace/project.json") + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Issues: []WorkspaceIssue{{ + URI: issueURI, + Message: "project is invalid", + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(nil), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + require.NoError(t, srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: file, + LanguageID: LanguageIDThrift, + Text: "struct API {}", + }, + })) + assert.Empty(t, srv.session.Views()) + assert.True(t, srv.session.HasOverlay(file)) + }) +} + +func TestWorkspaceLoaderHandlesWorkspaceFolderChanges(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + folderA := uri.File("/workspace-a") + folderB := uri.File("/workspace-b") + projectRoot := func(folder uri.URI) uri.URI { + return uri.File(filepath.Join(folder.FsPath(), "service")) + } + target := func(folder uri.URI) uri.URI { + return uri.File(filepath.Join(projectRoot(folder).FsPath(), "api.thrift")) + } + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + if folder != folderA && folder != folderB { + return WorkspaceSnapshot{}, errors.New("unknown workspace folder") + } + + return WorkspaceSnapshot{Projects: []Project{{ + ConfigURI: uri.File(filepath.Join(folder.FsPath(), "project.json")), + RootURI: projectRoot(folder), + TargetFiles: []uri.URI{target(folder)}, + }}}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + target(folderA): []byte("struct A {}"), + target(folderB): []byte("struct B {}"), + }), nil, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: folderA}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Added: []protocol.WorkspaceFolder{{URI: folderB}}, + }, + })) + synctest.Wait() + assertViewPresent(t, srv, projectRoot(folderA)) + assertViewPresent(t, srv, projectRoot(folderB)) + + require.NoError(t, srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Removed: []protocol.WorkspaceFolder{{URI: folderB}}, + }, + })) + assertViewPresent(t, srv, projectRoot(folderA)) + assertViewMissing(t, srv, projectRoot(folderB)) + + err = srv.DidChangeWorkspaceFolders(t.Context(), &protocol.DidChangeWorkspaceFoldersParams{ + Event: protocol.WorkspaceFoldersChangeEvent{ + Added: []protocol.WorkspaceFolder{{URI: uri.File("/unknown")}}, + }, + }) + require.NoError(t, err) + synctest.Wait() + assertViewPresent(t, srv, projectRoot(folderA)) + }) +} + +func TestWorkspaceLoaderRejectsInvalidProjectsAndConflictingRoots(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + workspace := uri.File("/workspace") + validRoot := uri.File("/workspace/service") + validTarget := uri.File("/workspace/service/api.thrift") + invalidRootTarget := uri.File("/invalid-root/api.thrift") + invalidConfigTarget := uri.File("/invalid-config/api.thrift") + conflictingTarget := uri.File("/workspace/service/conflicting.thrift") + client := &diagClient{} + + loader := WorkspaceLoader(func(ctx context.Context, folder uri.URI) (WorkspaceSnapshot, error) { + return WorkspaceSnapshot{Projects: []Project{ + { + ConfigURI: uri.File("/workspace/invalid-root.json"), + RootURI: uri.URI("not a URI"), + TargetFiles: []uri.URI{invalidRootTarget}, + }, + { + RootURI: uri.File("/workspace/invalid-config"), + TargetFiles: []uri.URI{invalidConfigTarget}, + }, + { + 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}, + IncludePaths: []string{"/second/includes"}, + }, + }}, nil + }) + + srv := NewServer(cache.NewMemFS(map[uri.URI][]byte{ + validTarget: []byte("struct Valid {}"), + invalidRootTarget: []byte("struct InvalidRoot {}"), + invalidConfigTarget: []byte("struct InvalidConfig {}"), + conflictingTarget: []byte("struct Conflicting {}"), + }), client, Options{WorkspaceLoader: loader, ConfigPath: "pinned"}) + + _, err := srv.Initialize(t.Context(), testInitializeParams([]protocol.WorkspaceFolder{{URI: workspace}})) + require.NoError(t, err) + require.NoError(t, srv.Initialized(t.Context(), &protocol.InitializedParams{})) + synctest.Wait() + + views := srv.session.Views() + require.Len(t, views, 1) + assert.Equal(t, validRoot, views[0].Folder()) + assert.Equal(t, []string{"/first/includes"}, views[0].Resolver().IncludePaths()) + assert.True(t, views[0].FileKnown(validTarget)) + assert.False(t, views[0].FileKnown(invalidRootTarget)) + assert.False(t, views[0].FileKnown(invalidConfigTarget)) + assert.False(t, views[0].FileKnown(conflictingTarget)) + + assert.NotEmpty(t, client.last(uri.File("/workspace/invalid-root.json"))) + assert.NotEmpty(t, client.last(workspace), "an invalid project without a config URI is still reported") + assert.NotEmpty(t, client.last(uri.File("/workspace/conflicting.json"))) + }) +} + +func assertViewPresent(t *testing.T, srv *Server, folder uri.URI) { + t.Helper() + + for _, view := range srv.session.Views() { + if view.Folder() == folder { + return + } + } + + t.Errorf("view %s is missing", folder) +} + +func assertViewMissing(t *testing.T, srv *Server, folder uri.URI) { + t.Helper() + + for _, view := range srv.session.Views() { + if view.Folder() == folder { + t.Errorf("view %s is still present", folder) + } + } +} diff --git a/main.go b/main.go index 4aabbc2..a9a0ef3 100644 --- a/main.go +++ b/main.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "io" "io/fs" "log/slog" "os" @@ -23,23 +22,11 @@ import ( "github.com/karitham/thrift-ls/sema" "github.com/karitham/thrift-ls/syntax" - "go.lsp.dev/jsonrpc2" - "go.lsp.dev/pkg/fakenet" "go.lsp.dev/protocol" "go.lsp.dev/uri" ) -func main() { - cmd := rootCommand() - - if err := cmd.Run(context.Background(), os.Args); err != nil { - fmt.Fprintln(os.Stderr, "thrift-ls:", err) - os.Exit(1) - } -} - -// rootCommand is the CLI: the default (no subcommand) is the language -// server, with format, dump, and check as subcommands. +// rootCommand returns the thrift-ls urfave command. func rootCommand() *cli.Command { return &cli.Command{ Name: "thrift-ls", @@ -47,13 +34,13 @@ func rootCommand() *cli.Command { Version: lsp.ServerVersion, Flags: lspFlags(), // No subcommand: run the language server. - Action: lspAction, + Action: runLSP, Commands: []*cli.Command{ { Name: "lsp", Usage: "run the language server on stdio", Flags: lspFlags(), - Action: lspAction, + Action: runLSP, }, { Name: "format", @@ -62,39 +49,47 @@ func rootCommand() *cli.Command { Flags: formatFlags(), Action: formatAction, }, - { - Name: "dump", - Usage: "dump the parse tree and document IR of a thrift file", - ArgsUsage: "", - Flags: []cli.Flag{ - &cli.BoolFlag{ - Name: "ir", - Usage: "also dump the formatted document IR with layout decisions", - }, - &cli.BoolFlag{ - Name: "ast", - Usage: "dump only the parse tree (tokens, trivia, node spans)", - }, - &cli.BoolFlag{ - Name: "includes", - Usage: "show how each include resolves instead of dumping the tree", - }, - &cli.IntFlag{ - Name: "printWidth", - Usage: "line width for the IR dump", - Value: 80, - }, - }, - Action: dumpAction, + dumpCommand(), + checkCommand(), + }, + } +} + +func dumpCommand() *cli.Command { + return &cli.Command{ + Name: "dump", + Usage: "dump the parse tree and document IR of a thrift file", + ArgsUsage: "", + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: "ir", + Usage: "also dump the formatted document IR with layout decisions", }, - { - Name: "check", - Usage: "report parse, semantic, and lint diagnostics on thrift files", - ArgsUsage: "", - Flags: lspFlags(), - Action: checkAction, + &cli.BoolFlag{ + Name: "ast", + Usage: "dump only the parse tree (tokens, trivia, node spans)", + }, + &cli.BoolFlag{ + Name: "includes", + Usage: "show how each include resolves instead of dumping the tree", + }, + &cli.IntFlag{ + Name: "printWidth", + Usage: "line width for the IR dump", + Value: 80, }, }, + Action: dumpAction, + } +} + +func checkCommand() *cli.Command { + return &cli.Command{ + Name: "check", + Usage: "report parse, semantic, and lint diagnostics on thrift files", + ArgsUsage: "", + Flags: lspFlags(), + Action: checkAction, } } @@ -180,8 +175,8 @@ func formatFlags() []cli.Flag { return flags } -// lspAction serves the language server on stdio. -func lspAction(ctx context.Context, cmd *cli.Command) error { +// runLSP serves the language server on stdio. +func runLSP(ctx context.Context, cmd *cli.Command) error { // Degrade, don't die: editors launch this process, so a broken // thrift-ls.json here would kill every buffer's language server at // startup. The reason goes to stderr (open before initialize) and @@ -215,16 +210,7 @@ func lspAction(ctx context.Context, cmd *cli.Command) error { CLI: cliPatch, } - ss := lsp.NewStreamServer(lspOpts) - stream := jsonrpc2.NewStream(fakenet.NewConn("stdio", os.Stdin, os.Stdout)) - conn := jsonrpc2.NewConn(stream) - - err = ss.ServeStream(ctx, conn) - if errors.Is(err, io.EOF) { - return nil - } - - return err + return lsp.ServeStdio(ctx, lspOpts, os.Stdin, os.Stdout) } // formatAction formats a single thrift file. @@ -236,7 +222,23 @@ func formatAction(ctx context.Context, cmd *cli.Command) error { return err } - return formatFile(file, cmd.Writer, cmd.Bool("w"), cmd.Bool("d"), cmd.String("config"), cliPatch) + return formatter.FormatFile(file, formatter.FileOptions{ + Output: cmd.Writer, + Write: cmd.Bool("w"), + Diff: cmd.Bool("d"), + ConfigPath: cmd.String("config"), + Patch: cliPatch.FormatPatch, + ResolveConfig: resolveFormatConfig, + }) +} + +func resolveFormatConfig(path, dir string) (formatter.FormatPatch, error) { + cfg, err := loadConfig(path, dir) + if err != nil { + return formatter.FormatPatch{}, err + } + + return options.Effective(cfg).FormatPatch, nil } // dumpAction prints the parse tree, and optionally the formatted document @@ -323,7 +325,10 @@ func dumpIncludes(ctx context.Context, file string, cmd *cli.Command) error { return err } - cfg := loadConfig(cmd.String("config"), filepath.Dir(abs)) + cfg, err := loadConfig(cmd.String("config"), filepath.Dir(abs)) + if err != nil { + return err + } patch := options.Effective(cfg) cliPatch, err := lspPatch(cmd) @@ -417,7 +422,10 @@ func checkAction(ctx context.Context, cmd *cli.Command) error { return fmt.Errorf("no thrift files found in %s", path) } - cfg := loadConfig(cmd.String("config"), ".") + cfg, err := loadConfig(cmd.String("config"), ".") + if err != nil { + return err + } patch := options.Effective(cfg) cliPatch, err := lspPatch(cmd) @@ -649,30 +657,30 @@ func formatPatch(cmd *cli.Command) (options.Patch, error) { // loadConfig loads the explicit config path, or finds one walking up from // dir. A missing config is not an error. -func loadConfig(path, dir string) *options.Patch { +func loadConfig(path, dir string) (*options.Patch, error) { if path == "" { var err error path, err = options.FindConfig(dir) if err != nil { - fatal(err) + return nil, err } } if path == "" { - return nil + return nil, nil } cfg, err := options.Load(path) if err != nil { - fatal(err) + return nil, err } - return cfg + return cfg, nil } // loadConfigLax is loadConfig for the language server: any failure degrades -// to nil (defaults) after printing why, never os.Exit — see lspAction. The +// to nil (defaults) after printing why, never os.Exit — see runLSP. The // strict variant stays for format/check, where a config typo should fail // fast rather than silently reformat against defaults. func loadConfigLax(path, dir string) *options.Patch { @@ -707,71 +715,6 @@ func loadConfigLax(path, dir string) *options.Patch { return cfg } -func fatal(err error) { - fmt.Fprintln(os.Stderr, "thrift-ls:", err) - os.Exit(1) -} - -// formatFile formats a single file: read, parse, resolve options, format, -// self-validate, and write, diff, or print to w. -func formatFile(file string, w io.Writer, write, diffOut bool, configPath string, cli options.Patch) error { - if file == "" { - return errors.New("must specify a thrift file to format, e.g. thrift-ls format file.thrift") - } - - src, err := os.ReadFile(file) - if err != nil { - return err - } - - absFile, err := filepath.Abs(file) - if err != nil { - return err - } - - cfg := loadConfig(configPath, filepath.Dir(absFile)) - patch := options.Effective(cfg) - patch = cli.Apply(patch) - - fopts, err := patch.FormatPatch.Options() - if err != nil { - return err - } - - parsed, errs := syntax.Parse(src) - if parseErrors(errs) { - return fmt.Errorf("%s: file does not parse:\n%s", file, formatErrors(errs)) - } - - out, err := formatter.Format(parsed, fopts) - if err != nil { - return fmt.Errorf("%s: %w", file, err) - } - - // Self-validation: the formatted output must parse cleanly. - if _, errs := syntax.Parse([]byte(out)); parseErrors(errs) { - return fmt.Errorf("%s: formatting produced invalid output", file) - } - - switch { - case write: - perms := os.FileMode(0o644) - if info, err := os.Stat(file); err == nil { - perms = info.Mode() - } - - return os.WriteFile(file, []byte(out), perms) - case diffOut: - fmt.Fprint(w, string(Diff("old", src, "new", []byte(out)))) - - return nil - default: - fmt.Fprint(w, out) - - return nil - } -} - func parseErrors(errs []syntax.Error) bool { for _, e := range errs { if e.Severity == syntax.SeverityError { @@ -822,3 +765,10 @@ func lintConfigOf(l *options.LintConfig) sema.Config { return sema.ConfigFromLint(disabled, severity) } + +func main() { + if err := rootCommand().Run(context.Background(), os.Args); err != nil { + fmt.Fprintln(os.Stderr, "thrift-ls:", err) + os.Exit(1) + } +} diff --git a/options/options.go b/options/options.go index 87ee734..b570598 100644 --- a/options/options.go +++ b/options/options.go @@ -166,18 +166,8 @@ func Load(path string) (*Patch, error) { // because relative include paths in the config anchor to it. func FindConfig(dir string) (string, error) { path := os.Getenv("THRIFT_LS_CONFIG") - if path == "" { - var err error - - path, err = findConfig(dir) - if err != nil { - return "", err - } - } - - if path == "" { - return "", nil + return FindNearestConfig(dir) } abs, err := filepath.Abs(path) @@ -188,9 +178,16 @@ func FindConfig(dir string) (string, error) { return abs, nil } -// findConfig walks up from dir to the nearest thrift-ls.json. -func findConfig(dir string) (string, error) { - for d := dir; ; d = filepath.Dir(d) { +// FindNearestConfig returns the nearest thrift-ls.json found by walking from +// dir toward the filesystem root. It returns an absolute path, or an empty +// path when no config exists. It does not read THRIFT_LS_CONFIG. +func FindNearestConfig(dir string) (string, error) { + abs, err := filepath.Abs(dir) + if err != nil { + return "", err + } + + for d := abs; ; d = filepath.Dir(d) { path := filepath.Join(d, ConfigFileName) _, err := os.Stat(path) if err == nil { diff --git a/options/options_test.go b/options/options_test.go index b3d644c..2f0a65c 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -9,56 +9,65 @@ import ( "github.com/stretchr/testify/require" ) -func TestFindConfig(t *testing.T) { +func TestFindNearestConfig(t *testing.T) { dir := t.TempDir() - sub := filepath.Join(dir, "a", "b") - if err := os.MkdirAll(sub, 0o755); err != nil { - t.Fatal(err) - } + require.NoError(t, os.MkdirAll(sub, 0o755)) - // No config anywhere. - got, err := FindConfig(sub) - if err != nil || got != "" { - t.Fatalf("FindConfig = %q, %v; want empty", got, err) - } + t.Setenv("THRIFT_LS_CONFIG", filepath.Join(dir, "hostile.json")) + + got, err := FindNearestConfig(sub) + require.NoError(t, err) + assert.Empty(t, got) - // Config in an ancestor directory is found walking up. cfgPath := filepath.Join(dir, "thrift-ls.json") - if err := os.WriteFile(cfgPath, []byte("{}"), 0o644); err != nil { - t.Fatal(err) - } + require.NoError(t, os.WriteFile(cfgPath, []byte("{}"), 0o644)) - got, err = FindConfig(sub) - if err != nil || got != cfgPath { - t.Fatalf("FindConfig = %q, %v; want %q", got, err, cfgPath) - } + got, err = FindNearestConfig(sub) + require.NoError(t, err) + assert.Equal(t, cfgPath, got) + assert.True(t, filepath.IsAbs(got)) - // A nearer config wins. near := filepath.Join(dir, "a", "thrift-ls.json") - if err := os.WriteFile(near, []byte("{}"), 0o644); err != nil { - t.Fatal(err) - } + require.NoError(t, os.WriteFile(near, []byte("{}"), 0o644)) - got, err = FindConfig(sub) - if err != nil || got != near { - t.Fatalf("FindConfig = %q, %v; want %q", got, err, near) - } + got, err = FindNearestConfig(sub) + require.NoError(t, err) + assert.Equal(t, near, got) +} + +func TestFindNearestConfigReturnsAbsolutePathForRelativeDirectory(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, ConfigFileName) + require.NoError(t, os.WriteFile(cfgPath, []byte("{}"), 0o644)) - // Discovery from a relative dir must still return an absolute path. wd, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(dir); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) defer func() { _ = os.Chdir(wd) }() - got, err = FindConfig(".") - if err != nil || got != cfgPath { - t.Fatalf("FindConfig(\".\") = %q, %v; want absolute %q", got, err, cfgPath) - } + got, err := FindNearestConfig(".") + require.NoError(t, err) + assert.Equal(t, cfgPath, got) +} + +func TestFindConfigHonorsEnvironment(t *testing.T) { + dir := t.TempDir() + nearest := filepath.Join(dir, ConfigFileName) + environment := filepath.Join(dir, "environment.json") + require.NoError(t, os.WriteFile(nearest, []byte("{}"), 0o644)) + require.NoError(t, os.WriteFile(environment, []byte("{}"), 0o644)) + t.Setenv("THRIFT_LS_CONFIG", "") + + got, err := FindConfig(dir) + require.NoError(t, err) + assert.Equal(t, nearest, got) + + t.Setenv("THRIFT_LS_CONFIG", environment) + + got, err = FindConfig(dir) + require.NoError(t, err) + assert.Equal(t, environment, got) } // A relative THRIFT_LS_CONFIG also comes back absolute. diff --git a/sema/pipeline.go b/sema/pipeline.go index e606963..c9c5fcb 100644 --- a/sema/pipeline.go +++ b/sema/pipeline.go @@ -199,6 +199,14 @@ func New(cfg Config, analyzers []Analyzer) *Pipeline { return &Pipeline{analyzers: analyzers, cfg: cfg} } +// WithAnalyzers returns a copy of the pipeline with analyzers appended. +func (p *Pipeline) WithAnalyzers(analyzers ...Analyzer) *Pipeline { + out := *p + out.analyzers = append(slices.Clone(p.analyzers), analyzers...) + + return &out +} + // WithFixers returns a copy of the pipeline with the fixers added. func (p *Pipeline) WithFixers(fs ...Fixer) *Pipeline { out := *p diff --git a/sema/pipeline_test.go b/sema/pipeline_test.go index 263688d..f68785c 100644 --- a/sema/pipeline_test.go +++ b/sema/pipeline_test.go @@ -11,6 +11,8 @@ import ( "github.com/karitham/thrift-ls/lsp/cache" ) +var _ func(Config) *Pipeline = DefaultPipeline + // runOne runs a single analyzer over view and files: the shape check // tests use to pin one check's behavior in isolation. func runOne(t *testing.T, a Analyzer, view *cache.View, files ...uri.URI) Report { diff --git a/syntax/parser.go b/syntax/parser.go index 6f33f16..09d8e00 100644 --- a/syntax/parser.go +++ b/syntax/parser.go @@ -234,7 +234,7 @@ func (n *Service) setStructured(a []*StructuredAnnotation) { // --- headers --------------------------------------------------------------- func (p *parser) parseInclude() Node { - n := &Include{first: p.nextReal(p.pos)} + n := &Include{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // include if !p.at(TokenStringLiteral) { @@ -254,7 +254,7 @@ func (p *parser) parseInclude() Node { } func (p *parser) parseCPPInclude() Node { - n := &CPPInclude{first: p.nextReal(p.pos)} + n := &CPPInclude{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // cpp_include if !p.at(TokenStringLiteral) { @@ -274,7 +274,7 @@ func (p *parser) parseCPPInclude() Node { } func (p *parser) parseNamespace() Node { - n := &Namespace{first: p.nextReal(p.pos)} + n := &Namespace{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // namespace if !p.at(TokenIdentifier) && !p.at(TokenStar) { @@ -302,7 +302,7 @@ func (p *parser) parseNamespace() Node { // --- definitions ----------------------------------------------------------- func (p *parser) parseConst() Node { - n := &Const{first: p.nextReal(p.pos)} + n := &Const{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // const n.Type = p.parseFieldType() @@ -340,7 +340,7 @@ func (p *parser) parseConst() Node { } func (p *parser) parseTypedef() Node { - n := &Typedef{first: p.nextReal(p.pos)} + n := &Typedef{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // typedef n.Type = p.parseFieldType() @@ -366,7 +366,7 @@ func (p *parser) parseTypedef() Node { } func (p *parser) parseEnum() Node { - n := &Enum{first: p.nextReal(p.pos)} + n := &Enum{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // enum n.Name = p.expectIdentifier("enum name") @@ -406,7 +406,7 @@ func (p *parser) parseEnumValue() *EnumValue { return nil } - v := &EnumValue{first: p.nextReal(p.pos)} + v := &EnumValue{nodeBase: nodeBase{first: p.nextReal(p.pos)}} v.Name = p.identifier() if p.at(TokenEqual) { @@ -428,7 +428,7 @@ func (p *parser) parseEnumValue() *EnumValue { } func (p *parser) parseStruct() Node { - n := &Struct{first: p.nextReal(p.pos), Kind: StructKind(p.cur().Kind)} + n := &Struct{nodeBase: nodeBase{first: p.nextReal(p.pos)}, Kind: StructKind(p.cur().Kind)} p.advance() // struct | union | exception n.Name = p.expectIdentifier("struct name") @@ -452,7 +452,7 @@ func (p *parser) parseStruct() Node { } func (p *parser) parseService() Node { - n := &Service{first: p.nextReal(p.pos)} + n := &Service{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // service n.Name = p.expectIdentifier("service name") @@ -500,7 +500,7 @@ func (p *parser) parseService() Node { func (p *parser) parseFunction() *Function { annos := p.parseStructuredAnnotations() - f := &Function{first: p.nextReal(p.pos), Structured: annos} + f := &Function{nodeBase: nodeBase{first: p.nextReal(p.pos)}, Structured: annos} if len(annos) > 0 { f.first = annos[0].TokStart() } @@ -549,7 +549,7 @@ func (p *parser) parseFunction() *Function { return nil } - f.Throws = &Throws{first: p.pos - 1} + f.Throws = &Throws{nodeBase: nodeBase{first: p.pos - 1}} f.Throws.Fields = p.parseFieldList(TokenRParen) p.expect(TokenRParen, "')' to close throws") @@ -588,7 +588,7 @@ func (p *parser) parseFieldList(term TokenKind) []*Field { func (p *parser) parseField() (*Field, bool) { annos := p.parseStructuredAnnotations() - f := &Field{first: p.nextReal(p.pos), Structured: annos} + f := &Field{nodeBase: nodeBase{first: p.nextReal(p.pos)}, Structured: annos} if len(annos) > 0 { f.first = annos[0].TokStart() } @@ -653,7 +653,7 @@ func (p *parser) parseField() (*Field, bool) { // --- types ----------------------------------------------------------------- func (p *parser) parseFieldType() *FieldType { - t := &FieldType{first: p.nextReal(p.pos)} + t := &FieldType{nodeBase: nodeBase{first: p.nextReal(p.pos)}} switch p.cur().Kind { case TokenMap, TokenList, TokenSet: @@ -764,7 +764,7 @@ func isBaseType(k TokenKind) bool { // --- constant values ------------------------------------------------------- func (p *parser) parseConstValue() *ConstValue { - v := &ConstValue{first: p.nextReal(p.pos)} + v := &ConstValue{nodeBase: nodeBase{first: p.nextReal(p.pos)}} switch p.cur().Kind { case TokenIntConstant, TokenTrue, TokenFalse: @@ -862,7 +862,7 @@ func (p *parser) parseStructuredAnnotations() []*StructuredAnnotation { var out []*StructuredAnnotation for p.at(TokenAt) { - sa := &StructuredAnnotation{first: p.nextReal(p.pos)} + sa := &StructuredAnnotation{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // @ sa.Name = p.expectIdentifier("annotation name") @@ -910,7 +910,7 @@ func (p *parser) parseAnnotationsIfPresent() *Annotations { // literal (a bare name means an implicit value of "1"), and each may end // with an optional ',' or ';'. func (p *parser) parseAnnotations() *Annotations { - a := &Annotations{first: p.nextReal(p.pos)} + a := &Annotations{nodeBase: nodeBase{first: p.nextReal(p.pos)}} p.advance() // ( for !p.at(TokenRParen) && !p.at(TokenEOF) { @@ -921,7 +921,7 @@ func (p *parser) parseAnnotations() *Annotations { continue } - item := &Annotation{first: p.nextReal(p.pos)} + item := &Annotation{nodeBase: nodeBase{first: p.nextReal(p.pos)}} item.Name = p.identifier() if p.at(TokenEqual) { @@ -966,7 +966,7 @@ func (p *parser) identifier() *Identifier { i := p.nextReal(p.pos) t := p.advance() - return &Identifier{first: i, last: i, Text: t.Text} + return &Identifier{nodeBase: nodeBase{first: i, last: i}, Text: t.Text} } // expectIdentifier parses a plain identifier name and reports an error when