From 556e9a36bc0d5a6306dd3611d335bc119a6c693e Mon Sep 17 00:00:00 2001 From: Albina Blazhko Date: Thu, 20 Aug 2026 20:06:21 +0300 Subject: [PATCH 1/6] feat: add no-illogical-composition-keywords rule --- .../no-illogical-composition-keywords.md | 7 + docs/@v2/rules/built-in-rules.md | 1 + docs/@v2/rules/configurable-rules.md | 13 + .../oas/no-illogical-composition-keywords.md | 203 +++ docs/@v2/rules/recommended.md | 1 + docs/@v2/rules/ruleset-templates.md | 3 + docs/@v2/v2.sidebars.yaml | 1 + .../__snapshots__/redocly-yaml.test.ts.snap | 3 + .../config-resolvers.test.ts.snap | 6 + .../core/src/config/__tests__/load.test.ts | 18 + packages/core/src/config/all.ts | 3 + packages/core/src/config/minimal.ts | 3 + .../core/src/config/recommended-strict.ts | 3 + packages/core/src/config/recommended.ts | 3 + packages/core/src/config/spec.ts | 3 + .../no-illogical-composition-keywords.test.ts | 1397 +++++++++++++++++ packages/core/src/rules/oas3/index.ts | 2 + .../oas3/no-illogical-composition-keywords.ts | 508 ++++++ packages/core/src/types/index.ts | 2 +- packages/core/src/types/oas3.ts | 25 +- packages/core/src/types/oas3_1.ts | 6 +- packages/core/src/types/redocly-yaml.ts | 1 + packages/core/src/visitors.ts | 3 + .../snapshot.txt | 62 +- tests/e2e/lint/oas3.2/snapshot.txt | 22 +- 25 files changed, 2276 insertions(+), 23 deletions(-) create mode 100644 .changeset/no-illogical-composition-keywords.md create mode 100644 docs/@v2/rules/oas/no-illogical-composition-keywords.md create mode 100644 packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts create mode 100644 packages/core/src/rules/oas3/no-illogical-composition-keywords.ts diff --git a/.changeset/no-illogical-composition-keywords.md b/.changeset/no-illogical-composition-keywords.md new file mode 100644 index 0000000000..ce5593dcaa --- /dev/null +++ b/.changeset/no-illogical-composition-keywords.md @@ -0,0 +1,7 @@ +--- +'@redocly/cli': minor +--- + +Added the `no-illogical-composition-keywords` rule. + +**Note**: the rule is set to `warn` in the `recommended` ruleset and to `error` in `recommended-strict`, so existing API descriptions may report new problems. diff --git a/docs/@v2/rules/built-in-rules.md b/docs/@v2/rules/built-in-rules.md index 0578380a79..c7dce4d053 100644 --- a/docs/@v2/rules/built-in-rules.md +++ b/docs/@v2/rules/built-in-rules.md @@ -87,6 +87,7 @@ The rules list is split into sections. - [no-duplicated-enum-values](./common/no-duplicated-enum-values.md): All values in an `enum` must be unique - [no-enum-type-mismatch](./common/no-enum-type-mismatch.md): Enum options must match the data type declared in the schema - [no-example-value-and-externalValue](./oas/no-example-value-and-externalValue.md): Either the `value` or `externalValue` may be present, but not both +- [no-illogical-composition-keywords](./oas/no-illogical-composition-keywords.md): `oneOf`, `anyOf`, and `allOf` must combine schemas a value can actually match - [no-invalid-media-type-examples](./oas/no-invalid-media-type-examples.md): Example request bodies must match the declared schema - [no-mixed-number-range-constraints](./common/no-mixed-number-range-constraints.md): Ensures that schemas do not use both `maximum` and `exclusiveMaximum` (or both `minimum` and `exclusiveMinimum`) at the same time. - [no-invalid-schema-examples](./oas/no-invalid-schema-examples.md): Schema examples must match declared types diff --git a/docs/@v2/rules/configurable-rules.md b/docs/@v2/rules/configurable-rules.md index 133dc8f4b9..2e7e61dd2b 100644 --- a/docs/@v2/rules/configurable-rules.md +++ b/docs/@v2/rules/configurable-rules.md @@ -627,6 +627,19 @@ OpenAPI 3.0, 3.1, and 3.2 mostly share a type tree. Learn more about the [OpenAPI node types](https://redocly.com/docs/openapi-visual-reference/openapi-node-types/). +The `OneOf`, `AnyOf`, and `AllOf` node types each match the list of schemas under the keyword of the same name. +Use them to assert on one composition keyword without matching the other two. + +```yaml +rules: + rule/oneof-needs-two-schemas: + subject: + type: OneOf + message: Use at least two schemas in oneOf + assertions: + minLength: 2 +``` + ### `any` example The following example asserts that the maximum length of each description is 20 characters. diff --git a/docs/@v2/rules/oas/no-illogical-composition-keywords.md b/docs/@v2/rules/oas/no-illogical-composition-keywords.md new file mode 100644 index 0000000000..56b44ccea2 --- /dev/null +++ b/docs/@v2/rules/oas/no-illogical-composition-keywords.md @@ -0,0 +1,203 @@ +--- +slug: /docs/cli/rules/oas/no-illogical-composition-keywords +--- + +# no-illogical-composition-keywords + +Ensures that `oneOf`, `anyOf`, and `allOf` combine schemas that a value can actually resolve against. + +The rule reports: + +- A `oneOf` or `anyOf` with fewer than two schemas. +- An `allOf` with fewer than two schemas that neither declares another keyword of its own nor extends a discriminated schema. +- A schema repeated inside the same keyword. +- An empty schema (`{}`) used as a member. +- Two `oneOf` schemas that a single value can match at the same time. +- A `discriminator` whose property is missing from `required` in every member schema. +- An inline `oneOf` or `anyOf` member that a `discriminator` cannot select. + +| OAS | Compatibility | +| --- | ------------- | +| 2.0 | ❌ | +| 3.0 | ✅ | +| 3.1 | ✅ | +| 3.2 | ✅ | + +```mermaid +flowchart TD + +Root ==> components --> NamedSchemas --> Schema + +Schema ==> OneOf +Schema ==> AnyOf +Schema ==> AllOf + +style OneOf fill:#codaf9,stroke:#0044d4,stroke-width:5px +style AnyOf fill:#codaf9,stroke:#0044d4,stroke-width:5px +style AllOf fill:#codaf9,stroke:#0044d4,stroke-width:5px +``` + +## API design principles + +`oneOf` means "exactly one". +When a value matches two of the listed schemas, no tool can tell which one was intended, and validators, code generators, and documentation all disagree about the result. + +The most common version of this is nullability: if a referenced schema already accepts `null` and the `oneOf` also lists `type: 'null'`, a null value matches both branches. + +Deciding whether two arbitrary schemas overlap is not solvable in general, so the comparison stays deliberately narrow. +It reads `type`, `enum`, `const`, `properties`, `required`, and `additionalProperties: false`. +When a member uses any other constraint, such as `not`, `pattern`, `minimum`, or a nested `allOf`, that constraint may be what separates the schemas, so the rule reports nothing for the pair. + +A `discriminator` names the property that tells the members apart, so the rule trusts it and checks only what the specification requires. +The property must be listed in `required` in every member schema, because a value can otherwise omit it and nothing decides which schema applies. +Every member must also be a `$ref`: a `discriminator` selects a schema by its component name, and the specification states that inline `oneOf` and `anyOf` subschemas are not considered, so an inline member can never be selected. +A member that declares `$id` is exempt, because a `mapping` entry can name it by URI. + +Wrapping one schema in `allOf` to attach sibling keywords, such as `description` or `readOnly` next to a `$ref`, stays common because support for `$ref` siblings is uneven across tools. +Referencing a schema that declares a `discriminator` carries meaning of its own too: the discriminator resolves the subtype by its schema name, so the wrapper declares a subtype even when it adds no properties. +The rule reports an `allOf` wrapper only when neither applies. + +## Configuration + +| Option | Type | Description | +| -------- | ------ | ----------------------------------------------------------------------------------------- | +| severity | string | Possible values: `off`, `warn`, `error`. Default `warn` (in `recommended` configuration). | + +An example configuration: + +```yaml +rules: + no-illogical-composition-keywords: error +``` + +## Examples + +Given this configuration: + +```yaml +rules: + no-illogical-composition-keywords: error +``` + +Example of an **incorrect** `oneOf` where both schemas accept `null`: + +```yaml +components: + schemas: + TimeShift: + type: [object, 'null'] + Invoice: + oneOf: + - $ref: '#/components/schemas/TimeShift' + - type: 'null' +``` + +Example of a **correct** `oneOf`: + +```yaml +components: + schemas: + TimeShift: + type: object + Invoice: + oneOf: + - $ref: '#/components/schemas/TimeShift' + - type: 'null' +``` + +Example of an **incorrect** `discriminator` with an inline member: + +```yaml +components: + schemas: + Cat: + type: object + properties: + petType: + type: string + required: [petType] + Pet: + discriminator: + propertyName: petType + oneOf: + - $ref: '#/components/schemas/Cat' + - type: object + properties: + petType: + type: string + required: [petType] +``` + +> Move the inline schema into `components/schemas` and reference it with a `$ref`. + +Example of an **incorrect** `discriminator`, where `petType` is optional: + +```yaml +components: + schemas: + Pet: + discriminator: + propertyName: petType + oneOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Dog' + Cat: + type: object + properties: + petType: + type: string + Dog: + type: object + properties: + petType: + type: string +``` + +> Add `petType` to `required` in both `Cat` and `Dog` to fix this. + +Example of a **correct** single-schema `allOf` that declares a subtype: + +```yaml +components: + schemas: + Pet: + type: object + required: [petType] + properties: + petType: + type: string + discriminator: + propertyName: petType + Cat: + allOf: + - $ref: '#/components/schemas/Pet' +``` + +Example of **incorrect** composition keywords: + +```yaml +components: + schemas: + Pet: + oneOf: + - $ref: '#/components/schemas/Cat' + Animal: + allOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Cat' + - {} +``` + +> `Pet` wraps a single schema, and `Animal` repeats one schema and adds an empty one that matches any value. + +## Related rules + +- [no-schema-type-mismatch](../common/no-schema-type-mismatch.md) +- [no-required-schema-properties-undefined](../common/no-required-schema-properties-undefined.md) +- [spec-discriminator-defaultMapping](./spec-discriminator-defaultMapping.md) + +## Resources + +- [Rule source](https://github.com/Redocly/redocly-cli/blob/main/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts) +- [Schema object docs](https://redocly.com/docs/openapi-visual-reference/schemas/) +- [Discriminator object docs](https://redocly.com/docs/openapi-visual-reference/discriminator/) diff --git a/docs/@v2/rules/recommended.md b/docs/@v2/rules/recommended.md index d607d24f85..9df4032702 100644 --- a/docs/@v2/rules/recommended.md +++ b/docs/@v2/rules/recommended.md @@ -51,6 +51,7 @@ Warnings: - [no-ambiguous-paths](./oas/no-ambiguous-paths.md) - [no-duplicated-enum-values](./common/no-duplicated-enum-values.md) - [no-duplicated-tag-names](./oas/no-duplicated-tag-names.md) +- [no-illogical-composition-keywords](./oas/no-illogical-composition-keywords.md) - [no-invalid-media-type-examples](./oas/no-invalid-media-type-examples.md) - [no-invalid-parameter-examples](./oas/no-invalid-parameter-examples.md) - [no-invalid-schema-examples](./oas/no-invalid-schema-examples.md) diff --git a/docs/@v2/rules/ruleset-templates.md b/docs/@v2/rules/ruleset-templates.md index c18cc53b7b..71d53bf384 100644 --- a/docs/@v2/rules/ruleset-templates.md +++ b/docs/@v2/rules/ruleset-templates.md @@ -187,6 +187,7 @@ rules: no-empty-servers: error no-enum-type-mismatch: error no-identical-paths: error + no-illogical-composition-keywords: warn no-invalid-media-type-examples: warn no-invalid-parameter-examples: warn no-invalid-schema-examples: warn @@ -237,6 +238,7 @@ rules: no-enum-type-mismatch: error no-example-value-and-externalValue: error no-identical-paths: error + no-illogical-composition-keywords: warn no-invalid-media-type-examples: warn no-invalid-parameter-examples: warn no-invalid-schema-examples: warn @@ -283,6 +285,7 @@ rules: no-enum-type-mismatch: error no-example-value-and-externalValue: error no-identical-paths: error + no-illogical-composition-keywords: warn no-invalid-media-type-examples: warn no-invalid-parameter-examples: warn no-invalid-schema-examples: warn diff --git a/docs/@v2/v2.sidebars.yaml b/docs/@v2/v2.sidebars.yaml index a7720f844e..f0744958f3 100644 --- a/docs/@v2/v2.sidebars.yaml +++ b/docs/@v2/v2.sidebars.yaml @@ -122,6 +122,7 @@ - page: rules/oas/no-example-value-and-externalValue.md - page: rules/oas/no-http-verbs-in-paths.md - page: rules/oas/no-identical-paths.md + - page: rules/oas/no-illogical-composition-keywords.md - page: rules/oas/no-invalid-media-type-examples.md - page: rules/oas/no-invalid-parameter-examples.md - page: rules/oas/no-invalid-schema-examples.md diff --git a/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap b/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap index bc90eff282..611644abdf 100644 --- a/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap +++ b/packages/core/src/__tests__/__snapshots__/redocly-yaml.test.ts.snap @@ -1383,6 +1383,9 @@ exports[`createConfigTypes > matches snapshot for the default config schema 1`] "EncodingMap", "HeadersMap", "Link", + "AllOf", + "AnyOf", + "OneOf", "DiscriminatorMapping", "Discriminator", "Components", diff --git a/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap b/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap index e131e1d901..16aef564c3 100644 --- a/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap +++ b/packages/core/src/config/__tests__/__snapshots__/config-resolvers.test.ts.snap @@ -172,6 +172,7 @@ exports[`resolveConfig > should ignore minimal from the root and read local file "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "error", @@ -239,6 +240,7 @@ exports[`resolveConfig > should ignore minimal from the root and read local file "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "error", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", @@ -302,6 +304,7 @@ exports[`resolveConfig > should ignore minimal from the root and read local file "no-enum-type-mismatch": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "error", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", @@ -620,6 +623,7 @@ exports[`resolveConfig > should resolve extends with local file config which con "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "warn", @@ -687,6 +691,7 @@ exports[`resolveConfig > should resolve extends with local file config which con "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", @@ -750,6 +755,7 @@ exports[`resolveConfig > should resolve extends with local file config which con "no-enum-type-mismatch": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", diff --git a/packages/core/src/config/__tests__/load.test.ts b/packages/core/src/config/__tests__/load.test.ts index f8c15df694..e4b95963b9 100644 --- a/packages/core/src/config/__tests__/load.test.ts +++ b/packages/core/src/config/__tests__/load.test.ts @@ -300,6 +300,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "warn", @@ -369,6 +370,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "off", @@ -434,6 +436,7 @@ describe('loadConfig', () => { "no-enum-type-mismatch": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "off", @@ -681,6 +684,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "warn", @@ -748,6 +752,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", @@ -811,6 +816,7 @@ describe('loadConfig', () => { "no-enum-type-mismatch": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", @@ -1071,6 +1077,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "warn", @@ -1138,6 +1145,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "error", @@ -1201,6 +1209,7 @@ describe('loadConfig', () => { "no-enum-type-mismatch": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "error", @@ -1549,6 +1558,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "warn", @@ -1618,6 +1628,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "off", @@ -1683,6 +1694,7 @@ describe('loadConfig', () => { "no-enum-type-mismatch": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "off", @@ -1930,6 +1942,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "warn", @@ -1997,6 +2010,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", @@ -2060,6 +2074,7 @@ describe('loadConfig', () => { "no-enum-type-mismatch": "error", "no-http-verbs-in-paths": "off", "no-identical-paths": "error", + "no-illogical-composition-keywords": "warn", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "warn", "no-invalid-schema-examples": "warn", @@ -2320,6 +2335,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": { "allowAdditionalProperties": false, "severity": "warn", @@ -2387,6 +2403,7 @@ describe('loadConfig', () => { "no-example-value-and-externalValue": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "error", @@ -2450,6 +2467,7 @@ describe('loadConfig', () => { "no-enum-type-mismatch": "warn", "no-http-verbs-in-paths": "off", "no-identical-paths": "warn", + "no-illogical-composition-keywords": "off", "no-invalid-media-type-examples": "warn", "no-invalid-parameter-examples": "off", "no-invalid-schema-examples": "error", diff --git a/packages/core/src/config/all.ts b/packages/core/src/config/all.ts index 6655b7ef1f..23b3ec8f2b 100644 --- a/packages/core/src/config/all.ts +++ b/packages/core/src/config/all.ts @@ -76,6 +76,7 @@ const all: RawGovernanceConfig<'built-in'> = { 'no-enum-type-mismatch': 'error', 'no-required-schema-properties-undefined': 'error', 'no-schema-type-mismatch': 'error', + 'no-illogical-composition-keywords': 'error', 'no-invalid-media-type-examples': 'error', 'no-server-example.com': 'error', 'no-server-trailing-slash': 'error', @@ -144,6 +145,7 @@ const all: RawGovernanceConfig<'built-in'> = { 'no-enum-type-mismatch': 'error', 'no-required-schema-properties-undefined': 'error', 'no-schema-type-mismatch': 'error', + 'no-illogical-composition-keywords': 'error', 'no-invalid-media-type-examples': 'error', 'no-mixed-number-range-constraints': 'error', 'no-server-example.com': 'error', @@ -212,6 +214,7 @@ const all: RawGovernanceConfig<'built-in'> = { 'no-enum-type-mismatch': 'error', 'no-required-schema-properties-undefined': 'error', 'no-schema-type-mismatch': 'error', + 'no-illogical-composition-keywords': 'error', 'no-invalid-media-type-examples': 'error', 'no-mixed-number-range-constraints': 'error', 'no-server-example.com': 'error', diff --git a/packages/core/src/config/minimal.ts b/packages/core/src/config/minimal.ts index bb18b843af..69861c7426 100644 --- a/packages/core/src/config/minimal.ts +++ b/packages/core/src/config/minimal.ts @@ -68,6 +68,7 @@ const minimal: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'warn', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'warn', + 'no-illogical-composition-keywords': 'off', 'no-invalid-media-type-examples': { severity: 'warn', allowAdditionalProperties: false, @@ -133,6 +134,7 @@ const minimal: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'warn', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'warn', + 'no-illogical-composition-keywords': 'off', 'no-invalid-media-type-examples': 'warn', 'no-invalid-parameter-examples': 'off', 'no-invalid-schema-examples': 'off', @@ -194,6 +196,7 @@ const minimal: RawGovernanceConfig<'built-in'> = { 'no-enum-type-mismatch': 'warn', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'warn', + 'no-illogical-composition-keywords': 'off', 'no-invalid-media-type-examples': 'warn', 'no-invalid-parameter-examples': 'off', 'no-invalid-schema-examples': 'off', diff --git a/packages/core/src/config/recommended-strict.ts b/packages/core/src/config/recommended-strict.ts index f04c6196d1..5ea44ab8d0 100644 --- a/packages/core/src/config/recommended-strict.ts +++ b/packages/core/src/config/recommended-strict.ts @@ -68,6 +68,7 @@ const recommendedStrict: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'error', 'no-invalid-media-type-examples': { severity: 'error', allowAdditionalProperties: false, @@ -133,6 +134,7 @@ const recommendedStrict: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'error', 'no-invalid-media-type-examples': 'error', 'no-invalid-parameter-examples': 'error', 'no-invalid-schema-examples': 'error', @@ -194,6 +196,7 @@ const recommendedStrict: RawGovernanceConfig<'built-in'> = { 'no-enum-type-mismatch': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'error', 'no-invalid-media-type-examples': 'error', 'no-invalid-parameter-examples': 'error', 'no-invalid-schema-examples': 'error', diff --git a/packages/core/src/config/recommended.ts b/packages/core/src/config/recommended.ts index 44fbe65768..d0c8823ce5 100644 --- a/packages/core/src/config/recommended.ts +++ b/packages/core/src/config/recommended.ts @@ -68,6 +68,7 @@ const recommended: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'warn', 'no-invalid-media-type-examples': { severity: 'warn', allowAdditionalProperties: false, @@ -133,6 +134,7 @@ const recommended: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'warn', 'no-invalid-media-type-examples': 'warn', 'no-invalid-parameter-examples': 'warn', 'no-invalid-schema-examples': 'warn', @@ -194,6 +196,7 @@ const recommended: RawGovernanceConfig<'built-in'> = { 'no-enum-type-mismatch': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'warn', 'no-invalid-media-type-examples': 'warn', 'no-invalid-parameter-examples': 'warn', 'no-invalid-schema-examples': 'warn', diff --git a/packages/core/src/config/spec.ts b/packages/core/src/config/spec.ts index b6e8056668..4ee2d06a92 100644 --- a/packages/core/src/config/spec.ts +++ b/packages/core/src/config/spec.ts @@ -68,6 +68,7 @@ const spec: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'off', 'no-invalid-media-type-examples': { severity: 'off', allowAdditionalProperties: false, @@ -133,6 +134,7 @@ const spec: RawGovernanceConfig<'built-in'> = { 'no-example-value-and-externalValue': 'error', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'off', 'no-invalid-media-type-examples': 'off', 'no-invalid-parameter-examples': 'off', 'no-invalid-schema-examples': 'off', @@ -194,6 +196,7 @@ const spec: RawGovernanceConfig<'built-in'> = { 'no-enum-type-mismatch': 'off', 'no-http-verbs-in-paths': 'off', 'no-identical-paths': 'error', + 'no-illogical-composition-keywords': 'off', 'no-invalid-media-type-examples': 'off', 'no-invalid-parameter-examples': 'off', 'no-invalid-schema-examples': 'off', diff --git a/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts b/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts new file mode 100644 index 0000000000..e03d6cbf7c --- /dev/null +++ b/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts @@ -0,0 +1,1397 @@ +import { outdent } from 'outdent'; + +import { parseYamlToDocument, replaceSourceWithRef } from '../../../../__tests__/utils.js'; +import { createConfig } from '../../../config/index.js'; +import { lintDocument } from '../../../lint.js'; +import { BaseResolver } from '../../../resolve.js'; + +describe('Oas3 no-illogical-composition-keywords', () => { + async function lint(yaml: string) { + const document = parseYamlToDocument(outdent`${yaml}`, 'foobar.yaml'); + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + return replaceSourceWithRef(results); + } + + describe('oneOf', () => { + it('should report when oneOf has only one schema', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: string + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "\`oneOf\` should have at least two schemas. Use the schema directly instead.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when oneOf has only one schema but the parent declares a discriminator', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + discriminator: + propertyName: petType + oneOf: + - $ref: '#/components/schemas/Cat' + Cat: + type: object + properties: + petType: + type: string + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should report duplicated schemas that are not next to each other', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - $ref: '#/components/schemas/Cat' + - type: integer + - $ref: '#/components/schemas/Cat' + Cat: + type: object + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/2", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` duplicates the schema at position 1.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report distinct refs that resolve to equal schemas as overlapping', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Kitten' + Cat: + type: object + properties: + id: + type: string + Kitten: + type: object + properties: + id: + type: string + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: \`#/components/schemas/Cat\` and \`#/components/schemas/Kitten\`. Both schemas define \`id\` without constraints that exclude each other. Add a discriminator, or constrain the shared properties to different values.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should stay quiet when the discriminator `propertyName` is not a string', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + discriminator: + propertyName: + name: + type: string + oneOf: + - type: object + properties: + name: + type: string + required: [name] + - type: object + properties: + name: + type: string + description: + type: string + required: [name] + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/0", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report inline members that a discriminator cannot select', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Cat: + type: object + properties: + petType: + type: string + required: [petType] + Test: + discriminator: + propertyName: petType + oneOf: + - $ref: '#/components/schemas/Cat' + - type: object + properties: + petType: + type: string + required: [petType] + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report an empty schema inside oneOf', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: string + - {} + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` is empty, so it matches any value.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report when a referenced schema and a null schema both accept null', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - $ref: '#/components/schemas/InvoiceTimeShift' + - type: 'null' + InvoiceTimeShift: + type: + - object + - 'null' + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: \`#/components/schemas/InvoiceTimeShift\` and schema at position 2. Both schemas accept \`null\`.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report when type sets overlap', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: string + - type: + - string + - integer + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and schema at position 2. Both schemas accept \`string\`.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when a shared property uses not to exclude the other value', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - $ref: '#/components/schemas/Lizard' + - $ref: '#/components/schemas/OtherPet' + Lizard: + type: object + required: [petType] + properties: + petType: + type: string + enum: ['Lizard'] + OtherPet: + type: object + required: [petType] + properties: + petType: + not: + enum: ['Lizard'] + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should not crash on boolean schemas used as members or property schemas', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Members: + oneOf: + - true + - type: string + Properties: + oneOf: + - type: object + required: [kind] + properties: + kind: true + - type: object + required: [kind] + properties: + kind: + type: string + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should not report when members carry constraints these checks do not model', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Patterns: + oneOf: + - type: string + pattern: '^a' + - type: string + pattern: '^b' + Ranges: + oneOf: + - type: integer + maximum: 10 + - type: integer + minimum: 11 + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should report when both members declare an empty required list', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + required: [] + properties: + name: + type: string + - type: object + title: Second + required: [] + properties: + name: + type: string + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and \`Second\`. Both schemas define \`name\` without constraints that exclude each other. Add a discriminator, or constrain the shared properties to different values.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when one member forbids a property the other requires', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + additionalProperties: false + required: [a] + properties: + a: + type: string + - type: object + required: [a, b] + properties: + a: + type: string + b: + type: string + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should report when an exclusive shared property is not required, without a discriminator', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + properties: + kind: + type: string + enum: [cat] + - type: object + properties: + kind: + type: string + enum: [dog] + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and schema at position 2. Add \`kind\` to \`required\` in every schema; an optional property cannot distinguish the schemas.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report each overlapping pair of a oneOf separately', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: string + - type: string + title: Second + - type: string + title: Third + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and \`Second\`. Both schemas accept \`string\`.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and \`Third\`. Both schemas accept \`string\`.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: \`Second\` and \`Third\`. Both schemas accept \`string\`.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report when both members require the same undeclared property', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + required: [documentType, fileId] + properties: + fileId: + type: string + - type: object + title: Second + required: [documentType, fileId] + properties: + fileId: + type: string + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and \`Second\`. Both schemas define \`fileId\` without constraints that exclude each other. Add a discriminator, or constrain the shared properties to different values.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when members are separated by a nested allOf', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + properties: + id: + type: string + allOf: + - type: object + required: [kind] + properties: + kind: + enum: [dog] + - type: object + properties: + id: + type: string + allOf: + - type: object + required: [kind] + properties: + kind: + enum: [cat] + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should report the discriminator property as required when members do not declare it', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Pet: + discriminator: + propertyName: petType + oneOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Dog' + Cat: + type: object + properties: + name: + type: string + Dog: + type: object + properties: + name: + type: string + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Pet/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: \`#/components/schemas/Cat\` and \`#/components/schemas/Dog\`. Add \`petType\` to \`required\` in every schema; the \`discriminator\` cannot read a property a value may omit.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when the discriminator property is required in every member', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Pet: + discriminator: + propertyName: petType + oneOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Dog' + Cat: + type: object + required: [petType, name] + properties: + name: + type: string + Dog: + type: object + required: [petType, name] + properties: + name: + type: string + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should not report members that differ only by format', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + required: [id] + properties: + id: + type: string + format: uuid + - type: object + required: [id] + properties: + id: + type: string + format: uri + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and schema at position 2. Both schemas define \`id\` without constraints that exclude each other. Add a discriminator, or constrain the shared properties to different values.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when the parent is nullable and a single member accepts null', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + nullable: true + oneOf: + - type: string + nullable: true + - type: integer + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should not report when a required property with exclusive values tells the members apart', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + additionalProperties: + type: string + properties: + kind: + type: string + enum: [cat] + required: [kind] + - type: object + additionalProperties: + type: string + properties: + kind: + type: string + enum: [dog] + required: [kind] + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should report when members share a property with no constraint that tells them apart', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: object + additionalProperties: false + properties: + name: + type: string + required: [name] + - type: object + properties: + name: + type: string + required: [name] + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and schema at position 2. Both schemas define \`name\` without constraints that exclude each other. Add a discriminator, or constrain the shared properties to different values.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report when the discriminator property is not required in every member', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + discriminator: + propertyName: petType + oneOf: + - type: object + properties: + petType: + type: string + enum: [cat] + - type: object + properties: + petType: + type: string + enum: [dog] + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/0", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and schema at position 2. Add \`petType\` to \`required\` in every schema; the \`discriminator\` cannot read a property a value may omit.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report when members have different types', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: string + - type: integer + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should report both a structural problem and an overlap in the same oneOf', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: string + - type: string + title: Text + - {} + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf/2", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`oneOf\` is empty, so it matches any value.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and \`Text\`. Both schemas accept \`string\`.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + }); + + describe('anyOf', () => { + it('should report when anyOf has only one schema', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + anyOf: + - type: string + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/anyOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "\`anyOf\` should have at least two schemas. Use the schema directly instead.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report duplicated and empty schemas inside anyOf', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + anyOf: + - type: string + - type: string + - {} + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/anyOf/2", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`anyOf\` is empty, so it matches any value.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/anyOf/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`anyOf\` duplicates the schema at position 1.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report anyOf used with a discriminator by default', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + discriminator: + propertyName: petType + anyOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Dog' + Cat: + type: object + Dog: + type: string + `) + ).toMatchInlineSnapshot(`[]`); + }); + }); + + describe('allOf', () => { + it('should report an allOf wrapper that adds nothing', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + allOf: + - $ref: '#/components/schemas/Cat' + Cat: + type: object + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/allOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "\`allOf\` should have at least two schemas. Use the schema directly instead.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should not report a single-schema allOf used to attach sibling keywords', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + type: object + properties: + customerId: + readOnly: true + allOf: + - $ref: '#/components/schemas/CustomerId' + CustomerId: + type: string + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should not report a single-schema allOf declaring a subtype of a discriminated schema', async () => { + expect( + await lint(` + openapi: 3.0.3 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Pet: + type: object + required: [petType] + properties: + petType: + type: string + discriminator: + propertyName: petType + Cat: + allOf: + - $ref: '#/components/schemas/Pet' + `) + ).toMatchInlineSnapshot(`[]`); + }); + + it('should report an empty allOf', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + allOf: [] + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/allOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "\`allOf\` should have at least two schemas.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should report duplicated and empty schemas inside allOf', async () => { + expect( + await lint(` + openapi: 3.0.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + allOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Cat' + - {} + Cat: + type: object + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/allOf/2", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`allOf\` is empty, so it matches any value.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/allOf/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`allOf\` duplicates the schema at position 1.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + }); + + it('should check every composition keyword used on the same schema', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Test: + oneOf: + - type: string + allOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Cat' + Cat: + type: object + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Test/allOf/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Schema in \`allOf\` duplicates the schema at position 1.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/components/schemas/Test/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "\`oneOf\` should have at least two schemas. Use the schema directly instead.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); +}); diff --git a/packages/core/src/rules/oas3/index.ts b/packages/core/src/rules/oas3/index.ts index 05def43644..ce7066f7e2 100644 --- a/packages/core/src/rules/oas3/index.ts +++ b/packages/core/src/rules/oas3/index.ts @@ -49,6 +49,7 @@ import { BooleanParameterPrefixes } from './boolean-parameter-prefixes.js'; import { ComponentNameUnique } from './component-name-unique.js'; import { NoEmptyServers } from './no-empty-servers.js'; import { NoExampleValueAndExternalValue } from './no-example-value-and-externalValue.js'; +import { NoIllogicalCompositionKeywords } from './no-illogical-composition-keywords.js'; import { ValidContentExamples } from './no-invalid-media-type-examples.js'; import { NoServerExample } from './no-server-example.com.js'; import { NoServerTrailingSlash } from './no-server-trailing-slash.js'; @@ -106,6 +107,7 @@ export const rules: Oas3RuleSet<'built-in'> = { 'paths-kebab-case': PathsKebabCase as Oas3Rule, 'boolean-parameter-prefixes': BooleanParameterPrefixes, 'path-http-verbs-order': PathHttpVerbsOrder as Oas3Rule, + 'no-illogical-composition-keywords': NoIllogicalCompositionKeywords, 'no-invalid-media-type-examples': ValidContentExamples, 'no-identical-paths': NoIdenticalPaths as Oas3Rule, 'no-ambiguous-paths': NoAmbiguousPaths as Oas3Rule, diff --git a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts new file mode 100644 index 0000000000..35f2ca0baf --- /dev/null +++ b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts @@ -0,0 +1,508 @@ +import { isRef } from '../../ref-utils.js'; +import type { + Oas3Discriminator, + Oas3Schema, + Oas3_1Schema, + Referenced, +} from '../../typings/openapi.js'; +import { dequal } from '../../utils/dequal.js'; +import { getOwn } from '../../utils/get-own.js'; +import { isDefined } from '../../utils/is-defined.js'; +import { isPlainObject } from '../../utils/is-plain-object.js'; +import type { Oas3Rule } from '../../visitors.js'; +import type { UserContext, ResolveFn } from '../../walk.js'; + +type CompositionSchema = Oas3Schema | Oas3_1Schema; +type CompositionKeyword = 'oneOf' | 'anyOf' | 'allOf'; + +type SourcedSchema = { + schema: CompositionSchema; + source: string | undefined; +}; + +type SchemaProperties = NonNullable; + +type PropertyComparison = { + left: SourcedSchema; + right: SourcedSchema; + leftProperties: SchemaProperties; + rightProperties: SchemaProperties; + leftRequired: Set; + rightRequired: Set; + sharedNames: string[]; +}; + +// Keywords these checks model, plus annotations that don't constrain which values match. +// A schema using anything else is left alone, because the unmodelled keyword may separate it. +const UNDERSTOOD_KEYWORDS: ReadonlySet = new Set([ + 'type', + 'format', + 'enum', + 'const', + 'nullable', + 'properties', + 'required', + 'additionalProperties', + 'discriminator', + '$ref', + '$id', + '$schema', + '$anchor', + '$comment', + 'title', + 'description', + 'default', + 'example', + 'examples', + 'deprecated', + 'readOnly', + 'writeOnly', + 'externalDocs', + 'xml', +]); + +export const NoIllogicalCompositionKeywords: Oas3Rule = () => { + return { + AllOf(members, ctx) { + const parentSchema = getParentSchema(ctx); + if (!Array.isArray(members) || !parentSchema) return; + + // A lone member is redundant only when the wrapper carries nothing else of its own + // and is not declaring a subtype of a discriminated schema. + if ( + members.length === 0 || + (members.length === 1 && + Object.keys(parentSchema).length === 1 && + !isSubtypeOfDiscriminatedSchema(members[0], ctx.resolve)) + ) { + reportSingleSchema(members, 'allOf', ctx); + } + + reportEmptyMembers(members, 'allOf', ctx); + reportDuplicateMembers(members, 'allOf', ctx); + }, + + AnyOf(members, ctx) { + const parentSchema = getParentSchema(ctx); + if (!Array.isArray(members) || !parentSchema) return; + + const hasDiscriminator = isPlainObject(parentSchema.discriminator); + + if (members.length < 2 && !hasDiscriminator) { + reportSingleSchema(members, 'anyOf', ctx); + } + + reportEmptyMembers(members, 'anyOf', ctx); + reportDuplicateMembers(members, 'anyOf', ctx); + + if (hasDiscriminator) { + reportInlineMembers(members, 'anyOf', ctx); + } + }, + + OneOf(members, ctx) { + const parentSchema = getParentSchema(ctx); + if (!Array.isArray(members) || !parentSchema) return; + + const { discriminator } = parentSchema; + const hasDiscriminator = isPlainObject(discriminator); + + if (members.length < 2 && !hasDiscriminator) { + reportSingleSchema(members, 'oneOf', ctx); + } + + reportEmptyMembers(members, 'oneOf', ctx); + reportDuplicateMembers(members, 'oneOf', ctx); + + if (hasDiscriminator) { + reportInlineMembers(members, 'oneOf', ctx); + } + + reportAmbiguousMembers(members, hasDiscriminator ? discriminator : undefined, ctx); + }, + }; +}; + +// `struct` is not guaranteed to have rejected a malformed composition first. +function getParentSchema(ctx: UserContext): CompositionSchema | undefined { + return isPlainObject(ctx.parent) ? ctx.parent : undefined; +} + +function reportSingleSchema( + members: CompositionSchema[], + keyword: CompositionKeyword, + ctx: UserContext +) { + const suggestion = members.length === 1 ? ' Use the schema directly instead.' : ''; + ctx.report({ + message: `\`${keyword}\` should have at least two schemas.${suggestion}`, + location: ctx.location.key(), + }); +} + +// A schema whose `allOf` references a schema with a `discriminator` declares a subtype: the +// discriminator resolves the subtype by name, so the wrapper carries meaning of its own. +function isSubtypeOfDiscriminatedSchema( + member: Referenced, + resolve: ResolveFn +): boolean { + return isPlainObject(resolveSchema(member, resolve)?.schema.discriminator); +} + +function reportEmptyMembers( + members: CompositionSchema[], + keyword: CompositionKeyword, + ctx: UserContext +) { + for (const [index, member] of members.entries()) { + if (isPlainObject(member) && Object.keys(member).length === 0) { + ctx.report({ + message: `Schema in \`${keyword}\` is empty, so it matches any value.`, + location: ctx.location.child([index]), + }); + } + } +} + +function reportInlineMembers( + members: CompositionSchema[], + keyword: CompositionKeyword, + ctx: UserContext +) { + for (const [index, member] of members.entries()) { + if (isRef(member) || (isPlainObject(member) && '$id' in member)) continue; + + ctx.report({ + message: `Schema in \`${keyword}\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.`, + location: ctx.location.child([index]), + }); + } +} + +function reportDuplicateMembers( + members: CompositionSchema[], + keyword: CompositionKeyword, + ctx: UserContext +) { + for (let index = 1; index < members.length; index++) { + const firstIndex = members.findIndex((other) => dequal(other, members[index])); + if (firstIndex < index) { + ctx.report({ + message: `Schema in \`${keyword}\` duplicates the schema at position ${firstIndex + 1}.`, + location: ctx.location.child([index]), + }); + } + } +} + +function reportAmbiguousMembers( + members: CompositionSchema[], + discriminator: Oas3Discriminator | undefined, + ctx: UserContext +) { + const { resolve, report, location } = ctx; + const resolvedMembers = members.map((member) => resolveSchema(member, resolve)); + + for (let leftIndex = 0; leftIndex < resolvedMembers.length - 1; leftIndex++) { + const left = resolvedMembers[leftIndex]; + if (!left) continue; + + for (let rightIndex = leftIndex + 1; rightIndex < resolvedMembers.length; rightIndex++) { + const right = resolvedMembers[rightIndex]; + if (!right) continue; + + // Skip only pairs `reportDuplicateMembers` already reported, which compares raw nodes too. + if (dequal(members[leftIndex], members[rightIndex])) continue; + + const reason = findOverlapReason(left, right, resolve, discriminator); + if (!reason) continue; + + report({ + message: `Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: ${describeMember( + members[leftIndex], + leftIndex + )} and ${describeMember(members[rightIndex], rightIndex)}. ${reason}`, + location: location.key(), + }); + } + } +} + +function describeMember(member: CompositionSchema, index: number): string { + if (isRef(member)) return `\`${member.$ref}\``; + if (member.title) return `\`${member.title}\``; + return `schema at position ${index + 1}`; +} + +function findOverlapReason( + left: SourcedSchema, + right: SourcedSchema, + resolve: ResolveFn, + discriminator?: Oas3Discriminator +): string | null { + const leftSchema = left.schema; + const rightSchema = right.schema; + + if (hasUnsupportedConstraint(leftSchema) || hasUnsupportedConstraint(rightSchema)) return null; + + if (areExclusive(leftSchema, rightSchema)) return null; + + if (schemaAllowsNull(leftSchema) && schemaAllowsNull(rightSchema)) { + return 'Both schemas accept `null`.'; + } + + const leftEnum = leftSchema.enum; + const rightEnum = rightSchema.enum; + if (leftEnum && rightEnum) { + const shared = leftEnum.filter((value) => rightEnum.some((other) => dequal(value, other))); + return `Both schemas allow the values ${JSON.stringify(shared)}.`; + } + + const sharedConst = readConst(leftSchema); + if (isDefined(sharedConst) && isDefined(readConst(rightSchema))) { + return `Both schemas require the same \`const\` value ${JSON.stringify(sharedConst)}.`; + } + + if (discriminator) { + const { propertyName } = discriminator; + // `struct` is not guaranteed to have rejected a non-string `propertyName` first. + return typeof propertyName === 'string' + ? describeDiscriminatorGap(leftSchema, rightSchema, propertyName) + : null; + } + + const propertyOverlap = findPropertyOverlap(left, right, resolve); + if (propertyOverlap) return propertyOverlap; + + if (!leftSchema.properties && !rightSchema.properties) { + const sharedTypes = getSharedTypes(leftSchema, rightSchema); + if (sharedTypes) { + return `Both schemas accept ${sharedTypes.map((type) => `\`${type}\``).join(', ')}.`; + } + } + + return null; +} + +function describeDiscriminatorGap( + left: CompositionSchema, + right: CompositionSchema, + propertyName: string +): string | null { + const requiredInBoth = + !!left.required?.includes(propertyName) && !!right.required?.includes(propertyName); + if (requiredInBoth) return null; + + return `Add \`${propertyName}\` to \`required\` in every schema; the \`discriminator\` cannot read a property a value may omit.`; +} + +function findPropertyOverlap( + left: SourcedSchema, + right: SourcedSchema, + resolve: ResolveFn +): string | null { + const comparison = collectPropertyComparison(left, right); + if (!comparison) return null; + + if (forbidsPropertyRequiredBy(left.schema, comparison.leftProperties, comparison.rightRequired)) { + return null; + } + if ( + forbidsPropertyRequiredBy(right.schema, comparison.rightProperties, comparison.leftRequired) + ) { + return null; + } + + if (requiredSetsDistinguish(comparison)) return null; + if (comparison.sharedNames.length === 0) return null; + + const shared = classifySharedProperties(comparison, resolve); + if (!shared) return null; + + const { optionalDistinguishingNames, ambiguousNames } = shared; + + if (optionalDistinguishingNames.length > 0) { + const alsoShared = + ambiguousNames.length > 0 ? ` Other shared properties: ${quoteAll(ambiguousNames)}.` : ''; + return `Add ${quoteAll( + optionalDistinguishingNames + )} to \`required\` in every schema; an optional property cannot distinguish the schemas.${alsoShared}`; + } + + if (ambiguousNames.length > 0) { + return `Both schemas define ${quoteAll( + ambiguousNames + )} without constraints that exclude each other. Add a discriminator, or constrain the shared properties to different values.`; + } + + return null; +} + +function collectPropertyComparison( + left: SourcedSchema, + right: SourcedSchema +): PropertyComparison | null { + const leftProperties = left.schema.properties; + const rightProperties = right.schema.properties; + if (!leftProperties || !rightProperties) return null; + + return { + left, + right, + leftProperties, + rightProperties, + leftRequired: new Set(left.schema.required ?? []), + rightRequired: new Set(right.schema.required ?? []), + sharedNames: Object.keys(leftProperties).filter((name) => getOwn(rightProperties, name)), + }; +} + +function requiredSetsDistinguish({ + leftProperties, + rightProperties, + leftRequired, + rightRequired, + sharedNames, +}: PropertyComparison): boolean { + if (leftRequired.size === 0 || rightRequired.size === 0) return false; + + const sharedNameIsRequired = sharedNames.some( + (name) => leftRequired.has(name) || rightRequired.has(name) + ); + if (!hasIntersection(leftRequired, rightRequired) && !sharedNameIsRequired) return true; + + // A property required by both sides distinguishes nothing, no matter where it is declared. + const leftRequiresUndeclared = [...leftRequired].some( + (name) => !getOwn(rightProperties, name) && !rightRequired.has(name) + ); + const rightRequiresUndeclared = [...rightRequired].some( + (name) => !getOwn(leftProperties, name) && !leftRequired.has(name) + ); + return leftRequiresUndeclared && rightRequiresUndeclared; +} + +function classifySharedProperties( + { + left, + right, + leftProperties, + rightProperties, + leftRequired, + rightRequired, + sharedNames, + }: PropertyComparison, + resolve: ResolveFn +): { optionalDistinguishingNames: string[]; ambiguousNames: string[] } | null { + const optionalDistinguishingNames: string[] = []; + const ambiguousNames: string[] = []; + + for (const name of sharedNames) { + const requiredInBoth = leftRequired.has(name) && rightRequired.has(name); + + const leftProperty = resolveSchema(getOwn(leftProperties, name), resolve, left.source); + const rightProperty = resolveSchema(getOwn(rightProperties, name), resolve, right.source); + if (!leftProperty || !rightProperty) continue; + if ( + hasUnsupportedConstraint(leftProperty.schema) || + hasUnsupportedConstraint(rightProperty.schema) + ) { + continue; + } + + if (areExclusive(leftProperty.schema, rightProperty.schema)) { + if (requiredInBoth) return null; + optionalDistinguishingNames.push(name); + continue; + } + + ambiguousNames.push(name); + } + + return { optionalDistinguishingNames, ambiguousNames }; +} + +function forbidsPropertyRequiredBy( + schema: CompositionSchema, + declaredProperties: SchemaProperties, + required: Set +): boolean { + if (schema.additionalProperties !== false) return false; + return [...required].some((name) => !getOwn(declaredProperties, name)); +} + +function areExclusive(left: CompositionSchema, right: CompositionSchema): boolean { + const leftEnum = left.enum; + const rightEnum = right.enum; + if (leftEnum && rightEnum) { + return !leftEnum.some((value) => rightEnum.some((other) => dequal(value, other))); + } + + const leftConst = readConst(left); + const rightConst = readConst(right); + if (isDefined(leftConst) && isDefined(rightConst)) return !dequal(leftConst, rightConst); + + const leftTypes = getTypeSet(left); + const rightTypes = getTypeSet(right); + // `format` is an annotation, not an assertion, so differing formats prove nothing. + return !!leftTypes && !!rightTypes && !hasIntersection(leftTypes, rightTypes); +} + +function resolveSchema( + member: Referenced, + resolve: ResolveFn, + from?: string +): SourcedSchema | undefined { + if (isRef(member)) { + const { node, location } = resolve(member, from); + return isPlainObject(node) ? { schema: node, source: location?.source.absoluteRef } : undefined; + } + // JSON Schema allows `true` and `false` as schemas; these checks only read object schemas. + return isPlainObject(member) ? { schema: member, source: from } : undefined; +} + +function hasUnsupportedConstraint(schema: CompositionSchema): boolean { + return Object.keys(schema).some( + (keyword) => !UNDERSTOOD_KEYWORDS.has(keyword) && !keyword.startsWith('x-') + ); +} + +function getTypeSet(schema: CompositionSchema): Set | undefined { + const declaredType = schema.type; + if (declaredType === undefined) return undefined; + + const types = new Set(Array.isArray(declaredType) ? declaredType : [declaredType]); + if ('nullable' in schema && schema.nullable === true) types.add('null'); + return types; +} + +function getSharedTypes(left: CompositionSchema, right: CompositionSchema): string[] | undefined { + if (hasUnsupportedConstraint(left) || hasUnsupportedConstraint(right)) return undefined; + + const leftTypes = getTypeSet(left); + const rightTypes = getTypeSet(right); + if (!leftTypes || !rightTypes) return undefined; + + const shared = [...leftTypes].filter((type) => rightTypes.has(type)); + return shared.length > 0 ? shared : undefined; +} + +function schemaAllowsNull(schema: CompositionSchema): boolean { + if ('nullable' in schema && schema.nullable === true) return true; + const declaredType = schema.type; + return Array.isArray(declaredType) ? declaredType.includes('null') : declaredType === 'null'; +} + +function readConst(schema: CompositionSchema): unknown { + return 'const' in schema ? schema.const : undefined; +} + +function hasIntersection(left: Set, right: Set): boolean { + for (const value of left) { + if (right.has(value)) return true; + } + return false; +} + +function quoteAll(names: string[]): string { + return names.map((name) => `\`${name}\``).join(', '); +} diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts index 7014ac1518..073e1208dc 100755 --- a/packages/core/src/types/index.ts +++ b/packages/core/src/types/index.ts @@ -56,7 +56,7 @@ type NormalizedResolveTypeFn = (value: any, key: string) => NormalizedPropType; export function listOf( typeName: string, - opts: { description?: string; documentationLink?: string } = {} + opts: { name?: string; description?: string; documentationLink?: string } = {} ) { return { name: `${typeName}List`, diff --git a/packages/core/src/types/oas3.ts b/packages/core/src/types/oas3.ts index c23569005b..4adbcc03dc 100755 --- a/packages/core/src/types/oas3.ts +++ b/packages/core/src/types/oas3.ts @@ -565,6 +565,12 @@ const Link: NodeType = { 'The Link object represents a possible design-time link for a response. The presence of a link does not guarantee the caller’s ability to successfully invoke it, rather it provides a known relationship and traversal mechanism between responses and other operations.', }; +const compositionDescription = + 'Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema.'; +const AllOf = listOf('Schema', { name: 'AllOf', description: compositionDescription }); +const AnyOf = listOf('Schema', { name: 'AnyOf', description: compositionDescription }); +const OneOf = listOf('Schema', { name: 'OneOf', description: compositionDescription }); + // draft-00 const Schema: NodeType = { properties: { @@ -597,18 +603,9 @@ const Schema: NodeType = { enum: ['object', 'array', 'string', 'number', 'integer', 'boolean'], description: 'Value MUST be a string. Multiple types via an array are not supported.', }, - allOf: listOf('Schema', { - description: - 'Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema.', - }), - anyOf: listOf('Schema', { - description: - 'Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema.', - }), - oneOf: listOf('Schema', { - description: - 'Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema.', - }), + allOf: AllOf, + anyOf: AnyOf, + oneOf: OneOf, not: 'Schema', properties: 'SchemaProperties', items: (value: unknown) => { @@ -911,6 +908,10 @@ export const Oas3Types = { Link, Logo, Schema, + // Registered so rules and configurable rules can target these node names. + AllOf, + AnyOf, + OneOf, Xml, SchemaProperties, DiscriminatorMapping, diff --git a/packages/core/src/types/oas3_1.ts b/packages/core/src/types/oas3_1.ts index 2419c84613..79f0a924f5 100755 --- a/packages/core/src/types/oas3_1.ts +++ b/packages/core/src/types/oas3_1.ts @@ -140,9 +140,9 @@ const Schema: NodeType = { }; } }, - allOf: listOf('Schema'), - anyOf: listOf('Schema'), - oneOf: listOf('Schema'), + allOf: listOf('Schema', { name: 'AllOf' }), + anyOf: listOf('Schema', { name: 'AnyOf' }), + oneOf: listOf('Schema', { name: 'OneOf' }), not: 'Schema', if: 'Schema', then: 'Schema', diff --git a/packages/core/src/types/redocly-yaml.ts b/packages/core/src/types/redocly-yaml.ts index 5b6561dacc..01010f44f2 100644 --- a/packages/core/src/types/redocly-yaml.ts +++ b/packages/core/src/types/redocly-yaml.ts @@ -107,6 +107,7 @@ const builtInOAS3Rules = [ 'component-name-unique', 'no-empty-servers', 'no-example-value-and-externalValue', + 'no-illogical-composition-keywords', 'no-invalid-media-type-examples', 'no-server-example.com', 'no-server-trailing-slash', diff --git a/packages/core/src/visitors.ts b/packages/core/src/visitors.ts index 0d54e67bf4..2624817790 100644 --- a/packages/core/src/visitors.ts +++ b/packages/core/src/visitors.ts @@ -215,6 +215,9 @@ type Oas3FlatVisitor = { Response?: VisitFunctionOrObject>; Link?: VisitFunctionOrObject; Schema?: VisitFunctionOrObject; + AllOf?: VisitFunctionOrObject>; + AnyOf?: VisitFunctionOrObject>; + OneOf?: VisitFunctionOrObject>; Xml?: VisitFunctionOrObject; SchemaProperties?: VisitFunctionOrObject>; DiscriminatorMapping?: VisitFunctionOrObject>; diff --git a/tests/e2e/lint/no-invalid-media-type-examples-recursion/snapshot.txt b/tests/e2e/lint/no-invalid-media-type-examples-recursion/snapshot.txt index 15b939a25f..72c84bb2f6 100644 --- a/tests/e2e/lint/no-invalid-media-type-examples-recursion/snapshot.txt +++ b/tests/e2e/lint/no-invalid-media-type-examples-recursion/snapshot.txt @@ -1,4 +1,18 @@ -[1] openapi.json:29:28 at #/paths/~1test/get/responses/202/content/application~1json/example +[1] openapi.json:68:13 at #/components/schemas/PetWithProps/properties/b/oneOf + +`oneOf` should have at least two schemas. Use the schema directly instead. + +66 | }, +67 | "b": { +68 | "oneOf": [{ "$ref": "#/components/schemas/PetWithProps" }] + | ^^^^^^^ +69 | } +70 | } + +Warning was generated by the no-illogical-composition-keywords rule. + + +[2] openapi.json:29:28 at #/paths/~1test/get/responses/202/content/application~1json/example Example value must conform to the schema: must have required property 'c'. @@ -21,7 +35,7 @@ Warning was generated by the no-invalid-media-type-examples rule. Reference: https://redocly.com/docs/cli/rules/oas/no-invalid-media-type-examples -[2] openapi.json:30:24 at #/paths/~1test/get/responses/202/content/application~1json/example/a +[3] openapi.json:30:24 at #/paths/~1test/get/responses/202/content/application~1json/example/a Example value must conform to the schema: `a` property must have required property 'c'. @@ -39,10 +53,52 @@ Warning was generated by the no-invalid-media-type-examples rule. Reference: https://redocly.com/docs/cli/rules/oas/no-invalid-media-type-examples +[4] openapi.json:49:9 at #/components/schemas/Pet/oneOf + +`oneOf` should have at least two schemas. Use the schema directly instead. + +47 | "Pet": { +48 | "type": "object", +49 | "oneOf": [{ "$ref": "#/components/schemas/Dog" }] + | ^^^^^^^ +50 | }, +51 | "Dog": { + +Warning was generated by the no-illogical-composition-keywords rule. + + +[5] openapi.json:53:9 at #/components/schemas/Dog/oneOf + +`oneOf` should have at least two schemas. Use the schema directly instead. + +51 | "Dog": { +52 | "type": "object", +53 | "oneOf": [{ "$ref": "#/components/schemas/Pet" }] + | ^^^^^^^ +54 | }, +55 | "PetDirect": { + +Warning was generated by the no-illogical-composition-keywords rule. + + +[6] openapi.json:57:9 at #/components/schemas/PetDirect/oneOf + +`oneOf` should have at least two schemas. Use the schema directly instead. + +55 | "PetDirect": { +56 | "type": "object", +57 | "oneOf": [{ "$ref": "#/components/schemas/PetDirect" }] + | ^^^^^^^ +58 | }, +59 | "PetWithProps": { + +Warning was generated by the no-illogical-composition-keywords rule. + + validating openapi.json using lint rules for api 'main'... openapi.json: validated in ms Woohoo! Your API description is valid. 🎉 -You have 2 warnings. +You have 6 warnings. diff --git a/tests/e2e/lint/oas3.2/snapshot.txt b/tests/e2e/lint/oas3.2/snapshot.txt index eeebd82515..a7fa1a2d8c 100644 --- a/tests/e2e/lint/oas3.2/snapshot.txt +++ b/tests/e2e/lint/oas3.2/snapshot.txt @@ -303,7 +303,21 @@ Warning was generated by the operation-4xx-response rule. Reference: https://redocly.com/docs/cli/rules/oas/operation-4xx-response -[21] openapi.yaml:149:5 at #/components/schemas/MyResponseType +[21] openapi.yaml:150:7 at #/components/schemas/MyResponseType/oneOf + +Schemas in `oneOf` must be mutually exclusive. Found overlapping schemas: `#/components/schemas/Lizard` and `#/components/schemas/OtherPet`. Add `petType` to `required` in every schema; the `discriminator` cannot read a property a value may omit. + +148 | schemas: +149 | MyResponseType: +150 | oneOf: + | ^^^^^ +151 | - $ref: '#/components/schemas/Lizard' +152 | - $ref: '#/components/schemas/OtherPet' + +Warning was generated by the no-illogical-composition-keywords rule. + + +[22] openapi.yaml:149:5 at #/components/schemas/MyResponseType Component: "MyResponseType" is never used. @@ -319,7 +333,7 @@ Warning was generated by the no-unused-components rule. Reference: https://redocly.com/docs/cli/rules/oas/no-unused-components -[22] openapi.yaml:125:5 at #/components/securitySchemes/petstore_auth +[23] openapi.yaml:125:5 at #/components/securitySchemes/petstore_auth Security scheme: "petstore_auth" is never used. @@ -335,7 +349,7 @@ Warning was generated by the no-unused-components rule. Reference: https://redocly.com/docs/cli/rules/oas/no-unused-components -[23] openapi.yaml:136:5 at #/components/securitySchemes/api_key +[24] openapi.yaml:136:5 at #/components/securitySchemes/api_key Security scheme: "api_key" is never used. @@ -355,6 +369,6 @@ Reference: https://redocly.com/docs/cli/rules/oas/no-unused-components validating openapi.yaml using lint rules for api 'main'... openapi.yaml: validated in ms -❌ Validation failed with 10 errors and 13 warnings. +❌ Validation failed with 10 errors and 14 warnings. run `redocly lint --generate-ignore-file` to add all problems to the ignore file. From b0c7308d1393491953841dd51f428705c2369b73 Mon Sep 17 00:00:00 2001 From: Albina Blazhko Date: Thu, 20 Aug 2026 20:35:56 +0300 Subject: [PATCH 2/6] fix: compare const against enum in no-illogical-composition-keywords --- .../no-illogical-composition-keywords.test.ts | 40 +++++++++++++++++++ .../oas3/no-illogical-composition-keywords.ts | 31 ++++++-------- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts b/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts index e03d6cbf7c..e4d69f6fc4 100644 --- a/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts +++ b/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts @@ -264,6 +264,46 @@ describe('Oas3 no-illogical-composition-keywords', () => { `); }); + it('should compare `const` against `enum` as a single allowed value', async () => { + expect( + await lint(` + openapi: 3.1.0 + info: + title: Test + version: '1.0' + paths: {} + components: + schemas: + Exclusive: + oneOf: + - type: string + enum: [card] + - type: string + const: bank + Overlapping: + oneOf: + - enum: [card] + - const: card + `) + ).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Overlapping/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and schema at position 2. Both schemas allow the values ["card"].", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + it('should report an empty schema inside oneOf', async () => { expect( await lint(` diff --git a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts index 35f2ca0baf..1f73c3ba91 100644 --- a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts +++ b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts @@ -251,18 +251,13 @@ function findOverlapReason( return 'Both schemas accept `null`.'; } - const leftEnum = leftSchema.enum; - const rightEnum = rightSchema.enum; - if (leftEnum && rightEnum) { - const shared = leftEnum.filter((value) => rightEnum.some((other) => dequal(value, other))); + const leftValues = readAllowedValues(leftSchema); + const rightValues = readAllowedValues(rightSchema); + if (leftValues && rightValues) { + const shared = leftValues.filter((value) => rightValues.some((other) => dequal(value, other))); return `Both schemas allow the values ${JSON.stringify(shared)}.`; } - const sharedConst = readConst(leftSchema); - if (isDefined(sharedConst) && isDefined(readConst(rightSchema))) { - return `Both schemas require the same \`const\` value ${JSON.stringify(sharedConst)}.`; - } - if (discriminator) { const { propertyName } = discriminator; // `struct` is not guaranteed to have rejected a non-string `propertyName` first. @@ -431,16 +426,12 @@ function forbidsPropertyRequiredBy( } function areExclusive(left: CompositionSchema, right: CompositionSchema): boolean { - const leftEnum = left.enum; - const rightEnum = right.enum; - if (leftEnum && rightEnum) { - return !leftEnum.some((value) => rightEnum.some((other) => dequal(value, other))); + const leftValues = readAllowedValues(left); + const rightValues = readAllowedValues(right); + if (leftValues && rightValues) { + return !leftValues.some((value) => rightValues.some((other) => dequal(value, other))); } - const leftConst = readConst(left); - const rightConst = readConst(right); - if (isDefined(leftConst) && isDefined(rightConst)) return !dequal(leftConst, rightConst); - const leftTypes = getTypeSet(left); const rightTypes = getTypeSet(right); // `format` is an annotation, not an assertion, so differing formats prove nothing. @@ -492,8 +483,10 @@ function schemaAllowsNull(schema: CompositionSchema): boolean { return Array.isArray(declaredType) ? declaredType.includes('null') : declaredType === 'null'; } -function readConst(schema: CompositionSchema): unknown { - return 'const' in schema ? schema.const : undefined; +function readAllowedValues(schema: CompositionSchema): unknown[] | undefined { + if (schema.enum) return schema.enum; + const constValue = 'const' in schema ? schema.const : undefined; + return isDefined(constValue) ? [constValue] : undefined; } function hasIntersection(left: Set, right: Set): boolean { From 554f9f17b6b6a0cc7ef7eff3e28eefcc60c38ee9 Mon Sep 17 00:00:00 2001 From: Albina Blazhko Date: Thu, 20 Aug 2026 21:19:56 +0300 Subject: [PATCH 3/6] docs: improve rule docs --- docs/@v2/rules/oas/no-illogical-composition-keywords.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/@v2/rules/oas/no-illogical-composition-keywords.md b/docs/@v2/rules/oas/no-illogical-composition-keywords.md index 56b44ccea2..b7b1112c2a 100644 --- a/docs/@v2/rules/oas/no-illogical-composition-keywords.md +++ b/docs/@v2/rules/oas/no-illogical-composition-keywords.md @@ -8,7 +8,7 @@ Ensures that `oneOf`, `anyOf`, and `allOf` combine schemas that a value can actu The rule reports: -- A `oneOf` or `anyOf` with fewer than two schemas. +- A `oneOf` or `anyOf` with fewer than two schemas, unless the schema declares a `discriminator`. - An `allOf` with fewer than two schemas that neither declares another keyword of its own nor extends a discriminated schema. - A schema repeated inside the same keyword. - An empty schema (`{}`) used as a member. @@ -45,7 +45,8 @@ When a value matches two of the listed schemas, no tool can tell which one was i The most common version of this is nullability: if a referenced schema already accepts `null` and the `oneOf` also lists `type: 'null'`, a null value matches both branches. Deciding whether two arbitrary schemas overlap is not solvable in general, so the comparison stays deliberately narrow. -It reads `type`, `enum`, `const`, `properties`, `required`, and `additionalProperties: false`. +It reads `type`, `nullable`, `enum`, `const`, `properties`, `required`, and `additionalProperties: false`. +A `const` counts as a single-value `enum`, so one member can use `enum` and the other `const`. When a member uses any other constraint, such as `not`, `pattern`, `minimum`, or a nested `allOf`, that constraint may be what separates the schemas, so the rule reports nothing for the pair. A `discriminator` names the property that tells the members apart, so the rule trusts it and checks only what the specification requires. From c0ff85c51e178caa565591877e255c9d3e164aae Mon Sep 17 00:00:00 2001 From: Albina Blazhko Date: Thu, 20 Aug 2026 21:32:53 +0300 Subject: [PATCH 4/6] refactor: simplify and rename the required-property check in no-illogical-composition-keywords --- .../oas3/no-illogical-composition-keywords.ts | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts index 1f73c3ba91..822ec3b123 100644 --- a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts +++ b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts @@ -308,7 +308,7 @@ function findPropertyOverlap( return null; } - if (requiredSetsDistinguish(comparison)) return null; + if (schemasRequireDifferentProperties(comparison)) return null; if (comparison.sharedNames.length === 0) return null; const shared = classifySharedProperties(comparison, resolve); @@ -352,28 +352,19 @@ function collectPropertyComparison( }; } -function requiredSetsDistinguish({ +function schemasRequireDifferentProperties({ leftProperties, rightProperties, leftRequired, rightRequired, - sharedNames, }: PropertyComparison): boolean { - if (leftRequired.size === 0 || rightRequired.size === 0) return false; - - const sharedNameIsRequired = sharedNames.some( - (name) => leftRequired.has(name) || rightRequired.has(name) - ); - if (!hasIntersection(leftRequired, rightRequired) && !sharedNameIsRequired) return true; - - // A property required by both sides distinguishes nothing, no matter where it is declared. - const leftRequiresUndeclared = [...leftRequired].some( + const leftRequiresPropertyUnknownToRight = [...leftRequired].some( (name) => !getOwn(rightProperties, name) && !rightRequired.has(name) ); - const rightRequiresUndeclared = [...rightRequired].some( + const rightRequiresPropertyUnknownToLeft = [...rightRequired].some( (name) => !getOwn(leftProperties, name) && !leftRequired.has(name) ); - return leftRequiresUndeclared && rightRequiresUndeclared; + return leftRequiresPropertyUnknownToRight && rightRequiresPropertyUnknownToLeft; } function classifySharedProperties( From 3938123dabc44a07faf90f52cb0b93fea8e7d5cf Mon Sep 17 00:00:00 2001 From: Albina Blazhko Date: Thu, 20 Aug 2026 21:43:10 +0300 Subject: [PATCH 5/6] refactor: remove dead check and unread data from no-illogical-composition-keywords --- .../oas3/no-illogical-composition-keywords.ts | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts index 822ec3b123..247b51eda2 100644 --- a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts +++ b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts @@ -23,8 +23,8 @@ type SourcedSchema = { type SchemaProperties = NonNullable; type PropertyComparison = { - left: SourcedSchema; - right: SourcedSchema; + leftSource: string | undefined; + rightSource: string | undefined; leftProperties: SchemaProperties; rightProperties: SchemaProperties; leftRequired: Set; @@ -259,11 +259,7 @@ function findOverlapReason( } if (discriminator) { - const { propertyName } = discriminator; - // `struct` is not guaranteed to have rejected a non-string `propertyName` first. - return typeof propertyName === 'string' - ? describeDiscriminatorGap(leftSchema, rightSchema, propertyName) - : null; + return describeDiscriminatorGap(leftSchema, rightSchema, discriminator); } const propertyOverlap = findPropertyOverlap(left, right, resolve); @@ -282,8 +278,12 @@ function findOverlapReason( function describeDiscriminatorGap( left: CompositionSchema, right: CompositionSchema, - propertyName: string + discriminator: Oas3Discriminator ): string | null { + const { propertyName } = discriminator; + // `struct` is not guaranteed to have rejected a non-string `propertyName` first. + if (typeof propertyName !== 'string') return null; + const requiredInBoth = !!left.required?.includes(propertyName) && !!right.required?.includes(propertyName); if (requiredInBoth) return null; @@ -342,8 +342,8 @@ function collectPropertyComparison( if (!leftProperties || !rightProperties) return null; return { - left, - right, + leftSource: left.source, + rightSource: right.source, leftProperties, rightProperties, leftRequired: new Set(left.schema.required ?? []), @@ -369,8 +369,8 @@ function schemasRequireDifferentProperties({ function classifySharedProperties( { - left, - right, + leftSource, + rightSource, leftProperties, rightProperties, leftRequired, @@ -385,8 +385,8 @@ function classifySharedProperties( for (const name of sharedNames) { const requiredInBoth = leftRequired.has(name) && rightRequired.has(name); - const leftProperty = resolveSchema(getOwn(leftProperties, name), resolve, left.source); - const rightProperty = resolveSchema(getOwn(rightProperties, name), resolve, right.source); + const leftProperty = resolveSchema(getOwn(leftProperties, name), resolve, leftSource); + const rightProperty = resolveSchema(getOwn(rightProperties, name), resolve, rightSource); if (!leftProperty || !rightProperty) continue; if ( hasUnsupportedConstraint(leftProperty.schema) || @@ -458,8 +458,6 @@ function getTypeSet(schema: CompositionSchema): Set | undefined { } function getSharedTypes(left: CompositionSchema, right: CompositionSchema): string[] | undefined { - if (hasUnsupportedConstraint(left) || hasUnsupportedConstraint(right)) return undefined; - const leftTypes = getTypeSet(left); const rightTypes = getTypeSet(right); if (!leftTypes || !rightTypes) return undefined; From 6b78552eef347448cc56b57dc5b65cce2a74092e Mon Sep 17 00:00:00 2001 From: Albina Blazhko Date: Fri, 21 Aug 2026 16:31:21 +0300 Subject: [PATCH 6/6] refactor: simplify no-illogical-composition-keywords and report a nullable schema whose oneOf accepts null --- .../oas/no-illogical-composition-keywords.md | 2 + .../no-illogical-composition-keywords.test.ts | 887 +++++++++++------- .../oas3/no-illogical-composition-keywords.ts | 61 +- 3 files changed, 576 insertions(+), 374 deletions(-) diff --git a/docs/@v2/rules/oas/no-illogical-composition-keywords.md b/docs/@v2/rules/oas/no-illogical-composition-keywords.md index b7b1112c2a..dd7a5afa46 100644 --- a/docs/@v2/rules/oas/no-illogical-composition-keywords.md +++ b/docs/@v2/rules/oas/no-illogical-composition-keywords.md @@ -13,6 +13,7 @@ The rule reports: - A schema repeated inside the same keyword. - An empty schema (`{}`) used as a member. - Two `oneOf` schemas that a single value can match at the same time. +- A nullable schema whose `oneOf` also accepts `null`. - A `discriminator` whose property is missing from `required` in every member schema. - An inline `oneOf` or `anyOf` member that a `discriminator` cannot select. @@ -43,6 +44,7 @@ style AllOf fill:#codaf9,stroke:#0044d4,stroke-width:5px When a value matches two of the listed schemas, no tool can tell which one was intended, and validators, code generators, and documentation all disagree about the result. The most common version of this is nullability: if a referenced schema already accepts `null` and the `oneOf` also lists `type: 'null'`, a null value matches both branches. +The same ambiguity appears one level up, when the schema holding the `oneOf` is itself nullable and a member accepts `null` too. Deciding whether two arbitrary schemas overlap is not solvable in general, so the comparison stays deliberately narrow. It reads `type`, `nullable`, `enum`, `const`, `properties`, `required`, and `additionalProperties: false`. diff --git a/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts b/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts index e4d69f6fc4..168ca86719 100644 --- a/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts +++ b/packages/core/src/rules/oas3/__tests__/no-illogical-composition-keywords.test.ts @@ -6,22 +6,10 @@ import { lintDocument } from '../../../lint.js'; import { BaseResolver } from '../../../resolve.js'; describe('Oas3 no-illogical-composition-keywords', () => { - async function lint(yaml: string) { - const document = parseYamlToDocument(outdent`${yaml}`, 'foobar.yaml'); - const results = await lintDocument({ - externalRefResolver: new BaseResolver(), - document, - config: await createConfig({ - rules: { 'no-illogical-composition-keywords': 'error' }, - }), - }); - return replaceSourceWithRef(results); - } - describe('oneOf', () => { it('should report when oneOf has only one schema', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -32,8 +20,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { Test: oneOf: - type: string - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -53,8 +52,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should not report when oneOf has only one schema but the parent declares a discriminator', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -72,13 +71,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { properties: petType: type: string - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should report duplicated schemas that are not next to each other', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -93,8 +103,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { - $ref: '#/components/schemas/Cat' Cat: type: object - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -114,8 +135,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report distinct refs that resolve to equal schemas as overlapping', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -137,8 +158,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { properties: id: type: string - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -157,9 +189,9 @@ describe('Oas3 no-illogical-composition-keywords', () => { `); }); - it('should stay quiet when the discriminator `propertyName` is not a string', async () => { - expect( - await lint(` + it('should not report a discriminator gap when `propertyName` is not a string', async () => { + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -167,60 +199,46 @@ describe('Oas3 no-illogical-composition-keywords', () => { paths: {} components: schemas: + Cat: + type: object + properties: + name: + type: string + required: [name] + Dog: + type: object + properties: + name: + type: string + bark: + type: string + required: [name] Test: discriminator: propertyName: name: type: string oneOf: - - type: object - properties: - name: - type: string - required: [name] - - type: object - properties: - name: - type: string - description: - type: string - required: [name] - `) - ).toMatchInlineSnapshot(` - [ - { - "location": [ - { - "pointer": "#/components/schemas/Test/oneOf/0", - "reportOnKey": false, - "source": "foobar.yaml", - }, - ], - "message": "Schema in \`oneOf\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.", - "ruleId": "no-illogical-composition-keywords", - "severity": "error", - "suggest": [], - }, - { - "location": [ - { - "pointer": "#/components/schemas/Test/oneOf/1", - "reportOnKey": false, - "source": "foobar.yaml", - }, - ], - "message": "Schema in \`oneOf\` is inline, so the \`discriminator\` cannot select it. Use a \`$ref\` to a named schema.", - "ruleId": "no-illogical-composition-keywords", - "severity": "error", - "suggest": [], - }, - ] - `); + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Dog' + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should report inline members that a discriminator cannot select', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -244,8 +262,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { petType: type: string required: [petType] - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -265,8 +294,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should compare `const` against `enum` as a single allowed value', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -284,8 +313,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { oneOf: - enum: [card] - const: card - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -305,8 +345,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report an empty schema inside oneOf', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -318,8 +358,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { oneOf: - type: string - {} - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -339,8 +390,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report when a referenced schema and a null schema both accept null', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -350,14 +401,25 @@ describe('Oas3 no-illogical-composition-keywords', () => { schemas: Test: oneOf: - - $ref: '#/components/schemas/InvoiceTimeShift' + - $ref: '#/components/schemas/ContainsNull' - type: 'null' - InvoiceTimeShift: + ContainsNull: type: - object - 'null' - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -367,7 +429,7 @@ describe('Oas3 no-illogical-composition-keywords', () => { "source": "foobar.yaml", }, ], - "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: \`#/components/schemas/InvoiceTimeShift\` and schema at position 2. Both schemas accept \`null\`.", + "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: \`#/components/schemas/ContainsNull\` and schema at position 2. Both schemas accept \`null\`.", "ruleId": "no-illogical-composition-keywords", "severity": "error", "suggest": [], @@ -377,8 +439,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report when type sets overlap', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -392,8 +454,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { - type: - string - integer - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -412,9 +485,9 @@ describe('Oas3 no-illogical-composition-keywords', () => { `); }); - it('should not report when a shared property uses not to exclude the other value', async () => { - expect( - await lint(` + it('should not report when a shared property uses an unmodelled keyword such as `not`', async () => { + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -440,13 +513,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { petType: not: enum: ['Lizard'] - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should not crash on boolean schemas used as members or property schemas', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -469,39 +553,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { properties: kind: type: string - `) - ).toMatchInlineSnapshot(`[]`); - }); + `, + 'foobar.yaml' + ); - it('should not report when members carry constraints these checks do not model', async () => { - expect( - await lint(` - openapi: 3.1.0 - info: - title: Test - version: '1.0' - paths: {} - components: - schemas: - Patterns: - oneOf: - - type: string - pattern: '^a' - - type: string - pattern: '^b' - Ranges: - oneOf: - - type: integer - maximum: 10 - - type: integer - minimum: 11 - `) - ).toMatchInlineSnapshot(`[]`); + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should report when both members declare an empty required list', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -522,8 +591,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { properties: name: type: string - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -543,8 +623,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should not report when one member forbids a property the other requires', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -567,13 +647,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { type: string b: type: string - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should report when an exclusive shared property is not required, without a discriminator', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -593,8 +684,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { kind: type: string enum: [dog] - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -614,8 +716,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report each overlapping pair of a oneOf separately', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -630,8 +732,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { title: Second - type: string title: Third - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -676,88 +789,9 @@ describe('Oas3 no-illogical-composition-keywords', () => { `); }); - it('should report when both members require the same undeclared property', async () => { - expect( - await lint(` - openapi: 3.1.0 - info: - title: Test - version: '1.0' - paths: {} - components: - schemas: - Test: - oneOf: - - type: object - required: [documentType, fileId] - properties: - fileId: - type: string - - type: object - title: Second - required: [documentType, fileId] - properties: - fileId: - type: string - `) - ).toMatchInlineSnapshot(` - [ - { - "location": [ - { - "pointer": "#/components/schemas/Test/oneOf", - "reportOnKey": true, - "source": "foobar.yaml", - }, - ], - "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and \`Second\`. Both schemas define \`fileId\` without constraints that exclude each other. Add a discriminator, or constrain the shared properties to different values.", - "ruleId": "no-illogical-composition-keywords", - "severity": "error", - "suggest": [], - }, - ] - `); - }); - - it('should not report when members are separated by a nested allOf', async () => { - expect( - await lint(` - openapi: 3.1.0 - info: - title: Test - version: '1.0' - paths: {} - components: - schemas: - Test: - oneOf: - - type: object - properties: - id: - type: string - allOf: - - type: object - required: [kind] - properties: - kind: - enum: [dog] - - type: object - properties: - id: - type: string - allOf: - - type: object - required: [kind] - properties: - kind: - enum: [cat] - `) - ).toMatchInlineSnapshot(`[]`); - }); - it('should report the discriminator property as required when members do not declare it', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -774,15 +808,26 @@ describe('Oas3 no-illogical-composition-keywords', () => { Cat: type: object properties: - name: + petType: type: string Dog: type: object properties: - name: + petType: type: string - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -802,8 +847,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should not report when the discriminator property is required in every member', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -829,13 +874,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { properties: name: type: string - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); - it('should not report members that differ only by format', async () => { - expect( - await lint(` + it('should report members that differ only by `format`', async () => { + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -857,8 +913,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { id: type: string format: uri - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -877,9 +944,9 @@ describe('Oas3 no-illogical-composition-keywords', () => { `); }); - it('should not report when the parent is nullable and a single member accepts null', async () => { - expect( - await lint(` + it('should report when the schema and a `oneOf` member both accept null', async () => { + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -887,19 +954,51 @@ describe('Oas3 no-illogical-composition-keywords', () => { paths: {} components: schemas: - Test: + NullableParent: nullable: true oneOf: - type: string nullable: true - type: integer - `) - ).toMatchInlineSnapshot(`[]`); + PlainParent: + oneOf: + - type: string + nullable: true + - type: integer + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/NullableParent/oneOf", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "The schema and a schema in \`oneOf\` both accept \`null\`, so nothing decides which one applies to a null value.", + "ruleId": "no-illogical-composition-keywords", + "severity": "error", + "suggest": [], + }, + ] + `); }); it('should not report when a required property with exclusive values tells the members apart', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -925,13 +1024,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { type: string enum: [dog] required: [kind] - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should report when members share a property with no constraint that tells them apart', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -952,8 +1062,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { name: type: string required: [name] - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -973,8 +1094,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report when the discriminator property is not required in every member', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -996,8 +1117,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { petType: type: string enum: [dog] - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -1043,8 +1175,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should not report when members have different types', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -1056,64 +1188,26 @@ describe('Oas3 no-illogical-composition-keywords', () => { oneOf: - type: string - type: integer - `) - ).toMatchInlineSnapshot(`[]`); - }); + `, + 'foobar.yaml' + ); - it('should report both a structural problem and an overlap in the same oneOf', async () => { - expect( - await lint(` - openapi: 3.0.0 - info: - title: Test - version: '1.0' - paths: {} - components: - schemas: - Test: - oneOf: - - type: string - - type: string - title: Text - - {} - `) - ).toMatchInlineSnapshot(` - [ - { - "location": [ - { - "pointer": "#/components/schemas/Test/oneOf/2", - "reportOnKey": false, - "source": "foobar.yaml", - }, - ], - "message": "Schema in \`oneOf\` is empty, so it matches any value.", - "ruleId": "no-illogical-composition-keywords", - "severity": "error", - "suggest": [], - }, - { - "location": [ - { - "pointer": "#/components/schemas/Test/oneOf", - "reportOnKey": true, - "source": "foobar.yaml", - }, - ], - "message": "Schemas in \`oneOf\` must be mutually exclusive. Found overlapping schemas: schema at position 1 and \`Text\`. Both schemas accept \`string\`.", - "ruleId": "no-illogical-composition-keywords", - "severity": "error", - "suggest": [], - }, - ] - `); + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); }); describe('anyOf', () => { it('should report when anyOf has only one schema', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -1124,8 +1218,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { Test: anyOf: - type: string - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -1145,8 +1250,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report duplicated and empty schemas inside anyOf', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -1159,8 +1264,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { - type: string - type: string - {} - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -1193,8 +1309,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should not report anyOf used with a discriminator by default', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -1212,15 +1328,26 @@ describe('Oas3 no-illogical-composition-keywords', () => { type: object Dog: type: string - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); }); describe('allOf', () => { it('should report an allOf wrapper that adds nothing', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -1233,8 +1360,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { - $ref: '#/components/schemas/Cat' Cat: type: object - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -1254,8 +1392,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should not report a single-schema allOf used to attach sibling keywords', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -1272,13 +1410,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { - $ref: '#/components/schemas/CustomerId' CustomerId: type: string - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should not report a single-schema allOf declaring a subtype of a discriminated schema', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.3 info: title: Test @@ -1297,13 +1446,24 @@ describe('Oas3 no-illogical-composition-keywords', () => { Cat: allOf: - $ref: '#/components/schemas/Pet' - `) - ).toMatchInlineSnapshot(`[]`); + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); it('should report an empty allOf', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -1313,8 +1473,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { schemas: Test: allOf: [] - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -1334,8 +1505,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should report duplicated and empty schemas inside allOf', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.0.0 info: title: Test @@ -1350,8 +1521,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { - {} Cat: type: object - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ @@ -1385,8 +1567,8 @@ describe('Oas3 no-illogical-composition-keywords', () => { }); it('should check every composition keyword used on the same schema', async () => { - expect( - await lint(` + const document = parseYamlToDocument( + outdent` openapi: 3.1.0 info: title: Test @@ -1402,8 +1584,19 @@ describe('Oas3 no-illogical-composition-keywords', () => { - $ref: '#/components/schemas/Cat' Cat: type: object - `) - ).toMatchInlineSnapshot(` + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ + rules: { 'no-illogical-composition-keywords': 'error' }, + }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` [ { "location": [ diff --git a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts index 247b51eda2..b65441e970 100644 --- a/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts +++ b/packages/core/src/rules/oas3/no-illogical-composition-keywords.ts @@ -32,8 +32,9 @@ type PropertyComparison = { sharedNames: string[]; }; -// Keywords these checks model, plus annotations that don't constrain which values match. -// A schema using anything else is left alone, because the unmodelled keyword may separate it. +// Every keyword a schema may carry without stopping the comparison. A keyword outside this set +// could be the one that separates two schemas, so `hasUnsupportedConstraint` gives up on the pair. +// Membership therefore has two reasons: the checks read it, or it constrains no value at all. const UNDERSTOOD_KEYWORDS: ReadonlySet = new Set([ 'type', 'format', @@ -67,8 +68,6 @@ export const NoIllogicalCompositionKeywords: Oas3Rule = () => { const parentSchema = getParentSchema(ctx); if (!Array.isArray(members) || !parentSchema) return; - // A lone member is redundant only when the wrapper carries nothing else of its own - // and is not declaring a subtype of a discriminated schema. if ( members.length === 0 || (members.length === 1 && @@ -118,6 +117,10 @@ export const NoIllogicalCompositionKeywords: Oas3Rule = () => { reportInlineMembers(members, 'oneOf', ctx); } + if (schemaAllowsNull(parentSchema)) { + reportNullableParent(members, ctx); + } + reportAmbiguousMembers(members, hasDiscriminator ? discriminator : undefined, ctx); }, }; @@ -140,8 +143,6 @@ function reportSingleSchema( }); } -// A schema whose `allOf` references a schema with a `discriminator` declares a subtype: the -// discriminator resolves the subtype by name, so the wrapper carries meaning of its own. function isSubtypeOfDiscriminatedSchema( member: Referenced, resolve: ResolveFn @@ -195,6 +196,20 @@ function reportDuplicateMembers( } } +function reportNullableParent(members: CompositionSchema[], ctx: UserContext) { + const nullableMember = members.some((member) => { + const resolved = resolveSchema(member, ctx.resolve); + return resolved ? schemaAllowsNull(resolved.schema) : false; + }); + if (!nullableMember) return; + + ctx.report({ + message: + 'The schema and a schema in `oneOf` both accept `null`, so nothing decides which one applies to a null value.', + location: ctx.location.key(), + }); +} + function reportAmbiguousMembers( members: CompositionSchema[], discriminator: Oas3Discriminator | undefined, @@ -267,7 +282,7 @@ function findOverlapReason( if (!leftSchema.properties && !rightSchema.properties) { const sharedTypes = getSharedTypes(leftSchema, rightSchema); - if (sharedTypes) { + if (sharedTypes?.length) { return `Both schemas accept ${sharedTypes.map((type) => `\`${type}\``).join(', ')}.`; } } @@ -284,9 +299,9 @@ function describeDiscriminatorGap( // `struct` is not guaranteed to have rejected a non-string `propertyName` first. if (typeof propertyName !== 'string') return null; - const requiredInBoth = + const isRequiredInBoth = !!left.required?.includes(propertyName) && !!right.required?.includes(propertyName); - if (requiredInBoth) return null; + if (isRequiredInBoth) return null; return `Add \`${propertyName}\` to \`required\` in every schema; the \`discriminator\` cannot read a property a value may omit.`; } @@ -383,7 +398,7 @@ function classifySharedProperties( const ambiguousNames: string[] = []; for (const name of sharedNames) { - const requiredInBoth = leftRequired.has(name) && rightRequired.has(name); + const isRequiredInBoth = leftRequired.has(name) && rightRequired.has(name); const leftProperty = resolveSchema(getOwn(leftProperties, name), resolve, leftSource); const rightProperty = resolveSchema(getOwn(rightProperties, name), resolve, rightSource); @@ -396,7 +411,7 @@ function classifySharedProperties( } if (areExclusive(leftProperty.schema, rightProperty.schema)) { - if (requiredInBoth) return null; + if (isRequiredInBoth) return null; optionalDistinguishingNames.push(name); continue; } @@ -423,10 +438,8 @@ function areExclusive(left: CompositionSchema, right: CompositionSchema): boolea return !leftValues.some((value) => rightValues.some((other) => dequal(value, other))); } - const leftTypes = getTypeSet(left); - const rightTypes = getTypeSet(right); // `format` is an annotation, not an assertion, so differing formats prove nothing. - return !!leftTypes && !!rightTypes && !hasIntersection(leftTypes, rightTypes); + return getSharedTypes(left, right)?.length === 0; } function resolveSchema( @@ -448,12 +461,16 @@ function hasUnsupportedConstraint(schema: CompositionSchema): boolean { ); } +function isNullable(schema: CompositionSchema): boolean { + return 'nullable' in schema && schema.nullable === true; +} + function getTypeSet(schema: CompositionSchema): Set | undefined { const declaredType = schema.type; if (declaredType === undefined) return undefined; const types = new Set(Array.isArray(declaredType) ? declaredType : [declaredType]); - if ('nullable' in schema && schema.nullable === true) types.add('null'); + if (isNullable(schema)) types.add('null'); return types; } @@ -462,14 +479,11 @@ function getSharedTypes(left: CompositionSchema, right: CompositionSchema): stri const rightTypes = getTypeSet(right); if (!leftTypes || !rightTypes) return undefined; - const shared = [...leftTypes].filter((type) => rightTypes.has(type)); - return shared.length > 0 ? shared : undefined; + return [...leftTypes].filter((type) => rightTypes.has(type)); } function schemaAllowsNull(schema: CompositionSchema): boolean { - if ('nullable' in schema && schema.nullable === true) return true; - const declaredType = schema.type; - return Array.isArray(declaredType) ? declaredType.includes('null') : declaredType === 'null'; + return isNullable(schema) || getTypeSet(schema)?.has('null') === true; } function readAllowedValues(schema: CompositionSchema): unknown[] | undefined { @@ -478,13 +492,6 @@ function readAllowedValues(schema: CompositionSchema): unknown[] | undefined { return isDefined(constValue) ? [constValue] : undefined; } -function hasIntersection(left: Set, right: Set): boolean { - for (const value of left) { - if (right.has(value)) return true; - } - return false; -} - function quoteAll(names: string[]): string { return names.map((name) => `\`${name}\``).join(', '); }