Repository navigation
Conversation
…iler
Hover, completion, signature help, and go to definition all go through
`v -vls-mode -line-info`, which the V launcher routes to the V1
compatibility compiler. When that compiler is missing, the launcher
refuses with a single line and exits:
`-vls-mode` requires the compatibility compiler, but no usable V
0.5.2 fallback was found and make is unavailable. Install make,
then run `make v1` in `C:\Users\me\v`.
That refusal is not an "unknown option" line, so neither
`compiler_rejects_line_info` nor `compiler_refused_and_stopped`
recognized it. `line_info_mode` stayed `.direct`, so VLS kept spawning
the compiler once per request for an answer that can never arrive, and
every one of those lookups resolved to empty with nothing said about why.
Recognize the refusal, retire the lookups as `.missing` so they are
answered from VLS's own index instead of paying a process launch each,
and tell the user what to install. A launcher that can build the
fallback itself announces "running `make v1` now" and then answers, so
that form is explicitly not a dead end.
The notice needs no "already warned" flag: the caller sets
`line_info_mode` to `.missing` first, and from then on `run_v_line_info`
returns from its early `.missing` check without reaching this point, so
it is sent at most once per session.
vlang/v#29369 (merged as 76d88b9) rewrites the tail of the launcher's refusal: the sentence after "make is unavailable" is now a platform-specific hint, and the comma became a period. On Windows the clause reads On Windows, install GNU make in MSYS2 (`make` or `mingw32-make`) and put its tools, including `sh`, on PATH. and elsewhere it is still "Install make.". `compiler_lacks_compatibility_compiler` keys on "requires the compatibility compiler", which both spellings contain, so detection is unaffected. This test pinned the old wording verbatim, so it now asserts both: green before the compiler change and after it, and the coupling is written down instead of being rediscovered the next time the wording shifts. Validated against V 0.5.2 76d88b9, the merged compiler: the Windows sample above is its output verbatim. `interop_test.v` passes; the module suite reports the same 3 failures with and without this change - `index_test.v`, `handlers_test.v` and `integration_test.v`, none of them in this file. `integration_test.v` fails on an empty completion list, which is the symptom of the missing V1 fallback this refusal describes, and is what vlang/v#29369 and vlang/vscode-vlang#543 are about.
…tics - Use compile_target as exec_dir and diagnostic_source_dir so the compiler checks the correct module directory - Add base_dir parameter to source_path_from_overlay so relative paths resolve against the compile target, not the project root - Escape backslashes in JSON rootPath for Windows paths - Fix u64 to int conversion in test file size calculations
When the 'v' on PATH is a forwarding wrapper (v.bat -> v.exe), find_v_dir returned the wrapper directory, so vlib module lookups (os, builtin, time, strings) resolved nowhere and indexed member completions came back empty. Unwrap .bat/.cmd shims to their quoted executable target in resolve_v_compiler_exe and cover it with tests.
…sing-compat-compiler
…h split_check_errors
…update source_path_from_overlay callers
Every edit now schedules two jobs: a fast index-only unknown-import check (150ms debounce) and the full compiler check (800ms). The slow answer replaces the fast one, so clean files see no extra traffic while a bad import is flagged in milliseconds. Adds opt-in VLS_PERF_LOG timing for requests and diagnostics jobs, plus the comparison/roadmap notes behind the change.
run_v_check fingerprints the program (compiler id plus sorted per-file content hashes, buffers winning over disk) before overlay or compiler work. Identical states reuse answers from memory or a per-program JSON file in the OS cache dir, across restarts. Reopening this repo unchanged answers in about 15ms from disk instead of 5s cold. Also hardens compiler spawn: a missing compiler returns an error result instead of crashing the server.
find_v_dir_from_exe walks up to a directory holding vlib. The fake compiler tree had none, so there was no V home to find and the test failed everywhere. Creating base/vlib mirrors a real installation.
run_v_line_info_once tries the warm shared server and its program copy first, falling back to the one-shot compiler path on any failure. Compat and missing modes keep their existing path untouched, and the pool never changes mode probing. Hover, signature help, and compiler-backed completions stop paying overlay plus spawn per request when a server is warm.
Extends the compiler-free fast check with bracket/string/comment/interpolation balance scanning that mirrors the index tokenizer. Only certain findings publish (stray closers, unclosed constructs at EOF with no continuation); mid-keystroke states stay silent and the slow check corrects the rest.
Stub compilers for modern-direct, old-flag, and dead-end generations assert the probed LineInfoMode, diagnostics results, and mode stability across repeated probes.
About 115 new focused tests for previously uncovered pure and harness-level helpers: occurrences, declarations, completions, positions, import parsing, fingerprint and disk-cache paths, JSON framing, inlay stamps. Coverage heuristic up from 38 percent to about 71 percent of functions mentioned in tests.
…path collisions Direct tests for the remaining pure helpers in rename, rename_conflicts, module_imports (now fully covered) and expression_types. make_unique_temp_path gains a per-process counter suffix: the Windows clock returned the same tick 19987 times in 20000 probes, so pid plus timestamp collided by construction.
Issues 474 and 473 both reproduce clean on this tree (compile-target rework and sibling-aware completion cover them), so these are guards with breaking controls: a nested two-file program checks clean and maps diagnostics to real paths, and an unopened sibling resolves through completion, definition, diagnostics, and didOpen.
run_v_check logs one VLS_PERF_LOG line per slow check with fingerprint, overlay prep, compiler run, and parse milliseconds. First measurement on this host: the compiler run is 97 percent of a 360ms single-file check.
reindex_uri capped disk files at 2MB but indexed open buffers of any size, allowing one huge buffer to force an unbounded index build. The gate now covers both paths; oversized entries are dropped and marked skipped exactly like oversized disk files.
The overlay directory persists per program instead of being wiped per check; buffers rewrite only when their content hash differs, and stale copies are pruned. Warm prep time measured 90ms down to 21ms on a 52-file overlay.
vls.inlayHints gains variableTypes and parameterNames options matching the two kinds the compiler emits. Defaults preserve current behavior; runtime toggles apply without restart.
Per-symbol trigram caches maintained incrementally inside index entries, ranked by overlap then match class, case and underscore insensitive with a bounded per-symbol cap.
Inserting thousands of entries one by one doubles the map backing store repeatedly; a late doubling can abort under a fragmented heap (GC_alloc_large on big trees). Reserve once from the walk's file count.
Renaming to a name already live in the scope, or renaming a member of an interface its implementations must keep, is refused with a message naming the conflict, instead of silently producing code that will not compile.
A struct literal that omits fields offers a quickfix adding them with their zero values. Existing fields are left untouched, fields are inserted in declaration order, and the action is skipped when the literal is complete or its type is unknown.
…linked editing All three are implemented and tested: range formatting returns a hunk only inside the requested range, inline values type a := literal binding, and linked editing is now the occurrence set a rename would edit, across files, instead of same-line text matches that had nothing to do with the symbol.
…puted A pull returns the same items the last push published, through the same dedup and cap path, so there is one truncation rule; a repeated pull whose previousResultId matches answers unchanged. The result id is derived from the uri and the encoded items rather than stored, so push and pull agree without a second write path. A document never checked answers empty, not an error.
A workspace pull reports every document a check has answered for, from the same cache the per-file pull reads, so no compiler runs and no second dedup or cap exists. An id the client already holds is answered unchanged, and the answer is bounded: at five hundred documents the report says how many were left out instead of silently dropping them.
@medvednikov's review of the index-memory change asked for three things, and this branch predates those fixes. Ported across, adapted to this tree: index_reserve_target clamps to index_max_files instead of dropping the reservation when the projection crosses the cap; occurrences_for applies the same oversized-source gate reindex_uri uses; index_is_complete_for_scope and index_is_complete treat a skipped URI still in open_files as not indexed, so an unsaved oversized buffer stops a rename. Verified: v -check, v fmt -verify, and the full index_test.v green on Windows.
A cursor inside a nested call, on the inner paren, three levels deep, on a method chain and past every closing paren is what source_call_target answers, so the signature-help fallback cannot regress to reporting the outer function.
Adds `ssr.v`: a rule written as a comment on the line the selection
starts on, `pattern ==>> template`, with `$name` placeholders carrying
the text they matched. The action is offered as `refactor.rewrite` when
the selection sits on such a line.
Two invariants hold it together:
* Scope-negative. A rule is refused as a whole, and the reason is
logged, when any match sits in a `$if` branch, spans a `${...}`
interpolation, or starts where V writes a type — a struct field's
type, a generic argument, an annotation, or a return type.
* Visible. Every match is inside the selection, and the action's
preview is the edit set itself.
Three bugs found while making the tests green, each pinned by a test
that fails without the fix:
* The match's start was the offset the scan arrived from, so the edit
range began in the whitespace before the match and would have
replaced the preceding newline.
* The refusal read the whole document with spans relative to the
selected region, so a `$if` branch below the selection's start was
rewritten instead of refused.
* A trailing separator skip ate the pattern's own next token, so
`f($a, $b)` did not match `f(1, 2)`.
The render decides parentheses from the binding's own operators rather
than from a re-parse, because vls has no parser to re-read it with, and
a comma in the template is followed by a space as V code is written.
Deliberately absent, as each cannot be made safe on the index alone:
typed wildcards, rules that span files, and declaration-shape rewrites.
The full suite is green apart from nine failures that reproduce on a
clean checkout, where the V1 compatibility compiler is unavailable.
The check for a match inside an interpolation only looked for a `${`
inside the match's own span, so the common case — a pattern that
matched an interpolation's expression — was not refused at all. The
two bytes before the match are the opening `${` there, and the check
never looked there. `'a${x + f(1)}c'` matched with no refusal, and the
replacement would have been written inside the string.
`ssr_in_interpolation` scans the offset's own line instead: the quote
that opened the string and the brace depth inside it decide where the
interpolation runs, so a match anywhere inside one is caught, and a
plain `if x { f(1) }` still passes. A span that merely holds a `${` is
refused separately, because rewriting across a string breaks it.
The second branch of the old check claimed to stop a closing brace
being produced inside an interpolation, but its condition tested the
two bytes before the match for `}{`, which is not that. It is gone.
Also drops `SsrRefusal`, which nothing used.
Four defects found by running a shape matrix over the constructs a
structural replace touches, rather than by reasoning about them. Each
one is pinned by a test that fails without it.
A match inside `$else` was allowed. The comptime scan counted a `}`
that starts a line as a branch closing, but `} $else {` closes one
branch and opens the next on the same line, so the opener was never
settled and a match under it read as below the pair. The compiler does
not build the branch the flags do not choose, so a wrong rewrite there
is invisible to the slow check — the exact failure this layer exists to
prevent, in the one construct it did not name.
An attribute now refuses as an attribute. It was refused before, but
only because the prefix ended in `[`, which the type-position rule
answered — so the log told the user a match in `@[export]` was in a
type position, which is false and describes a different refusal.
An annotated field type was allowed. `item x T @[json: json]` ends with
an attribute rather than a bare name, so the field rule's last check
matched nothing and the type passed. The type is now the name the match
starts on rather than the last name on the line.
A match inside an array literal was refused as a type position. `[`
after `:=` opens a literal, not a generic argument. `[` after a name is
indistinguishable from an index by text alone, so both are refused;
`[` after an operator is now allowed.
The return-type rule no longer requires the line to open with `fn`,
which also covers an interface's `foo() T`.
Also fixes a comment on `ssr_find_matches` that described a
`code_only` parameter the function has never had.
…d-to-end test `ssr_skip_trivia` had an infinite loop. Its comment branch tested the loop bound on `pos`, the offset the function was called with, but incremented `at`; `pos` never moves, so the condition was always true and the loop ran until the counter wrapped. Every V file carries `//` comments, and the rule a structural replace reads is written as one, so the feature hung on its own usage. Found by the end-to-end test, which hung for fifteen minutes before it was killed. The rewrite also searched from the selection's start, whose region holds the rule's own comment, and the rule's text matches its pattern. A selection whose only match is that comment was offered a rewrite of it, which destroys the rule the user typed. The region now starts after the rule's line. Three tests pin these. The comment one hangs rather than fails when the loop returns, which is the only way an infinite loop can be pinned; a regression shows up as a run that does not finish, and CI's job timeout turns that red. The end-to-end test goes through `handle_code_action`, so it also pins the registration, the `refactor.rewrite` kind filter, and the LSP positions of the edits, none of which were covered before.
… an interpolation is not
Four cases were reasoned about and never measured.
`g($a + 1)` on `f(1 + 2)` renders `g((1 + 2)+1)`: the binding is
wrapped, which is the point, and the spacing is tight because the
tokenizer drops the rule's own spaces. `v fmt` unwinds the spacing, so
the tests assert the wrap rather than the whitespace, and say so.
`\${` is an escaped dollar, and a backtick string is raw, so `${` inside
either is text rather than an interpolation. A rewrite there is an
ordinary one and must not be refused: refusing it would make a rewrite
inside an escaped string impossible. Both are pinned with tests that
fail when the scanner starts counting those forms as interpolations.
The `$if` refusal now names `$else` too. It already refused a match in
one — `} $else {` closes a branch and opens the next on the same line —
but the message said only `$if`, which is the same imprecision the
attribute refusal was corrected for.
The rule is a comment on the line the selection starts on, and the region searched began after that line so the rule's own text could not be matched. That also excluded the code *before* the comment on the same line, which is the natural way to write it: annotate the call you want rewritten and select it. `f(1) // f($a) ==>> g($a)` offered nothing at all, with nothing said about why. The region searched is now the selection, and a match that reaches into the rule's comment is dropped instead. The interval test is `start < end of the rule's line` and `end > start of its comment`, so a match ending exactly at the comment — the call before it — is kept. Both directions are pinned: a test that the call is offered, and the existing one that the comment itself is not rewritten, which now fails without the drop.
…iler
The fingerprint hashes a file's bytes, so rewriting the words of a
comment — the commonest edit that provably cannot change a diagnostic —
invalidates the cached answer and pays a full compiler check.
The fingerprint now hashes the file's code with its comments dropped:
diag_code_sketch keeps every newline a comment held, because those move
every line under them, and drops the rest of the comment. An edit to the
words changes nothing; an edit that adds a line still does, since every
line below moves and a diagnostic's line number moves with it.
A string's body is kept verbatim, so an interpolation inside one stays
code to the fingerprint and editing it still reaches the compiler. That
is why the sketch is not the existing masked_v_code, which blanks string
bodies: an edit inside `${...}` must not be free, because reusing an
answer there would leave a stale error on screen.
The sketch is cache-key material rather than an analysis input, so it
does not preserve offsets — which is what makes a comment's length
irrelevant. The first version did preserve them, and thereby made an
edit to a comment's length cheap but not its words, which is the case
that actually happens.
Two tests, both controlled by turning the dropping off: a comment edit
leaves the fingerprint alone and an interpolation edit does not.
An action carried an edit, which is computed by the server. Writing a test body is not that kind of work: the file is written by `v cover --write-tests`, and the server only points at the function. LSP has a field for exactly this, so add it rather than smuggle the command through an edit. `main.v` advertises the new command alongside the two it already had.
A test-generation action has to know which functions a run never entered, and that answer lives in a coverage directory. The tiers that decide what a check runs with are the natural place to ask, so a fourth key joins them: the editor's own settings, the project's `vls.json`, the environment, and nothing. The editor tier wins, which is what lets an extension that already runs coverage point VLS at the directory it recorded into instead of running the tests a second time.
A coverage recording is what tells VLS which functions a run never entered. With a coverage directory configured, a `.v` file offers one action per such function, plus one for the whole file when there is more than one. The action runs `v cover --write-tests`, which is the one tool that generates the body; the server names the file and reports what the tool wrote, and never composes a test itself. What it writes is a starting point and not a test: a call with zero values and a `// TODO:` line, because the tool cannot know what a function should return. `-no-skip-unused` belongs with the recording and is documented as such. Without it a function nothing calls is dropped from the build, so its coverage cannot be reported at all. No directory configured means no action offered, rather than an action that opens a message: a directory that does not exist gives the same answer, and the report it produces is read through a decodable struct so a newer report cannot break an older server.
Contributor
|
Findings High: Pull diagnostics never see background results. Workers use a private High: Read requests can answer before indexing completes. High: Per-project settings leak across workspace roots. check_defines applies project settings to global Medium: Disabling diagnostics does not clear already-published diagnostics. The configuration handler only cancels scheduled jobs in Medium: Range formatting can add a newline to files that originally had none. The changed hunk always appends Medium: Workspace diagnostics omit clean documents in |
Contributor
|
Definitely time to merge main, as well. |
Contributor
|
#548 supersedes this PR. Closing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tiered diagnostics and everything that has built on them
This branch began as tiered diagnostics and grew into most of the request
path, so here is what it now holds.
Diagnostics. Each edit schedules two jobs: a fast index-only check (150ms
debounce) and the existing compiler check (800ms idle). The slow answer
replaces the fast one, so anything the fast tier reports that the compiler
does not reproduce is corrected within a second; clean files see no extra
traffic. The fast tier reports unresolvable imports and certain parse
errors from the buffer alone, in the compiler's own wording so the existing
"Remove unknown import" quickfix fires on it. The slow path fingerprints the
program and reuses answers from memory or a per-program JSON file in the OS
cache dir, across restarts. A missing compiler returns an error result
instead of crashing the server.
Performance. A stable overlay directory built at save time, a background
index refresh that answers requests from the index as it stands, memoized
project listings with hashes gated on file metadata, a symbol index pre-sized
before bulk walks, and the index size gate extended to open buffers so one
huge buffer cannot force an unbounded index build.
Requests. Hover, signature help and compiler-backed completions answer from
the shared diagnostics-server pool, with the one-shot path as fallback. A
waiting slow check is rushed when a compiler request arrives, and while a
check is in flight interactive requests are answered from the index and
followed up with the compiler's answer on the same request id. Completion is
deliberately untouched by that follow-up. A change confined to a function
body checks that file's module rather than the whole tree.
New surface. documentLink for imports and the URLs in comments; the LSP 3.17
pull diagnostics, answering from what the last push published through the
same dedup and cap path; a combined workspace/diagnostic report for the
Problems pane; trigram fuzzy ranking for workspace symbols; granular
inlay-hint toggles; and layered configuration (editor settings > project
vls.json>VLS_DEFINES) with defines reaching the check path.Refactors and assists. Rename refuses names that shadow a binding or break
an interface, and renames an interface's members across its implementors.
The lightbulb offers fill struct literal, extract or inline a variable,
extract function, add an import, implement missing interface members, and
extract a declaration to a new file. Range formatting, inline values and
linked editing are advertised now they work — linked editing is the
occurrence set a rename would edit, across files.
Structural search and replace (ssr.v). A rule written as a comment on the
line the selection starts on,
pattern ==>> template, with$nameplaceholders carrying the text they matched. Every match is inside the
selection and the preview is the edit set itself. It is deliberately
scope-negative: the whole rule is refused, with the reason logged, when any
match sits in an
$ifor$elsebranch, inside a${...}interpolation,inside an
@[...]attribute, or where V writes a type. Typed wildcards,workspace-spanning rules and declaration-shape rewrites are left out, since
each cannot be made safe on the index alone.
Test generation (uncovered_tests.v). A lightbulb action that writes the
tests a test run never called. It reads a coverage directory —
vls.coverageDirectoryin the editor's settings, orcoverageDirectoryinthe project's
vls.json— which av -no-skip-unused -coverage vcov/ test ./run recorded, and offers one action per function no run entered plus onefor the whole file. The action runs
v cover --write-tests, the tool thatgenerates the body, so the server never composes a test itself and the file
it writes is the one the tool writes everywhere else. That is also why the
bodies are a starting point rather than a test: a call with zero values and a
// TODO:line, because the tool cannot know what a function should return.-no-skip-unusedbelongs with the recording and is documented as such,since without it a function nothing calls is dropped from the build and its
coverage cannot be reported at all.
Two supporting changes:
CodeActiongained acommandfield, so an actioncan point at tool work instead of an edit the server computes — standard LSP
that this server had no use for until now; and
vls.generateTestjoinsvls.runFileandvls.runTestsin the advertised commands.Verified on this host.
v -checkandv fmt -verifyclean. Full suite, eachfile on its own: 20 passed, 1 skipped, 3 failed. The 3 are handlers_test.v,
integration_test.v and snapshot_test.v, and all three fail the same way —
they need the V1 compatibility compiler, which this host cannot build because
makeis not installed. Each was confirmed against a pristine base checkoutwith this branch's work removed, so the failures predate it. CI's Windows
jobs previously failed building V itself, from vlang/v#30251; #30252 restored
the missing
sys/stat.hinclude and is merged, so the queued runs should nowreach the test steps. Two upstream merges are folded in; the branch no
longer stacks on #540, which was closed without landing. #547 carries the
index-memory findings on their own branch.
The test-generation work adds one suite,
uncovered_tests_test.v(11 tests),and 4 tests to
vls_config_test.vcovering the three configuration tiers.