Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(typed-enum): modify spectral rule to look only at schemas #719

Merged
merged 1 commit into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
}
});
});
});
Loading