From 74345c7a6dd4c55373a7312dffb3ba25e4731107 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Sat, 28 Mar 2026 16:39:52 +0100 Subject: [PATCH 01/15] Navigate through if/then/else conditional schemas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../org/kson/SchemaCompletionLocationTest.kt | 98 +++++++++- .../kotlin/org/kson/schema/SchemaIdLookup.kt | 166 ++++++++++++---- .../org/kson/schema/SchemaNavigationTest.kt | 182 +++++++++++++++++- 3 files changed, 402 insertions(+), 44 deletions(-) diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt index 1d57a494..1e8877ae 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt @@ -1495,10 +1495,11 @@ class SchemaCompletionLocationTest { val schema = searchExpressionSchema() // Cursor inside a [] delimited list, but schema expects objects (SearchTerm - // or AndExpression). The structural mismatch means no branch is compatible. - // This also exercises the SQUARE_BRACKET_L guard in the path builder: the - // path must point at /query (not drop to root), otherwise the filter would - // see root-level completions leak through. + // or AndExpression). Like testNoCompletionsInsideEmptyDelimitedDashListWhenSchemaExpectsObject, + // the structural mismatch (list where object expected) eliminates every anyOf + // branch, so we should return no completions rather than leak object-property + // suggestions into a list context. Also exercises the SQUARE_BRACKET_L guard in + // the path builder: the path must target /query (not drop to root). val completions = getCompletionsAtCaret(schema, $$""" '$schema': test query: @@ -1508,7 +1509,11 @@ class SchemaCompletionLocationTest { """.trimIndent()) assertNotNull(completions) - assertTrue(completions.isEmpty(), "Should have no completions when document structure doesn't match schema, got: ${completions.map { it.label }}") + assertTrue( + completions.isEmpty(), + "Should have no completions: the path must target /query, and list-at-object " + + "filters out every anyOf branch. Got: ${completions.map { it.label }}" + ) } @Test @@ -1560,6 +1565,89 @@ class SchemaCompletionLocationTest { assertTrue("config" !in labels, "Should NOT include 'config' (parent property)") } + @Test + fun testIfThenCompletionsIncludeConditionalProperties() { + // Test that properties from if/then branches appear in completions + val schema = """ + { + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "if": { + "properties": { + "kind": { "const": "dog" } + } + }, + "then": { + "properties": { + "bark": { "type": "boolean" } + } + } + } + """ + + val completions = getCompletionsAtCaret(schema, """ + { + "kind": "dog", + + } + """.trimIndent()) + + assertNotNull(completions, "Should return completions") + val labels = completions.map { it.label } + assertTrue("bark" in labels, "Should include 'bark' from then branch, got: $labels") + } + + @Test + fun testAllOfWithIfThenCompletionsIncludeConditionalProperties() { + // Test the orchestra.schema.kson pattern: allOf containing if/then blocks + val schema = """ + { + "${'$'}defs": { + "DogParams": { + "type": "object", + "properties": { + "treats": { "type": "integer" } + } + } + }, + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "allOf": [ + { + "if": { + "properties": { + "kind": { "const": "dog" } + } + }, + "then": { + "properties": { + "params": { + "${'$'}ref": "#/${'$'}defs/DogParams" + } + } + } + } + ] + } + """ + + // Completions at the root level should include "params" from the allOf if/then branch + val completions = getCompletionsAtCaret(schema, """ + { + "kind": "dog", + + } + """.trimIndent()) + + assertNotNull(completions, "Should return completions") + val labels = completions.map { it.label } + assertTrue("params" in labels, "Should include 'params' from allOf if/then branch, got: $labels") + } + private fun searchExpressionSchema() = $$""" type: object additionalProperties: false diff --git a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt index dcde3671..f954a9fd 100644 --- a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt +++ b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt @@ -90,49 +90,75 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { val expanded = mutableListOf() for (ref in schemas) { - val schemaObj = ref.resolvedValue as? KsonObject - - if (schemaObj != null) { - var addedBranches = false - // Track where to insert the parent (to add it first) - val branchesStartIndex = expanded.size - - // Check for oneOf - (schemaObj.propertyLookup["oneOf"] as? KsonList)?.elements?.forEach { branch -> - val resolved = resolveRefIfPresent(branch, ref.resolvedValueBaseUri) - expanded.add(ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, SchemaResolutionType.ONE_OF)) - addedBranches = true - } + expandSingleSchema(ref, expanded) + } - // Check for anyOf - (schemaObj.propertyLookup["anyOf"] as? KsonList)?.elements?.forEach { branch -> - val resolved = resolveRefIfPresent(branch, ref.resolvedValueBaseUri) - expanded.add(ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, SchemaResolutionType.ANY_OF)) - addedBranches = true - } + return expanded + } - // Check for allOf - (schemaObj.propertyLookup["allOf"] as? KsonList)?.elements?.forEach { branch -> - val resolved = resolveRefIfPresent(branch, ref.resolvedValueBaseUri) - expanded.add(ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, SchemaResolutionType.ALL_OF)) - addedBranches = true - } + /** + * Recursively expand a single schema's combinators and conditionals. + * + * Expands oneOf/anyOf/allOf into individual branches and if/then/else into + * conditional branches, resolving any $ref along the way. Branches are + * themselves expanded recursively so that nested structures (e.g. allOf + * containing if/then) are fully flattened. + * + * The parent schema is preserved alongside its branches so that its own + * properties (title, description, constraints) remain available. + */ + private fun expandSingleSchema(ref: ResolvedRef, expanded: MutableList) { + val schemaObj = ref.resolvedValue as? KsonObject - if (addedBranches) { - // Include the parent schema to preserve its properties (e.g., description, title, constraints) - // Insert at the start so it appears first in hover info - expanded.add(branchesStartIndex, ref) - } else { - // If we didn't add any branches, keep the original schema - expanded.add(ref) - } - } else { - // Not an object, keep as-is - expanded.add(ref) + if (schemaObj == null) { + expanded.add(ref) + return + } + + var addedBranches = false + // Track where to insert the parent (to add it first) + val branchesStartIndex = expanded.size + + fun addBranch(branch: KsonValue, baseUri: String, resolutionType: SchemaResolutionType) { + val resolved = resolveRefIfPresent(branch, baseUri) + val branchRef = ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, resolutionType) + expandSingleSchema(branchRef, expanded) + addedBranches = true + } + + // Check for oneOf + (schemaObj.propertyLookup["oneOf"] as? KsonList)?.elements?.forEach { branch -> + addBranch(branch, ref.resolvedValueBaseUri, SchemaResolutionType.ONE_OF) + } + + // Check for anyOf + (schemaObj.propertyLookup["anyOf"] as? KsonList)?.elements?.forEach { branch -> + addBranch(branch, ref.resolvedValueBaseUri, SchemaResolutionType.ANY_OF) + } + + // Check for allOf + (schemaObj.propertyLookup["allOf"] as? KsonList)?.elements?.forEach { branch -> + addBranch(branch, ref.resolvedValueBaseUri, SchemaResolutionType.ALL_OF) + } + + // Check for if/then/else conditionals + if (schemaObj.propertyLookup.containsKey("if")) { + schemaObj.propertyLookup["then"]?.let { thenBranch -> + addBranch(thenBranch, ref.resolvedValueBaseUri, SchemaResolutionType.IF_THEN) + } + schemaObj.propertyLookup["else"]?.let { elseBranch -> + addBranch(elseBranch, ref.resolvedValueBaseUri, SchemaResolutionType.IF_ELSE) } } - return expanded + if (addedBranches) { + // Include the parent schema to preserve its properties (e.g., description, title, constraints) + // Insert at the start so it appears first in hover info + expanded.add(branchesStartIndex, ref) + } else { + // If we didn't add any branches, keep the original schema + expanded.add(ref) + } } @@ -254,6 +280,15 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { )) } + // If no items found, search through if/then/else conditionals + if (results.isEmpty() && schemaNode.propertyLookup.containsKey("if")) { + results.addAll(navigateThroughConditionals( + schemaNode = schemaNode, + currentBaseUri = currentBaseUri, + recursiveNavigate = { schema, baseUri -> navigateArrayItems(schema, baseUri) } + )) + } + return results } @@ -330,6 +365,46 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { return results } + /** + * Navigate through if/then/else conditional schemas. + * + * Navigates into both the "then" and "else" sub-schemas (when present), + * resolving any $ref in each branch before delegating to the caller's + * navigation function. + * + * @param schemaNode The schema node containing if/then/else + * @param currentBaseUri The current base URI for $ref resolution + * @param recursiveNavigate How to continue navigation within each branch + */ + private fun navigateThroughConditionals( + schemaNode: KsonObject, + currentBaseUri: String, + recursiveNavigate: (schema: KsonObject, baseUri: String) -> List + ): List { + val results = mutableListOf() + + fun processConditionalBranch(branch: KsonValue?, resolutionType: SchemaResolutionType) { + branch ?: return + val resolved = resolveRefIfPresent(branch, currentBaseUri) + if (resolved.resolvedValue is KsonObject) { + val nestedResults = recursiveNavigate(resolved.resolvedValue, resolved.resolvedValueBaseUri) + // Unlike navigateThroughCombinators, we always tag results with the + // conditional type. Combinators need selective tagging because their + // branches are validated individually (anyOf/oneOf filtering). + // Conditional branches are not yet filtered by evaluating the "if" + // condition, so preserving inner resolution types has no benefit today. + results.addAll(nestedResults.map { ref -> + ref.copy(resolutionType = resolutionType) + }) + } + } + + processConditionalBranch(schemaNode.propertyLookup["then"], SchemaResolutionType.IF_THEN) + processConditionalBranch(schemaNode.propertyLookup["else"], SchemaResolutionType.IF_ELSE) + + return results + } + /** * Navigate an object schema to find all sub-schemas for a property. * @@ -337,11 +412,13 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { * 1. Direct property lookup in "properties" * 2. Pattern matching via "patternProperties" (can match multiple patterns) * 3. Combinator schemas ("allOf", "anyOf", "oneOf") - * 4. Fallback to "additionalProperties" + * 4. Conditional schemas ("if"/"then"/"else") + * 5. Fallback to "additionalProperties" * * Returns a list because a property can be defined in multiple places: * - Multiple patternProperties can match * - Property can exist in multiple combinator branches + * - Property can exist in both then and else conditional branches */ private fun navigateObjectProperty( schemaNode: KsonObject, @@ -386,6 +463,15 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { )) } + // Try if/then/else conditionals + if (schemaNode.propertyLookup.containsKey("if")) { + results.addAll(navigateThroughConditionals( + schemaNode = schemaNode, + currentBaseUri = currentBaseUri, + recursiveNavigate = { schema, baseUri -> navigateObjectProperty(schema, propertyName, baseUri) } + )) + } + // Fallback to additionalProperties if nothing found if (results.isEmpty()) { schemaNode.propertyLookup["additionalProperties"]?.let { @@ -603,6 +689,10 @@ enum class SchemaResolutionType { ANY_OF, /** Schema from "oneOf" combinator - exactly one branch must be valid */ ONE_OF, + /** Schema from "then" branch of an if/then conditional */ + IF_THEN, + /** Schema from "else" branch of an if/then/else conditional */ + IF_ELSE, /** Root schema or schema resolved via $ref */ ROOT } diff --git a/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt b/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt index 92b304d4..ce38ce5e 100644 --- a/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt +++ b/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt @@ -2,6 +2,8 @@ package org.kson.schema import org.kson.KsonCore import org.kson.value.navigation.json_pointer.JsonPointer +import org.kson.schema.ResolvedRef +import org.kson.schema.SchemaResolutionType import org.kson.value.KsonValue as InternalKsonValue import org.kson.value.KsonObject as InternalKsonObject import org.kson.value.KsonString as InternalKsonString @@ -16,9 +18,15 @@ class SchemaNavigationTest { * Helper to navigate schema and get all result values */ private fun navigateSchema(schema: String, path: List): List { + return navigateSchemaFull(schema, path).map { it.resolvedValue } + } + + /** + * Helper to navigate schema and get full [ResolvedRef] results (including resolution type) + */ + private fun navigateSchemaFull(schema: String, path: List): List { return KsonCore.parseToAst(schema).ksonValue?.let { SchemaIdLookup(it).navigateByDocumentPointer(JsonPointer.fromTokens(path)) - .map { it.resolvedValue } } ?: emptyList() } @@ -637,6 +645,178 @@ class SchemaNavigationTest { assertEquals("string", ((emailResults.single() as InternalKsonObject).propertyLookup["type"] as? InternalKsonString)?.value) } + @Test + fun testNavigateIfThen() { + // Use JSON syntax inside { } to avoid KSON plain-object termination issues with if/then + val schema = """ + { + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "if": { + "properties": { + "kind": { "const": "dog" } + } + }, + "then": { + "properties": { + "bark": { "type": "boolean", "description": "Does it bark?" } + } + } + } + """ + + // "bark" is only reachable via the then branch + val barkResults = navigateSchema(schema, listOf("bark")) + assertEquals(1, barkResults.size, "Expected to find 'bark' through if/then") + val barkSchema = barkResults.single() as InternalKsonObject + assertEquals("boolean", (barkSchema.propertyLookup["type"] as? InternalKsonString)?.value) + assertEquals("Does it bark?", (barkSchema.propertyLookup["description"] as? InternalKsonString)?.value) + } + + @Test + fun testNavigateIfThenElse() { + val schema = """ + { + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "if": { + "properties": { + "kind": { "const": "dog" } + } + }, + "then": { + "properties": { + "bark": { "type": "boolean" } + } + }, + "else": { + "properties": { + "meow": { "type": "boolean" } + } + } + } + """ + + // Both then and else branches should be navigable, with correct resolution types + val barkResults = navigateSchemaFull(schema, listOf("bark")) + assertEquals(1, barkResults.size, "Expected to find 'bark' through then branch") + assertEquals(SchemaResolutionType.IF_THEN, barkResults.single().resolutionType) + assertEquals("boolean", ((barkResults.single().resolvedValue as InternalKsonObject).propertyLookup["type"] as? InternalKsonString)?.value) + + val meowResults = navigateSchemaFull(schema, listOf("meow")) + assertEquals(1, meowResults.size, "Expected to find 'meow' through else branch") + assertEquals(SchemaResolutionType.IF_ELSE, meowResults.single().resolutionType) + assertEquals("boolean", ((meowResults.single().resolvedValue as InternalKsonObject).propertyLookup["type"] as? InternalKsonString)?.value) + } + + @Test + fun testNavigateAllOfWithIfThen() { + // This mirrors the orchestra.schema.kson pattern: + // allOf contains if/then blocks that conditionally constrain properties + val schema = """ + { + "${'$'}defs": { + "DogParams": { + "type": "object", + "properties": { + "treats": { "type": "integer" } + } + }, + "CatParams": { + "type": "object", + "properties": { + "naps": { "type": "integer" } + } + } + }, + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "allOf": [ + { + "if": { + "properties": { + "kind": { "const": "dog" } + } + }, + "then": { + "properties": { + "params": { + "${'$'}ref": "#/${'$'}defs/DogParams" + } + } + } + }, + { + "if": { + "properties": { + "kind": { "const": "cat" } + } + }, + "then": { + "properties": { + "params": { + "${'$'}ref": "#/${'$'}defs/CatParams" + } + } + } + } + ] + } + """ + + // Navigate to "params" — should find it in both allOf if/then branches + val paramsResults = navigateSchema(schema, listOf("params")) + assertEquals(2, paramsResults.size, "Expected params from both if/then branches") + + // Navigate deeper: params.treats should resolve through the DogParams $ref + val treatsResults = navigateSchema(schema, listOf("params", "treats")) + assertEquals(1, treatsResults.size, "Expected to find 'treats' through DogParams ref") + assertEquals("integer", ((treatsResults.single() as InternalKsonObject).propertyLookup["type"] as? InternalKsonString)?.value) + + // Navigate deeper: params.naps should resolve through the CatParams $ref + val napsResults = navigateSchema(schema, listOf("params", "naps")) + assertEquals(1, napsResults.size, "Expected to find 'naps' through CatParams ref") + assertEquals("integer", ((napsResults.single() as InternalKsonObject).propertyLookup["type"] as? InternalKsonString)?.value) + } + + @Test + fun testNavigateIfThenWithArrayItems() { + val schema = """ + { + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "if": { + "properties": { + "kind": { "const": "list" } + } + }, + "then": { + "properties": { + "items": { + "type": "array", + "items": { "type": "string", "description": "A list item" } + } + } + } + } + """ + + // Navigate through if/then to array items + val itemResults = navigateSchema(schema, listOf("items", "0")) + assertEquals(1, itemResults.size, "Expected to navigate through if/then into array items") + val itemSchema = itemResults.single() as InternalKsonObject + assertEquals("string", (itemSchema.propertyLookup["type"] as? InternalKsonString)?.value) + assertEquals("A list item", (itemSchema.propertyLookup["description"] as? InternalKsonString)?.value) + } + @Test fun testNavigateMixedPropertiesAndCombinators() { val schema = """ From 3f351716efeb5db25a281faedf674671b49452ef Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Sat, 28 Mar 2026 16:59:05 +0100 Subject: [PATCH 02/15] Filter if/then/else branches by evaluating conditions against the document 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. --- .../kotlin/org/kson/tooling/KsonTooling.kt | 2 +- .../kson/tooling/SchemaFilteringService.kt | 72 +++++++++----- .../org/kson/SchemaCompletionLocationTest.kt | 95 +++++++++++++++++++ .../org/kson/SchemaFilteringServiceTest.kt | 67 +++++++++++-- .../kotlin/org/kson/SchemaInfoLocationTest.kt | 58 +++++++++++ .../kotlin/org/kson/schema/SchemaIdLookup.kt | 51 +++++++--- 6 files changed, 300 insertions(+), 45 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt index 403be3fa..c36dada0 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt @@ -306,7 +306,7 @@ object KsonTooling { documentPointer: JsonPointer ): ResolvedSchemaContext { val schemaIdLookup = SchemaIdLookup(parsedSchema) - val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer) + val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, documentValue) val filteringService = SchemaFilteringService(schemaIdLookup) val validSchemas = filteringService.getValidSchemas(candidateSchemas, documentValue, documentPointer) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt index 060ba557..6f83dd1d 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt @@ -22,8 +22,8 @@ import kotlin.js.JsExport * Service for filtering schemas based on validation against document content. * * This service handles the validation-based filtering logic for JSON Schema combinators - * (oneOf/anyOf/allOf), ensuring that only compatible schemas are used for IDE features - * like completions, hover info, and jump-to-definition. + * (oneOf/anyOf/allOf) and conditionals (if/then/else), ensuring that only compatible + * schemas are used for IDE features like completions, hover info, and jump-to-definition. * * The filtering uses a "soft" validation approach: a schema is included if the existing * properties don't contradict it, even if required properties are missing. @@ -31,16 +31,16 @@ import kotlin.js.JsExport class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { /** - * Get valid schemas for a document path, applying combinator expansion and filtering. + * Get valid schemas for a document path, applying expansion and filtering. * * This function: - * 1. Expands combinator schemas (oneOf/anyOf/allOf) into individual branches - * 2. Filters branches based on validation against the current document (for oneOf/anyOf) + * 1. Expands combinator schemas (oneOf/anyOf/allOf) and conditionals (if/then/else) into individual branches + * 2. Filters branches based on validation against the current document (for oneOf/anyOf/if-then-else) * 3. Returns all branches for allOf and direct properties (no filtering needed) * * @param candidateSchemas The schemas found at the document path * @param documentValue The pre-parsed document value, or null if the document - * couldn't be parsed. When null, combinator filtering is skipped and all + * couldn't be parsed. When null, filtering is skipped and all * expanded schemas are returned. * @param documentPointer The [JsonPointer] to the location in the document * @return List of valid schemas after expansion and filtering @@ -50,22 +50,16 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { documentValue: KsonValue?, documentPointer: JsonPointer ): List { - // Check if we need to filter based on combinators - // This includes both schemas directly tagged as combinators AND schemas that contain combinator properties - // Note: Only oneOf/anyOf require validation-based filtering. - // allOf always includes all branches (no filtering needed). - val hasCombinatorsThatRequireValidation = candidateSchemas.any { ref -> + val hasBranchesThatRequireValidation = candidateSchemas.any { ref -> requiresValidationFiltering(ref) } - // Always expand combinators to get individual branches + // Always expand to get individual branches val expandedSchemas = schemaIdLookup.expandCombinators(candidateSchemas) - // Filter if needed (for oneOf/anyOf that require validation) - return if (hasCombinatorsThatRequireValidation && documentValue != null) { + return if (hasBranchesThatRequireValidation && documentValue != null) { filterByValidation(expandedSchemas, documentValue, documentPointer) } else { - // No filtering needed (no combinators, or document unavailable) expandedSchemas } } @@ -73,25 +67,27 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { /** * Checks if a schema reference requires validation-based filtering. * - * Only oneOf/anyOf combinators require validation filtering. allOf combinators - * always include all branches. + * oneOf/anyOf combinators and if/then/else conditionals require validation filtering. + * allOf combinators always include all branches (no filtering needed). * * @param ref The schema reference to check * @return true if the schema requires validation filtering */ private fun requiresValidationFiltering(ref: ResolvedRef): Boolean { - return ref.resolutionType == SchemaResolutionType.ANY_OF || - ref.resolutionType == SchemaResolutionType.ONE_OF || - (ref.resolvedValue as? KsonObject)?.let { obj -> - obj.propertyLookup.containsKey("oneOf") || obj.propertyLookup.containsKey("anyOf") + return ref.resolutionType in FILTERABLE_RESOLUTION_TYPES || + (ref.resolvedValue as? org.kson.value.KsonObject)?.let { obj -> + obj.propertyLookup.containsKey("oneOf") || + obj.propertyLookup.containsKey("anyOf") || + obj.propertyLookup.containsKey("if") } ?: false } /** * Filters schemas based on validation against the current document. * - * For schemas resolved via combinators (anyOf/oneOf), this validates them against - * the parent object to ensure only compatible schemas contribute completions. + * For schemas resolved via combinators (anyOf/oneOf) or conditionals (if/then/else), + * this validates them against the document value to ensure only compatible schemas + * contribute completions. * * This uses a "soft" validation approach: a schema is included if the existing * properties don't contradict it, even if required properties are missing. @@ -113,15 +109,24 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { val targetValue = KsonValueWalker.navigateWithJsonPointer(documentValue, documentPointer) ?: return candidateSchemas - return candidateSchemas.filter { ref -> + val filtered = candidateSchemas.filter { ref -> when (ref.resolutionType) { - // For anyOf/oneOf, check if the current document state is compatible + // For anyOf/oneOf and if/then/else, check if the current document state is compatible SchemaResolutionType.ANY_OF, - SchemaResolutionType.ONE_OF -> isSchemaValidForDocument(ref, targetValue) + SchemaResolutionType.ONE_OF, + SchemaResolutionType.IF_THEN, + SchemaResolutionType.IF_ELSE -> isSchemaValidForDocument(ref, targetValue) // For all other types (direct property, allOf, etc.), include them else -> true } } + + // If filtering eliminated every filterable branch, the document value likely + // doesn't match the expected shape yet (e.g. a list where objects are expected). + // Fall back to unfiltered schemas so completions remain available. + val filterableBranchesBefore = candidateSchemas.count { isFilterableBranch(it) } + val filterableBranchesAfter = filtered.count { isFilterableBranch(it) } + return if (filterableBranchesBefore > 0 && filterableBranchesAfter == 0) candidateSchemas else filtered } /** @@ -183,6 +188,9 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { } } + private fun isFilterableBranch(ref: ResolvedRef): Boolean = + ref.resolutionType in FILTERABLE_RESOLUTION_TYPES + companion object { /** * Error types that should be ignored during validation-based filtering. @@ -192,5 +200,17 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { MessageType.SCHEMA_REQUIRED_PROPERTY_MISSING, MessageType.SCHEMA_MISSING_REQUIRED_DEPENDENCIES ) + + /** + * Resolution types that require validation-based filtering. + * These are schema branches where multiple alternatives exist and + * only compatible ones should be shown. + */ + private val FILTERABLE_RESOLUTION_TYPES = setOf( + SchemaResolutionType.ANY_OF, + SchemaResolutionType.ONE_OF, + SchemaResolutionType.IF_THEN, + SchemaResolutionType.IF_ELSE + ) } } diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt index 1e8877ae..bf235412 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt @@ -6,6 +6,7 @@ import org.kson.tooling.KsonTooling import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertFalse import kotlin.test.assertTrue class SchemaCompletionLocationTest { @@ -1648,6 +1649,100 @@ class SchemaCompletionLocationTest { assertTrue("params" in labels, "Should include 'params' from allOf if/then branch, got: $labels") } + @Test + fun testOrchestraPatternCompletionsFilterToMatchingParameterModel() { + // Minimal reproduction of the orchestra.schema.kson pattern: + // - TaskModel has a base "parameters" with additionalProperties: true + // - allOf contains if/then blocks mapping integrationJob to specific parameter models + // - Parameter models have additionalProperties: false + // - Document is nested: pipeline > taskId > parameters + val schema = """ + { + "${'$'}defs": { + "TaskModel": { + "type": "object", + "additionalProperties": false, + "properties": { + "integrationJob": { "type": "string" }, + "parameters": { + "additionalProperties": true, + "type": "object" + } + }, + "allOf": [ + { + "if": { + "properties": { "integrationJob": { "const": "RUN_QUERY" } }, + "required": ["integrationJob"] + }, + "then": { + "properties": { + "parameters": { "${'$'}ref": "#/${'$'}defs/RunQueryParams" } + } + } + }, + { + "if": { + "properties": { "integrationJob": { "const": "SYNC_JOB" } }, + "required": ["integrationJob"] + }, + "then": { + "properties": { + "parameters": { "${'$'}ref": "#/${'$'}defs/SyncJobParams" } + } + } + } + ] + }, + "RunQueryParams": { + "type": "object", + "additionalProperties": false, + "properties": { + "statement": { "type": "string", "description": "SQL to execute" } + }, + "required": ["statement"] + }, + "SyncJobParams": { + "type": "object", + "additionalProperties": false, + "properties": { + "connector_id": { "type": "string", "description": "Connector to sync" } + }, + "required": ["connector_id"] + } + }, + "type": "object", + "properties": { + "pipeline": { + "type": "object", + "additionalProperties": { "${'$'}ref": "#/${'$'}defs/TaskModel" } + } + } + } + """ + + // Document: user picked RUN_QUERY and is inside parameters + val completions = getCompletionsAtCaret(schema, """ + { + "pipeline": { + "myTask": { + "integrationJob": "RUN_QUERY", + "parameters": { + + } + } + } + } + """.trimIndent()) + + assertNotNull(completions, "Should return completions") + val labels = completions.map { it.label } + + // Should see "statement" from RunQueryParams, NOT "connector_id" from SyncJobParams + assertTrue("statement" in labels, "Should include 'statement' from RunQueryParams, got: $labels") + assertFalse("connector_id" in labels, "Should NOT include 'connector_id' from SyncJobParams, got: $labels") + } + private fun searchExpressionSchema() = $$""" type: object additionalProperties: false diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt index efce9496..ad200ebc 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt @@ -270,9 +270,65 @@ class SchemaFilteringServiceTest { } @Test - fun testGetValidSchemas_withTypeMismatchAtTarget_filtersOutIncompatibleBranches() { - // Document is a list; both branches expect objects. No branch survives — - // only the parent (oneOf container) remains. + fun testGetValidSchemas_withIfThen_filtersIncompatibleConditionalBranches() { + // Schema with allOf containing if/then blocks — the orchestra pattern. + // The params property is only reachable via if/then (no anyOf fallback), + // so this test isolates the if/then filtering behavior. + val schema = """ + { + "${'$'}defs": { + "DogParams": { + "type": "object", + "additionalProperties": false, + "properties": { + "treats": { "type": "integer" } + } + }, + "CatParams": { + "type": "object", + "additionalProperties": false, + "properties": { + "naps": { "type": "integer" } + } + } + }, + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "allOf": [ + { + "if": { "properties": { "kind": { "const": "dog" } } }, + "then": { "properties": { "params": { "${'$'}ref": "#/${'$'}defs/DogParams" } } } + }, + { + "if": { "properties": { "kind": { "const": "cat" } } }, + "then": { "properties": { "params": { "${'$'}ref": "#/${'$'}defs/CatParams" } } } + } + ] + } + """.trimIndent() + + val document = """ + { + "kind": "dog", + "params": { + "treats": 5 + } + } + """.trimIndent() + + val validSchemas = getValidSchemasForDocument(schema, document, JsonPointer("/params")) + + assertEquals(1, validSchemas.size, "Only DogParams should survive filtering") + + val schema0 = validSchemas.single() as org.kson.value.KsonObject + val propertyNames = (schema0.propertyLookup["properties"] as? org.kson.value.KsonObject)?.propertyLookup?.keys ?: emptySet() + assertTrue("treats" in propertyNames, "DogParams properties should be present, got: $propertyNames") + } + + @Test + fun testGetValidSchemas_withTypeMismatchAtTarget_fallsBackToAllBranches() { val schema = """ oneOf: - type: object @@ -291,9 +347,6 @@ class SchemaFilteringServiceTest { val validSchemas = getValidSchemasForDocument(schema, document) - assertEquals( - 1, validSchemas.size, - "Only the parent schema should remain when target type doesn't match any branch" - ) + assertEquals(3, validSchemas.size, "Parent + both oneOf branches should be returned when target type doesn't match any branch") } } diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt index a226735e..fa9faf04 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt @@ -520,4 +520,62 @@ class SchemaInfoLocationTest { // Should have separator between the two assertTrue(hoverInfo.contains("---"), "Expected separator between branches. Got: $hoverInfo") } + + @Test + fun testGetSchemaInfoAtLocation_ifThenShowsMatchingBranch() { + // Hover on a property reached via if/then should show the specific + // parameter model, not a generic "any object" schema. + val schema = """ + { + "${'$'}defs": { + "DogParams": { + "type": "object", + "title": "Dog Parameters", + "description": "Parameters for dogs", + "additionalProperties": false, + "properties": { + "treats": { "type": "integer", "description": "Number of treats" } + } + }, + "CatParams": { + "type": "object", + "title": "Cat Parameters", + "description": "Parameters for cats", + "additionalProperties": false, + "properties": { + "naps": { "type": "integer", "description": "Number of naps" } + } + } + }, + "type": "object", + "properties": { + "kind": { "type": "string" } + }, + "allOf": [ + { + "if": { "properties": { "kind": { "const": "dog" } } }, + "then": { "properties": { "params": { "${'$'}ref": "#/${'$'}defs/DogParams" } } } + }, + { + "if": { "properties": { "kind": { "const": "cat" } } }, + "then": { "properties": { "params": { "${'$'}ref": "#/${'$'}defs/CatParams" } } } + } + ] + } + """ + + val hoverInfo = getInfoAtCaret(schema, """ + { + "kind": "dog", + "params": { + "treats": 5 + } + } + """.trimIndent()) + + assertNotNull(hoverInfo, "Should show hover info for params") + assertTrue(hoverInfo.contains("Dog Parameters"), "Should show DogParams title, got: $hoverInfo") + assertTrue(hoverInfo.contains("Parameters for dogs"), "Should show DogParams description, got: $hoverInfo") + assertFalse(hoverInfo.contains("Cat Parameters"), "Should NOT show CatParams, got: $hoverInfo") + } } diff --git a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt index f954a9fd..247d1529 100644 --- a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt +++ b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt @@ -1,5 +1,6 @@ package org.kson.schema +import org.kson.parser.MessageSink import org.kson.value.navigation.json_pointer.JsonPointer import org.kson.value.KsonList import org.kson.value.KsonObject @@ -191,6 +192,7 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { */ fun navigateByDocumentPointer( documentPointer: JsonPointer, + documentValue: KsonValue? = null, ): List { val startingBaseUri = "" val documentPathTokens = documentPointer.tokens @@ -202,6 +204,8 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { // Track all current schema nodes we're exploring (can branch out due to combinators) var currentNodes = listOf(resolveRefIfPresent(schemaRootValue, startingBaseUri)) + // Track the document value at the current navigation level for if/then evaluation + var currentDocumentValue = documentValue for (token in documentPathTokens) { val nextNodes = mutableListOf() @@ -229,7 +233,7 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { navigateArrayItems(node, updatedBaseUri) } else { // Object navigation: go to "properties" wrapper, then the property - navigateObjectProperty(node, token, updatedBaseUri) + navigateObjectProperty(node, token, updatedBaseUri, currentDocumentValue) } // Resolve $ref for each navigated node @@ -243,6 +247,11 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { if (currentNodes.isEmpty()) { break } + + // Advance document value in parallel with schema navigation + currentDocumentValue = currentDocumentValue?.let { docVal -> + KsonValueWalker.navigateWithJsonPointer(docVal, JsonPointer.fromTokens(listOf(token))) + } } return currentNodes @@ -368,17 +377,19 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { /** * Navigate through if/then/else conditional schemas. * - * Navigates into both the "then" and "else" sub-schemas (when present), - * resolving any $ref in each branch before delegating to the caller's - * navigation function. + * When [documentValue] is available, evaluates the "if" condition against it + * and only includes the matching branch (then or else). When unavailable, + * includes both branches so that all possible schemas are discoverable. * * @param schemaNode The schema node containing if/then/else * @param currentBaseUri The current base URI for $ref resolution + * @param documentValue The document value at this schema level, used to evaluate the "if" condition * @param recursiveNavigate How to continue navigation within each branch */ private fun navigateThroughConditionals( schemaNode: KsonObject, currentBaseUri: String, + documentValue: KsonValue? = null, recursiveNavigate: (schema: KsonObject, baseUri: String) -> List ): List { val results = mutableListOf() @@ -389,16 +400,32 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { if (resolved.resolvedValue is KsonObject) { val nestedResults = recursiveNavigate(resolved.resolvedValue, resolved.resolvedValueBaseUri) // Unlike navigateThroughCombinators, we always tag results with the - // conditional type. Combinators need selective tagging because their - // branches are validated individually (anyOf/oneOf filtering). - // Conditional branches are not yet filtered by evaluating the "if" - // condition, so preserving inner resolution types has no benefit today. + // conditional type. This tagging is what SchemaFilteringService uses + // to identify branches that need validation-based filtering. results.addAll(nestedResults.map { ref -> ref.copy(resolutionType = resolutionType) }) } } + // When we have a document value, evaluate the "if" condition to determine + // which branch to include. This is critical when the "if" condition + // checks a sibling property that isn't visible from the target property. + val ifCondition = schemaNode.propertyLookup["if"] + if (documentValue != null && ifCondition != null) { + val ifSchema = SchemaParser.parseSchemaElement(ifCondition, MessageSink(), currentBaseUri, this) + if (ifSchema != null) { + val conditionMatches = ifSchema.isValid(documentValue, MessageSink()) + if (conditionMatches) { + processConditionalBranch(schemaNode.propertyLookup["then"], SchemaResolutionType.IF_THEN) + } else { + processConditionalBranch(schemaNode.propertyLookup["else"], SchemaResolutionType.IF_ELSE) + } + return results + } + } + + // No document value or couldn't parse if condition — include both branches processConditionalBranch(schemaNode.propertyLookup["then"], SchemaResolutionType.IF_THEN) processConditionalBranch(schemaNode.propertyLookup["else"], SchemaResolutionType.IF_ELSE) @@ -423,7 +450,8 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { private fun navigateObjectProperty( schemaNode: KsonObject, propertyName: String, - currentBaseUri: String + currentBaseUri: String, + documentValue: KsonValue? = null ): List { val results = mutableListOf() @@ -454,7 +482,7 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { results.addAll(navigateThroughCombinators( schemaNode = schemaNode, currentBaseUri = currentBaseUri, - recursiveNavigate = { schema, baseUri -> navigateObjectProperty(schema, propertyName, baseUri) }, + recursiveNavigate = { schema, baseUri -> navigateObjectProperty(schema, propertyName, baseUri, documentValue) }, shouldTagWithCombinator = { it in listOf( SchemaResolutionType.DIRECT_PROPERTY, SchemaResolutionType.PATTERN_PROPERTY, @@ -468,7 +496,8 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { results.addAll(navigateThroughConditionals( schemaNode = schemaNode, currentBaseUri = currentBaseUri, - recursiveNavigate = { schema, baseUri -> navigateObjectProperty(schema, propertyName, baseUri) } + documentValue = documentValue, + recursiveNavigate = { schema, baseUri -> navigateObjectProperty(schema, propertyName, baseUri, documentValue) } )) } From d8fcf8026f42e9e2d7a5a05bee0da4812434e372 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 11:15:24 -0700 Subject: [PATCH 03/15] test: condense verbose if/then and orchestra-pattern fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several fixture schemas (DogParams/CatParams, long orchestra examples) were exploratory — useful while building if/then navigation, noisy now that the assertions are stable. Shrink them to minimal repros so the intent of each test is obvious at a glance. Also pass the parsed document through in SchemaFilteringServiceTest's navigate helper so it exercises if/then condition evaluation. --- .../org/kson/SchemaCompletionLocationTest.kt | 126 ++++-------------- .../org/kson/SchemaFilteringServiceTest.kt | 7 +- .../kotlin/org/kson/SchemaInfoLocationTest.kt | 51 ++----- .../org/kson/schema/SchemaNavigationTest.kt | 3 +- 4 files changed, 43 insertions(+), 144 deletions(-) diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt index bf235412..33d26fea 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt @@ -1602,145 +1602,77 @@ class SchemaCompletionLocationTest { @Test fun testAllOfWithIfThenCompletionsIncludeConditionalProperties() { - // Test the orchestra.schema.kson pattern: allOf containing if/then blocks + // allOf if/then should surface properties from matching branches val schema = """ { - "${'$'}defs": { - "DogParams": { - "type": "object", - "properties": { - "treats": { "type": "integer" } - } - } - }, "type": "object", - "properties": { - "kind": { "type": "string" } - }, + "properties": { "kind": { "type": "string" } }, "allOf": [ { - "if": { - "properties": { - "kind": { "const": "dog" } - } - }, - "then": { - "properties": { - "params": { - "${'$'}ref": "#/${'$'}defs/DogParams" - } - } - } + "if": { "properties": { "kind": { "const": "dog" } } }, + "then": { "properties": { "bark": { "type": "boolean" } } } } ] } """ - // Completions at the root level should include "params" from the allOf if/then branch val completions = getCompletionsAtCaret(schema, """ - { - "kind": "dog", - - } + { "kind": "dog", } """.trimIndent()) - assertNotNull(completions, "Should return completions") - val labels = completions.map { it.label } - assertTrue("params" in labels, "Should include 'params' from allOf if/then branch, got: $labels") + assertNotNull(completions) + assertTrue("bark" in completions.map { it.label }, "Should include 'bark' from then branch") } @Test - fun testOrchestraPatternCompletionsFilterToMatchingParameterModel() { - // Minimal reproduction of the orchestra.schema.kson pattern: - // - TaskModel has a base "parameters" with additionalProperties: true - // - allOf contains if/then blocks mapping integrationJob to specific parameter models - // - Parameter models have additionalProperties: false - // - Document is nested: pipeline > taskId > parameters + fun testIfThenFiltersNestedPropertyCompletionsBySiblingValue() { + // A nested property's completions should be narrowed by if/then evaluation + // against a sibling at the parent level. The base "config" allows any + // properties; the if/then narrows to a specific $ref based on "kind". val schema = """ { "${'$'}defs": { - "TaskModel": { + "Item": { "type": "object", - "additionalProperties": false, "properties": { - "integrationJob": { "type": "string" }, - "parameters": { - "additionalProperties": true, - "type": "object" - } + "kind": { "type": "string" }, + "config": { "additionalProperties": true, "type": "object" } }, "allOf": [ { - "if": { - "properties": { "integrationJob": { "const": "RUN_QUERY" } }, - "required": ["integrationJob"] - }, - "then": { - "properties": { - "parameters": { "${'$'}ref": "#/${'$'}defs/RunQueryParams" } - } - } + "if": { "properties": { "kind": { "const": "a" } }, "required": ["kind"] }, + "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigA" } } } }, { - "if": { - "properties": { "integrationJob": { "const": "SYNC_JOB" } }, - "required": ["integrationJob"] - }, - "then": { - "properties": { - "parameters": { "${'$'}ref": "#/${'$'}defs/SyncJobParams" } - } - } + "if": { "properties": { "kind": { "const": "b" } }, "required": ["kind"] }, + "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigB" } } } } ] }, - "RunQueryParams": { - "type": "object", - "additionalProperties": false, - "properties": { - "statement": { "type": "string", "description": "SQL to execute" } - }, - "required": ["statement"] + "ConfigA": { + "type": "object", "additionalProperties": false, + "properties": { "alpha": { "type": "string" } } }, - "SyncJobParams": { - "type": "object", - "additionalProperties": false, - "properties": { - "connector_id": { "type": "string", "description": "Connector to sync" } - }, - "required": ["connector_id"] + "ConfigB": { + "type": "object", "additionalProperties": false, + "properties": { "beta": { "type": "string" } } } }, "type": "object", "properties": { - "pipeline": { - "type": "object", - "additionalProperties": { "${'$'}ref": "#/${'$'}defs/TaskModel" } - } + "items": { "type": "object", "additionalProperties": { "${'$'}ref": "#/${'$'}defs/Item" } } } } """ - // Document: user picked RUN_QUERY and is inside parameters val completions = getCompletionsAtCaret(schema, """ - { - "pipeline": { - "myTask": { - "integrationJob": "RUN_QUERY", - "parameters": { - - } - } - } - } + { "items": { "x": { "kind": "a", "config": { } } } } """.trimIndent()) - assertNotNull(completions, "Should return completions") + assertNotNull(completions) val labels = completions.map { it.label } - - // Should see "statement" from RunQueryParams, NOT "connector_id" from SyncJobParams - assertTrue("statement" in labels, "Should include 'statement' from RunQueryParams, got: $labels") - assertFalse("connector_id" in labels, "Should NOT include 'connector_id' from SyncJobParams, got: $labels") + assertTrue("alpha" in labels, "Should include 'alpha' from ConfigA, got: $labels") + assertFalse("beta" in labels, "Should NOT include 'beta' from ConfigB, got: $labels") } private fun searchExpressionSchema() = $$""" diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt index ad200ebc..6b965899 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt @@ -30,7 +30,7 @@ class SchemaFilteringServiceTest { val parsedDocument = KsonCore.parseToAst(document).ksonValue val schemaIdLookup = SchemaIdLookup(parsedSchema) val filteringService = SchemaFilteringService(schemaIdLookup) - val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer) + val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, parsedDocument) return filteringService.getValidSchemas(candidateSchemas, parsedDocument, documentPointer).map { it.resolvedValue } } @@ -271,9 +271,8 @@ class SchemaFilteringServiceTest { @Test fun testGetValidSchemas_withIfThen_filtersIncompatibleConditionalBranches() { - // Schema with allOf containing if/then blocks — the orchestra pattern. - // The params property is only reachable via if/then (no anyOf fallback), - // so this test isolates the if/then filtering behavior. + // allOf with if/then blocks that select a $ref based on a sibling property. + // params is only reachable via if/then (no anyOf), isolating the filtering. val schema = """ { "${'$'}defs": { diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt index fa9faf04..6d8f06a9 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt @@ -523,59 +523,28 @@ class SchemaInfoLocationTest { @Test fun testGetSchemaInfoAtLocation_ifThenShowsMatchingBranch() { - // Hover on a property reached via if/then should show the specific - // parameter model, not a generic "any object" schema. + // Hover should show the specific $ref from the matching if/then branch val schema = """ { "${'$'}defs": { - "DogParams": { - "type": "object", - "title": "Dog Parameters", - "description": "Parameters for dogs", - "additionalProperties": false, - "properties": { - "treats": { "type": "integer", "description": "Number of treats" } - } - }, - "CatParams": { - "type": "object", - "title": "Cat Parameters", - "description": "Parameters for cats", - "additionalProperties": false, - "properties": { - "naps": { "type": "integer", "description": "Number of naps" } - } - } + "ConfigA": { "type": "object", "title": "Config A", "description": "Settings for A" }, + "ConfigB": { "type": "object", "title": "Config B", "description": "Settings for B" } }, "type": "object", - "properties": { - "kind": { "type": "string" } - }, + "properties": { "kind": { "type": "string" } }, "allOf": [ - { - "if": { "properties": { "kind": { "const": "dog" } } }, - "then": { "properties": { "params": { "${'$'}ref": "#/${'$'}defs/DogParams" } } } - }, - { - "if": { "properties": { "kind": { "const": "cat" } } }, - "then": { "properties": { "params": { "${'$'}ref": "#/${'$'}defs/CatParams" } } } - } + { "if": { "properties": { "kind": { "const": "a" } } }, "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigA" } } } }, + { "if": { "properties": { "kind": { "const": "b" } } }, "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigB" } } } } ] } """ val hoverInfo = getInfoAtCaret(schema, """ - { - "kind": "dog", - "params": { - "treats": 5 - } - } + { "kind": "a", "config": {} } """.trimIndent()) - assertNotNull(hoverInfo, "Should show hover info for params") - assertTrue(hoverInfo.contains("Dog Parameters"), "Should show DogParams title, got: $hoverInfo") - assertTrue(hoverInfo.contains("Parameters for dogs"), "Should show DogParams description, got: $hoverInfo") - assertFalse(hoverInfo.contains("Cat Parameters"), "Should NOT show CatParams, got: $hoverInfo") + assertNotNull(hoverInfo) + assertTrue(hoverInfo.contains("Config A"), "Should show matching branch title, got: $hoverInfo") + assertFalse(hoverInfo.contains("Config B"), "Should NOT show non-matching branch, got: $hoverInfo") } } diff --git a/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt b/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt index ce38ce5e..29fda6da 100644 --- a/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt +++ b/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt @@ -715,8 +715,7 @@ class SchemaNavigationTest { @Test fun testNavigateAllOfWithIfThen() { - // This mirrors the orchestra.schema.kson pattern: - // allOf contains if/then blocks that conditionally constrain properties + // allOf contains if/then blocks that select a $ref based on a sibling property val schema = """ { "${'$'}defs": { From 8369bd1b2815cf07fcc01c49b8577f99164d535f Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 11:15:50 -0700 Subject: [PATCH 04/15] Support const value completions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../tooling/navigation/SchemaInformation.kt | 14 +++++ .../org/kson/SchemaCompletionLocationTest.kt | 63 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt index 353ccb5a..3bea759f 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt @@ -234,6 +234,7 @@ internal fun InternalKsonValue.extractCompletions( * * Provides completions for: * - Object properties (if type is object) + * - Const value (if const is defined) * - Enum values (if enum is defined) * - Boolean values (if type is boolean) * - Null value (if type is null or includes null) @@ -255,6 +256,19 @@ private fun InternalKsonObject.extractValueCompletions(): List { val completions = mutableListOf() + // If const exists, offer that single value + propertyLookup["const"]?.let { constValue -> + completions.add( + CompletionItem( + label = constValue.formatValueForDisplay(), + detail = "const value", + documentation = this.extractSchemaInfo(), + kind = CompletionKind.VALUE + ) + ) + return completions + } + // If enum exists, offer those values (propertyLookup["enum"] as? InternalKsonList)?.let { enumList -> enumList.elements.forEach { enumValue -> diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt index 33d26fea..350ba428 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt @@ -30,6 +30,69 @@ class SchemaCompletionLocationTest { return KsonTooling.getCompletionsAtLocation(KsonTooling.parse(document), KsonTooling.parse(schema), line, column) } + @Test + fun testConstValueCompletions() { + val schema = """ + { + type: object + properties: { + status: { + const: "active" + description: "Always active" + } + } + } + """ + + val completions = getCompletionsAtCaret(schema, """ + { + status: "" + } + """.trimIndent()) + + assertNotNull(completions, "Should return completions for const value") + val labels = completions.map { it.label } + assertEquals(listOf("active"), labels, "Should offer only the const value") + } + + @Test + fun testIfThenNarrowsConstValueForSiblingProperty() { + // if/then narrows a property to a const based on a sibling value + val schema = """ + { + "type": "object", + "properties": { + "kind": { "type": "string" }, + "breed": { "type": "string" } + }, + "allOf": [ + { + "if": { + "properties": { "kind": { "const": "dog" } }, + "required": ["kind"] + }, + "then": { + "properties": { + "breed": { "const": "labrador" } + } + } + } + ] + } + """ + + val completions = getCompletionsAtCaret(schema, """ + { + "kind": "dog", + "breed": "" + } + """.trimIndent()) + + assertNotNull(completions, "Should return completions") + val labels = completions.map { it.label } + assertTrue("labrador" in labels, "Should include const from matching if/then, got: $labels") + } + @Test fun testEnumValueCompletions() { val schema = """ From 2b905631bda780c5fb244fde1c3db175ae960817 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 11:16:26 -0700 Subject: [PATCH 05/15] test: extract getCompletionsAtCaret into SchemaCompletionTest interface The same helper was duplicated across SchemaCompletionLocationTest and follow-up completion tests. Move it to a shared interface so new tests can mix it in instead of recopying. --- .../kotlin/org/kson/SchemaCompletionTest.kt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionTest.kt diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionTest.kt new file mode 100644 index 00000000..61102a51 --- /dev/null +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionTest.kt @@ -0,0 +1,26 @@ +package org.kson + +import org.kson.tooling.CompletionItem +import org.kson.tooling.KsonTooling + +/** + * Shared test interface for schema-driven completion tests. + * + * Provides a helper that parses a document with a `` marker, computes + * the cursor position, and returns the completions at that position. + */ +interface SchemaCompletionTest { + + fun getCompletionsAtCaret(schema: String, documentWithCaret: String): List? { + val caretMarker = "" + val caretIndex = documentWithCaret.indexOf(caretMarker) + require(caretIndex >= 0) { "Document must contain $caretMarker marker" } + + val beforeCaret = documentWithCaret.substring(0, caretIndex) + val line = beforeCaret.count { it == '\n' } + val column = caretIndex - (beforeCaret.lastIndexOf('\n') + 1) + val document = documentWithCaret.replace(caretMarker, "") + + return KsonTooling.getCompletionsAtLocation(KsonTooling.parse(document), KsonTooling.parse(schema), line, column) + } +} From 1e096cc4b0f57a9bc4af17273eaef0475bf413bf Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 11:16:38 -0700 Subject: [PATCH 06/15] Generalize completion narrowing from const-only to intersection semantics 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. --- .../tooling/navigation/SchemaInformation.kt | 80 ++++++++++++- .../org/kson/SchemaCompletionLocationTest.kt | 109 +++++++++++++++++- 2 files changed, 185 insertions(+), 4 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt index 3bea759f..bfd9eb85 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt @@ -6,6 +6,7 @@ import org.kson.parser.Location import org.kson.value.navigation.json_pointer.JsonPointer import org.kson.schema.ResolvedRef import org.kson.schema.SchemaIdLookup +import org.kson.schema.SchemaResolutionType import org.kson.walker.KsonValueWalker import org.kson.walker.navigateWithJsonPointer import org.kson.value.KsonValue as InternalKsonValue @@ -73,9 +74,7 @@ internal object SchemaInformation{ documentValue: InternalKsonValue? = null ): List { val resolvedSchemas = validSchemas ?: SchemaIdLookup(schemaValue).navigateByDocumentPointer(documentPointer) - val allCompletions = resolvedSchemas - .flatMap { it.resolvedValue.extractCompletions() } - .distinctBy { it.label } // Remove duplicates based on label + val allCompletions = extractCompletionsWithNarrowing(resolvedSchemas) // Only filter if: // 1. Document value is provided @@ -111,6 +110,81 @@ internal object SchemaInformation{ } } +/** + * Resolution types where schemas constrain each other (intersection semantics). + * A value must satisfy ALL reductive schemas simultaneously, so value completions + * from these schemas are intersected (e.g., a base property's enum intersected with + * an if/then branch's narrower enum or const). + */ +private val REDUCTIVE_RESOLUTION_TYPES = setOf( + SchemaResolutionType.DIRECT_PROPERTY, + SchemaResolutionType.PATTERN_PROPERTY, + SchemaResolutionType.ADDITIONAL_PROPERTY, + SchemaResolutionType.ARRAY_ITEMS, + SchemaResolutionType.ALL_OF, + SchemaResolutionType.IF_THEN, + SchemaResolutionType.IF_ELSE, + SchemaResolutionType.ROOT +) + +/** + * Extract completions from resolved schemas, applying JSON Schema narrowing semantics. + * + * JSON Schema combinators have different semantics for completions: + * - **Reductive** (allOf, if/then/else, direct properties): a value must satisfy all + * schemas simultaneously. When multiple reductive schemas provide value completions, + * only values present in ALL schemas are valid (intersection semantics). + * - **Additive** (anyOf, oneOf): a value must satisfy at least one branch. + * Completions from different branches are alternatives and are merged. + * + * @param resolvedSchemas The schemas found at the document path, with resolution type metadata + * @return Deduplicated list of completion items respecting narrowing semantics + */ +private fun extractCompletionsWithNarrowing(resolvedSchemas: List): List { + val reductive = resolvedSchemas.filter { it.resolutionType in REDUCTIVE_RESOLUTION_TYPES } + val additive = resolvedSchemas.filter { it.resolutionType !in REDUCTIVE_RESOLUTION_TYPES } + + // Get completions from each reductive schema separately + val perSchemaCompletions = reductive.map { it.resolvedValue.extractCompletions() } + + // Partition into value and property completions per schema + val perSchemaValueCompletions = perSchemaCompletions + .map { completions -> completions.filter { it.kind == CompletionKind.VALUE } } + .filter { it.isNotEmpty() } + + // Property completions are unioned (not intersected) because allOf schemas can each + // contribute additional properties — the full set is the union of all branches. + val allPropertyCompletions = perSchemaCompletions + .flatMap { completions -> completions.filter { it.kind == CompletionKind.PROPERTY } } + + // Reductive value completions: intersect when multiple schemas constrain the values. + // A value must satisfy ALL reductive schemas, so only values present in every schema's + // completions are valid. Falls back to union if intersection is empty (e.g., when + // multiple unfiltered if/then branches are all included because no document value + // was available to evaluate conditions). + val reductiveValueCompletions = if (perSchemaValueCompletions.size > 1) { + val labelSets = perSchemaValueCompletions.map { completions -> + completions.map { it.label }.toSet() + } + val intersection = labelSets.reduce { acc, set -> acc.intersect(set) } + + if (intersection.isNotEmpty()) { + // Keep completions from the narrowest schema (fewest values) for better detail/docs + val narrowest = perSchemaValueCompletions.minBy { it.size } + narrowest.filter { it.label in intersection } + } else { + perSchemaValueCompletions.flatten() + } + } else { + perSchemaValueCompletions.flatten() + } + + val reductiveCompletions = allPropertyCompletions + reductiveValueCompletions + val additiveCompletions = additive.flatMap { it.resolvedValue.extractCompletions() } + + return (reductiveCompletions + additiveCompletions).distinctBy { it.label } +} + /** * Extract schema information from a schema node. * diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt index 350ba428..f5453c80 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt @@ -90,7 +90,114 @@ class SchemaCompletionLocationTest { assertNotNull(completions, "Should return completions") val labels = completions.map { it.label } - assertTrue("labrador" in labels, "Should include const from matching if/then, got: $labels") + assertEquals(listOf("labrador"), labels, "Should narrow to only the const value, got: $labels") + } + + @Test + fun testIfThenNarrowsEnumValueForSiblingProperty() { + // if/then narrows a property's enum based on a sibling value. + // The base property has all enum values; the matching if/then branch + // constrains it to a subset via intersection semantics. + val schema = """ + { + "type": "object", + "properties": { + "integration": { "type": "string" }, + "job": { + "type": "string", + "enum": ["SNOW_QUERY", "SNOW_TEST", "DBT_RUN", "DBT_TEST"] + } + }, + "allOf": [ + { + "if": { + "properties": { "integration": { "const": "SNOWFLAKE" } }, + "required": ["integration"] + }, + "then": { + "properties": { + "job": { "enum": ["SNOW_QUERY", "SNOW_TEST"] } + } + } + }, + { + "if": { + "properties": { "integration": { "const": "DBT" } }, + "required": ["integration"] + }, + "then": { + "properties": { + "job": { "enum": ["DBT_RUN", "DBT_TEST"] } + } + } + } + ] + } + """ + + val completions = getCompletionsAtCaret(schema, """ + { + "integration": "SNOWFLAKE", + "job": "" + } + """.trimIndent()) + + assertNotNull(completions, "Should return completions") + val labels = completions.map { it.label } + assertEquals(listOf("SNOW_QUERY", "SNOW_TEST"), labels.sorted(), + "Should narrow to SNOWFLAKE jobs only, got: $labels") + } + + @Test + fun testIfThenEnumNarrowingFallsBackWhenNoSiblingValue() { + // When no sibling value is set, all enum values should be available + val schema = """ + { + "type": "object", + "properties": { + "integration": { "type": "string" }, + "job": { + "type": "string", + "enum": ["SNOW_QUERY", "DBT_RUN"] + } + }, + "allOf": [ + { + "if": { + "properties": { "integration": { "const": "SNOWFLAKE" } }, + "required": ["integration"] + }, + "then": { + "properties": { + "job": { "enum": ["SNOW_QUERY"] } + } + } + }, + { + "if": { + "properties": { "integration": { "const": "DBT" } }, + "required": ["integration"] + }, + "then": { + "properties": { + "job": { "enum": ["DBT_RUN"] } + } + } + } + ] + } + """ + + val completions = getCompletionsAtCaret(schema, """ + { + "job": "" + } + """.trimIndent()) + + assertNotNull(completions, "Should return completions") + val labels = completions.map { it.label } + assertEquals(listOf("DBT_RUN", "SNOW_QUERY"), labels.sorted(), + "Should include all enum values when integration is not set, got: $labels") } @Test From f4a19d9fef994b800d1863912d0710c0d54321f1 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 11:19:07 -0700 Subject: [PATCH 07/15] Use partial AST values for completion narrowing in broken documents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../kotlin/org/kson/tooling/KsonTooling.kt | 8 +- .../org/kson/tooling/ToolingDocument.kt | 14 ++ .../kson/BidirectionalIfThenCompletionTest.kt | 138 ++++++++++++++++++ .../kotlin/org/kson/value/KsonValue.kt | 82 +++++++---- 4 files changed, 212 insertions(+), 30 deletions(-) create mode 100644 kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt index c36dada0..ee319b64 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt @@ -159,9 +159,13 @@ object KsonTooling { ): List { val parsedSchema = schema.ksonValue ?: return emptyList() val documentPointer = KsonValuePathBuilder(document, Coordinates(line, column)).buildJsonPointerToPosition(includePropertyKeys = false) ?: return emptyList() - val context = ResolvedSchemaContext.resolveAndFilterSchemas(parsedSchema, document.ksonValue, documentPointer) + val schemaIdLookup = SchemaIdLookup(parsedSchema) + val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, document.partialKsonValue) + + val filteringService = SchemaFilteringService(schemaIdLookup) + val validSchemas = filteringService.getValidSchemas(candidateSchemas, document.ksonValue, documentPointer) - return SchemaInformation.getCompletions(context.schemaIdLookup.schemaRootValue, documentPointer, context.validSchemas, context.parsedDocument) + return SchemaInformation.getCompletions(parsedSchema, documentPointer, validSchemas, document.ksonValue) } /** diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/ToolingDocument.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/ToolingDocument.kt index 7c469f0c..273db134 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/ToolingDocument.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/ToolingDocument.kt @@ -13,6 +13,7 @@ import org.kson.validation.SourceContext import org.kson.value.KsonObject import org.kson.value.KsonString import org.kson.value.KsonValue +import org.kson.value.toKsonValueOrNull import kotlin.js.ExperimentalJsExport import kotlin.js.JsExport @@ -55,6 +56,19 @@ class ToolingDocument internal constructor(val content: String, internal val sou */ internal val rootAstNode: AstNode? get() = (parseResult.ast as? KsonRootImpl)?.rootNode + /** + * Partial [KsonValue] that skips error nodes rather than returning null. + * + * Falls back to [ksonValue] when available (no errors), otherwise builds a + * partial tree from the AST by silently dropping properties/elements that + * contain parse errors. This allows IDE features like completion narrowing + * to see successfully-parsed sibling values even when the cursor position + * has an incomplete value (e.g. `key:` with no value yet). + */ + internal val partialKsonValue: KsonValue? by lazy { + ksonValue ?: rootAstNode?.toKsonValueOrNull() + } + /** Full gap-free token list (includes WHITESPACE and COMMENT). */ internal val tokens: List get() = parseResult.lexedTokens diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt new file mode 100644 index 00000000..3227f535 --- /dev/null +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt @@ -0,0 +1,138 @@ +package org.kson + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull + +/** + * End-to-end tests for bidirectional if/then completion narrowing. + * + * Validates the full completions pipeline: schema navigation with partial + * document values, if/then condition evaluation, enum intersection, and the + * fallback to full enum when no sibling is set. + * + * The schema uses bidirectional if/then clauses where property A narrows + * property B and vice versa — a common pattern for interdependent enums. + */ +class BidirectionalIfThenCompletionTest : SchemaCompletionTest { + + private fun assertCompletionLabels(schema: String, doc: String, expected: List, message: String) { + val completions = getCompletionsAtCaret(schema, doc) + assertNotNull(completions, "$message: should return completions") + assertEquals(expected, completions.map { it.label }.sorted(), message) + } + + private val schema = """ + { + "type": "object", + "properties": { + "integration": { + "type": "string", + "enum": ["SNOWFLAKE", "DBT"] + }, + "job": { + "type": "string", + "enum": ["SNOW_QUERY", "SNOW_TEST", "DBT_RUN", "DBT_TEST"] + } + }, + "allOf": [ + { + "if": { "properties": { "integration": { "const": "SNOWFLAKE" } }, "required": ["integration"] }, + "then": { "properties": { "job": { "enum": ["SNOW_QUERY", "SNOW_TEST"] } } } + }, + { + "if": { "properties": { "integration": { "const": "DBT" } }, "required": ["integration"] }, + "then": { "properties": { "job": { "enum": ["DBT_RUN", "DBT_TEST"] } } } + }, + { + "if": { "properties": { "job": { "const": "SNOW_QUERY" } }, "required": ["job"] }, + "then": { "properties": { "integration": { "const": "SNOWFLAKE" } } } + }, + { + "if": { "properties": { "job": { "const": "SNOW_TEST" } }, "required": ["job"] }, + "then": { "properties": { "integration": { "const": "SNOWFLAKE" } } } + }, + { + "if": { "properties": { "job": { "const": "DBT_RUN" } }, "required": ["job"] }, + "then": { "properties": { "integration": { "const": "DBT" } } } + }, + { + "if": { "properties": { "job": { "const": "DBT_TEST" } }, "required": ["job"] }, + "then": { "properties": { "integration": { "const": "DBT" } } } + } + ] + } + """ + + @Test + fun testJobNarrowedByIntegrationSnowflake() { + assertCompletionLabels(schema, + """{"integration": "SNOWFLAKE", "job": ""}""", + listOf("SNOW_QUERY", "SNOW_TEST"), + "SNOWFLAKE integration should narrow job to SNOW_* values") + } + + @Test + fun testJobNarrowedByIntegrationDbt() { + assertCompletionLabels(schema, + """{"integration": "DBT", "job": ""}""", + listOf("DBT_RUN", "DBT_TEST"), + "DBT integration should narrow job to DBT_* values") + } + + @Test + fun testJobNarrowedWithEmptyKsonValue() { + assertCompletionLabels(schema, + "integration: SNOWFLAKE\njob: ", + listOf("SNOW_QUERY", "SNOW_TEST"), + "KSON empty value should still narrow via partial AST") + } + + @Test + fun testJobNarrowedWithPartialKsonValue() { + assertCompletionLabels(schema, + "integration: SNOWFLAKE\njob: S", + listOf("SNOW_QUERY", "SNOW_TEST"), + "KSON partial value should narrow") + } + + @Test + fun testIntegrationNarrowedByJobSnowQuery() { + assertCompletionLabels(schema, + """{"job": "SNOW_QUERY", "integration": ""}""", + listOf("SNOWFLAKE"), + "SNOW_QUERY job should narrow integration to SNOWFLAKE") + } + + @Test + fun testIntegrationNarrowedByJobDbtRun() { + assertCompletionLabels(schema, + """{"job": "DBT_RUN", "integration": ""}""", + listOf("DBT"), + "DBT_RUN job should narrow integration to DBT") + } + + @Test + fun testIntegrationNarrowedWithEmptyKsonValue() { + assertCompletionLabels(schema, + "job: SNOW_QUERY\nintegration: ", + listOf("SNOWFLAKE"), + "Reverse narrowing should work with KSON empty value") + } + + @Test + fun testIntegrationShowsAllWithoutSibling() { + assertCompletionLabels(schema, + """{"integration": ""}""", + listOf("DBT", "SNOWFLAKE"), + "Without job sibling, all integrations should be offered") + } + + @Test + fun testJobShowsAllWithoutSibling() { + assertCompletionLabels(schema, + """{"job": ""}""", + listOf("DBT_RUN", "DBT_TEST", "SNOW_QUERY", "SNOW_TEST"), + "Without integration sibling, all jobs should be offered") + } +} diff --git a/src/commonMain/kotlin/org/kson/value/KsonValue.kt b/src/commonMain/kotlin/org/kson/value/KsonValue.kt index 50caa4a3..3869fac2 100644 --- a/src/commonMain/kotlin/org/kson/value/KsonValue.kt +++ b/src/commonMain/kotlin/org/kson/value/KsonValue.kt @@ -214,48 +214,74 @@ class KsonNull(astNode: NullNode) : KsonValue(astNode) { } } -fun AstNode.toKsonValue(): KsonValue { +/** + * Strict AST → [KsonValue] conversion. Throws if the AST contains any error node + * or structural irregularity. Use when the AST is expected to be fully valid. + */ +fun AstNode.toKsonValue(): KsonValue = toKsonValueInternal(skipErrors = false) + ?: throw UnsupportedOperationException("Cannot create ${KsonValue::class.simpleName} from ${this::class.simpleName}") + +/** + * Error-tolerant AST → [KsonValue] conversion. + * + * Returns null when this node is itself an error (or otherwise unconvertible). + * For containers, silently drops child properties/elements that contain errors — + * preserving successfully-parsed siblings. Used for IDE features that need + * visibility into partial documents (e.g. completion narrowing where the cursor + * position has an incomplete value but sibling values must still be seen). + */ +fun AstNode.toKsonValueOrNull(): KsonValue? = toKsonValueInternal(skipErrors = true) + +private fun AstNode.toKsonValueInternal(skipErrors: Boolean): KsonValue? { if (this !is AstNodeImpl) { - /** - * Must have a fully valid [AstNodeImpl] to create an [KsonValue] for it - */ + if (skipErrors) return null throw RuntimeException("Cannot create ${KsonValue::class.simpleName} Node from a ${this::class.simpleName}") } return when (this) { - is KsonRootImpl -> rootNode.toKsonValue() + is AstNodeError -> { + if (skipErrors) null + else throw UnsupportedOperationException("Cannot create Valid Ast Node from ${this::class.simpleName}") + } + is KsonRootImpl -> rootNode.toKsonValueInternal(skipErrors) is ObjectNode -> { - KsonObject(properties.associate { prop -> + val props = properties.mapNotNull { prop -> val propImpl = prop as? ObjectPropertyNodeImpl - ?: throw ShouldNotHappenException("this AST is fully valid") + ?: if (skipErrors) return@mapNotNull null + else throw ShouldNotHappenException("this AST is fully valid") val propKey = propImpl.key as? ObjectKeyNodeImpl - ?: throw ShouldNotHappenException("this AST is fully valid") - val keyName = propKey.key.toKsonValue() as KsonString - keyName.value to KsonObjectProperty(keyName, propImpl.value.toKsonValue()) - }, - this) + ?: if (skipErrors) return@mapNotNull null + else throw ShouldNotHappenException("this AST is fully valid") + val keyName = propKey.key.toKsonValueInternal(skipErrors) as? KsonString + ?: if (skipErrors) return@mapNotNull null + else throw ShouldNotHappenException("object key must convert to KsonString") + val propValue = propImpl.value.toKsonValueInternal(skipErrors) + ?: if (skipErrors) return@mapNotNull null + else throw ShouldNotHappenException("property value must convert to a KsonValue") + keyName.value to KsonObjectProperty(keyName, propValue) + } + KsonObject(props.toMap(), this) + } + is ListNode -> { + val elems = elements.mapNotNull { elem -> + val listElementNode = elem as? ListElementNodeImpl + ?: if (skipErrors) return@mapNotNull null + else throw ShouldNotHappenException("this AST is fully valid") + listElementNode.value.toKsonValueInternal(skipErrors) + ?: if (skipErrors) return@mapNotNull null + else throw ShouldNotHappenException("list element must convert to a KsonValue") + } + KsonList(elems, this) } - is ListNode -> KsonList(elements.map { elem -> - val listElementNode = elem as? ListElementNodeImpl - ?: throw ShouldNotHappenException("this AST is fully valid") - listElementNode.value.toKsonValue() - - }, this) is EmbedBlockNode -> EmbedBlock(this) is StringNodeImpl -> KsonString(this) is NumberNode -> KsonNumber(this) is TrueNode -> KsonBoolean(this) is FalseNode -> KsonBoolean(this) is NullNode -> KsonNull(this) - is KsonValueNodeImpl -> this.toKsonValue() - is ObjectKeyNodeImpl -> { - throw ShouldNotHappenException("these properties are processed above in the ${ObjectNode::class.simpleName} case") - } - is ObjectPropertyNodeImpl -> { - throw ShouldNotHappenException("these properties are processed above in the ${ObjectNode::class.simpleName} case") - } - is ListElementNodeImpl -> { - throw ShouldNotHappenException("these elements are processed above in the ${ListNode::class.simpleName} case") + is KsonValueNodeImpl -> this.toKsonValueInternal(skipErrors) + is ObjectKeyNodeImpl, is ObjectPropertyNodeImpl, is ListElementNodeImpl -> { + if (skipErrors) null + else throw ShouldNotHappenException("these are processed above in their container case") } - is AstNodeError -> throw UnsupportedOperationException("Cannot create Valid Ast Node from ${this::class.simpleName}") } } From 5db60ec8a93a2d48057b2cfa56ef806db71591b1 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 11:19:25 -0700 Subject: [PATCH 08/15] refactor: simplify ResolvedSchemaContext to plain function The `schemaIdLookup` and `parsedDocument` fields on ResolvedSchemaContext were unused by callers once the partial-AST navigation was pulled inline into getCompletions. Drop the data class wrapper and make resolveAndFilterSchemas a plain function returning List. --- .../kotlin/org/kson/tooling/KsonTooling.kt | 54 ++++++------------- 1 file changed, 17 insertions(+), 37 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt index ee319b64..0c9916ac 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt @@ -49,9 +49,9 @@ object KsonTooling { ): String? { val parsedSchema = schema.ksonValue ?: return null val documentPointer = KsonValuePathBuilder(document, Coordinates(line, column)).buildJsonPointerToPosition() ?: return null - val context = ResolvedSchemaContext.resolveAndFilterSchemas(parsedSchema, document.ksonValue, documentPointer) + val validSchemas = resolveAndFilterSchemas(parsedSchema, document.ksonValue, documentPointer) - val schemaInfos = context.validSchemas.mapNotNull { ref -> + val schemaInfos = validSchemas.mapNotNull { ref -> ref.resolvedValue.extractSchemaInfo() } @@ -79,9 +79,9 @@ object KsonTooling { ): List { val parsedSchema = schema.ksonValue ?: return emptyList() val documentPointer = KsonValuePathBuilder(document, Coordinates(line, column)).buildJsonPointerToPosition() ?: return emptyList() - val context = ResolvedSchemaContext.resolveAndFilterSchemas(parsedSchema, document.ksonValue, documentPointer) + val validSchemas = resolveAndFilterSchemas(parsedSchema, document.ksonValue, documentPointer) - return context.validSchemas.map { + return validSchemas.map { Range( it.resolvedValue.location.start.line, it.resolvedValue.location.start.column, @@ -284,40 +284,20 @@ object KsonTooling { } /** - * Internal helper data class to hold the result of schema resolution and filtering. + * Navigate and filter schemas for a document path. + * + * Creates a [SchemaIdLookup], navigates to candidate schemas at the pointer, + * then filters them based on validation against the document value. */ - private data class ResolvedSchemaContext( - val schemaIdLookup: SchemaIdLookup, - val validSchemas: List, - val parsedDocument: KsonValue? - ){ - companion object { - /** - * Common helper to navigate and filter schemas based on a document path. - * - * Encapsulates the repeated pattern of: - * 1. Creating a SchemaIdLookup from the pre-parsed schema - * 2. Navigating to candidate schemas - * 3. Filtering schemas based on validation against the pre-parsed document - * - * @param parsedSchema The pre-parsed schema value - * @param documentValue The pre-parsed document value (may be null for broken documents) - * @param documentPointer The [JsonPointer] to navigate to in the schema - */ - fun resolveAndFilterSchemas( - parsedSchema: KsonValue, - documentValue: KsonValue?, - documentPointer: JsonPointer - ): ResolvedSchemaContext { - val schemaIdLookup = SchemaIdLookup(parsedSchema) - val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, documentValue) - - val filteringService = SchemaFilteringService(schemaIdLookup) - val validSchemas = filteringService.getValidSchemas(candidateSchemas, documentValue, documentPointer) - - return ResolvedSchemaContext(schemaIdLookup, validSchemas, documentValue) - } - } + private fun resolveAndFilterSchemas( + parsedSchema: KsonValue, + documentValue: KsonValue?, + documentPointer: JsonPointer + ): List { + val schemaIdLookup = SchemaIdLookup(parsedSchema) + val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, documentValue) + val filteringService = SchemaFilteringService(schemaIdLookup) + return filteringService.getValidSchemas(candidateSchemas, documentValue, documentPointer) } } From b884b2fd950de9c29d23b423090281b59e6e9aef Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 30 Mar 2026 12:54:36 +0200 Subject: [PATCH 09/15] Unify schema resolution: all features use partialKsonValue for navigation 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. --- .../kotlin/org/kson/tooling/KsonTooling.kt | 21 +++++++------- .../org/kson/SchemaDefinitionLocationTest.kt | 26 +++++++++++++++++ .../kotlin/org/kson/SchemaInfoLocationTest.kt | 28 +++++++++++++++++++ 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt index 0c9916ac..68674ec2 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt @@ -49,7 +49,7 @@ object KsonTooling { ): String? { val parsedSchema = schema.ksonValue ?: return null val documentPointer = KsonValuePathBuilder(document, Coordinates(line, column)).buildJsonPointerToPosition() ?: return null - val validSchemas = resolveAndFilterSchemas(parsedSchema, document.ksonValue, documentPointer) + val validSchemas = resolveAndFilterSchemas(parsedSchema, document, documentPointer) val schemaInfos = validSchemas.mapNotNull { ref -> ref.resolvedValue.extractSchemaInfo() @@ -79,7 +79,7 @@ object KsonTooling { ): List { val parsedSchema = schema.ksonValue ?: return emptyList() val documentPointer = KsonValuePathBuilder(document, Coordinates(line, column)).buildJsonPointerToPosition() ?: return emptyList() - val validSchemas = resolveAndFilterSchemas(parsedSchema, document.ksonValue, documentPointer) + val validSchemas = resolveAndFilterSchemas(parsedSchema, document, documentPointer) return validSchemas.map { Range( @@ -159,11 +159,7 @@ object KsonTooling { ): List { val parsedSchema = schema.ksonValue ?: return emptyList() val documentPointer = KsonValuePathBuilder(document, Coordinates(line, column)).buildJsonPointerToPosition(includePropertyKeys = false) ?: return emptyList() - val schemaIdLookup = SchemaIdLookup(parsedSchema) - val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, document.partialKsonValue) - - val filteringService = SchemaFilteringService(schemaIdLookup) - val validSchemas = filteringService.getValidSchemas(candidateSchemas, document.ksonValue, documentPointer) + val validSchemas = resolveAndFilterSchemas(parsedSchema, document, documentPointer) return SchemaInformation.getCompletions(parsedSchema, documentPointer, validSchemas, document.ksonValue) } @@ -288,16 +284,21 @@ object KsonTooling { * * Creates a [SchemaIdLookup], navigates to candidate schemas at the pointer, * then filters them based on validation against the document value. + * + * Uses [ToolingDocument.partialKsonValue] for navigation so that if/then + * conditions can evaluate sibling properties even when the document has + * parse errors. Uses [ToolingDocument.ksonValue] for validation filtering + * to avoid false matches from incomplete document state. */ private fun resolveAndFilterSchemas( parsedSchema: KsonValue, - documentValue: KsonValue?, + document: ToolingDocument, documentPointer: JsonPointer ): List { val schemaIdLookup = SchemaIdLookup(parsedSchema) - val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, documentValue) + val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, document.partialKsonValue) val filteringService = SchemaFilteringService(schemaIdLookup) - return filteringService.getValidSchemas(candidateSchemas, documentValue, documentPointer) + return filteringService.getValidSchemas(candidateSchemas, document.ksonValue, documentPointer) } } diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaDefinitionLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaDefinitionLocationTest.kt index 6314aab2..9ed4dac4 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaDefinitionLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaDefinitionLocationTest.kt @@ -594,4 +594,30 @@ class SchemaDefinitionLocationTest { ) } + @Test + fun testJumpToDefinition_ifThenResolvesInBrokenDocument() { + // "config" is only reachable via if/then, and the document has a + // parse error (missing value after "other":). The partial AST + // still recovers "kind": "a" so the if/then condition can evaluate. + assertDefinitionLocation( + schemaWithCaret = """ + { + "${'$'}defs": { + "ConfigA": { "type": "object", "description": "Settings for A" }, + "ConfigB": { "type": "object", "description": "Settings for B" } + }, + "type": "object", + "properties": { "kind": { "type": "string" } }, + "allOf": [ + { "if": { "properties": { "kind": { "const": "a" } } }, "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigA" } } } }, + { "if": { "properties": { "kind": { "const": "b" } } }, "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigB" } } } } + ] + } + """.trimIndent(), + documentWithCaret = """ + { "kind": "a", "other": , "config": {} } + """.trimIndent() + ) + } + } \ No newline at end of file diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt index 6d8f06a9..65cc4939 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaInfoLocationTest.kt @@ -547,4 +547,32 @@ class SchemaInfoLocationTest { assertTrue(hoverInfo.contains("Config A"), "Should show matching branch title, got: $hoverInfo") assertFalse(hoverInfo.contains("Config B"), "Should NOT show non-matching branch, got: $hoverInfo") } + + @Test + fun testGetSchemaInfoAtLocation_ifThenShowsMatchingBranchInBrokenDocument() { + val schema = """ + { + "${'$'}defs": { + "ConfigA": { "type": "object", "title": "Config A", "description": "Settings for A" }, + "ConfigB": { "type": "object", "title": "Config B", "description": "Settings for B" } + }, + "type": "object", + "properties": { "kind": { "type": "string" } }, + "allOf": [ + { "if": { "properties": { "kind": { "const": "a" } } }, "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigA" } } } }, + { "if": { "properties": { "kind": { "const": "b" } } }, "then": { "properties": { "config": { "${'$'}ref": "#/${'$'}defs/ConfigB" } } } } + ] + } + """ + + // The document has a parse error (missing value after "other":) + // so ksonValue is null, but partialKsonValue recovers "kind": "a" + // for if/then evaluation. + val hoverInfo = getInfoAtCaret(schema, """ + { "kind": "a", "other": , "config": {} } + """.trimIndent()) + assertNotNull(hoverInfo, "Hover should work in broken documents via partial AST") + assertTrue(hoverInfo.contains("Config A"), "Should show matching branch, got: $hoverInfo") + assertFalse(hoverInfo.contains("Config B"), "Should NOT show non-matching branch, got: $hoverInfo") + } } From 88afda134c8f35f21c6c927ebfeaec5063e505f0 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 30 Mar 2026 21:53:02 +0200 Subject: [PATCH 10/15] Filter oneOf/anyOf completions by sibling property values 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. --- .../kotlin/org/kson/tooling/KsonTooling.kt | 2 +- .../kson/tooling/SchemaFilteringService.kt | 132 +++++++++--- .../OneOfSiblingNarrowingCompletionTest.kt | 202 ++++++++++++++++++ .../org/kson/SchemaFilteringServiceTest.kt | 13 +- .../kotlin/org/kson/schema/SchemaIdLookup.kt | 52 +++-- 5 files changed, 349 insertions(+), 52 deletions(-) create mode 100644 kson-tooling-lib/src/commonTest/kotlin/org/kson/OneOfSiblingNarrowingCompletionTest.kt diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt index 68674ec2..8e122716 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/KsonTooling.kt @@ -298,7 +298,7 @@ object KsonTooling { val schemaIdLookup = SchemaIdLookup(parsedSchema) val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, document.partialKsonValue) val filteringService = SchemaFilteringService(schemaIdLookup) - return filteringService.getValidSchemas(candidateSchemas, document.ksonValue, documentPointer) + return filteringService.getValidSchemas(candidateSchemas, document, documentPointer) } } diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt index 6f83dd1d..a1bd52d3 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt @@ -6,13 +6,14 @@ package org.kson.tooling import org.kson.parser.LoggedMessage import org.kson.parser.MessageSink import org.kson.parser.messages.MessageType -import org.kson.value.navigation.json_pointer.JsonPointer import org.kson.schema.ResolvedRef import org.kson.schema.SchemaIdLookup import org.kson.schema.SchemaParser import org.kson.schema.SchemaResolutionType +import org.kson.value.KsonList import org.kson.value.KsonObject import org.kson.value.KsonValue +import org.kson.value.navigation.json_pointer.JsonPointer import org.kson.walker.KsonValueWalker import org.kson.walker.navigateWithJsonPointer import kotlin.js.ExperimentalJsExport @@ -31,51 +32,80 @@ import kotlin.js.JsExport class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { /** - * Get valid schemas for a document path, applying expansion and filtering. + * Get valid schemas for a document path, applying expansion and two-pass filtering. * * This function: * 1. Expands combinator schemas (oneOf/anyOf/allOf) and conditionals (if/then/else) into individual branches - * 2. Filters branches based on validation against the current document (for oneOf/anyOf/if-then-else) - * 3. Returns all branches for allOf and direct properties (no filtering needed) + * 2. Filters oneOf/anyOf branches by sibling property constraints (using [ToolingDocument.partialKsonValue]) + * 3. Filters remaining branches by leaf-level validation (using [ToolingDocument.ksonValue]) + * + * The two passes use different document values because sibling filtering must work + * even when the document has parse errors at the cursor position (where only the + * partial value is available), while leaf validation needs the fully parsed tree. * * @param candidateSchemas The schemas found at the document path - * @param documentValue The pre-parsed document value, or null if the document - * couldn't be parsed. When null, filtering is skipped and all - * expanded schemas are returned. + * @param document The parsed document providing both full and partial value trees * @param documentPointer The [JsonPointer] to the location in the document * @return List of valid schemas after expansion and filtering */ fun getValidSchemas( candidateSchemas: List, - documentValue: KsonValue?, + document: ToolingDocument, documentPointer: JsonPointer ): List { - val hasBranchesThatRequireValidation = candidateSchemas.any { ref -> - requiresValidationFiltering(ref) - } - - // Always expand to get individual branches val expandedSchemas = schemaIdLookup.expandCombinators(candidateSchemas) - return if (hasBranchesThatRequireValidation && documentValue != null) { - filterByValidation(expandedSchemas, documentValue, documentPointer) + val partialDocumentValue = document.partialKsonValue + val afterSiblingFilter = if (partialDocumentValue != null) { + filterBySiblingCompatibility(expandedSchemas, partialDocumentValue, documentPointer) } else { expandedSchemas } + + val hasBranchesThatRequireValidation = afterSiblingFilter.any { ref -> + requiresValidationFiltering(ref) + } + val documentValue = document.ksonValue + return if (hasBranchesThatRequireValidation && documentValue != null) { + filterByValidation(afterSiblingFilter, documentValue, documentPointer) + } else { + afterSiblingFilter + } } /** - * Checks if a schema reference requires validation-based filtering. + * Filters oneOf/anyOf branches by sibling property compatibility. * - * oneOf/anyOf combinators and if/then/else conditionals require validation filtering. - * allOf combinators always include all branches (no filtering needed). - * - * @param ref The schema reference to check - * @return true if the schema requires validation filtering + * For schemas with [ResolvedRef.parentBranch] set, checks whether the parent + * branch's property constraints (const/enum) are compatible with the document's + * sibling values. Falls back to unfiltered schemas if all branches are eliminated. + */ + private fun filterBySiblingCompatibility( + schemas: List, + documentValue: KsonValue, + documentPointer: JsonPointer + ): List { + val hasParentBranches = schemas.any { it.parentBranch != null } + if (!hasParentBranches) return schemas + + val filtered = schemas.filter { ref -> + val parentBranch = ref.parentBranch + parentBranch == null || isBranchCompatibleWithSiblings(parentBranch, documentValue, documentPointer) + } + + val branchesBefore = schemas.count { it.parentBranch != null } + val branchesAfter = filtered.count { it.parentBranch != null } + return if (branchesBefore > 0 && branchesAfter == 0) schemas else filtered + } + + /** + * True for filterable resolution types and for parent refs that still carry + * `oneOf`/`anyOf`/`if` keys after their children were extracted by navigation — + * pinned by `testGetValidSchemas_withTypeMismatchAtTarget_filtersOutAllBranches`. */ private fun requiresValidationFiltering(ref: ResolvedRef): Boolean { return ref.resolutionType in FILTERABLE_RESOLUTION_TYPES || - (ref.resolvedValue as? org.kson.value.KsonObject)?.let { obj -> + (ref.resolvedValue as? KsonObject)?.let { obj -> obj.propertyLookup.containsKey("oneOf") || obj.propertyLookup.containsKey("anyOf") || obj.propertyLookup.containsKey("if") @@ -121,12 +151,16 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { } } - // If filtering eliminated every filterable branch, the document value likely - // doesn't match the expected shape yet (e.g. a list where objects are expected). - // Fall back to unfiltered schemas so completions remain available. - val filterableBranchesBefore = candidateSchemas.count { isFilterableBranch(it) } - val filterableBranchesAfter = filtered.count { isFilterableBranch(it) } - return if (filterableBranchesBefore > 0 && filterableBranchesAfter == 0) candidateSchemas else filtered + // Scalar targets are often a placeholder being typed into; validating against + // it can spuriously eliminate every enum/const branch, so fall back to the + // unfiltered set in that case. Structural targets reflect committed intent. + val isScalarTarget = targetValue !is KsonObject && targetValue !is KsonList + if (isScalarTarget) { + val filterableBefore = candidateSchemas.count { it.resolutionType in FILTERABLE_RESOLUTION_TYPES } + val filterableAfter = filtered.count { it.resolutionType in FILTERABLE_RESOLUTION_TYPES } + if (filterableBefore > 0 && filterableAfter == 0) return candidateSchemas + } + return filtered } /** @@ -188,8 +222,46 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { } } - private fun isFilterableBranch(ref: ResolvedRef): Boolean = - ref.resolutionType in FILTERABLE_RESOLUTION_TYPES + /** + * Checks if a oneOf/anyOf branch is compatible with the document's sibling properties. + * + * For each property the branch declares, if that property has a value in the document, + * validates the value against its schema using [isSchemaValidForDocument] — the same + * soft-validation path used for leaf filtering. This catches const, enum, type, pattern, + * and any other constraint the branch puts on its sibling properties. + * + * The property being completed (last token of [documentPointer]) is excluded since its + * value is incomplete during completion — validating mid-typing input would spuriously + * reject the branch. + * + * @param parentBranch The oneOf/anyOf branch that contained this result + * @param documentValue The root document value + * @param documentPointer Path to the property being completed + */ + private fun isBranchCompatibleWithSiblings( + parentBranch: ResolvedRef, + documentValue: KsonValue, + documentPointer: JsonPointer + ): Boolean { + val propertyBeingCompleted = documentPointer.tokens.lastOrNull() ?: return true + val parentPointer = JsonPointer.fromTokens(documentPointer.tokens.dropLast(1)) + val parentDocValue = KsonValueWalker.navigateWithJsonPointer(documentValue, parentPointer) + as? KsonObject ?: return true + + val branchSchema = parentBranch.resolvedValue as? KsonObject ?: return true + val branchProperties = (branchSchema.propertyLookup["properties"] as? KsonObject) + ?: return true + + for ((propName, propSchemaValue) in branchProperties.propertyLookup) { + if (propName == propertyBeingCompleted) continue + + val docValue = parentDocValue.propertyLookup[propName] ?: continue + val propRef = schemaIdLookup.resolveRefIfPresent(propSchemaValue, parentBranch.resolvedValueBaseUri) + if (!isSchemaValidForDocument(propRef, docValue)) return false + } + + return true + } companion object { /** diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/OneOfSiblingNarrowingCompletionTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/OneOfSiblingNarrowingCompletionTest.kt new file mode 100644 index 00000000..af129062 --- /dev/null +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/OneOfSiblingNarrowingCompletionTest.kt @@ -0,0 +1,202 @@ +package org.kson + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull + +/** + * Tests that oneOf branches are filtered by sibling property values during + * completion. + * + * When a schema uses `allOf: [base, oneOf: [...]]` to couple interdependent + * properties (e.g., integration determines which job values are valid), setting + * one property should narrow completions for the other — even though the + * constraint lives on the parent oneOf branch, not on the property itself. + */ +class OneOfSiblingNarrowingCompletionTest : SchemaCompletionTest { + + private fun assertCompletionLabels(schema: String, doc: String, expected: List, message: String) { + val completions = getCompletionsAtCaret(schema, doc) + assertNotNull(completions, "$message: should return completions") + assertEquals(expected, completions.map { it.label }.sorted(), message) + } + + /** + * Schema that couples integration+job+parameters via oneOf, with shared + * properties in a base schema — the same pattern used for pipeline task + * definitions with many integration variants. + */ + private val schema = """ + { + "${'$'}defs": { + "Base": { + "type": "object", + "properties": { + "name": { "type": "string" } + } + }, + "AlphaParams": { + "type": "object", + "properties": { "url": { "type": "string" } } + }, + "BetaParams": { + "type": "object", + "properties": { "query": { "type": "string" } } + }, + "GammaRunParams": { + "type": "object", + "properties": { "script": { "type": "string" } } + }, + "GammaTestParams": { + "type": "object", + "properties": { "suite": { "type": "string" } } + } + }, + "allOf": [ + { "${'$'}ref": "#/${'$'}defs/Base" }, + { + "oneOf": [ + { + "properties": { + "integration": { "const": "ALPHA" }, + "job": { "const": "ALPHA_SYNC" }, + "parameters": { "${'$'}ref": "#/${'$'}defs/AlphaParams" } + }, + "required": ["integration", "job", "parameters"] + }, + { + "properties": { + "integration": { "const": "BETA" }, + "job": { "const": "BETA_QUERY" }, + "parameters": { "${'$'}ref": "#/${'$'}defs/BetaParams" } + }, + "required": ["integration", "job", "parameters"] + }, + { + "properties": { + "integration": { "const": "GAMMA" }, + "job": { "const": "GAMMA_RUN" }, + "parameters": { "${'$'}ref": "#/${'$'}defs/GammaRunParams" } + }, + "required": ["integration", "job", "parameters"] + }, + { + "properties": { + "integration": { "const": "GAMMA" }, + "job": { "const": "GAMMA_TEST" }, + "parameters": { "${'$'}ref": "#/${'$'}defs/GammaTestParams" } + }, + "required": ["integration", "job", "parameters"] + } + ] + } + ] + } + """ + + @Test + fun testJobNarrowedByIntegrationSingleMatch() { + assertCompletionLabels(schema, + """{"integration": "ALPHA", "job": ""}""", + listOf("ALPHA_SYNC"), + "ALPHA integration should narrow job to ALPHA_SYNC only") + } + + @Test + fun testJobNarrowedByIntegrationMultipleMatches() { + assertCompletionLabels(schema, + """{"integration": "GAMMA", "job": ""}""", + listOf("GAMMA_RUN", "GAMMA_TEST"), + "GAMMA integration should narrow job to both GAMMA_* values") + } + + @Test + fun testIntegrationNarrowedByJob() { + assertCompletionLabels(schema, + """{"job": "BETA_QUERY", "integration": ""}""", + listOf("BETA"), + "BETA_QUERY job should narrow integration to BETA only") + } + + @Test + fun testJobShowsAllWithoutSibling() { + assertCompletionLabels(schema, + """{"job": ""}""", + listOf("ALPHA_SYNC", "BETA_QUERY", "GAMMA_RUN", "GAMMA_TEST"), + "Without integration sibling, all jobs should be offered") + } + + @Test + fun testIntegrationShowsAllWithoutSibling() { + assertCompletionLabels(schema, + """{"integration": ""}""", + listOf("ALPHA", "BETA", "GAMMA"), + "Without job sibling, all integrations should be offered") + } + + @Test + fun testParametersNarrowedByIntegrationAndJob() { + val completions = getCompletionsAtCaret(schema, + """{"integration": "ALPHA", "job": "ALPHA_SYNC", "parameters": { "" }}""") + assertNotNull(completions, "Should return parameter completions") + assertEquals(listOf("url"), completions.map { it.label }.sorted(), + "Should offer AlphaParams properties, not all parameter types") + } + + @Test + fun testNarrowingWithKsonSyntax() { + assertCompletionLabels(schema, + "integration: ALPHA\njob: ", + listOf("ALPHA_SYNC"), + "Narrowing should work with KSON syntax via partial value") + } + + @Test + fun testPropertyNameCompletionsIncludeOneOfProperties() { + val completions = getCompletionsAtCaret(schema, + "name: foo\n") + assertNotNull(completions, "Should return completions") + val labels = completions.map { it.label }.sorted() + assertEquals( + listOf("integration", "job", "parameters"), + labels, + "Property completions should include oneOf branch properties (name filtered out as already filled)" + ) + } + + /** + * Sibling narrowing uses full schema validation, so non-const/non-enum discriminants + * (here, `type`) should filter branches the same way const/enum does. + */ + @Test + fun testNarrowingByTypeConstraint() { + val typeDiscriminatedSchema = """ + { + "oneOf": [ + { + "properties": { + "kind": { "type": "integer" }, + "color": { "enum": ["red", "green"] } + } + }, + { + "properties": { + "kind": { "type": "string" }, + "color": { "enum": ["blue", "yellow"] } + } + } + ] + } + """ + + assertCompletionLabels(typeDiscriminatedSchema, + """{"kind": 42, "color": ""}""", + listOf("green", "red"), + "integer kind should narrow to the type: integer branch's colors") + + assertCompletionLabels(typeDiscriminatedSchema, + """{"kind": "hello", "color": ""}""", + listOf("blue", "yellow"), + "string kind should narrow to the type: string branch's colors") + } +} diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt index 6b965899..f2aa7279 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt @@ -1,11 +1,12 @@ package org.kson -import org.kson.value.navigation.json_pointer.JsonPointer import org.kson.schema.SchemaIdLookup +import org.kson.tooling.KsonTooling import org.kson.tooling.SchemaFilteringService import org.kson.value.KsonObject import org.kson.value.KsonString import org.kson.value.KsonValue +import org.kson.value.navigation.json_pointer.JsonPointer import kotlin.test.* /** @@ -27,10 +28,10 @@ class SchemaFilteringServiceTest { documentPointer: JsonPointer = JsonPointer("") ): List { val parsedSchema = KsonCore.parseToAst(schema).ksonValue ?: fail("Schema should parse") - val parsedDocument = KsonCore.parseToAst(document).ksonValue + val parsedDocument = KsonTooling.parse(document) val schemaIdLookup = SchemaIdLookup(parsedSchema) val filteringService = SchemaFilteringService(schemaIdLookup) - val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, parsedDocument) + val candidateSchemas = schemaIdLookup.navigateByDocumentPointer(documentPointer, parsedDocument.ksonValue) return filteringService.getValidSchemas(candidateSchemas, parsedDocument, documentPointer).map { it.resolvedValue } } @@ -327,7 +328,7 @@ class SchemaFilteringServiceTest { } @Test - fun testGetValidSchemas_withTypeMismatchAtTarget_fallsBackToAllBranches() { + fun testGetValidSchemas_withTypeMismatchAtTarget_filtersOutAllBranches() { val schema = """ oneOf: - type: object @@ -346,6 +347,8 @@ class SchemaFilteringServiceTest { val validSchemas = getValidSchemasForDocument(schema, document) - assertEquals(3, validSchemas.size, "Parent + both oneOf branches should be returned when target type doesn't match any branch") + // Only the parent oneOf container remains — both branches expect objects + // but the document is a list, so neither branch is compatible. + assertEquals(1, validSchemas.size, "Only the parent oneOf should remain when no branch matches the document type") } } diff --git a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt index 247d1529..c8db5ede 100644 --- a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt +++ b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt @@ -122,7 +122,10 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { fun addBranch(branch: KsonValue, baseUri: String, resolutionType: SchemaResolutionType) { val resolved = resolveRefIfPresent(branch, baseUri) - val branchRef = ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, resolutionType) + // Preserve the parentBranch from navigation through expansion — the + // branch context from the original oneOf/anyOf is still the right one + // for sibling filtering regardless of how deeply the schema expands. + val branchRef = ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, resolutionType, ref.parentBranch) expandSingleSchema(branchRef, expanded) addedBranches = true } @@ -236,10 +239,10 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { navigateObjectProperty(node, token, updatedBaseUri, currentDocumentValue) } - // Resolve $ref for each navigated node + // Resolve $ref for each navigated node, preserving parentBranch context for (navNode in navigatedNodes) { val resolved = resolveRefIfPresent(navNode.resolvedValue, navNode.resolvedValueBaseUri) - nextNodes.add(ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, navNode.resolutionType)) + nextNodes.add(ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, navNode.resolutionType, navNode.parentBranch)) } } @@ -302,13 +305,16 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { } /** - * Resolves a $ref in a schema value if present. + * Resolves a `$ref` in a schema value if present. * - * @param value The schema value that might contain a $ref + * Public to support downstream `$ref` resolution within schema branches, e.g., + * when checking property constraints inside oneOf/anyOf branches. + * + * @param value The schema value that might contain a `$ref` * @param currentBaseUri The current base URI for resolving the reference - * @return A ResolvedRef with the resolved value and base URI + * @return A [ResolvedRef] with the resolved value and base URI */ - private fun resolveRefIfPresent(value: KsonValue, currentBaseUri: String): ResolvedRef { + fun resolveRefIfPresent(value: KsonValue, currentBaseUri: String): ResolvedRef { if (value is KsonObject) { val refValue = value.propertyLookup["\$ref"] as? KsonString if (refValue != null) { @@ -326,11 +332,9 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { * For each combinator branch, it resolves any $ref, then delegates to the caller's * navigation function to continue traversal. * - * Example for array navigation: - * ``` - * schema: { anyOf: [{ items: {...} }, { items: {...} }] } - * -> finds both items schemas and tags them as ANY_OF - * ``` + * For oneOf/anyOf results, the branch schema is attached as [ResolvedRef.parentBranch] + * so that downstream filtering can validate sibling property constraints that aren't + * visible from the leaf schema alone. * * @param schemaNode The schema node containing combinators * @param currentBaseUri The current base URI for $ref resolution @@ -347,18 +351,22 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { fun processCombinator(combinator: KsonValue?, combinatorType: SchemaResolutionType) { val combinatorList = combinator as? KsonList ?: return + val attachParentBranch = combinatorType in setOf(SchemaResolutionType.ONE_OF, SchemaResolutionType.ANY_OF) for (element in combinatorList.elements) { val resolved = resolveRefIfPresent(element, currentBaseUri) if (resolved.resolvedValue is KsonObject) { - // Continue navigation through this combinator branch val nestedResults = recursiveNavigate(resolved.resolvedValue, resolved.resolvedValueBaseUri) - // Tag results with combinator type if appropriate + // Attach parentBranch for oneOf/anyOf so downstream filtering + // can check sibling constraints. results.addAll(nestedResults.map { ref -> if (shouldTagWithCombinator(ref.resolutionType)) { - ref.copy(resolutionType = combinatorType) + ref.copy( + resolutionType = combinatorType, + parentBranch = if (attachParentBranch) resolved else null + ) } else { ref } @@ -726,10 +734,22 @@ enum class SchemaResolutionType { ROOT } +/** + * A schema node resolved during navigation, carrying the context of how it was found. + * + * @param resolvedValue The schema value at this location + * @param resolvedValueBaseUri The base URI for resolving `$ref` within this schema + * @param resolutionType How this schema was reached (direct property, combinator branch, etc.) + * @param parentBranch For schemas found inside a oneOf/anyOf branch, the branch schema that + * contained this result. This allows downstream filtering (e.g., [SchemaFilteringService]) + * to validate the branch against the parent document object — checking sibling property + * constraints that aren't visible from the leaf schema alone. + */ data class ResolvedRef( val resolvedValue: KsonValue, val resolvedValueBaseUri: String, - val resolutionType: SchemaResolutionType = SchemaResolutionType.ROOT + val resolutionType: SchemaResolutionType = SchemaResolutionType.ROOT, + val parentBranch: ResolvedRef? = null ) /** From e4291f75107a3757ba5fd1d54d5bf061bf234638 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 13 Apr 2026 13:27:05 -0700 Subject: [PATCH 11/15] Fix $ref resolution in schemas with root $id navigateByDocumentPointer always started with baseUri="" but schemas with $id store their root under that id in the lookup map. $ref resolution then failed because idMap[""] didn't exist. Use the root schema's $id as the starting base URI so refs resolve correctly. --- .../org/kson/SchemaCompletionLocationTest.kt | 170 ++++++++++++++++++ .../kotlin/org/kson/schema/SchemaIdLookup.kt | 16 +- 2 files changed, 177 insertions(+), 9 deletions(-) diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt index f5453c80..859129fb 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaCompletionLocationTest.kt @@ -1888,4 +1888,174 @@ class SchemaCompletionLocationTest { . . """ + + @Test + fun testOneOfWithRefAtRootProvidesPropertyCompletions() { + val schema = $$""" + oneOf: + - '$ref': '#/$defs/Model' + description: 'A model resource' + - '$ref': '#/$defs/View' + description: 'A view resource' + . + '$defs': + Model: + type: object + properties: + id: + type: string + . + type: + type: string + const: model + . + source: + type: string + . + . + . + View: + type: object + properties: + id: + type: string + . + type: + type: string + const: view + . + base: + type: string + . + . + . + . + """ + + val completions = getCompletionsAtCaret(schema, "") + + val labels = completions.map { it.label } + assertTrue(labels.contains("id"), "Should include shared property 'id', got: $labels") + assertTrue(labels.contains("type"), "Should include shared property 'type', got: $labels") + assertTrue(labels.contains("source"), "Should include Model-only property 'source', got: $labels") + assertTrue(labels.contains("base"), "Should include View-only property 'base', got: $labels") + } + + @Test + fun testOneOfWithRefAtRootWithSchemaMetadata() { + // Root schema with $schema, $id, title, description alongside oneOf + val schema = $$""" + '$schema': 'http://json-schema.org/draft-07/schema#' + '$id': 'test.schema.kson' + title: 'Test Resource' + description: 'A test resource that is either a Model or a View' + oneOf: + - '$ref': '#/$defs/Model' + description: 'A model resource' + - '$ref': '#/$defs/View' + description: 'A view resource' + . + '$defs': + Model: + type: object + properties: + id: + type: string + . + type: + type: string + const: model + . + source: + type: string + . + . + . + View: + type: object + properties: + id: + type: string + . + type: + type: string + const: view + . + base: + type: string + . + . + . + . + """ + + val completions = getCompletionsAtCaret(schema, "") + + val labels = completions.map { it.label } + assertTrue(labels.contains("id"), "Should include shared property 'id', got: $labels") + assertTrue(labels.contains("type"), "Should include shared property 'type', got: $labels") + assertTrue(labels.contains("source"), "Should include Model-only property 'source', got: $labels") + assertTrue(labels.contains("base"), "Should include View-only property 'base', got: $labels") + } + + @Test + fun testOneOfWithRefAtRootWithEmbedDescription() { + // Root schema with $id and %markdown embed block description alongside oneOf + val schema = $$""" + '$schema': 'http://json-schema.org/draft-07/schema#' + '$id': 'test.schema.kson' + title: 'Test Resource' + description: %markdown + # Test Resource + + Each file is either a `model` or a `view`. + Use `type: model` or `type: view` to discriminate. + %% + oneOf: + - '$ref': '#/$defs/Model' + description: 'A model resource' + - '$ref': '#/$defs/View' + description: 'A view resource' + . + '$defs': + Model: + type: object + properties: + id: + type: string + . + type: + type: string + const: model + . + source: + type: string + . + . + . + View: + type: object + properties: + id: + type: string + . + type: + type: string + const: view + . + base: + type: string + . + . + . + . + """ + + val completions = getCompletionsAtCaret(schema, "") + + assertEquals( + setOf("id", "type", "source", "base"), + completions.map { it.label }.toSet() + ) + } } diff --git a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt index c8db5ede..19cb9b6b 100644 --- a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt +++ b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt @@ -29,14 +29,7 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { idMap[KsonDraft7MetaSchema.ID] = KsonDraft7MetaSchema.schemaValue if (schemaRootValue is KsonObject) { - val rootBaseUri = schemaRootValue.propertyLookup["\$id"]?.let { idValue -> - if (idValue is KsonString) { - idValue.value - } else { - // this $id is completely invalid - null - } - } ?: "" + val rootBaseUri = rootBaseUri() // Store the root schema at its baseUri idMap[rootBaseUri] = schemaRootValue @@ -46,6 +39,11 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { } } + private fun rootBaseUri(): String = + (schemaRootValue as? KsonObject)?.propertyLookup["\$id"] + ?.let { (it as? KsonString)?.value } + ?: "" + /** * Resolves a `$ref` reference string to the corresponding schema value. * @@ -197,7 +195,7 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { documentPointer: JsonPointer, documentValue: KsonValue? = null, ): List { - val startingBaseUri = "" + val startingBaseUri = rootBaseUri() val documentPathTokens = documentPointer.tokens if (documentPathTokens.isEmpty()) { // Even at root, resolve $ref if present From 09d000205cff3bb980bf1ec997434c4ce09f31dd Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 00:07:20 -0700 Subject: [PATCH 12/15] refactor: move resolution-type taxonomy onto SchemaResolutionType enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two classifications — which types contribute reductive (intersected) value completions, and which types need validation-based filtering — lived as setOf() constants in two unrelated files. A reader had to cross-reference to understand what an enum value meant in the system. Lift both into properties on SchemaResolutionType itself (isReductive, requiresValidationFiltering). Call sites become self-explanatory; the taxonomy is discoverable from the enum. --- .../kson/tooling/SchemaFilteringService.kt | 33 +++++------------ .../tooling/navigation/SchemaInformation.kt | 22 ++---------- .../kotlin/org/kson/schema/SchemaIdLookup.kt | 35 ++++++++++++++++++- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt index a1bd52d3..d184a012 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt @@ -9,7 +9,6 @@ import org.kson.parser.messages.MessageType import org.kson.schema.ResolvedRef import org.kson.schema.SchemaIdLookup import org.kson.schema.SchemaParser -import org.kson.schema.SchemaResolutionType import org.kson.value.KsonList import org.kson.value.KsonObject import org.kson.value.KsonValue @@ -104,7 +103,7 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { * pinned by `testGetValidSchemas_withTypeMismatchAtTarget_filtersOutAllBranches`. */ private fun requiresValidationFiltering(ref: ResolvedRef): Boolean { - return ref.resolutionType in FILTERABLE_RESOLUTION_TYPES || + return ref.resolutionType.requiresValidationFiltering || (ref.resolvedValue as? KsonObject)?.let { obj -> obj.propertyLookup.containsKey("oneOf") || obj.propertyLookup.containsKey("anyOf") || @@ -140,14 +139,12 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { ?: return candidateSchemas val filtered = candidateSchemas.filter { ref -> - when (ref.resolutionType) { - // For anyOf/oneOf and if/then/else, check if the current document state is compatible - SchemaResolutionType.ANY_OF, - SchemaResolutionType.ONE_OF, - SchemaResolutionType.IF_THEN, - SchemaResolutionType.IF_ELSE -> isSchemaValidForDocument(ref, targetValue) - // For all other types (direct property, allOf, etc.), include them - else -> true + // Only check branches whose inclusion is conditional (oneOf/anyOf, if/then/else); + // direct properties, allOf, and friends always apply. + if (ref.resolutionType.requiresValidationFiltering) { + isSchemaValidForDocument(ref, targetValue) + } else { + true } } @@ -156,8 +153,8 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { // unfiltered set in that case. Structural targets reflect committed intent. val isScalarTarget = targetValue !is KsonObject && targetValue !is KsonList if (isScalarTarget) { - val filterableBefore = candidateSchemas.count { it.resolutionType in FILTERABLE_RESOLUTION_TYPES } - val filterableAfter = filtered.count { it.resolutionType in FILTERABLE_RESOLUTION_TYPES } + val filterableBefore = candidateSchemas.count { it.resolutionType.requiresValidationFiltering } + val filterableAfter = filtered.count { it.resolutionType.requiresValidationFiltering } if (filterableBefore > 0 && filterableAfter == 0) return candidateSchemas } return filtered @@ -272,17 +269,5 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { MessageType.SCHEMA_REQUIRED_PROPERTY_MISSING, MessageType.SCHEMA_MISSING_REQUIRED_DEPENDENCIES ) - - /** - * Resolution types that require validation-based filtering. - * These are schema branches where multiple alternatives exist and - * only compatible ones should be shown. - */ - private val FILTERABLE_RESOLUTION_TYPES = setOf( - SchemaResolutionType.ANY_OF, - SchemaResolutionType.ONE_OF, - SchemaResolutionType.IF_THEN, - SchemaResolutionType.IF_ELSE - ) } } diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt index bfd9eb85..6ef5f8c0 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/navigation/SchemaInformation.kt @@ -6,7 +6,6 @@ import org.kson.parser.Location import org.kson.value.navigation.json_pointer.JsonPointer import org.kson.schema.ResolvedRef import org.kson.schema.SchemaIdLookup -import org.kson.schema.SchemaResolutionType import org.kson.walker.KsonValueWalker import org.kson.walker.navigateWithJsonPointer import org.kson.value.KsonValue as InternalKsonValue @@ -110,23 +109,6 @@ internal object SchemaInformation{ } } -/** - * Resolution types where schemas constrain each other (intersection semantics). - * A value must satisfy ALL reductive schemas simultaneously, so value completions - * from these schemas are intersected (e.g., a base property's enum intersected with - * an if/then branch's narrower enum or const). - */ -private val REDUCTIVE_RESOLUTION_TYPES = setOf( - SchemaResolutionType.DIRECT_PROPERTY, - SchemaResolutionType.PATTERN_PROPERTY, - SchemaResolutionType.ADDITIONAL_PROPERTY, - SchemaResolutionType.ARRAY_ITEMS, - SchemaResolutionType.ALL_OF, - SchemaResolutionType.IF_THEN, - SchemaResolutionType.IF_ELSE, - SchemaResolutionType.ROOT -) - /** * Extract completions from resolved schemas, applying JSON Schema narrowing semantics. * @@ -141,8 +123,8 @@ private val REDUCTIVE_RESOLUTION_TYPES = setOf( * @return Deduplicated list of completion items respecting narrowing semantics */ private fun extractCompletionsWithNarrowing(resolvedSchemas: List): List { - val reductive = resolvedSchemas.filter { it.resolutionType in REDUCTIVE_RESOLUTION_TYPES } - val additive = resolvedSchemas.filter { it.resolutionType !in REDUCTIVE_RESOLUTION_TYPES } + val reductive = resolvedSchemas.filter { it.resolutionType.isReductive } + val additive = resolvedSchemas.filter { !it.resolutionType.isReductive } // Get completions from each reductive schema separately val perSchemaCompletions = reductive.map { it.resolvedValue.extractCompletions() } diff --git a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt index 19cb9b6b..5172a9cb 100644 --- a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt +++ b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt @@ -729,7 +729,40 @@ enum class SchemaResolutionType { /** Schema from "else" branch of an if/then/else conditional */ IF_ELSE, /** Root schema or schema resolved via $ref */ - ROOT + ROOT; + + /** + * True if this branch contributes value completions that must be intersected + * with other reductive branches — a value must satisfy all reductive schemas + * simultaneously (e.g., a base property's enum intersected with an if/then's + * narrower enum). Additive branches (oneOf/anyOf) merge their completions + * as alternatives instead. + * + * Exhaustive by design: adding a new enum entry forces a compile error here so + * the reductive-vs-additive classification is an explicit decision, not a default. + */ + val isReductive: Boolean + get() = when (this) { + DIRECT_PROPERTY, PATTERN_PROPERTY, ADDITIONAL_PROPERTY, + ARRAY_ITEMS, ALL_OF, IF_THEN, IF_ELSE, ROOT -> true + ANY_OF, ONE_OF -> false + } + + /** + * True if this branch needs validation-based filtering — alternatives that + * only apply when compatible with the document. allOf and direct properties + * always apply (no filtering needed); oneOf/anyOf and if/then/else branches + * are conditional on their constraints matching the document. + * + * Exhaustive by design: adding a new enum entry forces a compile error here so + * the filterable-vs-unconditional classification is an explicit decision, not a default. + */ + val requiresValidationFiltering: Boolean + get() = when (this) { + ANY_OF, ONE_OF, IF_THEN, IF_ELSE -> true + DIRECT_PROPERTY, PATTERN_PROPERTY, ADDITIONAL_PROPERTY, + ARRAY_ITEMS, ALL_OF, ROOT -> false + } } /** From fb8225383dd8882e78a1f19aa80d361bb9e26dfd Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 00:08:42 -0700 Subject: [PATCH 13/15] docs: clarify filterByValidation fallback pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three separate comments scattered across getValidSchemas and filterByValidation described individual "give up on filtering" cases. A reader had to piece the overall pipeline together themselves. Consolidate the four skip/fallback conditions into a single class-level KDoc section. Also document the load-bearing nature of the containsKey("oneOf"/ "anyOf"/"if") check in requiresValidationFiltering — it catches preserved parent schemas that would otherwise slip through filtering unchecked. --- .../org/kson/tooling/SchemaFilteringService.kt | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt index d184a012..648738ea 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt @@ -19,14 +19,13 @@ import kotlin.js.ExperimentalJsExport import kotlin.js.JsExport /** - * Service for filtering schemas based on validation against document content. + * Filters JSON Schema combinator (oneOf/anyOf/allOf) and conditional (if/then/else) + * branches by the document content, so IDE features only see compatible schemas. * - * This service handles the validation-based filtering logic for JSON Schema combinators - * (oneOf/anyOf/allOf) and conditionals (if/then/else), ensuring that only compatible - * schemas are used for IDE features like completions, hover info, and jump-to-definition. - * - * The filtering uses a "soft" validation approach: a schema is included if the existing - * properties don't contradict it, even if required properties are missing. + * Validation is "soft": a schema survives if existing properties don't contradict it, + * even if required properties are missing. [getValidSchemas] runs sibling-compat + * first (uses the partial value, works on broken documents) then leaf validation + * (uses the full value); each pass skips when its input has nothing to filter. */ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { From 37b96f266051b4454abd6e223272d0f029dc304c Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Mon, 20 Apr 2026 01:05:23 -0700 Subject: [PATCH 14/15] refactor: unify schema branch decomposition in SchemaNavigator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Navigation, expansion, and filter-time sibling checks each used to implement their own view of "decompose a schema's branches," with subtly different semantics. The worst effect: blind expansion at the target level leaked else-branch properties into completions when if matched (regression tests added at root and nested levels). Introduce SchemaNavigator as an inner class of SchemaIdLookup, mirroring TreeNavigator's shape from the walker package. It exposes one entry point, navigate(pointer, docVal), built from two primitives: - stepInto: structural per-token descent, no combinator logic - flatten: doc-aware decomposition of top-level branches (oneOf/anyOf/allOf unconditionally, if/then/else narrowed by strict isValid) Navigation calls flatten at every level, including the target, so the previous post-nav expandBranches pass disappears entirely. navigateThroughCombinators/Conditionals are absorbed into flatten. SchemaFilteringService drops its expandBranches call site — navigate already returns fully-flattened refs. Two existing SchemaNavigationTest cases are updated to reflect the new contract (target schema is now the head of the flattened result, followed by its expanded inner branches). --- .../kson/tooling/SchemaFilteringService.kt | 20 +- .../kson/BidirectionalIfThenCompletionTest.kt | 62 ++ .../kotlin/org/kson/schema/SchemaIdLookup.kt | 548 ++++++------------ .../org/kson/schema/SchemaNavigationTest.kt | 61 +- 4 files changed, 313 insertions(+), 378 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt index 648738ea..1f52cc28 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt @@ -30,34 +30,32 @@ import kotlin.js.JsExport class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { /** - * Get valid schemas for a document path, applying expansion and two-pass filtering. + * Get valid schemas for a document path, applying two-pass filtering. * - * This function: - * 1. Expands combinator schemas (oneOf/anyOf/allOf) and conditionals (if/then/else) into individual branches - * 2. Filters oneOf/anyOf branches by sibling property constraints (using [ToolingDocument.partialKsonValue]) - * 3. Filters remaining branches by leaf-level validation (using [ToolingDocument.ksonValue]) + * Navigation already flattens combinators and conditionals into individual branches + * at every level, so this function only needs to: + * 1. Filter oneOf/anyOf branches by sibling property constraints (using [ToolingDocument.partialKsonValue]) + * 2. Filter remaining branches by leaf-level validation (using [ToolingDocument.ksonValue]) * * The two passes use different document values because sibling filtering must work * even when the document has parse errors at the cursor position (where only the * partial value is available), while leaf validation needs the fully parsed tree. * - * @param candidateSchemas The schemas found at the document path + * @param candidateSchemas The schemas found at the document path (already flattened) * @param document The parsed document providing both full and partial value trees * @param documentPointer The [JsonPointer] to the location in the document - * @return List of valid schemas after expansion and filtering + * @return List of valid schemas after filtering */ fun getValidSchemas( candidateSchemas: List, document: ToolingDocument, documentPointer: JsonPointer ): List { - val expandedSchemas = schemaIdLookup.expandCombinators(candidateSchemas) - val partialDocumentValue = document.partialKsonValue val afterSiblingFilter = if (partialDocumentValue != null) { - filterBySiblingCompatibility(expandedSchemas, partialDocumentValue, documentPointer) + filterBySiblingCompatibility(candidateSchemas, partialDocumentValue, documentPointer) } else { - expandedSchemas + candidateSchemas } val hasBranchesThatRequireValidation = afterSiblingFilter.any { ref -> diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt index 3227f535..09a12270 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/BidirectionalIfThenCompletionTest.kt @@ -2,7 +2,9 @@ package org.kson import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertTrue /** * End-to-end tests for bidirectional if/then completion narrowing. @@ -135,4 +137,64 @@ class BidirectionalIfThenCompletionTest : SchemaCompletionTest { listOf("DBT_RUN", "DBT_TEST", "SNOW_QUERY", "SNOW_TEST"), "Without integration sibling, all jobs should be offered") } + + /** + * Regression: when `if` matches at the root, the `else` branch's properties + * must not leak into property completions. Previously a doc-blind expansion + * pass ran after navigation and emitted both branches; soft validation couldn't + * see the `if` predicate (which lives on the parent) and both properties + * survived. Navigation now flattens conditionals doc-aware at every level, + * including the target. + */ + @Test + fun testIfMatchesRootDoesNotLeakElseBranch() { + val schema = """ + { + "type": "object", + "properties": { "kind": { "type": "string" } }, + "if": { "properties": { "kind": { "const": "dog" } }, "required": ["kind"] }, + "then": { "properties": { "bark": { "type": "boolean" } } }, + "else": { "properties": { "meow": { "type": "boolean" } } } + } + """ + val completions = getCompletionsAtCaret(schema, """ + { + "kind": "dog", + + } + """.trimIndent()) + assertNotNull(completions) + val labels = completions.map { it.label }.sorted() + assertTrue("bark" in labels, "bark must be present when if matches, got: $labels") + assertFalse("meow" in labels, "meow must not leak from else when if matches, got: $labels") + } + + /** + * Analog to [testIfMatchesRootDoesNotLeakElseBranch] one level deeper: a property + * whose schema body is itself an if/then/else must narrow based on that property's + * own document value, not leak both branches. + */ + @Test + fun testIfMatchesNestedPropertyDoesNotLeakElseBranch() { + val schema = """ + { + "type": "object", + "properties": { + "config": { + "type": "object", + "properties": { "kind": { "type": "string" } }, + "if": { "properties": { "kind": { "const": "dog" } }, "required": ["kind"] }, + "then": { "properties": { "bark": { "type": "boolean" } } }, + "else": { "properties": { "meow": { "type": "boolean" } } } + } + } + } + """ + val completions = getCompletionsAtCaret(schema, + """{ "config": { "kind": "dog" } }""") + assertNotNull(completions) + val labels = completions.map { it.label }.sorted() + assertTrue("bark" in labels, "bark must be present when nested if matches, got: $labels") + assertFalse("meow" in labels, "meow must not leak from nested else when if matches, got: $labels") + } } diff --git a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt index 5172a9cb..628b6ee5 100644 --- a/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt +++ b/src/commonMain/kotlin/org/kson/schema/SchemaIdLookup.kt @@ -74,96 +74,6 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { } - /** - * Expands combinator schemas (oneOf/anyOf/allOf) in a list of resolved schemas. - * - * For each schema in the list: - * - If it contains oneOf/anyOf/allOf, expands them into individual branches - * - Resolves any $ref in the branches - * - Otherwise keeps the schema as-is - * - * @param schemas The list of schemas to expand - * @return Expanded list with combinator branches as separate items, with $ref resolved - */ - fun expandCombinators(schemas: List): List { - val expanded = mutableListOf() - - for (ref in schemas) { - expandSingleSchema(ref, expanded) - } - - return expanded - } - - /** - * Recursively expand a single schema's combinators and conditionals. - * - * Expands oneOf/anyOf/allOf into individual branches and if/then/else into - * conditional branches, resolving any $ref along the way. Branches are - * themselves expanded recursively so that nested structures (e.g. allOf - * containing if/then) are fully flattened. - * - * The parent schema is preserved alongside its branches so that its own - * properties (title, description, constraints) remain available. - */ - private fun expandSingleSchema(ref: ResolvedRef, expanded: MutableList) { - val schemaObj = ref.resolvedValue as? KsonObject - - if (schemaObj == null) { - expanded.add(ref) - return - } - - var addedBranches = false - // Track where to insert the parent (to add it first) - val branchesStartIndex = expanded.size - - fun addBranch(branch: KsonValue, baseUri: String, resolutionType: SchemaResolutionType) { - val resolved = resolveRefIfPresent(branch, baseUri) - // Preserve the parentBranch from navigation through expansion — the - // branch context from the original oneOf/anyOf is still the right one - // for sibling filtering regardless of how deeply the schema expands. - val branchRef = ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, resolutionType, ref.parentBranch) - expandSingleSchema(branchRef, expanded) - addedBranches = true - } - - // Check for oneOf - (schemaObj.propertyLookup["oneOf"] as? KsonList)?.elements?.forEach { branch -> - addBranch(branch, ref.resolvedValueBaseUri, SchemaResolutionType.ONE_OF) - } - - // Check for anyOf - (schemaObj.propertyLookup["anyOf"] as? KsonList)?.elements?.forEach { branch -> - addBranch(branch, ref.resolvedValueBaseUri, SchemaResolutionType.ANY_OF) - } - - // Check for allOf - (schemaObj.propertyLookup["allOf"] as? KsonList)?.elements?.forEach { branch -> - addBranch(branch, ref.resolvedValueBaseUri, SchemaResolutionType.ALL_OF) - } - - // Check for if/then/else conditionals - if (schemaObj.propertyLookup.containsKey("if")) { - schemaObj.propertyLookup["then"]?.let { thenBranch -> - addBranch(thenBranch, ref.resolvedValueBaseUri, SchemaResolutionType.IF_THEN) - } - schemaObj.propertyLookup["else"]?.let { elseBranch -> - addBranch(elseBranch, ref.resolvedValueBaseUri, SchemaResolutionType.IF_ELSE) - } - } - - if (addedBranches) { - // Include the parent schema to preserve its properties (e.g., description, title, constraints) - // Insert at the start so it appears first in hover info - expanded.add(branchesStartIndex, ref) - } else { - // If we didn't add any branches, keep the original schema - expanded.add(ref) - } - } - - /** * Navigate schema by document path tokens. * @@ -172,13 +82,15 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { * - For array indices: navigates to "items" schema (all array elements share the same schema) * - Falls back to "additionalProperties" or "patternProperties" when specific property not found * - Resolves `$ref` references to their target schemas - * - Handles combinators (allOf, anyOf, oneOf) which can create multiple schema branches + * - Handles combinators (allOf, anyOf, oneOf) and conditionals (if/then/else), flattening + * them at every level so callers receive fully decomposed branches * * Base URI tracking is handled internally to ensure correct `$ref` resolution. * * Returns a list because a single document path can match multiple schema locations: * - Property defined in multiple combinator branches * - Multiple patternProperties matching + * - Both `then` and `else` branches active when the `if` can't be evaluated * * Example: * ```kotlin @@ -195,111 +107,7 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { documentPointer: JsonPointer, documentValue: KsonValue? = null, ): List { - val startingBaseUri = rootBaseUri() - val documentPathTokens = documentPointer.tokens - if (documentPathTokens.isEmpty()) { - // Even at root, resolve $ref if present - val resolved = resolveRefIfPresent(schemaRootValue, startingBaseUri) - return listOf(resolved) - } - - // Track all current schema nodes we're exploring (can branch out due to combinators) - var currentNodes = listOf(resolveRefIfPresent(schemaRootValue, startingBaseUri)) - // Track the document value at the current navigation level for if/then evaluation - var currentDocumentValue = documentValue - - for (token in documentPathTokens) { - val nextNodes = mutableListOf() - - for ((node, baseUri) in currentNodes) { - if (node !is KsonObject) { - continue - } - - // Track $id changes for proper URI resolution - var updatedBaseUri = baseUri - node.propertyLookup[$$"$id"]?.let { idValue -> - if (idValue is KsonString) { - val fullyQualifiedId = resolveUri(idValue.value, baseUri) - updatedBaseUri = fullyQualifiedId.toString() - } - } - - // Determine if this is an array index or property name - val isArrayIndex = token.toIntOrNull() != null - - val navigatedNodes = if (isArrayIndex) { - // Array navigation: go to "items" schema - // We ignore the actual index - all array elements use the same schema - navigateArrayItems(node, updatedBaseUri) - } else { - // Object navigation: go to "properties" wrapper, then the property - navigateObjectProperty(node, token, updatedBaseUri, currentDocumentValue) - } - - // Resolve $ref for each navigated node, preserving parentBranch context - for (navNode in navigatedNodes) { - val resolved = resolveRefIfPresent(navNode.resolvedValue, navNode.resolvedValueBaseUri) - nextNodes.add(ResolvedRef(resolved.resolvedValue, resolved.resolvedValueBaseUri, navNode.resolutionType, navNode.parentBranch)) - } - } - - currentNodes = nextNodes - if (currentNodes.isEmpty()) { - break - } - - // Advance document value in parallel with schema navigation - currentDocumentValue = currentDocumentValue?.let { docVal -> - KsonValueWalker.navigateWithJsonPointer(docVal, JsonPointer.fromTokens(listOf(token))) - } - } - - return currentNodes - } - - /** - * Navigate to the schema for array items. - * - * Looks for "items" or "additionalItems" schema properties. - * Also searches through combinators (anyOf/oneOf/allOf) to find items schemas. - */ - private fun navigateArrayItems(schemaNode: KsonObject, currentBaseUri: String): List { - val results = mutableListOf() - - // Try "items" first (most common case) - schemaNode.propertyLookup["items"]?.let { - results.add(ResolvedRef(it, currentBaseUri, SchemaResolutionType.ARRAY_ITEMS)) - } - - // Fallback to "additionalItems" - schemaNode.propertyLookup["additionalItems"]?.let { - results.add(ResolvedRef(it, currentBaseUri, SchemaResolutionType.ARRAY_ITEMS)) - } - - // If no items found directly, search through combinators - if (results.isEmpty() && - (schemaNode.propertyLookup.containsKey("allOf") || - schemaNode.propertyLookup.containsKey("anyOf") || - schemaNode.propertyLookup.containsKey("oneOf"))) { - results.addAll(navigateThroughCombinators( - schemaNode = schemaNode, - currentBaseUri = currentBaseUri, - recursiveNavigate = { schema, baseUri -> navigateArrayItems(schema, baseUri) }, - shouldTagWithCombinator = { it == SchemaResolutionType.ARRAY_ITEMS } - )) - } - - // If no items found, search through if/then/else conditionals - if (results.isEmpty() && schemaNode.propertyLookup.containsKey("if")) { - results.addAll(navigateThroughConditionals( - schemaNode = schemaNode, - currentBaseUri = currentBaseUri, - recursiveNavigate = { schema, baseUri -> navigateArrayItems(schema, baseUri) } - )) - } - - return results + return SchemaNavigator(this).navigate(documentPointer, documentValue) } /** @@ -324,197 +132,196 @@ class SchemaIdLookup(val schemaRootValue: KsonValue) { } /** - * Navigate through combinator schemas (allOf, anyOf, oneOf) to find matching sub-schemas. + * Navigates a [JsonPointer] through a schema, returning all sub-schemas at the + * target location fully flattened (combinators exploded, conditionals narrowed by + * strict isValid against the document). * - * This is a shared implementation used by both array and object navigation. - * For each combinator branch, it resolves any $ref, then delegates to the caller's - * navigation function to continue traversal. + * The navigator is built on two primitives: + * - [stepInto]: structural per-token descent through a single schema, no combinator + * awareness. + * - [flatten]: doc-aware decomposition of a schema's top-level branches. * - * For oneOf/anyOf results, the branch schema is attached as [ResolvedRef.parentBranch] - * so that downstream filtering can validate sibling property constraints that aren't - * visible from the leaf schema alone. + * Every level applies `flatten` before stepping, including the root and the target, + * so no post-navigation expansion pass is needed. * - * @param schemaNode The schema node containing combinators - * @param currentBaseUri The current base URI for $ref resolution - * @param recursiveNavigate How to continue navigation (e.g., navigateArrayItems or navigateObjectProperty) - * @param shouldTagWithCombinator Which resolution types to overwrite with combinator type (e.g., ARRAY_ITEMS -> ANY_OF) + * Mirrors the shape of `TreeNavigator` in the walker package (see + * [org.kson.walker.navigateWithJsonPointer]) — one entry point, small internal + * helpers — so the pattern is recognizable. */ - private fun navigateThroughCombinators( - schemaNode: KsonObject, - currentBaseUri: String, - recursiveNavigate: (schema: KsonObject, baseUri: String) -> List, - shouldTagWithCombinator: (resolutionType: SchemaResolutionType) -> Boolean - ): List { - val results = mutableListOf() - - fun processCombinator(combinator: KsonValue?, combinatorType: SchemaResolutionType) { - val combinatorList = combinator as? KsonList ?: return - val attachParentBranch = combinatorType in setOf(SchemaResolutionType.ONE_OF, SchemaResolutionType.ANY_OF) - - for (element in combinatorList.elements) { - val resolved = resolveRefIfPresent(element, currentBaseUri) - - if (resolved.resolvedValue is KsonObject) { - val nestedResults = recursiveNavigate(resolved.resolvedValue, resolved.resolvedValueBaseUri) - - // Attach parentBranch for oneOf/anyOf so downstream filtering - // can check sibling constraints. - results.addAll(nestedResults.map { ref -> - if (shouldTagWithCombinator(ref.resolutionType)) { - ref.copy( - resolutionType = combinatorType, - parentBranch = if (attachParentBranch) resolved else null - ) - } else { - ref - } - }) - } - } - } + private class SchemaNavigator(private val idLookup: SchemaIdLookup) { - processCombinator(schemaNode.propertyLookup["allOf"], SchemaResolutionType.ALL_OF) - processCombinator(schemaNode.propertyLookup["anyOf"], SchemaResolutionType.ANY_OF) - processCombinator(schemaNode.propertyLookup["oneOf"], SchemaResolutionType.ONE_OF) + fun navigate(documentPointer: JsonPointer, documentValue: KsonValue?): List { + val rootBaseUri = idLookup.rootBaseUri() + val rootRef = idLookup.resolveRefIfPresent(idLookup.schemaRootValue, rootBaseUri) - return results - } + var current = flatten(rootRef, documentValue) + var currentDocValue = documentValue - /** - * Navigate through if/then/else conditional schemas. - * - * When [documentValue] is available, evaluates the "if" condition against it - * and only includes the matching branch (then or else). When unavailable, - * includes both branches so that all possible schemas are discoverable. - * - * @param schemaNode The schema node containing if/then/else - * @param currentBaseUri The current base URI for $ref resolution - * @param documentValue The document value at this schema level, used to evaluate the "if" condition - * @param recursiveNavigate How to continue navigation within each branch - */ - private fun navigateThroughConditionals( - schemaNode: KsonObject, - currentBaseUri: String, - documentValue: KsonValue? = null, - recursiveNavigate: (schema: KsonObject, baseUri: String) -> List - ): List { - val results = mutableListOf() - - fun processConditionalBranch(branch: KsonValue?, resolutionType: SchemaResolutionType) { - branch ?: return - val resolved = resolveRefIfPresent(branch, currentBaseUri) - if (resolved.resolvedValue is KsonObject) { - val nestedResults = recursiveNavigate(resolved.resolvedValue, resolved.resolvedValueBaseUri) - // Unlike navigateThroughCombinators, we always tag results with the - // conditional type. This tagging is what SchemaFilteringService uses - // to identify branches that need validation-based filtering. - results.addAll(nestedResults.map { ref -> - ref.copy(resolutionType = resolutionType) - }) + for (token in documentPointer.tokens) { + val stepped = current.flatMap { stepInto(it, token) } + currentDocValue = currentDocValue?.let { docVal -> + KsonValueWalker.navigateWithJsonPointer(docVal, JsonPointer.fromTokens(listOf(token))) + } + current = stepped.flatMap { flatten(it, currentDocValue) } + if (current.isEmpty()) break } + + return current } - // When we have a document value, evaluate the "if" condition to determine - // which branch to include. This is critical when the "if" condition - // checks a sibling property that isn't visible from the target property. - val ifCondition = schemaNode.propertyLookup["if"] - if (documentValue != null && ifCondition != null) { - val ifSchema = SchemaParser.parseSchemaElement(ifCondition, MessageSink(), currentBaseUri, this) - if (ifSchema != null) { - val conditionMatches = ifSchema.isValid(documentValue, MessageSink()) - if (conditionMatches) { - processConditionalBranch(schemaNode.propertyLookup["then"], SchemaResolutionType.IF_THEN) - } else { - processConditionalBranch(schemaNode.propertyLookup["else"], SchemaResolutionType.IF_ELSE) + /** + * Structural step by one pointer token. Looks at properties / patternProperties / + * additionalProperties (for names) or items / additionalItems (for integer indices). + * No combinator / conditional logic — [flatten] handles branching. + * + * Applies `$id` on [ref] to the base URI before property lookup, and resolves `$ref` + * on the stepped-into schema. + * + * Branch context inheritance: if [ref]'s resolutionType is a branch marker + * (ONE_OF / ANY_OF / ALL_OF / IF_THEN / IF_ELSE), stepped results keep that marker + * so downstream filtering still treats them as conditional. Otherwise the stepped + * result's resolutionType reflects how the step resolved the token + * (DIRECT_PROPERTY / PATTERN_PROPERTY / ADDITIONAL_PROPERTY / ARRAY_ITEMS). + * [ResolvedRef.parentBranch] is inherited unchanged. + */ + private fun stepInto(ref: ResolvedRef, token: String): List { + val schemaObj = ref.resolvedValue as? KsonObject ?: return emptyList() + + // Apply $id on the current node to the base URI before any lookup from this node. + var updatedBaseUri = ref.resolvedValueBaseUri + schemaObj.propertyLookup[$$"$id"]?.let { idValue -> + if (idValue is KsonString) { + updatedBaseUri = resolveUri(idValue.value, updatedBaseUri).toString() } - return results } - } - - // No document value or couldn't parse if condition — include both branches - processConditionalBranch(schemaNode.propertyLookup["then"], SchemaResolutionType.IF_THEN) - processConditionalBranch(schemaNode.propertyLookup["else"], SchemaResolutionType.IF_ELSE) - return results - } + val stepped = mutableListOf>() + val isArrayIndex = token.toIntOrNull() != null - /** - * Navigate an object schema to find all sub-schemas for a property. - * - * Handles multiple JSON Schema patterns: - * 1. Direct property lookup in "properties" - * 2. Pattern matching via "patternProperties" (can match multiple patterns) - * 3. Combinator schemas ("allOf", "anyOf", "oneOf") - * 4. Conditional schemas ("if"/"then"/"else") - * 5. Fallback to "additionalProperties" - * - * Returns a list because a property can be defined in multiple places: - * - Multiple patternProperties can match - * - Property can exist in multiple combinator branches - * - Property can exist in both then and else conditional branches - */ - private fun navigateObjectProperty( - schemaNode: KsonObject, - propertyName: String, - currentBaseUri: String, - documentValue: KsonValue? = null - ): List { - val results = mutableListOf() + if (isArrayIndex) { + schemaObj.propertyLookup["items"]?.let { + stepped.add(it to SchemaResolutionType.ARRAY_ITEMS) + } + schemaObj.propertyLookup["additionalItems"]?.let { + stepped.add(it to SchemaResolutionType.ARRAY_ITEMS) + } + } else { + val properties = schemaObj.propertyLookup["properties"] as? KsonObject + properties?.propertyMap?.get(token)?.let { + stepped.add(it.propValue to SchemaResolutionType.DIRECT_PROPERTY) + } - // Try direct property lookup in "properties" - val properties = schemaNode.propertyLookup["properties"] as? KsonObject - properties?.propertyMap?.get(propertyName)?.let { - results.add(ResolvedRef(it.propValue, currentBaseUri, SchemaResolutionType.DIRECT_PROPERTY)) - } + val patternProperties = schemaObj.propertyLookup["patternProperties"] as? KsonObject + patternProperties?.propertyMap?.forEach { (pattern, property) -> + try { + if (Regex(pattern).containsMatchIn(token)) { + stepped.add(property.propValue to SchemaResolutionType.PATTERN_PROPERTY) + } + } catch (_: Throwable) { + // Invalid regex pattern, skip it + // Use Throwable to catch JavaScript SyntaxError and other platform-specific errors + } + } - // Try pattern properties - check all patterns for a match - val patternProperties = schemaNode.propertyLookup["patternProperties"] as? KsonObject - patternProperties?.propertyMap?.forEach { (pattern, property) -> - try { - if (Regex(pattern).containsMatchIn(propertyName)) { - results.add(ResolvedRef(property.propValue, currentBaseUri, SchemaResolutionType.PATTERN_PROPERTY)) + if (stepped.isEmpty()) { + schemaObj.propertyLookup["additionalProperties"]?.let { + stepped.add(it to SchemaResolutionType.ADDITIONAL_PROPERTY) + } } - } catch (_: Throwable) { - // Invalid regex pattern, skip it - // Use Throwable to catch JavaScript SyntaxError and other platform-specific errors } - } - // Try combinators if we haven't found anything yet or if they exist - // (allOf should be merged with existing results, anyOf/oneOf provide alternatives) - if (schemaNode.propertyLookup.containsKey("allOf") || - schemaNode.propertyLookup.containsKey("anyOf") || - schemaNode.propertyLookup.containsKey("oneOf")) { - results.addAll(navigateThroughCombinators( - schemaNode = schemaNode, - currentBaseUri = currentBaseUri, - recursiveNavigate = { schema, baseUri -> navigateObjectProperty(schema, propertyName, baseUri, documentValue) }, - shouldTagWithCombinator = { it in listOf( - SchemaResolutionType.DIRECT_PROPERTY, - SchemaResolutionType.PATTERN_PROPERTY, - SchemaResolutionType.ADDITIONAL_PROPERTY - )} - )) + val inheritedType = ref.resolutionType.takeIf { it.isBranchMarker } + return stepped.map { (value, stepType) -> + val resolved = idLookup.resolveRefIfPresent(value, updatedBaseUri) + ResolvedRef( + resolved.resolvedValue, + resolved.resolvedValueBaseUri, + inheritedType ?: stepType, + ref.parentBranch + ) + } } - // Try if/then/else conditionals - if (schemaNode.propertyLookup.containsKey("if")) { - results.addAll(navigateThroughConditionals( - schemaNode = schemaNode, - currentBaseUri = currentBaseUri, - documentValue = documentValue, - recursiveNavigate = { schema, baseUri -> navigateObjectProperty(schema, propertyName, baseUri, documentValue) } - )) - } + /** + * Flatten a schema's top-level branches. Handles: + * - oneOf / anyOf / allOf: unconditional expansion, every branch emitted. + * - if / then / else: evaluates `if` strictly against [docVal] via + * [SchemaParser.parseSchemaElement] + [isValid]; emits only the matching branch + * when [docVal] is present, or both branches when [docVal] is null or the + * `if` can't be parsed. + * + * Recurses into each branch so nested combinators/conditionals are fully flattened. + * + * The parent [ref] is preserved at the head of the result so its title, description, + * and constraints remain available. For oneOf/anyOf branches, [ResolvedRef.parentBranch] + * is set to the resolved branch itself so [SchemaFilteringService] can validate sibling + * constraints. For allOf and conditional branches, `parentBranch` is inherited from + * [ref]. + */ + private fun flatten(ref: ResolvedRef, docVal: KsonValue?): List { + val schemaObj = ref.resolvedValue as? KsonObject ?: return listOf(ref) + + val results = mutableListOf() + var addedBranches = false + + fun addBranch(branch: KsonValue, resolutionType: SchemaResolutionType, attachAsParentBranch: Boolean) { + val resolved = idLookup.resolveRefIfPresent(branch, ref.resolvedValueBaseUri) + val branchRef = ResolvedRef( + resolved.resolvedValue, + resolved.resolvedValueBaseUri, + resolutionType, + if (attachAsParentBranch) resolved else ref.parentBranch + ) + results.addAll(flatten(branchRef, docVal)) + addedBranches = true + } - // Fallback to additionalProperties if nothing found - if (results.isEmpty()) { - schemaNode.propertyLookup["additionalProperties"]?.let { - results.add(ResolvedRef(it, currentBaseUri, SchemaResolutionType.ADDITIONAL_PROPERTY)) + (schemaObj.propertyLookup["oneOf"] as? KsonList)?.elements?.forEach { branch -> + addBranch(branch, SchemaResolutionType.ONE_OF, attachAsParentBranch = true) + } + + (schemaObj.propertyLookup["anyOf"] as? KsonList)?.elements?.forEach { branch -> + addBranch(branch, SchemaResolutionType.ANY_OF, attachAsParentBranch = true) + } + + (schemaObj.propertyLookup["allOf"] as? KsonList)?.elements?.forEach { branch -> + addBranch(branch, SchemaResolutionType.ALL_OF, attachAsParentBranch = false) + } + + val ifCondition = schemaObj.propertyLookup["if"] + if (ifCondition != null) { + val conditionMatches = docVal?.let { dv -> + val ifSchema = SchemaParser.parseSchemaElement( + ifCondition, MessageSink(), ref.resolvedValueBaseUri, idLookup + ) + ifSchema?.isValid(dv, MessageSink()) + } + when (conditionMatches) { + true -> schemaObj.propertyLookup["then"]?.let { + addBranch(it, SchemaResolutionType.IF_THEN, attachAsParentBranch = false) + } + false -> schemaObj.propertyLookup["else"]?.let { + addBranch(it, SchemaResolutionType.IF_ELSE, attachAsParentBranch = false) + } + null -> { + schemaObj.propertyLookup["then"]?.let { + addBranch(it, SchemaResolutionType.IF_THEN, attachAsParentBranch = false) + } + schemaObj.propertyLookup["else"]?.let { + addBranch(it, SchemaResolutionType.IF_ELSE, attachAsParentBranch = false) + } + } + } } - } - return results + if (addedBranches) { + results.add(0, ref) + } else { + results.add(ref) + } + + return results + } } companion object { @@ -763,6 +570,21 @@ enum class SchemaResolutionType { DIRECT_PROPERTY, PATTERN_PROPERTY, ADDITIONAL_PROPERTY, ARRAY_ITEMS, ALL_OF, ROOT -> false } + + /** + * True if this resolution type was produced by a branching construct (combinator + * or conditional). Stepping into a branch-marked ref preserves the marker so the + * downstream leaf still gets filtered by the branch's semantics. + * + * Exhaustive by design: adding a new enum entry forces a compile error here so + * the branch-vs-structural classification is an explicit decision, not a default. + */ + val isBranchMarker: Boolean + get() = when (this) { + ONE_OF, ANY_OF, ALL_OF, IF_THEN, IF_ELSE -> true + DIRECT_PROPERTY, PATTERN_PROPERTY, ADDITIONAL_PROPERTY, + ARRAY_ITEMS, ROOT -> false + } } /** diff --git a/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt b/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt index 29fda6da..47431300 100644 --- a/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt +++ b/src/commonTest/kotlin/org/kson/schema/SchemaNavigationTest.kt @@ -10,6 +10,7 @@ import org.kson.value.KsonString as InternalKsonString import org.kson.value.KsonBoolean as InternalKsonBoolean import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotNull class SchemaNavigationTest { @@ -447,6 +448,45 @@ class SchemaNavigationTest { assertEquals("string", ((uuidResults.single() as InternalKsonObject).propertyLookup["type"] as? InternalKsonString)?.value) } + /** + * When a property's schema body has its own oneOf, flatten's inner branches must + * carry *themselves* (the nearest-enclosing branch) as [ResolvedRef.parentBranch] — + * not the outer oneOf wrapper reached during navigation. Sibling-compatibility + * filtering validates the parentBranch's declared properties against document + * siblings; using the outer wrapper's properties instead of the inner branch would + * filter against the wrong constraint set. + */ + @Test + fun testNestedOneOfUsesNearestEnclosingAsParentBranch() { + val schema = """ + { + "oneOf": [ + { + "properties": { + "config": { + "oneOf": [ + { "title": "InnerA", "properties": { "foo": { "type": "string" } } }, + { "title": "InnerB", "properties": { "bar": { "type": "string" } } } + ] + } + } + } + ] + } + """ + val results = navigateSchemaFull(schema, listOf("config")) + // [configParent, innerA, innerB] + assertEquals(3, results.size) + val innerAParent = results[1].parentBranch?.resolvedValue as? InternalKsonObject + val innerBParent = results[2].parentBranch?.resolvedValue as? InternalKsonObject + assertNotNull(innerAParent) + assertNotNull(innerBParent) + assertEquals("InnerA", (innerAParent.propertyLookup["title"] as? InternalKsonString)?.value, + "parentBranch of inner A must be the inner branch itself, not the outer oneOf wrapper") + assertEquals("InnerB", (innerBParent.propertyLookup["title"] as? InternalKsonString)?.value, + "parentBranch of inner B must be the inner branch itself, not the outer oneOf wrapper") + } + @Test fun testNavigateAllOf() { val schema = """ @@ -537,9 +577,15 @@ class SchemaNavigationTest { - '${'$'}ref': '#/${'$'}defs/NumberType' """ - // This test verifies that navigation works when anyOf contains $ref + // Navigation flattens at every level. Stepping "0" lands on the array branch's + // items schema (which is itself an anyOf over NumberType), and flatten at the + // target expands that inner anyOf: parent items + one resolved NumberType branch. val results = navigateSchema(schema, listOf("0")) - assertEquals(1, results.size, "Root schema should return single result") + assertEquals(2, results.size, "Expected items parent + one flattened anyOf branch") + val itemsSchema = results[0] as InternalKsonObject + assertNotNull(itemsSchema.propertyLookup["anyOf"], "First result should be the items schema with its inner anyOf") + val numberBranch = results[1] as InternalKsonObject + assertEquals("number", (numberBranch.propertyLookup["type"] as? InternalKsonString)?.value) } @Test @@ -571,13 +617,20 @@ class SchemaNavigationTest { - '${'$'}ref': '#/${'$'}defs/ComplexRecipe' """ + // Navigation reaches the context property via the outer anyOf → $ref → properties. + // flatten runs at every level, so the context's own inner anyOf (object | null) + // is expanded too: context parent + 2 flattened branches = 3 results. val results = navigateSchema(schema, listOf("context")) - assertEquals(1, results.size, "Should find context property through anyOf → \$ref") + assertEquals(3, results.size, "Expected context schema + 2 inner anyOf branches") - val contextSchema = results.single() as InternalKsonObject + val contextSchema = results[0] as InternalKsonObject assertEquals("Context", (contextSchema.propertyLookup["title"] as? InternalKsonString)?.value) assertEquals("Defines arbitrary key-value pairs for Jinja interpolation", (contextSchema.propertyLookup["description"] as? InternalKsonString)?.value) + val branchTypes = results.drop(1).map { + ((it as InternalKsonObject).propertyLookup["type"] as? InternalKsonString)?.value + } + assertEquals(listOf("object", "null"), branchTypes, "Inner anyOf branches should be object and null") } @Test From 2e5eceef222c126a7cd9c7fb99876855f8352429 Mon Sep 17 00:00:00 2001 From: Bart Dubbeldam Date: Thu, 30 Apr 2026 09:29:58 +0200 Subject: [PATCH 15/15] Stop hiding incompatible-branch elimination behind a fallback filterBySiblingCompatibility used to fall back to the unfiltered set when every parent-branched ref was eliminated, which silently surfaced completions from branches the document genuinely contradicted. Removed the fallback so contradictions stay visible; the regression test documents the new behavior. Also clarified the scope of sibling-compat narrowing in isBranchCompatibleWithSiblings (properties: yes; required / dependencies / root-level type-enum-const: no), with a regression test pinning the required-only limitation. --- .../kson/tooling/SchemaFilteringService.kt | 25 ++++++-- .../org/kson/SchemaFilteringServiceTest.kt | 64 +++++++++++++++++++ 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt index 1f52cc28..4191ed09 100644 --- a/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt +++ b/kson-tooling-lib/src/commonMain/kotlin/org/kson/tooling/SchemaFilteringService.kt @@ -74,7 +74,9 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { * * For schemas with [ResolvedRef.parentBranch] set, checks whether the parent * branch's property constraints (const/enum) are compatible with the document's - * sibling values. Falls back to unfiltered schemas if all branches are eliminated. + * sibling values. When every branch contradicts the document, the result + * legitimately drops them all — completions from incompatible branches would + * be misleading. */ private fun filterBySiblingCompatibility( schemas: List, @@ -84,14 +86,10 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { val hasParentBranches = schemas.any { it.parentBranch != null } if (!hasParentBranches) return schemas - val filtered = schemas.filter { ref -> + return schemas.filter { ref -> val parentBranch = ref.parentBranch parentBranch == null || isBranchCompatibleWithSiblings(parentBranch, documentValue, documentPointer) } - - val branchesBefore = schemas.count { it.parentBranch != null } - val branchesAfter = filtered.count { it.parentBranch != null } - return if (branchesBefore > 0 && branchesAfter == 0) schemas else filtered } /** @@ -228,6 +226,21 @@ class SchemaFilteringService(private val schemaIdLookup: SchemaIdLookup) { * value is incomplete during completion — validating mid-typing input would spuriously * reject the branch. * + * **Scope of narrowing.** Only constraints reachable via the branch's `properties` map + * narrow. The following branch shapes are intentionally NOT considered: + * - Top-level `type` / `enum` / `const` on the branch itself (these constrain the parent + * object as a whole, not a sibling property). + * - `required` and `dependencies` — under soft validation these surface as + * [MessageType.SCHEMA_REQUIRED_PROPERTY_MISSING] / + * [MessageType.SCHEMA_MISSING_REQUIRED_DEPENDENCIES], which are intentionally + * ignored during completion (see [IGNORABLE_ERROR_TYPES]); they could only narrow + * if completion treated incomplete documents as final, which it doesn't. + * + * Discriminator-shaped unions (each branch pins one or more sibling properties via + * `properties: { … const/enum/type … }`) — the dominant pattern this filtering exists + * for — are fully covered. Branches that discriminate purely via the shapes above + * will not narrow. + * * @param parentBranch The oneOf/anyOf branch that contained this result * @param documentValue The root document value * @param documentPointer Path to the property being completed diff --git a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt index f2aa7279..964ed8e9 100644 --- a/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt +++ b/kson-tooling-lib/src/commonTest/kotlin/org/kson/SchemaFilteringServiceTest.kt @@ -327,6 +327,70 @@ class SchemaFilteringServiceTest { assertTrue("treats" in propertyNames, "DogParams properties should be present, got: $propertyNames") } + @Test + fun testGetValidSchemas_branchDiscriminatedByRequired_doesNotNarrow() { + // Branches discriminate via `required`, not via `properties..const/enum/type`. + // Sibling-compat filtering only inspects the branch's `properties` map, so the + // `required` arrays cannot narrow during completion — only the `a`-branch is + // satisfied by the document, but both branches still survive. Pins the + // documented limitation in `isBranchCompatibleWithSiblings`. + val schema = """ + oneOf: + - required: [a] + properties: + payload: + type: string + - required: [b] + properties: + payload: + type: number + """.trimIndent() + + val document = """ + a: present + """.trimIndent() + + val validSchemas = getValidSchemasForDocument(schema, document) + val survivingPayloadTypes = validSchemas.mapNotNull { propertyFieldOf(it, "payload", "type") } + assertEquals( + listOf("number", "string"), survivingPayloadTypes.sorted(), + "Both branches survive: required-based discrimination does not narrow during completion" + ) + } + + @Test + fun testGetValidSchemas_whenEveryBranchContradictsSiblings_dropsAllBranches() { + // Branches gate on `kind` const. Document says `kind: gamma`, which matches + // neither branch. Completing /payload, sibling-compat filtering must drop + // both branches' payload schemas — no silent fallback to the unfiltered set, + // which would surface completions from incompatible branches. + val schema = """ + oneOf: + - properties: + kind: + const: alpha + payload: + type: string + - properties: + kind: + const: beta + payload: + type: number + """.trimIndent() + + val document = """ + kind: gamma + """.trimIndent() + + val validSchemas = getValidSchemasForDocument(schema, document, JsonPointer("/payload")) + val survivingPayloadTypes = validSchemas.mapNotNull { (it as? KsonObject)?.propertyLookup?.get("type") as? KsonString } + .map { it.value } + assertEquals( + emptyList(), survivingPayloadTypes, + "No branch's payload schema should survive when sibling kind matches no branch's const" + ) + } + @Test fun testGetValidSchemas_withTypeMismatchAtTarget_filtersOutAllBranches() { val schema = """