Skip to content

fix(gen-jsonschema): fix allOf $ref schema naming and sort closure bugs#526

Merged
Peefy merged 1 commit intokcl-lang:mainfrom
project0:fix/allof-bugs
Mar 1, 2026
Merged

fix(gen-jsonschema): fix allOf $ref schema naming and sort closure bugs#526
Peefy merged 1 commit intokcl-lang:mainfrom
project0:fix/allof-bugs

Conversation

@project0
Copy link
Contributor

@project0 project0 commented Feb 28, 2026

hint: This bug was solved with the help of AI (Claude)

I discovered this problem when generating schemas for cloud-init: https://github.com/canonical/cloud-init/blob/main/cloudinit/config/schemas/schema-cloud-config-v1.json


Three bugs in the JSON schema to KCL generator caused schemas using the allOf + placeholder-properties pattern (e.g., cloud-init) to produce incorrect output:

  1. Sort closure variable shadowing: sort.SliceStable closures used i,j parameter names that shadowed the outer loop variable, causing $ref (order 0) to sort after properties (order 2), so properties were converted before allOf refs could merge in real types.

  2. Empty placeholder properties not replaced: when merging allOf member properties into an existing schema, properties with empty keyword sets (placeholder schemas like "count": {}) were silently skipped instead of being replaced with the typed schema from the allOf member.

  3. $ref residue causing wrong schema names: the first $ref encountered in an allOf fell through to default: and was stored in s.Keywords rather than resolved, causing the main keyword loop to later set reference = "#/$defs/first_ref" and derive the schema name from it instead of from $id or the filename. Fixed by adding case *jsonschema.Ref: to the outer allOf switch so all refs are always resolved into schs immediately.

Adds a minimal regression test (allof-placeholder-props) and the previously missing cloud-init/expect.k test fixture.

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

  • N (not sure)
  • Y

2. What is the scope of this PR (e.g. component or file name):

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Other

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

  • N
  • Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

  • Unit test
  • Integration test
  • Benchmark (add benchmark stats below)
  • Manual test (add detailed scripts or steps below)
  • Other

Three bugs in the JSON schema to KCL generator caused schemas using the
allOf + placeholder-properties pattern (e.g., cloud-init) to produce
incorrect output:

1. Sort closure variable shadowing: sort.SliceStable closures used i,j
   parameter names that shadowed the outer loop variable, causing $ref
   (order 0) to sort after properties (order 2), so properties were
   converted before allOf refs could merge in real types.

2. Empty placeholder properties not replaced: when merging allOf member
   properties into an existing schema, properties with empty keyword
   sets (placeholder schemas like "count": {}) were silently skipped
   instead of being replaced with the typed schema from the allOf member.

3. $ref residue causing wrong schema names: the first $ref encountered
   in an allOf fell through to default: and was stored in s.Keywords
   rather than resolved, causing the main keyword loop to later set
   reference = "#/$defs/first_ref" and derive the schema name from it
   instead of from $id or the filename. Fixed by adding case
   *jsonschema.Ref: to the outer allOf switch so all refs are always
   resolved into schs immediately.

Adds a minimal regression test (allof-placeholder-props) and the
previously missing cloud-init/expect.k test fixture.

Signed-off-by: Richard Hillmann <[email protected]>
@Peefy Peefy merged commit bc4db85 into kcl-lang:main Mar 1, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support Go API for GetSchemaTypeMapping

2 participants