fix: make $ref pattern check configurable - new rule incorrect_ref_pattern (#78)

Author: dpopp07

README.md

@@ -255,6 +255,7 @@ The supported rules are described below:
 | 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 |
+| incorrect_ref_pattern        | Flag internal `$ref` values that do not point to the section they should (e.g. referencing `parameters` from a `schema` field). | 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
@@ -407,6 +408,7 @@ The default values for each rule are described below.
 | has_circular_references       | warning |
 | $ref_siblings                 | off     |
 | duplicate_sibling_description | warning |
+| incorrect_ref_pattern        | warning |
 
 
 ## Turning off `update-notifier`

src/.defaultsForValidator.js

@@ -66,7 +66,8 @@ const defaults = {
       'no_empty_descriptions': 'error',
       'has_circular_references': 'warning',
       '$ref_siblings': 'off',
-      'duplicate_sibling_description': 'warning'
+      'duplicate_sibling_description': 'warning',
+      'incorrect_ref_pattern': 'warning'
     }
   },
   'swagger2': {

src/plugins/validation/2and3/semantic-validators/walker.js

@@ -15,8 +15,9 @@ const match = require('matcher');
 const walk = require('../../../utils/walk');
 
 module.exports.validate = function({ jsSpec, isOAS3 }, config) {
-  const errors = [];
-  const warnings = [];
+  const result = {};
+  result.error = [];
+  result.warning = [];
 
   config = config.walker;
 
@@ -44,7 +45,7 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
     ///// "type" should always have a string-type value, everywhere.
     if (obj.type && allowedParents.indexOf(path[path.length - 1]) === -1) {
       if (typeof obj.type !== 'string') {
-        errors.push({
+        result.error.push({
           path: [...path, 'type'],
           message: '"type" should be a string'
         });
@@ -55,7 +56,7 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
 
     if (obj.maximum && obj.minimum) {
       if (greater(obj.minimum, obj.maximum)) {
-        errors.push({
+        result.error.push({
           path: path.concat(['minimum']),
           message: 'Minimum cannot be more than maximum'
         });
@@ -64,7 +65,7 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
 
     if (obj.maxProperties && obj.minProperties) {
       if (greater(obj.minProperties, obj.maxProperties)) {
-        errors.push({
+        result.error.push({
           path: path.concat(['minProperties']),
           message: 'minProperties cannot be more than maxProperties'
         });
@@ -73,7 +74,7 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
 
     if (obj.maxLength && obj.minLength) {
       if (greater(obj.minLength, obj.maxLength)) {
-        errors.push({
+        result.error.push({
           path: path.concat(['minLength']),
           message: 'minLength cannot be more than maxLength'
         });
@@ -89,39 +90,33 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
       if (refBlacklist && refBlacklist.length && matches.length) {
         // Assertation 2
         // use the slice(1) to remove the `!` negator from the string
-        warnings.push({
-          path: [...path, '$ref'],
-          message: `${
-            blacklistPayload.location
-          } $refs must follow this format: ${refBlacklist[0].slice(1)}`
-        });
+        const checkStatus = config.incorrect_ref_pattern;
+        if (checkStatus !== 'off') {
+          result[checkStatus].push({
+            path: [...path, '$ref'],
+            message: `${
+              blacklistPayload.location
+            } $refs must follow this format: ${refBlacklist[0].slice(1)}`
+          });
+        }
       }
     }
 
     const keys = Object.keys(obj);
     keys.forEach(k => {
       if (keys.indexOf('$ref') > -1 && k !== '$ref') {
-        switch (config.$ref_siblings) {
-          case 'error':
-            errors.push({
-              path: path.concat([k]),
-              message: 'Values alongside a $ref will be ignored.'
-            });
-            break;
-          case 'warning':
-            warnings.push({
-              path: path.concat([k]),
-              message: 'Values alongside a $ref will be ignored.'
-            });
-            break;
-          default:
-            break;
+        const checkStatus = config.$ref_siblings;
+        if (checkStatus !== 'off') {
+          result[checkStatus].push({
+            path: path.concat([k]),
+            message: 'Values alongside a $ref will be ignored.'
+          });
         }
       }
     });
   });
 
-  return { errors, warnings };
+  return { errors: result.error, warnings: result.warning };
 };
 
 // values are globs!

test/plugins/validation/2and3/walker.js

@@ -586,11 +586,15 @@ describe('validation plugin - semantic - spec walker', () => {
           }
         };
 
-        const updatedConfig = Object.assign(config, {
-          walker: { $ref_siblings: 'warning' }
-        });
+        // temporarily configure $ref_siblings to be a warning
+        const originalValue = config.walker.$ref_siblings;
+        config.walker.$ref_siblings = 'warning';
+
+        const res = validate({ jsSpec: spec }, config);
+
+        // revert changes to config to avoid affecting other tests
+        config.walker.$ref_siblings = originalValue;
 
-        const res = validate({ jsSpec: spec }, updatedConfig);
         expect(res.errors.length).toEqual(0);
         expect(res.warnings.length).toEqual(1);
         expect(res.warnings[0].path).toEqual([
@@ -600,6 +604,7 @@ describe('validation plugin - semantic - spec walker', () => {
           'description'
         ]);
       });
+
       it('should return a problem for a links $ref that does not have the correct format', function() {
         const spec = {
           paths: {