diff --git a/tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js b/tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js index 339b067997..8a3e411574 100644 --- a/tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js +++ b/tools/spectral/ipa/__tests__/IPA125OneOfMustHaveDiscriminator.test.js @@ -2,34 +2,32 @@ import testRule from './__helpers__/testRule'; import { DiagnosticSeverity } from '@stoplight/types'; const componentSchemas = { - schemas: { - Dog: { - type: 'object', - properties: { - breed: { type: 'string' }, - age: { type: 'integer' }, - }, + Dog: { + type: 'object', + properties: { + breed: { type: 'string' }, + age: { type: 'integer' }, }, - Cat: { - type: 'object', - properties: { - color: { type: 'string' }, - livesLeft: { type: 'integer' }, - }, + }, + Cat: { + type: 'object', + properties: { + color: { type: 'string' }, + livesLeft: { type: 'integer' }, }, - Bird: { - type: 'object', - properties: { - species: { type: 'string' }, - wingspan: { type: 'number' }, - }, + }, + Bird: { + type: 'object', + properties: { + species: { type: 'string' }, + wingspan: { type: 'number' }, }, - Fish: { - type: 'object', - properties: { - species: { type: 'string' }, - waterType: { type: 'string' }, - }, + }, + Fish: { + type: 'object', + properties: { + species: { type: 'string' }, + waterType: { type: 'string' }, }, }, }; @@ -38,15 +36,17 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { name: 'valid oneOf with discriminator and matching mapping', document: { - components: componentSchemas, - schemas: { - Animal: { - oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], - discriminator: { - propertyName: 'type', - mapping: { - dog: '#/components/schemas/Dog', - cat: '#/components/schemas/Cat', + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + propertyName: 'type', + mapping: { + dog: '#/components/schemas/Dog', + cat: '#/components/schemas/Cat', + }, }, }, }, @@ -57,15 +57,17 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { name: 'invalid oneOf with discriminator but mismatched mapping', document: { - components: componentSchemas, - schemas: { - Animal: { - oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], - discriminator: { - propertyName: 'type', - mapping: { - dog: '#/components/schemas/Dog', - bird: '#/components/schemas/Bird', + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + propertyName: 'type', + mapping: { + dog: '#/components/schemas/Dog', + bird: '#/components/schemas/Bird', + }, }, }, }, @@ -76,7 +78,7 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ code: 'xgen-IPA-125-oneOf-must-have-discriminator', message: 'The discriminator mapping must match the oneOf references. Unmatched Discriminator mappings with oneOf references: #/components/schemas/Bird', - path: ['schemas', 'Animal'], + path: ['components', 'schemas', 'Animal'], severity: DiagnosticSeverity.Error, }, ], @@ -84,10 +86,12 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { name: 'invalid oneOf without discriminator', document: { - components: componentSchemas, - schemas: { - Animal: { - oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + }, }, }, }, @@ -95,7 +99,7 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { code: 'xgen-IPA-125-oneOf-must-have-discriminator', message: 'The schema has oneOf but no discriminator property.', - path: ['schemas', 'Animal'], + path: ['components', 'schemas', 'Animal'], severity: DiagnosticSeverity.Error, }, ], @@ -103,11 +107,13 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { name: 'invalid oneOf with non-object discriminator', document: { - components: componentSchemas, - schemas: { - Animal: { - oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], - discriminator: "I'm a string, not an object!", + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: "I'm a string, not an object!", + }, }, }, }, @@ -115,7 +121,7 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { code: 'xgen-IPA-125-oneOf-must-have-discriminator', message: 'Discriminator property is not an object.', - path: ['schemas', 'Animal'], + path: ['components', 'schemas', 'Animal'], severity: DiagnosticSeverity.Error, }, ], @@ -123,14 +129,16 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { name: 'invalid oneOf with discriminator but no propertyName', document: { - components: componentSchemas, - schemas: { - Animal: { - oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], - discriminator: { - mapping: { - dog: '#/components/schemas/Dog', - cat: '#/components/schemas/Cat', + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + mapping: { + dog: '#/components/schemas/Dog', + cat: '#/components/schemas/Cat', + }, }, }, }, @@ -140,7 +148,7 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { code: 'xgen-IPA-125-oneOf-must-have-discriminator', message: 'Discriminator has no propertyName defined.', - path: ['schemas', 'Animal'], + path: ['components', 'schemas', 'Animal'], severity: DiagnosticSeverity.Error, }, ], @@ -148,12 +156,14 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { name: 'invalid oneOf with discriminator but no mapping', document: { - components: componentSchemas, - schemas: { - Animal: { - oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], - discriminator: { - propertyName: 'type', + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + discriminator: { + propertyName: 'type', + }, }, }, }, @@ -162,7 +172,7 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { code: 'xgen-IPA-125-oneOf-must-have-discriminator', message: 'Discriminator must have a mapping object.', - path: ['schemas', 'Animal'], + path: ['components', 'schemas', 'Animal'], severity: DiagnosticSeverity.Error, }, ], @@ -170,12 +180,14 @@ testRule('xgen-IPA-125-oneOf-must-have-discriminator', [ { name: 'oneOf with discriminator exemption', document: { - components: componentSchemas, - schemas: { - Animal: { - oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], - 'x-xgen-IPA-exception': { - 'xgen-IPA-125-oneOf-must-have-discriminator': 'reason for exemption', + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + 'x-xgen-IPA-exception': { + 'xgen-IPA-125-oneOf-must-have-discriminator': 'reason for exemption', + }, }, }, }, diff --git a/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js new file mode 100644 index 0000000000..76513b9096 --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js @@ -0,0 +1,117 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + Dog: { + type: 'object', + properties: { + breed: { type: 'string' }, + age: { type: 'integer' }, + }, + }, + Cat: { + type: 'object', + properties: { + color: { type: 'string' }, + livesLeft: { type: 'integer' }, + }, + }, +}; + +testRule('xgen-IPA-125-oneOf-no-base-types', [ + { + name: 'valid oneOf with references only', + document: { + components: { + schemas: { + ...componentSchemas, + Animal: { + oneOf: [{ $ref: '#/components/schemas/Dog' }, { $ref: '#/components/schemas/Cat' }], + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid oneOf with object schema', + document: { + components: { + schemas: { + ...componentSchemas, + MixedObject: { + oneOf: [ + { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + { $ref: '#/components/schemas/Dog' }, + ], + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid oneOf with string type', + document: { + components: { + schemas: { + ...componentSchemas, + MixedType: { + oneOf: [{ type: 'string' }, { $ref: '#/components/schemas/Dog' }], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-no-base-types', + message: 'oneOf should not contain base types like integer, number, string, or boolean.', + path: ['components', 'schemas', 'MixedType'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'invalid oneOf with multiple base types', + document: { + components: { + schemas: { + ...componentSchemas, + BaseTypes: { + oneOf: [{ type: 'string' }, { type: 'integer' }, { type: 'boolean' }], + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-125-oneOf-no-base-types', + message: 'oneOf should not contain base types like integer, number, string, or boolean.', + path: ['components', 'schemas', 'BaseTypes'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'oneOf with exception', + document: { + components: { + schemas: { + ...componentSchemas, + MixedType: { + oneOf: [{ type: 'string' }, { type: 'integer' }], + 'x-xgen-IPA-exception': { + 'xgen-IPA-125-oneOf-no-base-types': 'reason for exemption', + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/ipa-spectral.yaml b/tools/spectral/ipa/ipa-spectral.yaml index 2b1a158f78..15891a2f41 100644 --- a/tools/spectral/ipa/ipa-spectral.yaml +++ b/tools/spectral/ipa/ipa-spectral.yaml @@ -10,6 +10,7 @@ extends: - ./rulesets/IPA-112.yaml - ./rulesets/IPA-113.yaml - ./rulesets/IPA-123.yaml + - ./rulesets/IPA-125.yaml overrides: - files: diff --git a/tools/spectral/ipa/rulesets/IPA-125.yaml b/tools/spectral/ipa/rulesets/IPA-125.yaml index 5f0154f8fd..e48c385311 100644 --- a/tools/spectral/ipa/rulesets/IPA-125.yaml +++ b/tools/spectral/ipa/rulesets/IPA-125.yaml @@ -3,6 +3,7 @@ functions: - IPA125OneOfMustHaveDiscriminator + - IPA125OneOfNoBaseTypes rules: xgen-IPA-125-oneOf-must-have-discriminator: @@ -36,6 +37,54 @@ rules: message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-125-oneOf-must-have-discriminator' severity: error - given: '$..[?(@.oneOf)]' + given: '$.components.schemas[*]' then: function: 'IPA125OneOfMustHaveDiscriminator' + + xgen-IPA-125-oneOf-no-base-types: + description: | + API producers should not use oneOf with base types like integer, string, boolean, or number. + + ##### Implementation details + Rule checks for the following conditions: + - Applies to schemas with `oneOf` arrays + - Ensures no element within oneOf has a type property that is a primitive/base type + - Base types considered are: integer, string, boolean, number + + ##### Rationale + Using oneOf with primitive types can lead to ambiguity and validation problems. Clients may not + be able to properly determine which type to use in which context. Instead, use more specific + object types with clear discriminators. + + ##### Example Violation + ```yaml + # Incorrect - Using oneOf with base types + type: object + properties: + value: + oneOf: + - type: string + - type: integer + ``` + + ##### Example Compliance + ```yaml + # Correct - Using oneOf with object types only + type: object + properties: + value: + oneOf: + - $ref: '#/components/schemas/StringValue' + - $ref: '#/components/schemas/IntegerValue' + discriminator: + propertyName: valueType + mapping: + string: '#/components/schemas/StringValue' + integer: '#/components/schemas/IntegerValue' + ``` + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-125-oneOf-no-base-types' + severity: error + given: '$.components.schemas[*]' + then: + function: 'IPA125OneOfNoBaseTypes' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index b99bc25806..11cfca9038 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -502,4 +502,83 @@ Rule checks for the following conditions: +### IPA-125 + +Rules are based on [http://go/ipa/IPA-125](http://go/ipa/IPA-125). + +#### xgen-IPA-125-oneOf-must-have-discriminator + + ![error](https://img.shields.io/badge/error-red) +Each oneOf property must include a discriminator property to define the exact type. + +##### Implementation details +Rule checks for the following conditions: + - Applies only to schemas with `oneOf` containing references + - Ensures a `discriminator` property is present with a valid `propertyName` + - Validates that `discriminator.mapping` contains exactly the same number of entries as `oneOf` references + - Validates that each `discriminator.mapping` value matches a reference in the `oneOf` array + - Ignores `oneOf` definitions with inline schemas + +##### Matching Logic +- The `discriminator.mapping` must have the same number of entries as there are references in the `oneOf` array +- Each value in the `discriminator.mapping` must match one of the `$ref` values in the `oneOf` array +- Each `$ref` in the `oneOf` array must have a corresponding entry in the `discriminator.mapping` +- Example: + ```yaml + oneOf: + - $ref: '#/components/schemas/Dog' + - $ref: '#/components/schemas/Cat' + discriminator: + propertyName: type + mapping: + dog: '#/components/schemas/Dog' + cat: '#/components/schemas/Cat' + ``` + This is valid because there are exactly 2 mappings for 2 oneOf references, and all values match. + +#### xgen-IPA-125-oneOf-no-base-types + + ![error](https://img.shields.io/badge/error-red) +API producers should not use oneOf with base types like integer, string, boolean, or number. + +##### Implementation details +Rule checks for the following conditions: + - Applies to schemas with `oneOf` arrays + - Ensures no element within oneOf has a type property that is a primitive/base type + - Base types considered are: integer, string, boolean, number + +##### Rationale +Using oneOf with primitive types can lead to ambiguity and validation problems. Clients may not +be able to properly determine which type to use in which context. Instead, use more specific +object types with clear discriminators. + +##### Example Violation +```yaml +# Incorrect - Using oneOf with base types +type: object +properties: + value: + oneOf: + - type: string + - type: integer +``` + +##### Example Compliance +```yaml +# Correct - Using oneOf with object types only +type: object +properties: + value: + oneOf: + - $ref: '#/components/schemas/StringValue' + - $ref: '#/components/schemas/IntegerValue' + discriminator: + propertyName: valueType + mapping: + string: '#/components/schemas/StringValue' + integer: '#/components/schemas/IntegerValue' +``` + + + diff --git a/tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js b/tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js index e1a6bdf256..f483cbeb5f 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js +++ b/tools/spectral/ipa/rulesets/functions/IPA125OneOfMustHaveDiscriminator.js @@ -11,51 +11,53 @@ const MAPPING_ERROR_MESSAGE = 'The discriminator mapping must match the oneOf references. Unmatched Discriminator mappings with oneOf references:'; export default (input, _, { path, documentInventory }) => { - const oas = documentInventory.unresolved; // Use unresolved document to access raw $ref - const schema = resolveObject(oas, path); - - if (!schema || !schema.oneOf || !Array.isArray(schema.oneOf)) { + if (!input.oneOf) { return; } - - // Check for exception first + const schema = resolveObject(documentInventory.unresolved, path); if (hasException(schema, RULE_NAME)) { return; } + const errors = checkViolationsAndReturnErrors(schema, path); + if (errors.length !== 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } + collectAdoption(path, RULE_NAME); +}; + +function checkViolationsAndReturnErrors(schema, path) { // Check if all oneOf items are objects with a $ref property const allReferences = schema.oneOf.every((item) => typeof item === 'object' && item.$ref); if (!allReferences) { - return; + return []; } // Validate the presence of a discriminator with more specific error messages if (!schema.discriminator) { - return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_DISCRIMINATOR_MESSAGE }]); + return [{ path, message: MISSING_DISCRIMINATOR_MESSAGE }]; } if (typeof schema.discriminator !== 'object') { - return collectAndReturnViolation(path, RULE_NAME, [{ message: INVALID_DISCRIMINATOR_MESSAGE }]); + return [{ path, message: INVALID_DISCRIMINATOR_MESSAGE }]; } if (!schema.discriminator.propertyName) { - return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_PROPERTY_NAME_MESSAGE }]); + return [{ path, message: MISSING_PROPERTY_NAME_MESSAGE }]; } if (!schema.discriminator.mapping) { - return collectAndReturnViolation(path, RULE_NAME, [{ message: MISSING_MAPPING_MESSAGE }]); + return [{ path, message: MISSING_MAPPING_MESSAGE }]; } + // Get the $ref values from the unresolved document const oneOfRefs = schema.oneOf.map((item) => item.$ref); const mappingValues = Object.values(schema.discriminator.mapping); // Check for discriminator mappings that don't match any oneOf reference const unmatchedMappings = mappingValues.filter((mapping) => !oneOfRefs.includes(mapping)); if (unmatchedMappings.length > 0) { - return collectAndReturnViolation(path, RULE_NAME, [ - { message: `${MAPPING_ERROR_MESSAGE} ${unmatchedMappings.join(', ')}` }, - ]); + return [{ path, message: `${MAPPING_ERROR_MESSAGE} ${unmatchedMappings.join(', ')}` }]; } - - collectAdoption(path, RULE_NAME); -}; + return []; +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js b/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js new file mode 100644 index 0000000000..c9aa54394f --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js @@ -0,0 +1,36 @@ +import { collectAdoption, collectAndReturnViolation } from './utils/collectionUtils.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { hasException } from './utils/exceptions.js'; + +const RULE_NAME = 'xgen-IPA-125-oneOf-no-base-types'; +const ERROR_MESSAGE = 'oneOf should not contain base types like integer, number, string, or boolean.'; + +export default (input, _, { path, documentInventory }) => { + if (!input.oneOf || !Array.isArray(input.oneOf)) { + return; + } + const schema = resolveObject(documentInventory.unresolved, path); + if (hasException(schema, RULE_NAME)) { + return; + } + + const errors = checkViolationsAndReturnErrors(schema, path); + if (errors.length !== 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } + collectAdoption(path, RULE_NAME); +}; + +function checkViolationsAndReturnErrors(schema, path) { + // Check if any oneOf item is a base type + const baseTypes = ['integer', 'number', 'string', 'boolean']; + const hasBaseType = schema.oneOf.some( + (item) => typeof item === 'object' && item.type && baseTypes.includes(item.type) + ); + + if (hasBaseType) { + return [{ path, message: ERROR_MESSAGE }]; + } + + return []; +}