Skip to content

Commit

Permalink
Intersection schema generation is order-dependent
Browse files Browse the repository at this point in the history
Description

- Given a schema that contains a named definition (`Level2B`),
- And that named definition is referenced in multiple locations,
- And that named schema is also an intersection type (`allOf` in this
  example),

Then when parsed, the generated TypeScript will contain the correct
reference only for the _first_ location in which the named schema is
encountered, during a depth-first traversal.

Subsequent references to the same schema will be generated as though
they were only the intersection type, and not the named schema.

Example

Given the following schema:

```yaml
description: Top Level
type: object
oneOf:
  - $ref: '#/definitions/Level2A'
  - $ref: '#/definitions/Level2B'

definitions:
  Level2A:
    description: Level2A
    type: object
    allOf: [$ref: '#/definitions/Base']
    properties:
      level_2A_ref: { $ref: '#/definitions/Level2B' }
  Level2B:
    description: Level2B
    type: object
    allOf: [$ref: '#/definitions/Base']
    properties:
      level_2B_prop: { const: xyzzy }
  Base:
    description: Base
    type: object
    properties:
      base_prop: { type: string }
```

The current resulting TypeScript will be (comments adjusted
for clarity):

```ts
// Incorrect: should be `type Demo = Level2A | Level2B`
// Note that the Level2B type at this location is the _second_ instance
// to Level2B during a depth-first traversal.
export type Demo = Level2A | Level2B1;

export type Level2A = Level2A1 & {
  // Correct in this location because this property is reached first in
  // a depth-first traversal.
  level_2A_ref?: Level2B;
  [k: string]: unknown;
};

export type Level2A1 = Base;

export type Level2B = Level2B1 & {
  level_2B_prop?: "xyzzy";
  [k: string]: unknown;
};

export type Level2B1 = Base;

export interface Base {
  base_prop?: string;
  [k: string]: unknown;
}
```

Root Cause

In `parser.ts`, [lines 57 - 75][1], when schema that matches multiple
"types" is encountered, the parser generates a new `ALL_OF` intersection
schema to contain each sub-type, then adds each sub-type to the new
`ALL_OF` schema.

Each sub-type is then parsed sequentially. During this process,
`maybeStripNameHints` is called, which mutates the schema by removing
the `$id`, `description`, and `name` properties.

Notably, these properties are used by `typesOfSchema` to detect the
`NAMED_SCHEMA` type. As a result, this schema object will never again be
detected as a `NAMED_SCHEMA` type.

Therefore, the _first_ instance of the schema object is correctly
handled as an intersection schema **and** a named schema, but all
subsequent instances are treated as though they are **only** an
intersection schema.

Proposed Solution

- The call to `typesOfSchema` is moved from `parser.ts` to
  `normalizer.ts`, with the goal of avoiding confusion due to a mutated
  schema object. The resulting list of schema types is persisted as a
  `$types` property on the schema.

- The generated intersection schema is _also_ moved from `parser.ts` to
  `normalizer.ts`. This is because it is advantageous to let the
  generated intersection schema participate in the caching mechanism
  (which it could not previously do, since it was generated dynamically
  during each encounter). Without this, multiple instances of the same
  schema are generated.

Related Issues

- bcherny#597

[1]: https://github.com/bcherny/json-schema-to-typescript/blob/31993def993b610ba238d3024260129e31ddc371/src/parser.ts#L57-L75 'parser.ts, lines 57 - 75'
  • Loading branch information
altearius committed Jun 23, 2024
1 parent 31993de commit 7118862
Show file tree
Hide file tree
Showing 31 changed files with 1,343 additions and 684 deletions.
20 changes: 20 additions & 0 deletions src/applySchemaTyping.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type {LinkedJSONSchema} from './types/JSONSchema'
import {typesOfSchema} from './typesOfSchema'

export function applySchemaTyping(schema: LinkedJSONSchema) {
const types = typesOfSchema(schema)
schema.$types = types

if (types.length > 1) {
schema.$intersection = {
$id: schema.$id,
allOf: [],
description: schema.description,
title: schema.title,
}

delete schema.$id
delete schema.description
delete schema.name
}
}
17 changes: 17 additions & 0 deletions src/normalizer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {JSONSchemaTypeName, LinkedJSONSchema, NormalizedJSONSchema, Parent} from './types/JSONSchema'
import {appendToDescription, escapeBlockComment, isSchemaLike, justName, toSafeString, traverse} from './utils'
import {Options} from './'
import {applySchemaTyping} from './applySchemaTyping'
import {DereferencedPaths} from './resolver'
import {isDeepStrictEqual} from 'util'

Expand Down Expand Up @@ -222,6 +223,22 @@ rules.set('Transform const to singleton enum', schema => {
}
})

// Precalculation of the schema types is necessary because the ALL_OF type
// is implemented in a way that mutates the schema object. Detection of the
// NAMED_SCHEMA type relies on the presence of the $id property, which is
// hoisted to a parent schema object during the ALL_OF type implementation,
// and becomes unavailable if the same schema is used in multiple places.
//
// Precalculation of the `ALL_OF` intersection schema is necessary because
// the intersection schema needs to participate in the schema cache during
// the parsing step, so it cannot be re-calculated every time the schema
// is encountered.
rules.set('Pre-calculate schema types and intersections', schema => {
if (schema !== null && typeof schema === 'object') {
applySchemaTyping(schema)
}
})

