feat: rework error filtering to preserve sibling additionalProperties errors

src/__tests__/__fixtures__/invalid-status-codes.oas3.json

@@ -0,0 +1,64 @@
+{
+  "openapi": "3.0.0",
+  "info": {
+    "title": "Invalid status codes",
+    "description": "Test for oas3-schema additionalProperties",
+    "version": "1.0.0",
+    "contact": {
+      "email": "[email protected]"
+    }
+  },
+  "servers": [
+    {
+      "url": "http://petstore.swagger.io/v1"
+    }
+  ],
+  "tags": [
+    {
+      "name": "pets"
+    }
+  ],
+  "paths": {
+    "/pets": {
+      "post": {
+        "description": "Add a new pet to the store",
+        "summary": "Create pet",
+        "operationId": "create_pet",
+        "tags": [
+          "pets"
+        ],
+        "requestBody": {
+          "description": "Pet object that needs to be added to the store",
+          "required": true,
+          "content": {
+            "application/json": {
+              "schema": {
+                "type": "object"
+              }
+            }
+          }
+        },
+        "responses": {
+          "204": {
+            "description": "Success"
+          },
+          "400": {
+            "description": "Bad request"
+          },
+          "42": {
+            "description": "The answer to life, the universe, and everything"
+          },
+          "9999": {
+            "description": "Four digits must be better than three"
+          },
+          "5xx": {
+            "description": "Sumpin bad happened"
+          },
+          "default": {
+            "description": "Error"
+          }
+        }
+      }
+    }
+  }
+}

src/__tests__/generate-assets.jest.test.ts

@@ -89,7 +89,7 @@ describe('generate-assets', () => {
       {
         code: 'oas3-schema',
         message: 'Property `500` is not expected to be here.',
-        path: ['paths', '/'],
+        path: ['paths', '/', '500'],
         range: expect.any(Object),
         severity: DiagnosticSeverity.Error,
       },
@@ -116,7 +116,7 @@ describe('generate-assets', () => {
       {
         code: 'oas2-schema',
         message: 'Property `500` is not expected to be here.',
-        path: ['paths', '/'],
+        path: ['paths', '/', '500'],
         range: expect.any(Object),
         severity: DiagnosticSeverity.Error,
       },

src/__tests__/linter.test.ts

@@ -11,6 +11,7 @@ import { IParser } from '../parsers/types';
 const invalidSchema = JSON.stringify(require('./__fixtures__/petstore.invalid-schema.oas3.json'));
 const todosInvalid = JSON.stringify(require('./__fixtures__/todos.invalid.oas2.json'));
 const petstoreMergeKeys = JSON.stringify(require('./__fixtures__/petstore.merge.keys.oas3.json'));
+const invalidStatusCodes = JSON.stringify(require('./__fixtures__/invalid-status-codes.oas3.json'));
 
 const fnName = 'fake';
 const fnName2 = 'fake2';
@@ -694,9 +695,19 @@ responses:: !!foo
       }),
       expect.objectContaining({
         code: 'oas3-schema',
-        message: 'Property `type` is not expected to be here.',
+        message: '`header-1` property should have required property `schema`.',
         path: ['paths', '/pets', 'get', 'responses', '200', 'headers', 'header-1'],
       }),
+      expect.objectContaining({
+        code: 'oas3-schema',
+        message: 'Property `type` is not expected to be here.',
+        path: ['paths', '/pets', 'get', 'responses', '200', 'headers', 'header-1', 'type'],
+      }),
+      expect.objectContaining({
+        code: 'oas3-schema',
+        message: 'Property `op` is not expected to be here.',
+        path: ['paths', '/pets', 'get', 'responses', '200', 'headers', 'header-1', 'op'],
+      }),
       expect.objectContaining({
         code: 'invalid-ref',
       }),
@@ -721,6 +732,32 @@ responses:: !!foo
     ]);
   });
 
