From 69021aa880b1d1f9071bc1936361688c6ee31df6 Mon Sep 17 00:00:00 2001 From: "mongodb-sage-bot[bot]" <247496174+mongodb-sage-bot[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 09:47:17 +0000 Subject: [PATCH 1/4] CLOUDP-456528: Allow schema-level IPA-112 is-prefix exceptions ## Proposed changes Allow the `xgen-IPA-112-boolean-field-names-avoid-is-prefix` exception to be set on a schema as well as on an individual field. Before this change, only an exception on the field itself was accepted, so every new `is*` field on an existing schema (such as project/group settings) needed its own exception and another round of API champion approval. - Add `findExceptionInParentSchema` to `utils/exceptions.js`, modeled on `findExceptionInPathHierarchy`. It looks up the exception on the schema that directly contains the property. If both the field and its schema have the exception, it returns an unnecessary-exception error for the field. - Add `evaluateAndCollectAdoptionStatusWithInheritedException` to `collectionUtils.js`. For a field that violates the rule, it records the inherited exception (with the schema's reason) against the field's path, so schema-level exceptions are still counted in the IPA metrics. Fields that follow the rule are counted as adoptions and are not flagged as having an unnecessary exception. - Update `IPA112BooleanFieldNamesAvoidIsPrefix` to use these helpers. Field-level exceptions keep working as before. - Add the schema-level option to the rule's description (IPA-112.yaml and the generated rulesets README) and to the exceptions section of CONTRIBUTING.md. A schema-level exception applies only to the schema's direct properties. Inline nested object schemas need their own exception. _Jira ticket:_ CLOUDP-456528 ## Checklist - [ ] I have signed the [MongoDB CLA](https://www.mongodb.com/legal/contributor-agreement) - [x] I have added tests that prove my fix is effective or that my feature works ### Changes to Spectral - [x] I have read the [README](../tools/spectral/README.md) file for Spectral Updates ## Further comments Releasing a new IPA validation version, and switching the MMS group settings schema to a schema-level exception (removing the per-field exceptions), are follow-up steps outside this repository change. --- tools/spectral/ipa/CONTRIBUTING.md | 3 + ...A112BooleanFieldNamesAvoidIsPrefix.test.js | 106 ++++++++++++++++++ .../__tests__/utils/collectionUtils.test.js | 49 ++++++++ .../ipa/__tests__/utils/exceptions.test.js | 48 +++++++- tools/spectral/ipa/rulesets/IPA-112.yaml | 1 + tools/spectral/ipa/rulesets/README.md | 1 + .../IPA112BooleanFieldNamesAvoidIsPrefix.js | 24 +++- .../functions/utils/collectionUtils.js | 25 +++++ .../rulesets/functions/utils/exceptions.js | 29 +++++ 9 files changed, 284 insertions(+), 2 deletions(-) diff --git a/tools/spectral/ipa/CONTRIBUTING.md b/tools/spectral/ipa/CONTRIBUTING.md index 3c9c2df596..34c8e1e724 100644 --- a/tools/spectral/ipa/CONTRIBUTING.md +++ b/tools/spectral/ipa/CONTRIBUTING.md @@ -24,6 +24,9 @@ Instead of using the [Spectral overrides approach](https://docs.stoplight.io/doc "xgen-IPA-104-resource-has-GET": "Legacy API, not used by infrastructure-as-code tooling", } ``` + +Some rules allow an exception to be inherited from a parent component. For example, an exception for `xgen-IPA-112-boolean-field-names-avoid-is-prefix` can be set on a schema instead of on each of its fields, and applies to all fields of that schema. Inherited exceptions are still counted in the IPA metrics. See the rule's implementation details in the [rulesets README](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/rulesets/README.md) to check whether it supports inherited exceptions. + --- ## Testing diff --git a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js index a353e018aa..fa7461a9f1 100644 --- a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js +++ b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js @@ -177,6 +177,112 @@ testRule('xgen-IPA-112-boolean-field-names-avoid-is-prefix', [ }, errors: [], }, + { + name: 'schema with schema-level exception - applies to all fields of the schema', + document: { + components: { + schemas: { + SchemaName: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + properties: { + isEnabled: { type: 'boolean' }, + isActive: { type: 'boolean' }, + visible: { type: 'boolean' }, + nested: { + type: 'object', + properties: { + isNested: { type: 'boolean' }, + }, + }, + }, + }, + OtherSchema: { + type: 'object', + properties: { + isOther: { type: 'boolean' }, + }, + }, + }, + }, + paths: { + '/users': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2024-01-01+json': { + schema: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + properties: { + isAvailable: { type: 'boolean' }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + message: 'Boolean field "isNested" should not use the "is" prefix. Use "nested" instead.', + path: ['components', 'schemas', 'SchemaName', 'properties', 'nested', 'properties', 'isNested'], + severity: DiagnosticSeverity.Error, + }, + { + code: 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + message: 'Boolean field "isOther" should not use the "is" prefix. Use "other" instead.', + path: ['components', 'schemas', 'OtherSchema', 'properties', 'isOther'], + severity: DiagnosticSeverity.Error, + }, + ], + }, + { + name: 'schema with both schema-level and field-level exception - field-level exception is unnecessary', + document: { + components: { + schemas: { + SchemaName: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + properties: { + isEnabled: { + type: 'boolean', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'SchemaName', + 'properties', + 'isEnabled', + 'x-xgen-IPA-exception', + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + ], + severity: DiagnosticSeverity.Error, + }, + ], + }, { name: 'schema with referenced property types', document: { diff --git a/tools/spectral/ipa/__tests__/utils/collectionUtils.test.js b/tools/spectral/ipa/__tests__/utils/collectionUtils.test.js index 6e98fa5042..57de22163d 100644 --- a/tools/spectral/ipa/__tests__/utils/collectionUtils.test.js +++ b/tools/spectral/ipa/__tests__/utils/collectionUtils.test.js @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, jest } from '@jest/globals'; import { evaluateAndCollectAdoptionStatus, + evaluateAndCollectAdoptionStatusWithInheritedException, evaluateAndCollectAdoptionStatusWithoutExceptions, } from '../../rulesets/functions/utils/collectionUtils.js'; import collector from '../../metrics/collector.js'; @@ -162,4 +163,52 @@ describe('tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js', () => expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.ADOPTION, testPath, testRuleName); }); }); + + describe('evaluateAndCollectAdoptionStatusWithInheritedException', () => { + const testRuleName = 'xgen-IPA-XXX-rule'; + const testPath = ['components', 'schemas', 'Schema', 'properties', 'field']; + const testParentObject = { + properties: { field: {} }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-XXX-rule': 'reason', + }, + }; + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('returns empty and collects exceptions for the object when there are errors', () => { + const testErrors = [ + { + path: testPath, + message: TEST_ERROR_MESSAGE, + }, + ]; + + const result = evaluateAndCollectAdoptionStatusWithInheritedException( + testErrors, + testRuleName, + testParentObject, + testPath + ); + + expect(result).toEqual(undefined); + expect(collector.add).toHaveBeenCalledTimes(1); + expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.EXCEPTION, testPath, testRuleName, 'reason'); + }); + + it('returns empty and collects adoptions when there are no errors', () => { + const result = evaluateAndCollectAdoptionStatusWithInheritedException( + [], + testRuleName, + testParentObject, + testPath + ); + + expect(result).toEqual(undefined); + expect(collector.add).toHaveBeenCalledTimes(1); + expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.ADOPTION, testPath, testRuleName); + }); + }); }); diff --git a/tools/spectral/ipa/__tests__/utils/exceptions.test.js b/tools/spectral/ipa/__tests__/utils/exceptions.test.js index 6646e861a6..75d9c8cd6c 100644 --- a/tools/spectral/ipa/__tests__/utils/exceptions.test.js +++ b/tools/spectral/ipa/__tests__/utils/exceptions.test.js @@ -1,5 +1,5 @@ import { describe, expect, it } from '@jest/globals'; -import { hasException } from '../../rulesets/functions/utils/exceptions'; +import { findExceptionInParentSchema, hasException } from '../../rulesets/functions/utils/exceptions'; const TEST_RULE_NAME_100 = 'xgen-IPA-100'; @@ -52,4 +52,50 @@ describe('tools/spectral/ipa/rulesets/functions/utils/exceptions.js', () => { expect(hasException(objectWithNestedIpa100Exception, TEST_RULE_NAME_100)).toBe(false); }); }); + describe('findExceptionInParentSchema', () => { + const oas = { + components: { + schemas: { + SchemaWithException: { + ...objectWithIpa100Exception, + properties: { + field: {}, + fieldWithException: { ...objectWithIpa100Exception }, + }, + }, + SchemaWithoutException: { + properties: { + field: {}, + }, + }, + }, + }, + }; + + it('returns the schema if the parent schema has an exception', () => { + const path = ['components', 'schemas', 'SchemaWithException', 'properties', 'field']; + expect(findExceptionInParentSchema(oas, {}, path, TEST_RULE_NAME_100)).toEqual({ + schema: oas.components.schemas.SchemaWithException, + }); + }); + it('returns an error if both the property and the parent schema have an exception', () => { + const path = ['components', 'schemas', 'SchemaWithException', 'properties', 'fieldWithException']; + expect(findExceptionInParentSchema(oas, objectWithIpa100Exception, path, TEST_RULE_NAME_100)).toEqual({ + error: [ + { + path: [...path, 'x-xgen-IPA-exception', TEST_RULE_NAME_100], + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + }, + ], + }); + }); + it('returns null if the parent schema does not have an exception', () => { + const path = ['components', 'schemas', 'SchemaWithoutException', 'properties', 'field']; + expect(findExceptionInParentSchema(oas, {}, path, TEST_RULE_NAME_100)).toBeNull(); + expect(findExceptionInParentSchema(oas, {}, path, 'xgen-IPA-101')).toBeNull(); + }); + it('returns null if the path does not point to a schema property', () => { + expect(findExceptionInParentSchema(oas, {}, ['components', 'schemas'], TEST_RULE_NAME_100)).toBeNull(); + }); + }); }); diff --git a/tools/spectral/ipa/rulesets/IPA-112.yaml b/tools/spectral/ipa/rulesets/IPA-112.yaml index 3b1115aae8..c7a56aa23e 100644 --- a/tools/spectral/ipa/rulesets/IPA-112.yaml +++ b/tools/spectral/ipa/rulesets/IPA-112.yaml @@ -64,6 +64,7 @@ rules: - Applies only to properties with type 'boolean' - Identifies property names that start with "is" followed by an uppercase letter - Suggests using the direct adjective form instead (e.g., "disabled" instead of "isDisabled") + - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all of its fields message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-112-boolean-field-names-avoid-is-prefix' severity: error resolved: false diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 0816e17fb6..6486067eac 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -863,6 +863,7 @@ Rule checks for the following conditions: - Applies only to properties with type 'boolean' - Identifies property names that start with "is" followed by an uppercase letter - Suggests using the direct adjective form instead (e.g., "disabled" instead of "isDisabled") + - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all of its fields diff --git a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js index ae7567ed13..54bd5c5cb2 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js +++ b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js @@ -1,8 +1,22 @@ -import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { + evaluateAndCollectAdoptionStatus, + evaluateAndCollectAdoptionStatusWithInheritedException, + handleInternalError, +} from './utils/collectionUtils.js'; import { resolveObject } from './utils/componentUtils.js'; +import { findExceptionInParentSchema } from './utils/exceptions.js'; const IS_PREFIX_REGEX = /^is[A-Z]/; +/** + * Checks that boolean field names do not use the "is" prefix + * + * The exception can be set on the field or on the schema containing the field. An exception on the schema applies to all of its fields. + * + * @param {string} input - The field name + * @param {object} options - Rule configuration options + * @param {object} context - The context object containing the path and documentInventory + */ export default (input, options, { path, documentInventory, rule }) => { const ruleName = rule.name; const oas = documentInventory.unresolved; @@ -14,7 +28,15 @@ export default (input, options, { path, documentInventory, rule }) => { return; } + const result = findExceptionInParentSchema(oas, property, path, ruleName); + if (result?.error) { + return result.error; + } + const errors = checkViolationsAndReturnErrors(input, path, ruleName); + if (result) { + return evaluateAndCollectAdoptionStatusWithInheritedException(errors, ruleName, result.schema, path); + } return evaluateAndCollectAdoptionStatus(errors, ruleName, property, path); }; diff --git a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js index eba0e27625..382d7e869a 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js @@ -27,6 +27,31 @@ export function evaluateAndCollectAdoptionStatus(validationErrors, ruleName, obj } } +/** + * Evaluates and collects adoptions and exceptions for an object inheriting an exception from a parent object, + * for example a schema property inheriting an exception from its schema. + * If the object is violating the rule, the validation error is ignored and the inherited exception is collected. + * If the object is adopting the rule, it is counted as adopting the rule. The inherited exception is not reported as + * unnecessary, since it may still be needed by other objects inheriting it. + * + * @param {Array<{path: Array, message: string}>} validationErrors the error results from the rule + * @param {string} ruleName the name of the rule + * @param {*} parentObject the parent object containing the exception + * @param {Array} objectPath the JSON path to the evaluated object + */ +export function evaluateAndCollectAdoptionStatusWithInheritedException( + validationErrors, + ruleName, + parentObject, + objectPath +) { + if (validationErrors.length !== 0) { + collectException(parentObject, ruleName, objectPath); + return; + } + collectAdoption(objectPath, ruleName); +} + /** * Evaluates and collects adoptions and violations based on the rule, evaluated object and the validation errors. * No exceptions are allowed. diff --git a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js index 1edf519735..40f1682bac 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js +++ b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js @@ -1,3 +1,5 @@ +import { resolveObject } from './componentUtils.js'; + export const EXCEPTION_EXTENSION = 'x-xgen-IPA-exception'; /** @@ -53,3 +55,30 @@ export function findExceptionInPathHierarchy(oas, currentPath, ruleName, jsonPat return null; } + +/** + * Finds an exception for a specified rule name in the schema containing the given property. + * An exception on the schema applies to all of its properties. + * + * @param {Object} oas - The OpenAPI Specification object. + * @param {Object} property - The property object being checked. + * @param {Array} propertyPath - The JSON path to the property, ending with ['properties', propertyName]. + * @param {string} ruleName - The name of the rule to check for exceptions. + * @return {null|Object} An object with the schema containing the rule exception if found, an object with an error + * if both the property and the schema have the exception, or null if the schema has no exception. + */ +export function findExceptionInParentSchema(oas, property, propertyPath, ruleName) { + if (propertyPath.length < 2 || propertyPath[propertyPath.length - 2] !== 'properties') { + return null; + } + + const schema = resolveObject(oas, propertyPath.slice(0, -2)); + if (!schema || !hasException(schema, ruleName)) { + return null; + } + + if (hasException(property, ruleName)) { + return { error: getUnnecessaryExceptionError(propertyPath, ruleName) }; + } + return { schema: schema }; +} From 0ddf006d04dad50aa8412532f89901dc522eb126 Mon Sep 17 00:00:00 2001 From: Maks Matsveyeu Date: Mon, 12 Oct 2026 12:40:07 +0100 Subject: [PATCH 2/4] fix(ipa): flag unnecessary schema-level exceptions and correct message - Report a schema-level IPA-112 exception as unnecessary when no field of the schema violates the rule - Report a field-level exception covered by a schema-level exception with an accurate message instead of "This component adopts the rule" - Guard findExceptionInParentSchema against undefined properties - Document that schema-level exceptions apply to direct properties only; fields in nested inline schemas or composition need their own exception --- tools/spectral/ipa/CONTRIBUTING.md | 2 +- ...A112BooleanFieldNamesAvoidIsPrefix.test.js | 38 +++++++++++++++++- .../ipa/__tests__/utils/exceptions.test.js | 9 ++++- tools/spectral/ipa/rulesets/IPA-112.yaml | 3 +- tools/spectral/ipa/rulesets/README.md | 3 +- .../IPA112BooleanFieldNamesAvoidIsPrefix.js | 28 ++++++++++++- .../rulesets/functions/utils/exceptions.js | 40 ++++++++++++++++++- 7 files changed, 114 insertions(+), 9 deletions(-) diff --git a/tools/spectral/ipa/CONTRIBUTING.md b/tools/spectral/ipa/CONTRIBUTING.md index 34c8e1e724..d7b90b31d0 100644 --- a/tools/spectral/ipa/CONTRIBUTING.md +++ b/tools/spectral/ipa/CONTRIBUTING.md @@ -25,7 +25,7 @@ Instead of using the [Spectral overrides approach](https://docs.stoplight.io/doc } ``` -Some rules allow an exception to be inherited from a parent component. For example, an exception for `xgen-IPA-112-boolean-field-names-avoid-is-prefix` can be set on a schema instead of on each of its fields, and applies to all fields of that schema. Inherited exceptions are still counted in the IPA metrics. See the rule's implementation details in the [rulesets README](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/rulesets/README.md) to check whether it supports inherited exceptions. +Some rules allow an exception to be inherited from a parent component. For example, an exception for `xgen-IPA-112-boolean-field-names-avoid-is-prefix` can be set on a schema instead of on each of its fields, and applies to all fields defined directly on that schema. Fields in nested inline schemas or in `allOf`, `anyOf` or `oneOf` composition need their own exception. Inherited exceptions are still counted in the IPA metrics. See the rule's implementation details in the [rulesets README](https://github.com/mongodb/openapi/tree/main/tools/spectral/ipa/rulesets/README.md) to check whether it supports inherited exceptions. --- ## Testing diff --git a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js index fa7461a9f1..22604893ec 100644 --- a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js +++ b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js @@ -269,7 +269,8 @@ testRule('xgen-IPA-112-boolean-field-names-avoid-is-prefix', [ errors: [ { code: 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', - message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + message: + 'This component is covered by an exception on its containing schema and does not need its own exception. Please remove the exception.', path: [ 'components', 'schemas', @@ -283,6 +284,41 @@ testRule('xgen-IPA-112-boolean-field-names-avoid-is-prefix', [ }, ], }, + { + name: 'schema with unnecessary schema-level exception - no violating fields', + document: { + components: { + schemas: { + SchemaName: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + properties: { + enabled: { type: 'boolean' }, + active: { type: 'boolean' }, + name: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + message: + 'This component does not need an exception because none of its fields violate the rule. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'SchemaName', + 'x-xgen-IPA-exception', + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + ], + severity: DiagnosticSeverity.Error, + }, + ], + }, { name: 'schema with referenced property types', document: { diff --git a/tools/spectral/ipa/__tests__/utils/exceptions.test.js b/tools/spectral/ipa/__tests__/utils/exceptions.test.js index 75d9c8cd6c..1ee27555d1 100644 --- a/tools/spectral/ipa/__tests__/utils/exceptions.test.js +++ b/tools/spectral/ipa/__tests__/utils/exceptions.test.js @@ -84,11 +84,18 @@ describe('tools/spectral/ipa/rulesets/functions/utils/exceptions.js', () => { error: [ { path: [...path, 'x-xgen-IPA-exception', TEST_RULE_NAME_100], - message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + message: + 'This component is covered by an exception on its containing schema and does not need its own exception. Please remove the exception.', }, ], }); }); + it('returns the schema if the property is undefined and the parent schema has an exception', () => { + const path = ['components', 'schemas', 'SchemaWithException', 'properties', 'field']; + expect(findExceptionInParentSchema(oas, undefined, path, TEST_RULE_NAME_100)).toEqual({ + schema: oas.components.schemas.SchemaWithException, + }); + }); it('returns null if the parent schema does not have an exception', () => { const path = ['components', 'schemas', 'SchemaWithoutException', 'properties', 'field']; expect(findExceptionInParentSchema(oas, {}, path, TEST_RULE_NAME_100)).toBeNull(); diff --git a/tools/spectral/ipa/rulesets/IPA-112.yaml b/tools/spectral/ipa/rulesets/IPA-112.yaml index c7a56aa23e..68255d8942 100644 --- a/tools/spectral/ipa/rulesets/IPA-112.yaml +++ b/tools/spectral/ipa/rulesets/IPA-112.yaml @@ -64,7 +64,8 @@ rules: - Applies only to properties with type 'boolean' - Identifies property names that start with "is" followed by an uppercase letter - Suggests using the direct adjective form instead (e.g., "disabled" instead of "isDisabled") - - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all of its fields + - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all fields defined directly on that schema + - Fields in nested inline schemas or in allOf, anyOf or oneOf composition need their own exception message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-112-boolean-field-names-avoid-is-prefix' severity: error resolved: false diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 6486067eac..93a36b9d9a 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -863,7 +863,8 @@ Rule checks for the following conditions: - Applies only to properties with type 'boolean' - Identifies property names that start with "is" followed by an uppercase letter - Suggests using the direct adjective form instead (e.g., "disabled" instead of "isDisabled") - - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all of its fields + - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all fields defined directly on that schema + - Fields in nested inline schemas or in allOf, anyOf or oneOf composition need their own exception diff --git a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js index 54bd5c5cb2..2055bea187 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js +++ b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js @@ -4,7 +4,7 @@ import { handleInternalError, } from './utils/collectionUtils.js'; import { resolveObject } from './utils/componentUtils.js'; -import { findExceptionInParentSchema } from './utils/exceptions.js'; +import { findExceptionInParentSchema, getUnnecessarySchemaExceptionError } from './utils/exceptions.js'; const IS_PREFIX_REGEX = /^is[A-Z]/; @@ -35,11 +35,35 @@ export default (input, options, { path, documentInventory, rule }) => { const errors = checkViolationsAndReturnErrors(input, path, ruleName); if (result) { - return evaluateAndCollectAdoptionStatusWithInheritedException(errors, ruleName, result.schema, path); + const inheritedResult = evaluateAndCollectAdoptionStatusWithInheritedException( + errors, + ruleName, + result.schema, + path + ); + if (errors.length === 0 && !schemaNeedsException(result.schema)) { + // No field of the schema needs the schema-level exception. Every field of the schema + // reports the same error, which Spectral deduplicates into a single result. + return getUnnecessarySchemaExceptionError(path.slice(0, -2), ruleName); + } + return inheritedResult; } return evaluateAndCollectAdoptionStatus(errors, ruleName, property, path); }; +/** + * Checks whether the schema-level exception is needed by at least one field defined directly on the schema. + * A field needs the exception if it is a boolean field using the "is" prefix. + * + * @param {object} schema - The schema containing the exception + * @returns {boolean} true if at least one field of the schema violates the rule, false otherwise + */ +function schemaNeedsException(schema) { + return Object.entries(schema.properties ?? {}).some( + ([fieldName, fieldSchema]) => fieldSchema?.type === 'boolean' && IS_PREFIX_REGEX.test(fieldName) + ); +} + function checkViolationsAndReturnErrors(input, path, ruleName) { try { if (IS_PREFIX_REGEX.test(input)) { diff --git a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js index 40f1682bac..ad4a8855e4 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js +++ b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js @@ -25,6 +25,42 @@ export function getUnnecessaryExceptionError(objectPath, ruleName) { ]; } +/** + * Creates an error for an exception on an object that is already covered by an exception + * on its containing schema. The object does not need its own exception, regardless of whether + * it violates or adopts the rule. + * + * @param {Array} objectPath the JSON path to the object with the unnecessary exception + * @param {string} ruleName the name of the rule + * @returns {Array<{path: Array, message: string}>} an array with the unnecessary exception error + */ +export function getUnnecessaryInheritedExceptionError(objectPath, ruleName) { + return [ + { + path: [...objectPath, EXCEPTION_EXTENSION, ruleName], + message: + 'This component is covered by an exception on its containing schema and does not need its own exception. Please remove the exception.', + }, + ]; +} + +/** + * Creates an error for a schema-level exception that is not needed by any field of the schema. + * + * @param {Array} schemaPath the JSON path to the schema with the unnecessary exception + * @param {string} ruleName the name of the rule + * @returns {Array<{path: Array, message: string}>} an array with the unnecessary exception error + */ +export function getUnnecessarySchemaExceptionError(schemaPath, ruleName) { + return [ + { + path: [...schemaPath, EXCEPTION_EXTENSION, ruleName], + message: + 'This component does not need an exception because none of its fields violate the rule. Please remove the exception.', + }, + ]; +} + /** * Finds an exception for a specified rule name in the current path or its parent paths within the given OpenAPI Specification (OAS) object. * @@ -77,8 +113,8 @@ export function findExceptionInParentSchema(oas, property, propertyPath, ruleNam return null; } - if (hasException(property, ruleName)) { - return { error: getUnnecessaryExceptionError(propertyPath, ruleName) }; + if (property && hasException(property, ruleName)) { + return { error: getUnnecessaryInheritedExceptionError(propertyPath, ruleName) }; } return { schema: schema }; } From fa68c9ee7483a9816704c392b799b3400055db99 Mon Sep 17 00:00:00 2001 From: Maks Matsveyeu Date: Mon, 12 Oct 2026 12:54:08 +0100 Subject: [PATCH 3/4] fix(ipa): flag unnecessary schema exceptions without direct boolean fields - Run the schema-level exception necessity check for non-boolean properties too, so a stale exception is reported even when the schema has no direct boolean fields - Document the unnecessary-exception behavior and metrics counting in the rule description --- ...A112BooleanFieldNamesAvoidIsPrefix.test.js | 45 +++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-112.yaml | 4 +- tools/spectral/ipa/rulesets/README.md | 4 +- .../IPA112BooleanFieldNamesAvoidIsPrefix.js | 17 +++++-- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js index 22604893ec..ac898612ad 100644 --- a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js +++ b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js @@ -319,6 +319,51 @@ testRule('xgen-IPA-112-boolean-field-names-avoid-is-prefix', [ }, ], }, + { + name: 'schema with schema-level exception whose only violating field is nested - exception is unnecessary', + document: { + components: { + schemas: { + SchemaName: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + properties: { + name: { type: 'string' }, + nested: { + type: 'object', + properties: { + isNested: { type: 'boolean' }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + message: + 'This component does not need an exception because none of its fields violate the rule. Please remove the exception.', + path: [ + 'components', + 'schemas', + 'SchemaName', + 'x-xgen-IPA-exception', + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + ], + severity: DiagnosticSeverity.Error, + }, + { + code: 'xgen-IPA-112-boolean-field-names-avoid-is-prefix', + message: 'Boolean field "isNested" should not use the "is" prefix. Use "nested" instead.', + path: ['components', 'schemas', 'SchemaName', 'properties', 'nested', 'properties', 'isNested'], + severity: DiagnosticSeverity.Error, + }, + ], + }, { name: 'schema with referenced property types', document: { diff --git a/tools/spectral/ipa/rulesets/IPA-112.yaml b/tools/spectral/ipa/rulesets/IPA-112.yaml index 68255d8942..ab3f6e1df6 100644 --- a/tools/spectral/ipa/rulesets/IPA-112.yaml +++ b/tools/spectral/ipa/rulesets/IPA-112.yaml @@ -64,7 +64,9 @@ rules: - Applies only to properties with type 'boolean' - Identifies property names that start with "is" followed by an uppercase letter - Suggests using the direct adjective form instead (e.g., "disabled" instead of "isDisabled") - - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all fields defined directly on that schema + - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all fields defined directly on that schema and is counted in the IPA exception metrics + - A field-level exception is unnecessary when the schema containing the field already has one + - A schema-level exception is unnecessary when no field defined directly on the schema violates the rule - Fields in nested inline schemas or in allOf, anyOf or oneOf composition need their own exception message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-112-boolean-field-names-avoid-is-prefix' severity: error diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 93a36b9d9a..64d07a86a7 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -863,7 +863,9 @@ Rule checks for the following conditions: - Applies only to properties with type 'boolean' - Identifies property names that start with "is" followed by an uppercase letter - Suggests using the direct adjective form instead (e.g., "disabled" instead of "isDisabled") - - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all fields defined directly on that schema + - Exceptions can be set on the field or on the schema containing the field. An exception on the schema applies to all fields defined directly on that schema and is counted in the IPA exception metrics + - A field-level exception is unnecessary when the schema containing the field already has one + - A schema-level exception is unnecessary when no field defined directly on the schema violates the rule - Fields in nested inline schemas or in allOf, anyOf or oneOf composition need their own exception diff --git a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js index 2055bea187..bca079e581 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js +++ b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js @@ -11,7 +11,8 @@ const IS_PREFIX_REGEX = /^is[A-Z]/; /** * Checks that boolean field names do not use the "is" prefix * - * The exception can be set on the field or on the schema containing the field. An exception on the schema applies to all of its fields. + * The exception can be set on the field or on the schema containing the field. An exception on the schema + * applies to all fields defined directly on that schema and is reported as unnecessary when no field needs it. * * @param {string} input - The field name * @param {object} options - Rule configuration options @@ -22,13 +23,23 @@ export default (input, options, { path, documentInventory, rule }) => { const oas = documentInventory.unresolved; const property = resolveObject(oas, path); + if (!property) { + return; + } + + const result = findExceptionInParentSchema(oas, property, path, ruleName); + // Skip schema references ($ref) and non-boolean fields: // Referenced schemas are validated separately to prevent duplicate violations - if (!property || property.type !== 'boolean') { + if (property.type !== 'boolean') { + // The rule only validates boolean fields, but the schema-level exception is still + // reported as unnecessary if none of the fields of the schema need it. + if (result && !schemaNeedsException(result.schema)) { + return getUnnecessarySchemaExceptionError(path.slice(0, -2), ruleName); + } return; } - const result = findExceptionInParentSchema(oas, property, path, ruleName); if (result?.error) { return result.error; } From 123d6210f1cd1c5dac0eb5097b180b78537df843 Mon Sep 17 00:00:00 2001 From: Maks Matsveyeu Date: Mon, 12 Oct 2026 17:28:30 +0100 Subject: [PATCH 4/4] refactor(ipa): compact exception helpers and guard non-boolean fields - Merge the three unnecessary-exception error constructors into one shared helper - Simplify the IPA-112 rule flow and guard against a missing parent schema when a field has its own exception (fixes a crash for non-boolean fields with both exceptions) - Add a regression test for the non-boolean field case --- ...A112BooleanFieldNamesAvoidIsPrefix.test.js | 25 ++++++++ .../IPA112BooleanFieldNamesAvoidIsPrefix.js | 38 +++++------- .../functions/utils/collectionUtils.js | 8 +-- .../rulesets/functions/utils/exceptions.js | 59 ++++++------------- 4 files changed, 62 insertions(+), 68 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js index ac898612ad..dc7b6c97cd 100644 --- a/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js +++ b/tools/spectral/ipa/__tests__/IPA112BooleanFieldNamesAvoidIsPrefix.test.js @@ -507,4 +507,29 @@ testRule('xgen-IPA-112-boolean-field-names-avoid-is-prefix', [ }, ], }, + { + name: 'non-boolean field with its own exception and a schema-level exception', + document: { + components: { + schemas: { + SchemaName: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + properties: { + isEnabled: { type: 'boolean' }, + name: { + type: 'string', + 'x-xgen-IPA-exception': { + 'xgen-IPA-112-boolean-field-names-avoid-is-prefix': 'Reason', + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, ]); diff --git a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js index bca079e581..62b8371500 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js +++ b/tools/spectral/ipa/rulesets/functions/IPA112BooleanFieldNamesAvoidIsPrefix.js @@ -29,15 +29,18 @@ export default (input, options, { path, documentInventory, rule }) => { const result = findExceptionInParentSchema(oas, property, path, ruleName); - // Skip schema references ($ref) and non-boolean fields: - // Referenced schemas are validated separately to prevent duplicate violations + // A schema-level exception is unnecessary if no field defined directly on the schema needs it. + // Every field of the schema reports the same error, which Spectral deduplicates into a single result. + const unnecessarySchemaException = + result?.schema && !schemaNeedsException(result.schema) + ? getUnnecessarySchemaExceptionError(path.slice(0, -2), ruleName) + : undefined; + + // Skip schema references ($ref) and non-boolean fields: referenced schemas are validated + // separately to prevent duplicate violations. The schema-level exception is still checked + // for non-boolean fields, since it applies to the whole schema. if (property.type !== 'boolean') { - // The rule only validates boolean fields, but the schema-level exception is still - // reported as unnecessary if none of the fields of the schema need it. - if (result && !schemaNeedsException(result.schema)) { - return getUnnecessarySchemaExceptionError(path.slice(0, -2), ruleName); - } - return; + return unnecessarySchemaException; } if (result?.error) { @@ -45,21 +48,12 @@ export default (input, options, { path, documentInventory, rule }) => { } const errors = checkViolationsAndReturnErrors(input, path, ruleName); - if (result) { - const inheritedResult = evaluateAndCollectAdoptionStatusWithInheritedException( - errors, - ruleName, - result.schema, - path - ); - if (errors.length === 0 && !schemaNeedsException(result.schema)) { - // No field of the schema needs the schema-level exception. Every field of the schema - // reports the same error, which Spectral deduplicates into a single result. - return getUnnecessarySchemaExceptionError(path.slice(0, -2), ruleName); - } - return inheritedResult; + if (!result) { + return evaluateAndCollectAdoptionStatus(errors, ruleName, property, path); } - return evaluateAndCollectAdoptionStatus(errors, ruleName, property, path); + + const inheritedResult = evaluateAndCollectAdoptionStatusWithInheritedException(errors, ruleName, result.schema, path); + return unnecessarySchemaException ?? inheritedResult; }; /** diff --git a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js index 382d7e869a..7c292549d1 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js @@ -28,11 +28,9 @@ export function evaluateAndCollectAdoptionStatus(validationErrors, ruleName, obj } /** - * Evaluates and collects adoptions and exceptions for an object inheriting an exception from a parent object, - * for example a schema property inheriting an exception from its schema. - * If the object is violating the rule, the validation error is ignored and the inherited exception is collected. - * If the object is adopting the rule, it is counted as adopting the rule. The inherited exception is not reported as - * unnecessary, since it may still be needed by other objects inheriting it. + * Evaluates and collects the adoption or the inherited exception for an object whose exception lives on a parent + * object, for example a schema property inheriting an exception from its schema. The inherited exception is not + * reported as unnecessary, since it may still be needed by other objects inheriting it. * * @param {Array<{path: Array, message: string}>} validationErrors the error results from the rule * @param {string} ruleName the name of the rule diff --git a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js index ad4a8855e4..ffd812ea2a 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js +++ b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js @@ -16,49 +16,27 @@ export function hasException(object, ruleName) { return false; } +const UNNECESSARY_EXCEPTION_MESSAGE = + 'This component adopts the rule and does not need an exception. Please remove the exception.'; +const UNNECESSARY_INHERITED_EXCEPTION_MESSAGE = + 'This component is covered by an exception on its containing schema and does not need its own exception. Please remove the exception.'; +const UNNECESSARY_SCHEMA_EXCEPTION_MESSAGE = + 'This component does not need an exception because none of its fields violate the rule. Please remove the exception.'; + +function unnecessaryExceptionError(objectPath, ruleName, message) { + return [{ path: [...objectPath, EXCEPTION_EXTENSION, ruleName], message }]; +} + export function getUnnecessaryExceptionError(objectPath, ruleName) { - return [ - { - path: [...objectPath, EXCEPTION_EXTENSION, ruleName], - message: 'This component adopts the rule and does not need an exception. Please remove the exception.', - }, - ]; + return unnecessaryExceptionError(objectPath, ruleName, UNNECESSARY_EXCEPTION_MESSAGE); } -/** - * Creates an error for an exception on an object that is already covered by an exception - * on its containing schema. The object does not need its own exception, regardless of whether - * it violates or adopts the rule. - * - * @param {Array} objectPath the JSON path to the object with the unnecessary exception - * @param {string} ruleName the name of the rule - * @returns {Array<{path: Array, message: string}>} an array with the unnecessary exception error - */ export function getUnnecessaryInheritedExceptionError(objectPath, ruleName) { - return [ - { - path: [...objectPath, EXCEPTION_EXTENSION, ruleName], - message: - 'This component is covered by an exception on its containing schema and does not need its own exception. Please remove the exception.', - }, - ]; + return unnecessaryExceptionError(objectPath, ruleName, UNNECESSARY_INHERITED_EXCEPTION_MESSAGE); } -/** - * Creates an error for a schema-level exception that is not needed by any field of the schema. - * - * @param {Array} schemaPath the JSON path to the schema with the unnecessary exception - * @param {string} ruleName the name of the rule - * @returns {Array<{path: Array, message: string}>} an array with the unnecessary exception error - */ -export function getUnnecessarySchemaExceptionError(schemaPath, ruleName) { - return [ - { - path: [...schemaPath, EXCEPTION_EXTENSION, ruleName], - message: - 'This component does not need an exception because none of its fields violate the rule. Please remove the exception.', - }, - ]; +export function getUnnecessarySchemaExceptionError(objectPath, ruleName) { + return unnecessaryExceptionError(objectPath, ruleName, UNNECESSARY_SCHEMA_EXCEPTION_MESSAGE); } /** @@ -93,15 +71,14 @@ export function findExceptionInPathHierarchy(oas, currentPath, ruleName, jsonPat } /** - * Finds an exception for a specified rule name in the schema containing the given property. - * An exception on the schema applies to all of its properties. + * Finds an exception for a specified rule name in the schema directly containing the given property. + * The exception applies to all fields defined directly on that schema. * * @param {Object} oas - The OpenAPI Specification object. * @param {Object} property - The property object being checked. * @param {Array} propertyPath - The JSON path to the property, ending with ['properties', propertyName]. * @param {string} ruleName - The name of the rule to check for exceptions. - * @return {null|Object} An object with the schema containing the rule exception if found, an object with an error - * if both the property and the schema have the exception, or null if the schema has no exception. + * @return {null|Object} the schema with the exception, an error if both the property and the schema have one, or null */ export function findExceptionInParentSchema(oas, property, propertyPath, ruleName) { if (propertyPath.length < 2 || propertyPath[propertyPath.length - 2] !== 'properties') {