The comment narrated the call below it. The non-obvious fact is that
the protocol has no clear operation, so an empty publish is the only
way to drop the client's diagnostics for a closed document.
The request, notification, and server construction helpers existed
byte-identical in both the server unit tests and the integration
tests, the didOpen notification blob was inlined seven times, the
redirect test rebuilt the download sequence download_request already
drives, and the request scan was duplicated at two sites. Move the
shared constructors into a test_support module and route the repeated
sequences through open, download_request, and find_request.
The initialize unit test asserted a strict subset of the integration
capability test plus the serverInfo name, so the name assertion moves
there and the unit test goes away.
The id-allocation plus pending-registration scaffolding for
server-initiated requests was repeated across pull_configuration,
redirect_prompt, and code_lens_refresh, the serialize-and-respond
pair was inlined at seven handler sites next to the existing error
helper, and the parse-params-or-reject match was copied verbatim in
four handlers. Centralize each pattern in one helper so a change to
any of them has a single site.
The parser accepts comments and trailing commas unconditionally, which
also meant a plain .json file full of JSONC syntax validated silently.
The languageId the client sends at didOpen now picks the dialect:
documents opened as json get a warning for each comment and trailing
comma, jsonc documents keep accepting both. The dialect only affects
reporting, the parse itself is unchanged, and clients sending unknown
language ids stay on the lenient side.
Every token carried an owned String copy of its source slice, one heap
allocation per token on every reparse, and the copy could only ever
agree with the source it was cut from. Tokens now hold just a kind and
range, and consumers slice the document text through Token::text, so
the text lives in exactly one place and the tree becomes plain Copy
data. Accessors that read text (key_text, as_string, as_f64, the
formatter, the validator) take the source alongside the node.
unescape_string built a fresh String for every call even though most
keys and values contain no escapes, and every key lookup funnels
through it on every reparse. key_text and as_string now return a Cow
that borrows the quote-stripped slice in the common case and only
allocates when an escape actually needs decoding.
eat_trivia and bump cloned each token, String and all, although every
token enters the tree at most once and the buffer is dropped right
after parsing. Taking the text out of the buffer slot avoids one heap
allocation per token per reparse.
The after-item comma-or-closer match in parse_object and parse_array
was the same four arms duplicated, each also repeating its loop's own
end-of-file close error.
LexError and SyntaxError were the same range-plus-message pair, and
parse converted one into the other field by field just to merge the
lists. Both stages now report through one SyntaxError in tree.rs.
Number validity was checked by f64::from_str, which accepts strings
that are not JSON numbers, so leading zeros like 05 and trailing dots
like 5. produced no diagnostic.
char::from_u32 rejects surrogate code points, so both halves of an
escaped pair like \uD83D\uDE00 fell through silently and the astral
character vanished from the decoded string. That corrupted key
matching, enum and const comparison, and $schema detection for any
string spelling a non-BMP character as escapes. Pairs now combine into
the supplementary code point, and lone or mismatched surrogates decode
to the replacement character instead of disappearing.
The escape arm consumed whatever byte followed the backslash, so a
backslash at the end of a line swallowed the newline and the lexer ran
into the following lines until the next quote, restructuring the rest
of the document into one string token. The newline guard now applies
inside escapes too.
Each \u escape collected its hex digits into a heap String before
parsing. Slicing the digits straight out of the raw input parses the
same text without the per-escape allocation.
The unknown-keyword arm pushed a LexError by hand while every other
error path goes through the error helper, and advance_char re-derived
UTF-8 character width from continuation bytes that char::len_utf8
already encodes.
Nothing removed a closed document from the store, so doc_schema
entries and waiting-set URIs accumulated for the server's lifetime and
a completed fetch nominated long-closed documents for diagnostics.
bind_document ran auto_loadable before consulting auto_attempted, so a
document whose schema never loads paid the workspace-containment
canonicalize() syscalls on every change. Recording the attempt first
also covers ineligible schemas, which previously never entered the set
and were re-probed forever.
A file schema outside one referring document's directory is no longer
retried when a second document in a different directory binds it while
no workspace root is known. That case keeps the explicit download lens.
A $ref fragment is percent-encoded per RFC 3986, so a def whose name
needs encoding, like #/$defs/foo%20bar, never resolved and its
constraints were silently dropped. Decode the fragment before the
pointer lookup, failing malformed escapes instead of passing them raw.
The redirect decision in fetch_one and the result bookkeeping in
on_fetched each split one decision across two control-flow constructs,
leaving a dead match arm that could only assert its own unreachability.
Deciding in a single match removes both arms and an extra url clone.
Share the root-object $schema member scan between URL detection and
the download lens range, the http(s) scheme test between the fetch
guards, the pointer-identity dedup push between navigation and
expansion, and the temp-dir, canned-response, and loopback-policy test
helpers between the fetch and store test modules. Also go through
Parse::value() like every other call site.
Validation reruns on every change, so the walk paid for keywords that
were not even present: counting characters without a length constraint,
building the present-property set without a required list, cloning the
pattern string on every regex cache hit, and probing every oneOf branch
after two matches had already decided the outcome.
schemaFetch.maxBytes is client-supplied and unclamped, so the over-read
probe's max_bytes + 1 could overflow: a panic that permanently kills
the fetch worker in debug builds, a wrap to take(0) in release.
is_valid failed a probe on any finding, so an advisory format warning
inside an anyOf branch rejected values the schema accepts, made oneOf
undercount matches, inverted not, and steered if/then/else wrong. Only
error findings fail a probe now, matching how external ref warnings
are already kept out of probes via the side channel.
Two known gaps in the schema engine worth more than a quick fix: the
duplicated property/item dispatch between validation and navigation,
and draft 2020-12 $ref sibling composition.
read_message allocated a fresh String per message even though the
stdin reader calls it in a loop. The caller now owns the scratch
buffer, so one allocation serves the whole stream.
write_message flushed after every message over a bare line-buffered
StdoutLock, so an event producing a response, diagnostics, and a code
lens refresh paid the syscall cost per message. Stdout now goes through
a BufWriter and the dispatch loop flushes once per batch.
A malformed value was folded into the missing-header error by
parse().ok(), so the one stderr line the dying reader thread produced
pointed at the wrong cause. And the declared length sized the body
allocation with no bound, letting a corrupt or hostile header force a
giant zeroed allocation before read_exact could fail. Malformed and
oversized values now fail with errors naming the offending value.
The &'static str fields threaded the same "2.0" literal through five
constructors and let a struct literal set a wrong version. A zero-sized
marker that serializes as "2.0" removes both.
No response the server produces carries error data, so every
construction went through ResponseError::new hardcoding None. The
field can return when a response actually needs it.
Missing fields deserialize to None for Option types without
#[serde(default)], and an untagged derive serializes each Outgoing
variant's payload directly, so the manual dispatch match duplicated
what serde generates.
The wrapper had a single caller and forced the callback to fabricate a
BrokenPipe error just to stop reading, which the caller then had to
filter back out. The inline loop returns directly instead.
Serialize-only types carried Option wrappers and skip_serializing_if
attributes on fields that every construction site sets, so the None
path never ran and callers paid for unwraps. Diagnostic.source also
becomes a static str because it is always the constant "json-ls"
and diagnostics are rebuilt on every edit. Option remains only where
a None is actually produced.
Per-field serde renames restate the snake-to-camel convention once
per field and invite a missed rename on future additions. A container
rename_all covers every field, while the LSP "type" fields keep their
explicit rename, which overrides the container attribute.
The URI helper, the completion request params, and the client
capability descent each restated a definition that already existed
elsewhere, namely schema::resolve::file_url_to_path,
TextDocumentPositionParams, and the sibling capability accessor's
traversal. Collapse each onto the single definition and import
completion_item_tag instead of spelling the crate path inline.
Every candidate property rendered its markdown documentation during
the completion request, walking refs and enumerating combinator
variants per item on each keystroke while clients show one item at a
time. Key items now carry the document URI and schema path in their
data payload, and the new completionItem/resolve handler builds the
documentation for the single item the client asks about.
Each conversion re-counted units from the line start, so publishing N
diagnostics on minified single-line JSON cost O(N x line length) per
change. Consecutive conversions on one line now continue from the last
offset, and the resume point resets with the index on every edit.
The key documentation tests in completion each rebuilt the same
document and schema setup to assert describe's rendering one layer
removed. Completion keeps a single wiring test and the rendering
assertions move next to the code in describe.
Hover and completion each walked the tree from an offset to a schema
path, with array branches that had already been copied verbatim and
object branches that had started to drift. The shared walker uses
completion's slot rule, so hovering between a colon and its value now
describes that member's schema instead of the enclosing object's.
A cursor between elements that touched neither fell through to the
element count, so positional schemas (prefixItems, tuple items) matched
the wrong index or none at all.
value_items looked only at the first name in a type array, so
["string", "boolean"] proposed nothing and ordering decided whether
booleans or null showed up.
The parser attaches trivia around a member's colon to the member node,
but fmt_member only re-emitted the key, colon, and value, silently
deleting any comment there and breaking the comment-preserving
contract. Line comments push the value to the next line so it does not
end up commented out.
Keys like $schema and const strings containing a dollar sign were
embedded verbatim in snippet bodies, so client snippet engines parsed
them as variables and inserted the wrong text.
Completing on the key of a member that already has a value replaced
only the key range with the full key-colon-value snippet, turning
{"na": 1} into the invalid {"name": "$0": 1}. The full member
snippet now only applies when the member has neither colon nor value.
Completion collected every ancestor object's keys on descent even when
the cursor lands in a value slot, filtered candidate properties with a
linear scan, and walked array elements twice to find the trailing
index. Duplicate-key detection cloned every key into the seen set even
though the text is only needed when a duplicate is found. All of this
runs on each keystroke, so the waste scales with document size.
oscar