fix(json-schema): stop forType custom attributes leaking across interned primitive types#3053
Open
schani wants to merge 4 commits into
Open
fix(json-schema): stop forType custom attributes leaking across interned primitive types#3053schani wants to merge 4 commits into
schani wants to merge 4 commits into
Conversation
…ned primitive types (#1268) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
No generated-output differences✅ This PR does not change generated outputs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
CustomAttributeProducers (as used by thequicktype-custom-*-examplepatternreferenced in the issue) can return
{ forType: someTypeAttributes }from aJSONSchemaAttributeProducerto attach custom type attributes to the typeproduced for a specific schema node. For plain, unrestricted primitive types
(most commonly
string), this leaked onto every occurrence of thatprimitive kind in the schema, not just the one the producer targeted.
Repro from the issue: a schema marks
emailAddressandphoneNumber(bothplain strings) as
deprecated: truevia a custom producer. An unrelatedplain-string property,
externalId, ends up markeddeprecatedtoo, eventhough nothing in the schema marks it as such.
Root cause
TypeBuilder.getOrAddTypeinterns types by aTypeIdentitycomputed from thetype kind plus only the identity attributes (
TypeAttributeKind.inIdentity === true). Custom attribute kinds defaultinIdentityandrequiresUniqueIdentitytofalse. Since most plain string properties in aschema resolve to the same identity (kind
"string"+StringTypes.unrestricted),getOrAddTypefinds and reuses the same internedPrimitiveType("string")node for all of them. When a
forTypeattribute is attached to oneoccurrence, it's actually merged onto that single shared node, so it "leaks"
to every other plain string property in the schema.
(Built-in producers avoid this today:
descriptionAttributeProducerroutesper-property descriptions through
forObject+ a property-keyed attributemap specifically to avoid this class of leak — but that workaround isn't
available to library consumers using the public
forTypeAPI.)Fix
In
packages/quicktype-core/src/input/JSONSchemaInput.ts, wrap any non-emptyforTypeattributes with an internalforTypeIdentityTypeAttributeKindmarker attribute (
requiresUniqueIdentity() => true) before combining theminto a type's attributes. This forces the type-graph builder to skip
interning for any type that carries occurrence-specific
forTypeattributes,so it always gets its own graph node instead of being silently shared with
unrelated occurrences of the same primitive kind. The marker attribute itself
is internal-only (default
combine/reconstitutebehavior, noaddToSchema), so it's invisible to renderers and generated output.Test coverage
Added
test/unit/json-schema-custom-attributes.test.ts, a focused unit testusing
JSONSchemaInputwith a customdeprecatedattribute producer (mirroringthe pattern from the linked example repos) that:
externalIdincorrectly ends updeprecated)externalIdhas nodeprecatedkey;emailAddressand
phoneNumbercorrectly do)This is unit-test-only because
CustomAttributeProducer/forTypeis aquicktype-corelibrary API feature — it requires registering a customTypeAttributeKindandJSONSchemaAttributeProducerprogrammatically, whichisn't reachable through the
quicktypeCLI or plain JSON/JSON-Schema fixtureinput files. Per repo conventions this is exactly the kind of API-level
behavior
test/unit/is for.Verification
npm run build: passesnpm run test:unit: 177/177 tests pass (including the new regression test)QUICKTEST=true FIXTURE=schema-typescript script/test: 71/71 pass locally,including the
description.schema,accessors.schema,comment-injection*.schema, andenum*.schemafixtures that alreadyexercise the built-in
forType-based description/accessor/enum-valuesproducers — no regression from widening
forTypehandling.before and after the fix:
externalIdno longer inheritsdeprecatedfromunrelated properties.
Fixes #1268
🤖 Generated with Claude Code