Fix get_capability typing overloads#595
Merged
alcarney merged 5 commits intoopenlawlibrary:mainfrom Mar 8, 2026
Merged
Conversation
tombh
approved these changes
Mar 6, 2026
6 tasks
44afbe3 to
c54eb0e
Compare
Collaborator
Author
|
Thanks! :) |
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
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.
Description (e.g. "Related to ...", etc.)
By trying to fix #592 I realised that the typing overrides we generate for
get_capabilitydon't make much sense.First there was the issue where
Sequence[str]was reported asstr.However, the script also tried to unpack types like
Union[DefinitionOptions, bool]I'd appreciate a second opinion, but I don't think this is correct?
Since the types in the union have different "shapes" you can't guarantee that
definition_provider.work_done_progressexists so wouldn't the safer option be to stop atdefinition_providerto encourage the caller to handle each type separately?Assuming the answer to this is "yes", I've tweaked the script to only unpack
Optional[T]capabilitiesAs a result, many overloads have been deleted.
This PR also tidies up the generate_code.py script and migrates away from deprecated typing syntax
Closes #592
Code review checklist (for code reviewer to complete)
Automated linters
You can run the lints that are run on CI locally with: