Skip to content

Commit

Permalink
fix(typed-enum): modify spectral rule to look only at schemas (#719)
Browse files Browse the repository at this point in the history
This Spectral rule checks any object that has a type and enum field,
regardless of whether it is in a valid schema location or not. This
fix modifies the given paths to run the same functionality against
schema objects only.

Signed-off-by: Dustin Popp <dpopp07@gmail.com>
  • Loading branch information
dpopp07 authored Jan 27, 2025
1 parent fe5f1af commit 4517084
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 2 deletions.
5 changes: 3 additions & 2 deletions packages/ruleset/src/ibm-oas.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ module.exports = {
'path-not-include-query': true,
// Enable with same severity as Spectral
'no-$ref-siblings': true,
// Enable with same severity as Spectral
'typed-enum': true,
// Enable with same settings as Spectral, but override the rule to modify
// the 'given' field to only check schemas - Spectral checks everything.
'typed-enum': ibmRules.typedEnum,
// Enable with same severity as Spectral
'oas2-api-host': true,
// Enable with same severity as Spectral
Expand Down
1 change: 1 addition & 0 deletions packages/ruleset/src/rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ module.exports = {
serverVariableDefaultValue: require('./server-variable-default-value'),
stringAttributes: require('./string-attributes'),
summarySentenceStyle: require('./summary-sentence-style'),
typedEnum: require('./typed-enum'),
unevaluatedProperties: require('./unevaluated-properties'),
unusedTags: require('./unused-tags'),
uniqueParameterRequestPropertyNames: require('./unique-parameter-request-property-names'),
Expand Down
26 changes: 26 additions & 0 deletions packages/ruleset/src/rules/typed-enum.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Copyright 2025 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const {
schemas,
} = require('@ibm-cloud/openapi-ruleset-utilities/src/collections');
const { oas } = require('@stoplight/spectral-rulesets');

// Spectral's "typed-enum" rule matches any object that happens to have a
// "type" and "enum" field on it. This results in false positives when
// APIs have metadata, like SDK generation metadata, contained in an
// extension, that contains objects with these keys are are not schemas.
// This solves the issue by replacing the "given" field with our "schemas"
// collection, modified to only give schemas with a "type" and "enum" field,
// while otherwise maintaining Spectral's implementation of the rule.
const typedEnum = oas.rules['typed-enum'];
typedEnum.given = schemas.map(s =>
s.replace(
'[*].schema',
'[?(@.schema && @.schema.type && @.schema.enum)].schema'
)
);

module.exports = typedEnum;
82 changes: 82 additions & 0 deletions packages/ruleset/test/rules/typed-enum.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* Copyright 2025 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const { typedEnum } = require('../../src/rules');
const {
makeCopy,
rootDocument,
testRule,
severityCodes,
} = require('../test-utils');

const rule = typedEnum;
const ruleId = 'typed-enum';
const expectedSeverity = severityCodes.warning;

describe(`Spectral rule: ${ruleId} (modified)`, () => {
describe('Should not yield errors', () => {
it('Clean spec', async () => {
const results = await testRule(ruleId, rule, rootDocument);
expect(results).toHaveLength(0);
});

// Spectral's rule, on its own, would not pass this test due to
// the fact that it looks at the whole spec for anything with a
// "type" and an "enum" field.
it('Ignore seemingly-violating non-schema objects', async () => {
const testDocument = makeCopy(rootDocument);

testDocument['x-sdk-apiref'] = {
type: 'int64',
enum: [
'this is based on real, internal metadata',
'that spectral would inappropriately flag',
],
};

const results = await testRule(ruleId, rule, testDocument);
expect(results).toHaveLength(0);
});
});

describe('Should yield errors', () => {
// This duplicates Spectral's negative test for this rule.
it('Schema enum field contains values that do not match the type', async () => {
const testDocument = makeCopy(rootDocument);

testDocument.paths['/v1/drinks'].parameters = [
{
schema: {
type: 'integer',
enum: [1, 'a string!', 3, 'and another one!'],
},
},
];

const results = await testRule(ruleId, rule, testDocument);
expect(results).toHaveLength(2);

const expectedResults = [
{
message: 'Enum value "a string!" must be "integer".',
path: 'paths./v1/drinks.parameters.0.schema.enum.1',
},
{
message: 'Enum value "and another one!" must be "integer".',
path: 'paths./v1/drinks.parameters.0.schema.enum.3',
},
];

for (const i in results) {
expect(results[i].code).toBe(ruleId);
expect(results[i].severity).toBe(expectedSeverity);

const { message, path } = expectedResults[i];
expect(results[i].message).toBe(message);
expect(results[i].path.join('.')).toBe(path);
}
});
});
});

0 comments on commit 4517084

Please sign in to comment.