export function normalize(
rootSchema: LinkedJSONSchema,
dereferencedPaths: DereferencedPaths,
Expand Down
40 changes: 22 additions & 18 deletions src/parser.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {JSONSchema4Type, JSONSchema4TypeName} from 'json-schema'
import {findKey, includes, isPlainObject, map, memoize, omit} from 'lodash'
import {findKey, includes, isPlainObject, map, memoize} from 'lodash'
import {format} from 'util'
import {Options} from './'
import {typesOfSchema} from './typesOfSchema'
import {applySchemaTyping} from './applySchemaTyping'
import {
AST,
T_ANY,
Expand All @@ -19,19 +19,20 @@ import {
getRootSchema,
isBoolean,
isPrimitive,
JSONSchema as LinkedJSONSchema,
JSONSchemaWithDefinitions,
type LinkedJSONSchema,
type NormalizedJSONSchema,
SchemaSchema,
SchemaType,
} from './types/JSONSchema'
import {generateName, log, maybeStripDefault, maybeStripNameHints} from './utils'
import {generateName, log, maybeStripDefault} from './utils'

export type Processed = Map<LinkedJSONSchema, Map<SchemaType, AST>>
export type Processed = Map<NormalizedJSONSchema, Map<SchemaType, AST>>

export type UsedNames = Set<string>

export function parse(
schema: LinkedJSONSchema | JSONSchema4Type,
schema: NormalizedJSONSchema | JSONSchema4Type,
options: Options,
keyName?: string,
processed: Processed = new Map(),
Expand All @@ -45,7 +46,7 @@ export function parse(
return parseLiteral(schema, keyName)
}

const types = typesOfSchema(schema)
const types = schema.$types
if (types.length === 1) {
const ast = parseAsTypeWithCache(schema, types[0], options, keyName, processed, usedNames)
log('blue', 'parser', 'Types:', types, 'Input:', schema, 'Output:', ast)
Expand All @@ -54,13 +55,13 @@ export function parse(

// Be careful to first process the intersection before processing its params,
// so that it gets first pick for standalone name.
const intersectionSchema = schema.$intersection
if (!intersectionSchema) {
throw new Error('Expected intersection schema')
}

const ast = parseAsTypeWithCache(
{
$id: schema.$id,
allOf: [],
description: schema.description,
title: schema.title,
},
intersectionSchema,
'ALL_OF',
options,
keyName,
Expand All @@ -71,15 +72,15 @@ export function parse(
ast.params = types.map(type =>
// We hoist description (for comment) and id/title (for standaloneName)
// to the parent intersection type, so we remove it from the children.
parseAsTypeWithCache(maybeStripNameHints(schema), type, options, keyName, processed, usedNames),
parseAsTypeWithCache(schema, type, options, keyName, processed, usedNames),
)

log('blue', 'parser', 'Types:', types, 'Input:', schema, 'Output:', ast)
return ast
}

function parseAsTypeWithCache(
schema: LinkedJSONSchema,
schema: NormalizedJSONSchema,
type: SchemaType,
options: Options,
keyName?: string,
Expand Down Expand Up @@ -131,7 +132,7 @@ function parseLiteral(schema: JSONSchema4Type, keyName: string | undefined): AST
}

function parseNonLiteral(
schema: LinkedJSONSchema,
schema: NormalizedJSONSchema,
type: SchemaType,
options: Options,
keyName: string | undefined,
Expand Down Expand Up @@ -288,8 +289,11 @@ function parseNonLiteral(
keyName,
standaloneName: standaloneName(schema, keyNameFromDefinition, usedNames, options),
params: (schema.type as JSONSchema4TypeName[]).map(type => {
const member: LinkedJSONSchema = {...omit(schema, '$id', 'description', 'title'), type}
return parse(maybeStripDefault(member as any), options, undefined, processed, usedNames)
const {$intersection, $id, description, title, $types, ...rest} = schema
const member: LinkedJSONSchema = {...rest, type}
maybeStripDefault(member)
applySchemaTyping(member)
return parse(member, options, undefined, processed, usedNames)
}),
type: 'UNION',
}
Expand Down
3 changes: 3 additions & 0 deletions src/types/JSONSchema.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {JSONSchema4, JSONSchema4Type, JSONSchema4TypeName} from 'json-schema'
import {isPlainObject, memoize} from 'lodash'
import type {typesOfSchema} from '../typesOfSchema'

export type SchemaType =
| 'ALL_OF'
Expand Down Expand Up @@ -71,6 +72,8 @@ export interface LinkedJSONSchema extends JSONSchema {
}

export interface NormalizedJSONSchema extends LinkedJSONSchema {
$intersection?: NormalizedJSONSchema
$types: ReturnType<typeof typesOfSchema>
additionalItems?: boolean | NormalizedJSONSchema
additionalProperties: boolean | NormalizedJSONSchema
extends?: string[]
Expand Down
20 changes: 0 additions & 20 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,26 +331,6 @@ export function maybeStripDefault(schema: LinkedJSONSchema): LinkedJSONSchema {
return schema
}

/**
* Removes the schema's `$id`, `name`, and `description` properties
* if they exist.
* Useful when parsing intersections.
*
* Mutates `schema`.
*/
export function maybeStripNameHints(schema: JSONSchema): JSONSchema {
if ('$id' in schema) {
delete schema.$id
}
if ('description' in schema) {
delete schema.description
}
if ('name' in schema) {
delete schema.name
}
return schema
}

export function appendToDescription(existingDescription: string | undefined, ...values: string[]): string {
if (existingDescription) {
return `${existingDescription}\n\n${values.join('\n')}`
Expand Down
Loading

0 comments on commit 7118862

Please sign in to comment.