Improved schema completions and navigation#360
Open
holodorum wants to merge 17 commits intokson-org:mainfrom
Open
Improved schema completions and navigation#360holodorum wants to merge 17 commits intokson-org:mainfrom
holodorum wants to merge 17 commits intokson-org:mainfrom
Conversation
KsonTreeWalker<N> is an interface that decouples tree-navigation algorithms from any specific tree representation. TreeNavigation provides generic algorithms parameterized by the walker: - navigateWithJsonPointer: follow a JSON Pointer through any tree - navigateToLocationWithPointer: find the deepest node at a cursor position and build the JSON Pointer path from root These algorithms are derived from the concrete implementations in KsonValueNavigation but work with any KsonTreeWalker, enabling both KsonValue trees and IntelliJ PSI trees to share the same navigation logic.
Two key changes that work together: 1. AstNodeWalker — a KsonTreeWalker<AstNode> that walks the parser's AST directly. Unlike KsonValueWalker (which requires a fully valid KsonValue tree), the AST includes error nodes for syntactically broken parts. Error nodes are treated as leaves that navigation passes through, so path building works on partially-typed documents where KsonValue conversion would fail. This replaces the old recovery mechanism (inserting `[]` and re-parsing) with a cleaner approach: the tree is always available, error nodes are just there. 2. Schema-aware KsonTooling methods now accept ToolingDocument instead of raw strings, eliminating redundant parsing. ToolingDocument stores the content string, exposes meaningfulTokens (WHITESPACE and COMMENT filtered out, reusing Lexer.ignoredTokens), and provides the root AST node for the walker. Internal refactoring: - KsonValuePathBuilder takes ToolingDocument, uses AstNodeWalker for navigation and meaningfulTokens for cursor-context analysis — zero re-parsing - SchemaFilteringService accepts KsonValue? instead of String - SchemaInformation uses TreeNavigation for document navigation - ResolvedSchemaContext uses pre-parsed values from ToolingDocument
Move the walker package (KsonTreeWalker, KsonValueWalker, AstNodeWalker, TreeNavigation) from kson-tooling-lib to the root project so that both projects can use the generic tree navigation algorithms. This enables eliminating KsonValueNavigation entirely: - navigateWithJsonPointer and navigateToLocationWithPointer were already duplicated in TreeNavigation as generic <N> algorithms - navigateWithJsonPointerGlob (with wildcard, glob pattern, and recursive descent support) is now generalized into TreeNavigation as well, using the KsonTreeWalker interface instead of KsonValue type checks EmbedBlockResolver and SchemaIdLookup now use TreeNavigation + KsonValueWalker directly. The KsonValueNavigation object, its LocationNavigationResult type, and its dedicated test file are deleted. All glob/wildcard/recursive-descent tests are migrated to TreeNavigationTest alongside the existing pointer and location tests.
…ng, polish tests Four focused improvements found during review of the treewalker commits: 1. Replace Pair<String, N> with TreeProperty<N> in KsonTreeWalker. getObjectProperties now returns List<TreeProperty<N>> instead of List<Pair<String, N>>, giving callers .name and .value instead of the anonymous .first and .second. 2. Extract processedStringContent() helper in KsonValuePathBuilder. The duplicated QuotedStringNode construction for escape-processing STRING_CONTENT tokens is consolidated into a single private method. 3. Rename navigateToLocation test methods in TreeNavigationTest to use backtick-quoted descriptive names, matching the style of the surrounding navigateWithJsonPointer and navigateWithJsonPointerGlob tests. 4. Add missing trailing newlines to TreeNavigationTest.kt and KsonValuePathBuilderTest.kt.
…eWalker The navigation algorithms (navigateWithJsonPointer, navigateWithJsonPointerGlob, navigateToLocationWithPointer) are now extension functions on KsonTreeWalker<N> rather than static methods on a TreeNavigation object. This eliminates the redundant `walker` parameter at every call site: the walker is the receiver, so callers write `walker.navigateWithJsonPointer(root, pointer)` instead of `TreeNavigation.navigateWithJsonPointer(walker, root, pointer)`. Internally, a private TreeNavigator class captures the walker once so the recursive helpers (navigateByParsedTokens, navigateRecursive, processSingleToken, etc.) can reference it directly without threading it as a parameter. The TreeNavigation object is eliminated entirely. TreeNavigationResult remains as a top-level data class in the same file.
KsonDocument previously maintained two separate parse paths: a strict Kson.analyze() for $schema extraction and a lazy error-tolerant ToolingDocument for editor features. This was redundant — both produced a KsonValue tree, and error-tolerant parsing is sufficient for $schema lookup. Move $schema extraction into ToolingDocument.schemaId on the Kotlin side (where it has proper access to the internal KsonValue types), and make KsonDocument a simple wrapper around a pre-parsed ToolingDocument passed in at construction. This eliminates one full parse per keystroke.
…elper Remove the processedStringContent helper in KsonValuePathBuilder in favor of direct QuotedStringContentTransformer usage. Tighten KDoc in TreeNavigation and KsonTreeWalker to avoid restating what's already documented on the referenced types.
The walker interface had 6 methods (isObject, isArray, getObjectProperties, getArrayElements, getStringValue, getObjectProperty) but every production call site followed the same pattern: check isObject/isArray, then call the corresponding get method. A sealed return type collapses the type check and data access into a single `when` match. The interface is now just two methods: getChildren (returning NodeChildren.Object, NodeChildren.Array, or NodeChildren.Leaf) and getLocation. This gives us compiler-enforced exhaustive matching at every call site, so adding a new node kind in the future would produce compile errors everywhere it needs handling. getStringValue had zero production callers and is removed. getObjectProperty had exactly one (the Literal branch in TreeNavigation), which now uses children.properties directly.
Two bugs combined to break completions for anyOf/oneOf schemas when the cursor was at a position where the document value didn't yet match the expected shape: 1. SchemaFilteringService fell back to validating against the root document when navigation to the pointer returned null (value not yet typed). Root properties would fail additionalProperties:false on every branch, filtering out all completions. Fix: return all expanded schemas when navigation returns null — nothing to filter against. 2. KsonValuePathBuilder.adjustPathForLocationContext unconditionally dropped the last path element when the cursor was outside a token. Inside an empty delimited list (`[<cursor>]`), this collapsed the pointer from the correct property path to root. Fix: skip the drop when lastToken is a container-opening delimiter (SQUARE_BRACKET_L or CURLY_BRACE_L), since the path already targets the container.
SchemaIdLookup.init stored the raw $id value (e.g. "pubmed.schema.kson")
as the idMap key, but navigateByDocumentPointer resolved the same $id via
resolveUri which produced a different string ("/pubmed.schema.kson" with
a leading slash). This mismatch caused $ref lookups to fail silently,
falling back to the unresolved schema object — which had no properties
or anyOf to expand, yielding no completions.
Fix: resolve the $id through resolveUri("...", "") during init, matching
the resolution path used during navigation. This ensures idMap keys are
consistent with later lookups.
Schema navigation (go-to-definition, completions, hover) previously dead-ended at if/then/else constructs because navigateObjectProperty and navigateArrayItems only knew about direct properties, pattern properties, and combinators (allOf/anyOf/oneOf). Schemas where allOf contains if/then blocks that conditionally map property values to parameter $refs were invisible to the tooling. Add navigateThroughConditionals (paralleling navigateThroughCombinators) which traverses into then and else sub-schemas. Wire it into both navigateObjectProperty and navigateArrayItems, and expand conditionals in expandCombinators so completions also discover conditional branches. Introduce IF_THEN and IF_ELSE SchemaResolutionType variants so downstream code (e.g. SchemaFilteringService) can distinguish how a schema was reached. These new types fall into the unfiltered path, matching allOf/direct-property behavior — condition evaluation against the document is left as a future enhancement.
…ument Two-layer filtering for conditional schemas: 1. During navigation, evaluate the "if" condition against the document value at the current schema level. This is critical when the condition checks a sibling property that is not visible from the target property. When a document value is available, only the matching branch (then or else) is included. 2. During post-navigation filtering, validate IF_THEN/IF_ELSE schemas against the document the same way ANY_OF/ONE_OF are already filtered. This catches cases where the resolved schema itself is incompatible. Consolidate scattered ANY_OF/ONE_OF checks into a shared FILTERABLE_RESOLUTION_TYPES set and rename isCombinatorBranch to isFilterableBranch.
extractValueCompletions handled enum, boolean, and null but not const.
A schema like { const: "SNOWFLAKE" } produced zero completions. This
matters for if/then schemas that narrow a property to a single const
value based on a sibling — the navigation correctly resolves the const
schema but completions were silently empty.
…tics The previous narrowing logic only kicked in when a reductive schema had a `const` value. This missed the common case where an if/then branch narrows a base property's enum to a subset. Replace the const-specific check with general intersection: when multiple reductive schemas (allOf, if/then, direct properties) provide value completions, only values present in ALL schemas survive. Falls back to union when the intersection is empty, which handles the case where no document value was available to evaluate if/then conditions. Also extracts the duplicated `getCompletionsAtCaret` test helper into a shared `SchemaCompletionTest` interface.
When the cursor is at an empty value position (e.g. `key:`), KSON can't fully parse the document, so `ksonValue` is null. Without a document value, if/then conditions can't evaluate sibling properties and all branches are included—defeating the narrowing. Add `toPartialKsonValue()` which walks the AST and silently skips error nodes, preserving successfully-parsed siblings. Use this for schema navigation during completions so that sibling values are visible for if/then evaluation even when the cursor position has no value. The partial value is only used for navigation (if/then evaluation). Validation filtering and property dedup continue to use `ksonValue` to avoid side effects from partial document state. Also simplifies `ResolvedSchemaContext` to a plain function returning `List<ResolvedRef>` — the `schemaIdLookup` and `parsedDocument` fields were unused by callers.
…tion Completions had its own inline schema resolution path to use partialKsonValue for if/then navigation while hover and go-to-definition used the shared resolveAndFilterSchemas with ksonValue (which is null when the document has parse errors). Change resolveAndFilterSchemas to take ToolingDocument directly and use partialKsonValue for navigation, ksonValue for validation filtering. All three features now share the same path, giving hover and go-to-definition the same broken-document resilience completions had.
When a schema uses allOf+oneOf to couple interdependent properties (e.g., integration determines which job values are valid), setting one property now narrows completions for the other. The mechanism: navigateThroughCombinators records which oneOf/anyOf branch each result came from (via the new parentBranch field on ResolvedRef). SchemaFilteringService uses this context in a dedicated first pass to check sibling property const/enum constraints against the document. A second pass does the existing leaf-level validation. The two passes use different document values: sibling filtering uses partialKsonValue (works even with parse errors at the cursor), while leaf validation uses the fully parsed ksonValue. Each pass has its own fallback so that completions degrade gracefully.
holodorum
changed the title
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.
if/then/elseconditional schemas — schema navigation (completions, hover,go-to-definition) now traverses
if/then/elseconstructs, evaluating conditions againstthe document to include only the matching branch
oneOf/anyOfwith sibling awareness — when a schema couples properties viaoneOf(e.g.,integrationdetermines validintegrationJobvalues), setting oneproperty narrows completions for the other by filtering branches whose sibling constraints
conflict with the document
(allOf, if/then) constrain a value, only completions present in ALL schemas survive,
replacing the previous const-only narrowing
error-tolerant partial AST values for schema navigation, so sibling property values are
visible for narrowing even when the cursor position has no value yet