diff --git a/.secrets.baseline b/.secrets.baseline
index 97e9df08..cb496540 100644
--- a/.secrets.baseline
+++ b/.secrets.baseline
@@ -3,7 +3,7 @@
"files": "package-lock.json|^.secrets.baseline$",
"lines": null
},
- "generated_at": "2024-12-19T16:14:03Z",
+ "generated_at": "2025-01-09T19:49:59Z",
"plugins_used": [
{
"name": "AWSKeyDetector"
diff --git a/docs/ibm-cloud-rules.md b/docs/ibm-cloud-rules.md
index 09c5d0bf..c0f01da2 100644
--- a/docs/ibm-cloud-rules.md
+++ b/docs/ibm-cloud-rules.md
@@ -7328,8 +7328,8 @@ paths:
This rule validates that any dictionary schemas are well defined and that all values share a single type.
Dictionaries are defined as object type schemas that have variable key names. They are distinct from model types,
which are objects with pre-defined properties. A schema must not define both concrete properties and variable key names.
- Practically, this means a schema must explicitly define a `properties` object or an `additionalProperties` schema, but not both.
- If used, the `additionalProperties` schema must define a concrete type. The concrete type of the values must not be a dictionary itself. See the IBM Cloud API Handbook documentation on types for more info.
+ Practically, this means a schema must explicitly define a `properties` object or an `(additional|pattern)Properties` schema, but not both.
+ If used, the `(additional|pattern)Properties` schema must define a concrete type. The concrete type of the values must not be a dictionary itself. See the IBM Cloud API Handbook documentation on types for more info.
diff --git a/packages/ruleset/src/functions/well-defined-dictionaries.js b/packages/ruleset/src/functions/well-defined-dictionaries.js
index e8661e59..cde853c5 100644
--- a/packages/ruleset/src/functions/well-defined-dictionaries.js
+++ b/packages/ruleset/src/functions/well-defined-dictionaries.js
@@ -1,5 +1,5 @@
/**
- * Copyright 2024 IBM Corporation.
+ * Copyright 2024 - 2025 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/
@@ -7,7 +7,6 @@ const {
isObject,
isObjectSchema,
schemaHasConstraint,
- schemaLooselyHasConstraint,
validateNestedSchemas,
} = require('@ibm-cloud/openapi-ruleset-utilities');
const { LoggerFactory } = require('../utils');
@@ -15,6 +14,13 @@ const { LoggerFactory } = require('../utils');
let ruleId;
let logger;
+/**
+ * The implementation for this rule makes assumptions that are dependent on the
+ * presence of the following other rules:
+ *
+ * - ibm-pattern-properties: patternProperties isn't empty or the wrong type
+ */
+
module.exports = function (schema, _opts, context) {
if (!logger) {
ruleId = context.rule.name;
@@ -32,7 +38,7 @@ function wellDefinedDictionaries(schema, path) {
// We will flag dictionaries of dictionaries, so we can skip
// providing guidance for directly nested dictionaries.
- if (path.at(-1) === 'additionalProperties') {
+ if (isDictionaryValueSchema(path)) {
return [];
}
@@ -40,15 +46,15 @@ function wellDefinedDictionaries(schema, path) {
`${ruleId}: checking object schema at location: ${path.join('.')}`
);
- // Dictionaries should have additionalProperties defined on them.
- // If the schema doesn't, make sure it has properties and then
- // abandon the check.
- if (!schemaDefinesField(schema, 'additionalProperties')) {
+ // Dictionaries should have additionalProperties or patternProperties
+ // defined on them. If the schema doesn't, make sure it has properties
+ // and then abandon the check.
+ if (!isDictionarySchema(schema)) {
if (!schemaDefinesField(schema, 'properties')) {
return [
{
message:
- 'Object schemas must define either properties, or additionalProperties with a concrete type',
+ 'Object schemas must define either properties, or (additional/pattern)Properties with a concrete type',
path,
},
];
@@ -79,8 +85,7 @@ function wellDefinedDictionaries(schema, path) {
// more strict in the future but this meets our current purposes.
if (schemaHasConstraint(schema, isAmbiguousDictionary)) {
errors.push({
- message:
- 'Dictionary schemas must have a single, well-defined value type in `additionalProperties`',
+ message: 'Dictionary schemas must have a single, well-defined value type',
path,
});
}
@@ -89,7 +94,7 @@ function wellDefinedDictionaries(schema, path) {
// should not be dictionaries themselves.
if (schemaHasConstraint(schema, isDictionaryOfDictionaries)) {
errors.push({
- message: 'Dictionaries must not have values that are also dictionaries.',
+ message: 'Dictionaries must not have values that are also dictionaries',
path,
});
}
@@ -102,29 +107,46 @@ function schemaDefinesField(schema, field) {
}
function isAmbiguousDictionary(schema) {
- if (!schema.additionalProperties) {
- return false;
- }
-
- // additionalProperties must be an object (not a boolean) value
- // and must define a `type` field in order to be considered an
- // unambiguous dictionary.
- return (
- !isObject(schema.additionalProperties) ||
- !schemaDefinesField(schema.additionalProperties, 'type')
+ return dictionaryValuesHaveConstraint(
+ schema,
+ valueSchema =>
+ !isObject(valueSchema) || !schemaDefinesField(valueSchema, 'type')
);
}
function isDictionaryOfDictionaries(schema) {
- if (!isObject(schema.additionalProperties)) {
+ return dictionaryValuesHaveConstraint(
+ schema,
+ valueSchema => isObject(valueSchema) && isDictionarySchema(valueSchema)
+ );
+}
+
+function dictionaryValuesHaveConstraint(schema, hasConstraint) {
+ return schemaHasConstraint(schema, s => {
+ if (s.additionalProperties !== undefined) {
+ return hasConstraint(s.additionalProperties);
+ }
+
+ if (s.patternProperties !== undefined) {
+ return Object.values(s.patternProperties).some(p => hasConstraint(p));
+ }
+
return false;
- }
+ });
+}
- // We don't want any schema that may define the dictionary values
- // to also be a dictionary, so we use a looser constraint that
- // checks against any oneOf/anyOf schema.
- return schemaLooselyHasConstraint(
- schema.additionalProperties,
- s => !!s['additionalProperties']
+// Check, *by path*, if the current schema is a dictionary value schema.
+function isDictionaryValueSchema(path) {
+ return (
+ path.at(-1) === 'additionalProperties' ||
+ path.at(-2) === 'patternProperties'
+ );
+}
+
+// Check, *by object fields* if the current schema is a dictionary or not.
+function isDictionarySchema(schema) {
+ return (
+ schemaDefinesField(schema, 'additionalProperties') ||
+ schemaDefinesField(schema, 'patternProperties')
);
}
diff --git a/packages/ruleset/test/rules/well-defined-dictionaries.test.js b/packages/ruleset/test/rules/well-defined-dictionaries.test.js
index b5517b0d..9da5c17d 100644
--- a/packages/ruleset/test/rules/well-defined-dictionaries.test.js
+++ b/packages/ruleset/test/rules/well-defined-dictionaries.test.js
@@ -1,5 +1,5 @@
/**
- * Copyright 2024 IBM Corporation.
+ * Copyright 2024 - 2025 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/
@@ -24,7 +24,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(0);
});
- it('Includes a well-defined dictionary with scalar values', async () => {
+ it('Includes a well-defined dictionary with scalar values (additionalProperties)', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.components.schemas.Movie.properties.metadata = {
@@ -39,6 +39,23 @@ describe(`Spectral rule: ${ruleId}`, () => {
expect(results).toHaveLength(0);
});
+ it('Includes a well-defined dictionary with scalar values (patternProperties)', async () => {
+ const testDocument = makeCopy(rootDocument);
+
+ testDocument.components.schemas.Movie.properties.metadata = {
+ description: 'a dictionary mapping keys to string values',
+ type: 'object',
+ patternProperties: {
+ '^[a-zA-Z]+$': {
+ type: 'string',
+ },
+ },
+ };
+
+ const results = await testRule(ruleId, rule, testDocument);
+ expect(results).toHaveLength(0);
+ });
+
it('Includes a well-defined dictionary with composed scalar values', async () => {
const testDocument = makeCopy(rootDocument);
@@ -139,14 +156,14 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Object schemas must define either properties, or additionalProperties with a concrete type'
+ 'Object schemas must define either properties, or (additional/pattern)Properties with a concrete type'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
}
});
- it('Includes a model/dictionary hybrid, which is not allowed', async () => {
+ it('Includes a model/dictionary hybrid, which is not allowed (additionalProperties)', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.components.schemas.Movie.properties.metadata = {
@@ -175,6 +192,37 @@ describe(`Spectral rule: ${ruleId}`, () => {
}
});
+ it('Includes a model/dictionary hybrid, which is not allowed (patternProperties)', async () => {
+ const testDocument = makeCopy(rootDocument);
+
+ testDocument.components.schemas.Movie.properties.metadata = {
+ description: 'a dictionary with no definition',
+ type: 'object',
+ properties: {
+ rating: {
+ type: 'integer',
+ },
+ },
+ patternProperties: {
+ '^[a-zA-Z]+$': {
+ type: 'string',
+ },
+ },
+ };
+
+ const results = await testRule(ruleId, rule, testDocument);
+ expect(results).toHaveLength(4);
+
+ for (const i in results) {
+ expect(results[i].code).toBe(ruleId);
+ expect(results[i].message).toMatch(
+ 'Object schemas must be either a model or a dictionary - they cannot be both'
+ );
+ expect(results[i].severity).toBe(expectedSeverity);
+ expect(results[i].path.join('.')).toBe(expectedPaths[i]);
+ }
+ });
+
it('Includes a dictionary with additionalProperties set to "true"', async () => {
const testDocument = makeCopy(rootDocument);
@@ -190,7 +238,36 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionary schemas must have a single, well-defined value type in `additionalProperties`'
+ 'Dictionary schemas must have a single, well-defined value type'
+ );
+ expect(results[i].severity).toBe(expectedSeverity);
+ expect(results[i].path.join('.')).toBe(expectedPaths[i]);
+ }
+ });
+
+ it('Includes a dictionary with patternProperties value that has no type', async () => {
+ const testDocument = makeCopy(rootDocument);
+
+ testDocument.components.schemas.Movie.properties.metadata = {
+ description: 'a dictionary with no definition',
+ type: 'object',
+ patternProperties: {
+ '^[a-zA-Z]+$': {
+ type: 'string',
+ },
+ '^wrong_[A-Z][a-z]+$': {
+ description: 'no type definition',
+ },
+ },
+ };
+
+ const results = await testRule(ruleId, rule, testDocument);
+ expect(results).toHaveLength(4);
+
+ for (const i in results) {
+ expect(results[i].code).toBe(ruleId);
+ expect(results[i].message).toMatch(
+ 'Dictionary schemas must have a single, well-defined value type'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
@@ -212,7 +289,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionary schemas must have a single, well-defined value type in `additionalProperties`'
+ 'Dictionary schemas must have a single, well-defined value type'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
@@ -244,7 +321,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionaries must not have values that are also dictionaries.'
+ 'Dictionaries must not have values that are also dictionaries'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedRulePaths[i]);
@@ -272,14 +349,14 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionary schemas must have a single, well-defined value type in `additionalProperties`'
+ 'Dictionary schemas must have a single, well-defined value type'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
}
});
- it('Includes a dictionary of dictionaries, which is not allowed', async () => {
+ it('Includes a dictionary of dictionaries, which is not allowed (additionalProperties)', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.components.schemas.Movie.properties.metadata = {
@@ -299,22 +376,28 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionaries must not have values that are also dictionaries.'
+ 'Dictionaries must not have values that are also dictionaries'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
}
});
- it('Flags dictionary of dictionaries but ignores dictionary value issue', async () => {
+ it('Includes a dictionary of dictionaries, which is not allowed (patternProperties)', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.components.schemas.Movie.properties.metadata = {
description: 'a dictionary with no definition',
type: 'object',
- additionalProperties: {
- type: 'object',
- additionalProperties: true,
+ patternProperties: {
+ '^[a-zA-Z]+$': {
+ type: 'object',
+ patternProperties: {
+ '^[a-zA-Z]+$': {
+ type: 'string',
+ },
+ },
+ },
},
};
@@ -324,14 +407,41 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionaries must not have values that are also dictionaries.'
+ 'Dictionaries must not have values that are also dictionaries'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
}
});
- it('Includes a composed dictionary of dictionaries, which is not allowed', async () => {
+ it('Includes a dictionary of dictionaries, which is not allowed (hybrid)', async () => {
+ const testDocument = makeCopy(rootDocument);
+
+ testDocument.components.schemas.Movie.properties.metadata = {
+ description: 'a dictionary with no definition',
+ type: 'object',
+ patternProperties: {
+ '^[a-zA-Z]+$': {
+ type: 'object',
+ additionalProperties: true,
+ },
+ },
+ };
+
+ const results = await testRule(ruleId, rule, testDocument);
+ expect(results).toHaveLength(4);
+
+ for (const i in results) {
+ expect(results[i].code).toBe(ruleId);
+ expect(results[i].message).toMatch(
+ 'Dictionaries must not have values that are also dictionaries'
+ );
+ expect(results[i].severity).toBe(expectedSeverity);
+ expect(results[i].path.join('.')).toBe(expectedPaths[i]);
+ }
+ });
+
+ it('Flags dictionary of dictionaries but ignores dictionary value issue', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.components.schemas.Movie.properties.metadata = {
@@ -339,20 +449,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
type: 'object',
additionalProperties: {
type: 'object',
- allOf: [
- {
- additionalProperties: {
- type: 'string',
- },
- },
- {
- properties: {
- name: {
- type: 'string',
- },
- },
- },
- ],
+ additionalProperties: true,
},
};
@@ -362,14 +459,14 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionaries must not have values that are also dictionaries.'
+ 'Dictionaries must not have values that are also dictionaries'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
}
});
- it('Includes a single oneOf element that creates a nested dictionary, which is not allowed', async () => {
+ it('Includes a composed dictionary of dictionaries, which is not allowed', async () => {
const testDocument = makeCopy(rootDocument);
testDocument.components.schemas.Movie.properties.metadata = {
@@ -377,7 +474,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
type: 'object',
additionalProperties: {
type: 'object',
- oneOf: [
+ allOf: [
{
additionalProperties: {
type: 'string',
@@ -400,7 +497,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionaries must not have values that are also dictionaries.'
+ 'Dictionaries must not have values that are also dictionaries'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedPaths[i]);
@@ -437,7 +534,7 @@ describe(`Spectral rule: ${ruleId}`, () => {
for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].message).toMatch(
- 'Dictionary schemas must have a single, well-defined value type in `additionalProperties`'
+ 'Dictionary schemas must have a single, well-defined value type'
);
expect(results[i].severity).toBe(expectedSeverity);
expect(results[i].path.join('.')).toBe(expectedRulePaths[i]);