diff --git a/README.md b/README.md index 55fa9e84f..12fedcfa4 100644 --- a/README.md +++ b/README.md @@ -235,11 +235,12 @@ The supported rules are described below: | invalid_non_empty_security_array | Flag any non-empty security array this is not of type OAuth2 | shared | ##### walker -| Rule | Description | Spec | -| --------------------------- | ---------------------------------------------------------------------------- | ------ | -| no_empty_descriptions | Flag any `description` field in the spec with an empty or whitespace string. | shared | -| has_circular_references | Flag any circular references found in the Swagger spec. | shared | -| $ref_siblings | Flag any properties that are siblings of a `$ref` property. | shared | +| Rule | Description | Spec | +| ----------------------------- | ---------------------------------------------------------------------------- | ------ | +| no_empty_descriptions | Flag any `description` field in the spec with an empty or whitespace string. | shared | +| has_circular_references | Flag any circular references found in the Swagger spec. | shared | +| $ref_siblings | Flag any properties that are siblings of a `$ref` property. | shared | +| duplicate_sibling_description | Flag descriptions sibling to `$ref` if identical to referenced description. | shared | [1]: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#dataTypeFormat [2]: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#parameter-object @@ -296,7 +297,7 @@ The default values for each rule are described below. ###### operations | Rule | Default | -| --------------------------- | --------| +| --------------------------- | ------- | | no_consumes_for_put_or_post | error | | get_op_has_consumes | warning | | no_produces | error | @@ -306,13 +307,13 @@ The default values for each rule are described below. ###### operations | Rule | Default | -| --------------------------- | --------| +| --------------------------- | ------- | | no_request_body_content | error | | no_request_body_name | warning | ###### parameters | Rule | Default | -| --------------------------- | --------| +| --------------------------- | ------- | | no_in_property | error | | invalid_in_property | error | | missing_schema_or_content | error | @@ -349,7 +350,7 @@ The default values for each rule are described below. ###### paths | Rule | Default | -| --------------------------- | --------| +| --------------------------- | ------- | | missing_path_parameter | error | | snake_case_only | warning | @@ -360,7 +361,7 @@ The default values for each rule are described below. ###### security_definitions | Rule | Default | -| --------------------------- | --------| +| --------------------------- | ------- | | unused_security_schemes | warning | | unused_security_scopes | warning | @@ -371,7 +372,7 @@ The default values for each rule are described below. ###### schemas | Rule | Default | -| --------------------------- | --------| +| --------------------------- | ------- | | invalid_type_format_pair | error | | snake_case_only | warning | | no_schema_description | warning | @@ -380,11 +381,12 @@ The default values for each rule are described below. | array_of_arrays | warning | ###### walker -| Rule | Default | -| --------------------------- | --------| -| no_empty_descriptions | error | -| has_circular_references | warning | -| $ref_siblings | off | +| Rule | Default | +| ----------------------------- | ------- | +| no_empty_descriptions | error | +| has_circular_references | warning | +| $ref_siblings | off | +| duplicate_sibling_description | warning | ## Turning off `update-notifier` diff --git a/src/.defaultsForValidator.js b/src/.defaultsForValidator.js index e232d715b..543d26ad3 100644 --- a/src/.defaultsForValidator.js +++ b/src/.defaultsForValidator.js @@ -61,7 +61,8 @@ const defaults = { 'walker': { 'no_empty_descriptions': 'error', 'has_circular_references': 'warning', - '$ref_siblings': 'off' + '$ref_siblings': 'off', + 'duplicate_sibling_description': 'warning' } }, 'swagger2': { diff --git a/src/plugins/validation/2and3/semantic-validators/walker-ibm.js b/src/plugins/validation/2and3/semantic-validators/walker-ibm.js index 63776d708..7295c7553 100644 --- a/src/plugins/validation/2and3/semantic-validators/walker-ibm.js +++ b/src/plugins/validation/2and3/semantic-validators/walker-ibm.js @@ -1,10 +1,14 @@ // Assertation 1: // The description, when present, should not be empty or contain empty space +// Assertation 2: +// Description siblings to $refs should not exist if identical to referenced description + +const at = require('lodash/at'); const walk = require('../../../utils/walk'); // Walks an entire spec. -module.exports.validate = function({ jsSpec }, config) { +module.exports.validate = function({ jsSpec, resolvedSpec }, config) { const result = {}; result.error = []; result.warning = []; @@ -15,6 +19,8 @@ module.exports.validate = function({ jsSpec }, config) { // check for empty descriptions if (obj.description !== undefined && obj.description !== null) { const description = obj.description.toString(); + + // verify description is not empty if (description.length === 0 || !description.trim()) { const checkStatus = config.no_empty_descriptions; if (checkStatus !== 'off') { @@ -24,6 +30,27 @@ module.exports.validate = function({ jsSpec }, config) { }); } } + + // check description siblings to $refs + // note: in general, siblings to $refs are discouraged and validated elsewhere. + // this is a more specific check to flag duplicated descriptions for referenced schemas + // (probably most useful when users turn of the $ref sibling validation) + if (obj.$ref) { + const referencedSchema = at(resolvedSpec, [path])[0]; + if ( + referencedSchema.description && + referencedSchema.description === description + ) { + const checkStatus = config.duplicate_sibling_description; + if (checkStatus !== 'off') { + result[checkStatus].push({ + path: [...path, 'description'], + message: + 'Description sibling to $ref matches that of the referenced schema. This is redundant and should be removed.' + }); + } + } + } } // check for and flag null values - they are not allowed by the spec and are likely mistakes diff --git a/test/plugins/validation/2and3/walker-ibm.js b/test/plugins/validation/2and3/walker-ibm.js index 930126ef0..65aac34e9 100644 --- a/test/plugins/validation/2and3/walker-ibm.js +++ b/test/plugins/validation/2and3/walker-ibm.js @@ -1,8 +1,8 @@ const expect = require('expect'); +const resolver = require('json-schema-ref-parser'); const { validate } = require('../../../../src/plugins/validation/2and3/semantic-validators/walker-ibm'); - const config = require('../../../../src/.defaultsForValidator').defaults.shared; describe('validation plugin - semantic - walker-ibm', () => { @@ -130,4 +130,53 @@ describe('validation plugin - semantic - walker-ibm', () => { ); expect(res.warnings.length).toEqual(0); }); + + it('should complain if a description sibling to a $ref matches the referenced schema description', async () => { + const spec = { + paths: { + '/stuff': { + get: { + summary: 'list stuff', + operationId: 'listStuff', + produces: ['application/json'], + responses: { + 200: { + description: 'successful operation', + schema: { + $ref: '#/responses/Success', + description: 'simple success response' + } + } + } + } + } + }, + responses: { + Success: { + type: 'string', + description: 'simple success response' + } + } + }; + + // clone spec, otherwise 'dereference' will change the spec by reference + const specCopy = JSON.parse(JSON.stringify(spec)); + const resolvedSpec = await resolver.dereference(specCopy); + + const res = validate({ jsSpec: spec, resolvedSpec }, config); + expect(res.errors.length).toEqual(0); + expect(res.warnings.length).toEqual(1); + expect(res.warnings[0].path).toEqual([ + 'paths', + '/stuff', + 'get', + 'responses', + '200', + 'schema', + 'description' + ]); + expect(res.warnings[0].message).toEqual( + 'Description sibling to $ref matches that of the referenced schema. This is redundant and should be removed.' + ); + }); });