Cover the four remaining language-server handlers - #71
Merged
Conversation
Definition, document symbols, document synchronisation and watched files had no direct tests. All five handlers are covered now. Go-to-definition resolves a variable use to its declaration, returns nothing for something that is not a variable, and degrades on a URI the server never opened. The outline lists the settings block, environments and variables, and excludes symbols coming from an included file — included statements are merged into the same AST, so without a file check they appeared in the outline at line numbers belonging to another file. The two notification handlers publish rather than return, so what they do is only visible through the facade. RecordingLanguageServer stands in for it and captures each publish with its URI and diagnostic count; every other channel throws, so a handler that starts using one fails loudly here instead of passing silently. The tests pin that didOpen publishes, that didChange republishes with the new diagnostics, that didClose clears the closed document's diagnostics last — the ordering fixed earlier, which nothing had verified — and that a watched-file change republishes the documents that include it while a file nobody includes publishes nothing. Building the stand-in took two attempts worth recording: PublishDiagnostics routes through the non-generic SendNotification(IRequest) overload, not the generic one, so the first version recorded nothing and every test reported zero published. That failure was the stub's, not the handlers'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
1 participant
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.
Definition, document symbols, synchronisation de document et fichiers surveillés n'avaient aucun test direct. Les cinq gestionnaires sont désormais couverts.
Definition et Document Symbol
« Aller à la définition » résout l'usage d'une variable vers sa déclaration, ne renvoie rien sur ce qui n'est pas une variable, et dégrade sur un URI jamais ouvert.
L'outline liste le bloc
settings, les environnements et les variables, et exclut les symboles venant d'un fichier inclus — les instructions incluses sont fusionnées dans le même AST, donc sans vérification de fichier elles apparaissaient dans l'outline, à des numéros de ligne appartenant à un autre fichier.Les deux gestionnaires de notification
Ils publient au lieu de retourner : ce qu'ils font n'est visible qu'à travers la façade.
RecordingLanguageServerla remplace et capture chaque publication avec son URI et son nombre de diagnostics.Tout autre canal lève — un gestionnaire qui se mettrait à en utiliser un échoue bruyamment ici plutôt que de passer en silence.
Les tests épinglent :
didOpenpublie ;didChangerepublie avec les nouveaux diagnostics ;didCloseefface les diagnostics du fichier fermé en dernier — l'ordre corrigé plus tôt, que rien ne vérifiait ; un changement de fichier surveillé republie les documents qui l'incluent, et un fichier que personne n'inclut ne publie rien.Sur la construction du double
Deux tentatives, et ça mérite d'être noté :
PublishDiagnosticspasse par la surcharge non génériqueSendNotification(IRequest), pas par la générique. La première version n'enregistrait donc rien et les cinq tests rapportaient « 0 publié ».C'était l'échec du double, pas des gestionnaires — le genre de faux négatif qui aurait pu me faire conclure que le code était cassé.
398 → 409 tests.
🤖 Generated with Claude Code