+  test('should preserve sibling additionalProperties errors', async () => {
+    spectral.registerFormat('oas2', isOpenApiv2);
+    spectral.registerFormat('oas3', isOpenApiv3);
+    await spectral.loadRuleset('spectral:oas');
+
+    const result = await spectral.run(invalidStatusCodes);
+
+    expect(result).toEqual([
+      expect.objectContaining({
+        code: 'oas3-schema',
+        message: 'Property `42` is not expected to be here.',
+        path: ['paths', '/pets', 'post', 'responses', '42'],
+      }),
+      expect.objectContaining({
+        code: 'oas3-schema',
+        message: 'Property `9999` is not expected to be here.',
+        path: ['paths', '/pets', 'post', 'responses', '9999'],
+      }),
+      expect.objectContaining({
+        code: 'oas3-schema',
+        message: 'Property `5xx` is not expected to be here.',
+        path: ['paths', '/pets', 'post', 'responses', '5xx'],
+      }),
+    ]);
+  });
+
   test('should report invalid schema $refs', async () => {
     spectral.registerFormat('oas2', isOpenApiv2);
     spectral.registerFormat('oas3', isOpenApiv3);

src/__tests__/spectral.jest.test.ts

@@ -110,15 +110,15 @@ describe('Spectral', () => {
       expect.arrayContaining([
         expect.objectContaining({
           code: 'oas2-schema',
-          path: ['paths', '/todos/{todoId}', 'get', 'responses', '200'],
+          path: ['paths', '/todos/{todoId}', 'get', 'responses', '200', 'description'],
           range: {
             end: {
-              character: 11,
-              line: 31,
+              character: 29,
+              line: 17,
             },
             start: {
-              character: 17,
-              line: 16,
+              character: 27,
+              line: 17,
             },
           },
           source: documentUri,

src/cli/services/__tests__/linter.test.ts

@@ -562,22 +562,22 @@ describe('Linter service', () => {
         expect.objectContaining({
           code: 'oas2-schema',
           message: 'Property `foo` is not expected to be here.',
-          path: ['paths'],
+          path: ['paths', 'foo'],
           range: {
             end: {
               character: 13,
               line: 8,
             },
             start: {
               character: 10,
-              line: 6,
+              line: 8,
             },
           },
           source: expect.stringContaining('__tests__/__fixtures__/draft-ref.oas2.json'),
         }),
         expect.objectContaining({
           code: 'oas2-schema',
-          message: 'Property `foo` is not expected to be here.',
+          message: '`info` property should have required property `title`.',
           path: ['definitions', 'info'],
           range: {
             end: {
@@ -591,6 +591,22 @@ describe('Linter service', () => {
           },
           source: expect.stringContaining('/__tests__/__fixtures__/refs/info.json'),
         }),
+        expect.objectContaining({
+          code: 'oas2-schema',
+          message: 'Property `foo` is not expected to be here.',
+          path: ['definitions', 'info', 'foo'],
+          range: {
+            end: {
+              character: 18,
+              line: 4,
+            },
+            start: {
+              character: 13,
+              line: 4,
+            },
+          },
+          source: expect.stringContaining('/__tests__/__fixtures__/refs/info.json'),
+        }),
         expect.objectContaining({
           code: 'info-description',
           message: 'OpenAPI object info `description` must be present and non-empty string.',
@@ -685,7 +701,7 @@ describe('Linter service', () => {
         }),
         expect.objectContaining({
           code: 'oas2-schema',
-          message: 'Property `response` is not expected to be here.',
+          message: '`get` property should have required property `responses`.',
           path: ['paths', '/test', 'get'],
           range: {
             end: {
@@ -747,6 +763,22 @@ describe('Linter service', () => {
           },
           source: expect.stringContaining('__tests__/__fixtures__/refs/paths.json'),
         }),
+        expect.objectContaining({
+          code: 'oas2-schema',
+          message: 'Property `response` is not expected to be here.',
+          path: ['paths', '/test', 'get', 'response'],
+          range: {
+            end: {
+              character: 25,
+              line: 4,
+            },
+            start: {
+              character: 20,
+              line: 4,
+            },
+          },
+          source: expect.stringContaining('__tests__/__fixtures__/refs/paths.json'),
+        }),
       ]);
     });
   });

src/functions/schema.ts

@@ -44,7 +44,8 @@ const logger = {
 const ajvInstances = {};
 
 function getAjv(oasVersion: Optional<number>, allErrors: Optional<boolean>): AJV.Ajv {
-  const type: string = oasVersion !== void 0 && oasVersion >= 2 ? 'oas' + oasVersion : 'jsonschema';
+  const qual = allErrors === true ? '-all' : '';
+  const type: string = (oasVersion !== void 0 && oasVersion >= 2 ? 'oas' + oasVersion : 'jsonschema') + qual;
   if (typeof ajvInstances[type] !== 'undefined') {
     return ajvInstances[type];
   }

src/rulesets/oas/functions/__tests__/oasDocumentSchema.test.ts

@@ -157,6 +157,13 @@ describe('oasDocumentSchema', () => {
           severity: DiagnosticSeverity.Error,
           range: expect.any(Object),
         },
+        {
+          code: 'oas3-schema',
+          message: 'Property `foo` is not expected to be here.',
+          path: ['components', 'securitySchemes', 'basic', 'foo'],
+          severity: DiagnosticSeverity.Error,
+          range: expect.any(Object),
+        },
       ]);
     });
 

src/rulesets/oas/functions/oasDocumentSchema.ts

@@ -31,6 +31,13 @@ const ERROR_MAP = [
 // As you can see, what we deal here wit is actually not really oneOf anymore - it's always the first member of oneOf we match against.
 // That being said, we always strip both oneOf and $ref, since we are always interested in the first error.
 export function prepareResults(errors: AJV.ErrorObject[]) {
+  // Update additionalProperties errors to make them more precise and prevent them from being treated as duplicates
+  for (const error of errors) {
+    if (error.keyword === 'additionalProperties') {
+      error.dataPath = `${error.dataPath}/${error.params['additionalProperty']}`;
+    }
+  }
+
   for (let i = 0; i < errors.length; i++) {
     const error = errors[i];
 

test-harness/scenarios/oas3-schema.scenario

@@ -57,13 +57,14 @@ rules:
 OpenAPI 3.x detected
 
 {document}
-  5:11  error  oas3-schema  Property `foo` is not expected to be here.                     info.contact
- 12:11  error  oas3-schema  Property `type` is not expected to be here.                    paths./user.get.parameters[0]
+  6:10  error  oas3-schema  Property `foo` is not expected to be here.                     info.contact.foo
+ 12:17  error  oas3-schema  Property `type` is not expected to be here.                    paths./user.get.parameters[0].type
  23:18  error  oas3-schema  `type` property type should be string.                         paths./user.get.parameters[1].schema.type
- 24:11  error  oas3-schema  Property `type` is not expected to be here.                    paths./user.get.parameters[2]
+ 24:11  error  oas3-schema  `2` property should have required property `schema`.           paths./user.get.parameters[2]
+ 26:17  error  oas3-schema  Property `type` is not expected to be here.                    paths./user.get.parameters[2].type
  37:28  error  oas3-schema  `user_id` property type should be object.                      paths./user.get.responses[200].content.application/json.schema.properties.user_id
  41:28  error  oas3-schema  `properties` property type should be object.                   paths./user.get.responses[200].content.application/yaml.schema.properties
  43:23  error  oas3-schema  `description` property type should be string.                  paths./user.get.responses[400].description
  46:17  error  oas3-schema  `responses` property should not have fewer than 1 properties.  paths./address.get.responses
 
-✖ 8 problems (8 errors, 0 warnings, 0 infos, 0 hints)
+✖ 9 problems (9 errors, 0 warnings, 0 infos, 0 hints)

test-harness/scenarios/results-format-junit.oas3.scenario

@@ -14,6 +14,6 @@ OpenAPI 3.x detected
 <?xml version="1.0" encoding="utf-8"?>
 <testsuites>
 <testsuite package="org.spectral" time="0" tests="1" errors="0" failures="1" name="{document}">
-<testcase time="0" name="org.spectral.oas3-schema" classname="{document|no-ext}"><failure message="Property `foo` is not expected to be here."><![CDATA[line 2, col 6, Property `foo` is not expected to be here. (oas3-schema) at path #/info]]></failure></testcase>
+<testcase time="0" name="org.spectral.oas3-schema" classname="{document|no-ext}"><failure message="Property `foo` is not expected to be here."><![CDATA[line 5, col 7, Property `foo` is not expected to be here. (oas3-schema) at path #/info/foo]]></failure></testcase>
 </testsuite>
 </testsuites>