diff --git a/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js index 401a2a7996..24867dfee1 100644 --- a/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js +++ b/tools/spectral/ipa/__tests__/IPA125OneOfNoBaseTypes.test.js @@ -70,7 +70,7 @@ testRule('xgen-IPA-125-oneOf-no-base-types', [ errors: [ { code: 'xgen-IPA-125-oneOf-no-base-types', - message: 'oneOf should not contain base types like integer, number, string, or boolean.', + message: 'oneOf should not mix base types with references.', path: ['components', 'schemas', 'MixedType'], severity: DiagnosticSeverity.Warning, }, @@ -91,12 +91,29 @@ testRule('xgen-IPA-125-oneOf-no-base-types', [ errors: [ { code: 'xgen-IPA-125-oneOf-no-base-types', - message: 'oneOf should not contain base types like integer, number, string, or boolean.', + message: 'oneOf should not contain multiple different base types.', path: ['components', 'schemas', 'BaseTypes'], severity: DiagnosticSeverity.Warning, }, ], }, + { + name: 'valid oneOf with same base type multiple times', + document: { + components: { + schemas: { + ...componentSchemas, + SameBaseType: { + oneOf: [ + { type: 'string', enum: ['one'] }, + { type: 'string', enum: ['two'] }, + ], + }, + }, + }, + }, + errors: [], + }, { name: 'oneOf with exception', document: { diff --git a/tools/spectral/ipa/rulesets/IPA-125.yaml b/tools/spectral/ipa/rulesets/IPA-125.yaml index 6c87d7fecf..0de89e318d 100644 --- a/tools/spectral/ipa/rulesets/IPA-125.yaml +++ b/tools/spectral/ipa/rulesets/IPA-125.yaml @@ -43,46 +43,21 @@ rules: xgen-IPA-125-oneOf-no-base-types: description: | - API producers should not use oneOf with base types like integer, string, boolean, or number. + API producers should not use oneOf with different base types like integer, string, boolean, or number or references at the same time. ##### 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 + - Ensures no mixing of base types with references + - Ensures no multiple different base types in the same oneOf - Base types considered are: integer, string, boolean, number + - Using the same base type multiple times is allowed (e.g., multiple string enums) ##### Rationale - Using oneOf with primitive types can lead to ambiguity and validation problems. Clients may not + Using oneOf with multiple 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: warn given: '$.components.schemas[*]' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 184732fd1f..962f8d4d32 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -560,46 +560,21 @@ Rule checks for the following conditions: #### xgen-IPA-125-oneOf-no-base-types ![warn](https://img.shields.io/badge/warning-yellow) -API producers should not use oneOf with base types like integer, string, boolean, or number. +API producers should not use oneOf with different base types like integer, string, boolean, or number or references at the same time. ##### 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 + - Ensures no mixing of base types with references + - Ensures no multiple different base types in the same oneOf - Base types considered are: integer, string, boolean, number + - Using the same base type multiple times is allowed (e.g., multiple string enums) ##### Rationale -Using oneOf with primitive types can lead to ambiguity and validation problems. Clients may not +Using oneOf with multiple 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/IPA125OneOfNoBaseTypes.js b/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js index c9aa54394f..9bb3b2f89c 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js +++ b/tools/spectral/ipa/rulesets/functions/IPA125OneOfNoBaseTypes.js @@ -3,7 +3,8 @@ 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.'; +const ERROR_MESSAGE_MIXED = 'oneOf should not mix base types with references.'; +const ERROR_MESSAGE_MULTIPLE = 'oneOf should not contain multiple different base types.'; export default (input, _, { path, documentInventory }) => { if (!input.oneOf || !Array.isArray(input.oneOf)) { @@ -22,14 +23,30 @@ export default (input, _, { path, documentInventory }) => { }; 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 }]; + const baseTypes = ['string', 'number', 'integer', 'boolean']; + const foundBaseTypes = new Set(); + let hasRef = false; + let hasBaseType = false; + + // Check each oneOf item + for (const item of schema.oneOf) { + if (item.$ref) { + hasRef = true; + continue; + } + + if (item.type && baseTypes.includes(item.type)) { + hasBaseType = true; + foundBaseTypes.add(item.type); + } + } + + if (hasRef && hasBaseType) { + return [{ path, message: ERROR_MESSAGE_MIXED }]; + } + + if (foundBaseTypes.size > 1) { + return [{ path, message: ERROR_MESSAGE_MULTIPLE }]; } return [];