fix: 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

@@ -14,6 +14,7 @@ const invalidSchema = JSON.stringify(require('./__fixtures__/petstore.invalid-sc
 const studioFixture = JSON.stringify(require('./__fixtures__/studio-default-fixture-oas3.json'), null, 2);
 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';
@@ -213,7 +214,7 @@ describe('linter', () => {
     expect(result).toEqual([
       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({
@@ -727,7 +728,7 @@ 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({
@@ -754,6 +755,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: {
@@ -607,22 +607,6 @@ describe('Linter service', () => {
           },
           source: expect.stringContaining('__tests__/__fixtures__/refs/info.json'),
         }),
-        expect.objectContaining({
-          code: 'oas2-schema',
-          message: '`description` property type should be string.',
-          path: ['definitions', 'info', 'description'],
-          range: {
-            end: {
-              character: 22,
-              line: 5,
-            },
-            start: {
-              character: 21,
-              line: 5,
-            },
-          },
-          source: expect.stringContaining('__tests__/__fixtures__/refs/info.json'),
-        }),
       ]);
     });
 
@@ -669,7 +653,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: {

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/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
+  errors.forEach(error => {
+    if (error.keyword === 'additionalProperties') {
+      error.dataPath = `${error.dataPath}/${error.params['additionalProperty']}`;
+    }
+  });
+
   for (let i = 0; i < errors.length; i++) {
     const error = errors[i];