Skip to content

Commit

Permalink
feat(new-validation): flag descriptions sibling to $ref if identica…
Browse files Browse the repository at this point in the history 
…l to referenced description
  • Loading branch information
@dpopp07
dpopp07 committed May 14, 2019
1 parent b81c4ee commit 91178cf
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 19 deletions.
34 changes: 18 additions & 16 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down Expand Up @@ -349,7 +350,7 @@ The default values for each rule are described below.

###### paths
| Rule | Default |
| --------------------------- | --------|
| --------------------------- | ------- |
| missing_path_parameter | error |
| snake_case_only | warning |

Expand All @@ -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 |

Expand All @@ -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 |
Expand All @@ -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`
Expand Down
3 changes: 2 additions & 1 deletion src/.defaultsForValidator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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': {
Expand Down
29 changes: 28 additions & 1 deletion src/plugins/validation/2and3/semantic-validators/walker-ibm.js
View file
Original file line number Diff line number Diff line change
@@ -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 = [];
Expand All @@ -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') {
Expand All @@ -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
Expand Down
51 changes: 50 additions & 1 deletion test/plugins/validation/2and3/walker-ibm.js
View file
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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.'
);
});
});

0 comments on commit 91178cf

Please sign in to comment.