expand oneOf/anyOf/allOf combinators and filter completions by context #283
Conversation
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
Collaborator
Author
|
rerun CI after #284 is merged |
holodorum
force-pushed
the
jump-to-schema-bug
branch
from
26451f7 to
014c538
Compare
Adds expandCombinators() method to SchemaIdLookup that expands oneOf/anyOf/allOf combinators into individual schema branches. This is the foundation for filtering schemas based on validation. The expansion allows tooling features (completions, hover, jump-to-definition) to process each combinator branch separately, enabling smart filtering based on document validation.
Implements filtering of schema branches based on document validation. For oneOf/anyOf combinators, only branches compatible with the existing document properties are included in completions and navigation. Uses a 'soft' validation approach: schemas are included if existing properties don't contradict them, even if required properties are missing. Adds: - getValidSchemas() for filtering logic - filterValidSchemas() for validation against document - Test coverage in SchemaCompletionLocationTest
Extends schema filtering to hover information. When hovering over a property in a oneOf/anyOf context, only show schema info from compatible branches. Updates getSchemaInfoAtLocation() to use getValidSchemas() for consistent filtering across all tooling features.
Extends schema filtering to jump-to-definition functionality. When jumping to definition in a oneOf/anyOf context, only navigate to schema locations from compatible branches. This ensures consistent behavior across completions, hover, and jump-to-definition features. VSCode integration updated to handle filtered definition results.
Implements resolveRefAtLocation() to support jump-to-definition for $ref references within schema documents. When the cursor is on a $ref value, jump to the referenced schema location. Currently supports internal references (starting with #). Includes comprehensive test coverage in SchemaRefResolutionTest.
Updates SchemaDefinitionLocationTest and KsonValuePathBuilderTest to use <caret> markers for clearer test expectations. The assertDefinitionLocation helper now inserts actual locations as markers, providing visual diff when tests fail. Makes test failures much easier to understand by showing expected vs actual locations with inline markers.
Major architectural improvements to the schema filtering logic: 1. Improved variable naming for clarity: - Renamed 'forDefinition' → 'includePropertyKeys' in KsonValuePathBuilder - Renamed 'needsFiltering' → 'hasCombinatorsThatRequireValidation' - Updated all KDoc comments and test files 2. Extracted validation methods to reduce nesting: - Created isSchemaValidForDocument() method - Created filterInsignificantErrors() helper - Extracted IGNORABLE_ERROR_TYPES constant - Reduced nesting from 4 levels to 2 levels 3. Extracted SchemaFilteringService from KsonTooling: - Moved ~130 lines of business logic to dedicated service - Better separation of concerns (API vs business logic) - Service is now testable in isolation and reusable - Removed god object anti-pattern from KsonTooling Benefits: - Improved code readability and maintainability - Better encapsulation and single responsibility - Easier to test individual components - Clearer intent through better naming All 2,500+ tests pass successfully.
Major improvements to code quality and testing:
1. Reduced code duplication:
- Extracted `resolveAndFilterSchemas()` helper method
- Eliminates triple duplication across getSchemaInfoAtLocation,
getSchemaLocationAtLocation, and getCompletionsAtLocation
- Encapsulates common pattern: parse → navigate → filter
2. Fixed return type consistency:
- Changed `getSchemaLocationAtLocation()` to return `List<Range>` (never null)
- Changed `resolveRefAtLocation()` to return `List<Range>` (never null)
- Updated TypeScript DefinitionService to handle empty arrays
- Updated test assertions to match new behavior
- Eliminates ambiguity between null, empty, and non-empty results
3. Added unit tests for SchemaFilteringService:
- 7 new unit tests covering core filtering logic
- Tests for oneOf/anyOf/allOf combinator handling
- Tests for invalid document fallback
- Tests for required property error filtering
- Tests for type violation filtering
- Provides isolated testing without going through public API
Benefits:
- Reduced code from ~100 duplicated lines to single helper
- Clearer, more consistent API semantics
- Better test coverage for core business logic
- Easier future maintenance and debugging
All 109 tests pass successfully.
Navigation of the document was done with JsonPointer tokens. Which was a list of Strings. Instead this has been refactored to use a `JsonPointer` class.
holodorum
force-pushed
the
jump-to-schema-bug
branch
from
014c538 to
26eaf65
Compare
Contributor
|
@holodorum if this passes those last few validations we discussed, this is good to merge whenever you think it's ready |
With the recent changes in main a `"`, is not part of the string. Hence, we needed to update some test-assumptions and the way we 'recover' a broken document
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.
In the original implementation of
kson-tooling-libin #243 we did not expand combinators likeoneOf,anyOfandallOf. So we could get hover info, completions or jump to definitions of schema's that involved these combinators.In PR #243 we also did not filter out possible completions. We would show all possible completions, no matter which keys already were used. We only could jump to a single definition, which wasn't a problem since we did not follow allOf or anyOf combinators.
This PR fixes both problems by expanding the combinators, and only filtering the 'possible' branches one could arrive at. So the completions, hover info and jumping to definition greatly improved.
This PR also introduces the possibility to jump to refs within a schema in 2a9e81e.
The later commits are all directed at improving the code quality, with refactors, improving tests and fixing small bugs.