Repository: Redocly/redocly-cli
Pull request #876 (2022-09-19 00:34:48)

fix: fix additional properties reported incorrectly with allOf
## What/Why/How?

The replaced default behavior for `allowAdditionalProperteis` with the new more correct implementation we did in our fork of AJV: `defaultUnevaluatedProperties` (https://github.com/Redocly/ajv/pull/12)

**Important**: awaits new release of ajv fork. I built and copied the new AJV locally and tested it: all works!

## Reference

Fixes https://github.com/Redocly/redocly-cli/issues/552

## Testing

Covered by tests in AJV fork + a few tests here.

## Check yourself

- [x] Code is linted
- [x] Tested with redoc/reference-docs/workflows
- [x] All new/updated code is covered with tests

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 

Multiple errors occurred
```
 1 | Error: The process '/usr/local/bin/npm' failed with exit code 1
 2 | Error: The process '/usr/local/bin/npm' failed with exit code 1
```


| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage                                                     | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :------------------------------------------------------------- | :-------------: |
|                                                                           🟡                                                                           | Statements | <div title="Base coverage is: 73.02%">70.26% (-2.76% 🔻)</div> |    3080/4384    |
|                                                                           🟡                                                                           | Branches   | <div title="Base coverage is: 64.93%">61.97% (-2.95% 🔻)</div> |    1677/2706    |
|                                                                           🟡                                                                           | Functions  | <div title="Base coverage is: 63.95%">60.17% (-3.78% 🔻)</div> |     494/821     |
|                                                                           🟡                                                                           | Lines      | <div title="Base coverage is: 72.98%">70.07% (-2.92% 🔻)</div> |    2858/4079    |


<details><summary>Show files with reduced coverage 🔻</summary>
<br/>

| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | File                                                                | Statements                                                      | Branches                                                        | Functions                                                       | Lines                                                          |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :------------------------------------------------------------------ | :-------------------------------------------------------------- | :-------------------------------------------------------------- | :-------------------------------------------------------------- | :------------------------------------------------------------- |
|                                                                           🟢                                                                           | <div title="core/src/rules/ajv.ts">`...` / ajv.ts</div>             | <div title="Base coverage is: 89.47%">81.58% (-7.89% 🔻)</div>  | <div title="Base coverage is: 71.43%">61.11% (-10.32% 🔻)</div> | 85.71%                                                          | <div title="Base coverage is: 97.14%">88.57% (-8.57% 🔻)</div> |
|                                                                           🔴                                                                           | cli/src/utils.ts                                                    | <div title="Base coverage is: 31.85%">28.15% (-3.7% 🔻)</div>   | <div title="Base coverage is: 13.1%">11.9% (-1.19% 🔻)</div>    | <div title="Base coverage is: 26.67%">13.33% (-13.33% 🔻)</div> | <div title="Base coverage is: 31.85%">28.15% (-3.7% 🔻)</div>  |
|                                                                           🟡                                                                           | cli/src/js-utils.ts                                                 | <div title="Base coverage is: 90.91%">54.55% (-36.36% 🔻)</div> | <div title="Base coverage is: 83.33%">0% (-83.33% 🔻)</div>     | <div title="Base coverage is: 100%">25% (-75% 🔻)</div>         | <div title="Base coverage is: 100%">60% (-40% 🔻)</div>        |
|                                                                           🔴                                                                           | <div title="cli/src/commands/split/index.ts">`...` / index.ts</div> | <div title="Base coverage is: 67.7%">0% (-67.7% 🔻)</div>       | <div title="Base coverage is: 58.4%">0% (-58.4% 🔻)</div>       | <div title="Base coverage is: 85.71%">0% (-85.71% 🔻)</div>     | <div title="Base coverage is: 71.33%">0% (-71.33% 🔻)</div>    |

</details>

## Test suite run failed

<details><summary>Failed tests: 1/491. Failed suites: 2/85.</summary>
<br/>

```
  ● no-invalid-media-type-examples › should report on invalid example with allowAdditionalProperties

    expect(received).toMatchInlineSnapshot(snapshot)

    Snapshot name: `no-invalid-media-type-examples should report on invalid example with allowAdditionalProperties 1`

    - Snapshot  - 19
    + Received  +  1

    - Array [
    -   Object {
    -     "from": Object {
    -       "pointer": "#/paths/~1pet/get/responses/200/content/application~1json",
    -       "source": "foobar.yaml",
    -     },
    -     "location": Array [
    -       Object {
    -         "pointer": "#/paths/~1pet/get/responses/200/content/application~1json/example/c",
    -         "reportOnKey": true,
    -         "source": "foobar.yaml",
    -       },
    -     ],
    -     "message": "Example value must conform to the schema: must NOT have unevaluated properties `c`.",
    -     "ruleId": "no-invalid-media-type-examples",
    -     "severity": "error",
    -     "suggest": Array [],
    -   },
    - ]
    + Array []

      115 |     });
      116 |
    > 117 |     expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`
          |                                           ^
      118 |       Array [
      119 |         Object {
      120 |           "from": Object {

      at packages/core/src/rules/oas3/__tests__/no-invalid-media-type-examples.test.ts:117:43
      at fulfilled (packages/core/src/rules/oas3/__tests__/no-invalid-media-type-examples.test.ts:5:58)

```
---
```
  ● Test suite failed to run

    Cannot find module '@redocly/openapi-core' from 'packages/cli/src/commands/split/__tests__/index.test.ts'

    Require stack:
      packages/cli/src/commands/split/index.ts
      packages/cli/src/commands/split/__tests__/index.test.ts

      13 |
      14 | jest.mock('@redocly/openapi-core', () => ({
    > 15 |   ...jest.requireActual('@redocly/openapi-core'),
         |           ^
      16 |   isRef: jest.fn(),
      17 | }));
      18 |

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:306:11)
      at packages/cli/src/commands/split/__tests__/index.test.ts:15:11
      at Object.<anonymous> (packages/cli/src/commands/split/index.ts:3:1)

```

</details>


<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from ef0e5321b7bf4dce28660519f80495fb2b90a26d</p>

tatomyr: LGTM

Patch:
diff --git a/package-lock.json b/package-lock.json
index 154d4618..52968dc9 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1229,9 +1229,9 @@
       }
     },
     "node_modules/@redocly/ajv": {
-      "version": "8.6.5",
-      "resolved": "https://registry.npmjs.org/@redocly/ajv/-/ajv-8.6.5.tgz",
-      "integrity": "sha512-3P2TY/u4c6OBqkP+1cTH1iGAEv0O34PV3vV2Wnos/nNHu62OTrtC4zcaxttG0pHtPtn42StrhGq7SsiFgP4Bfw==",
+      "version": "8.11.0",
+      "resolved": "https://registry.npmjs.org/@redocly/ajv/-/ajv-8.11.0.tgz",
+      "integrity": "sha512-9GWx27t7xWhDIR02PA18nzBdLcKQRgc46xNQvjFkrYk4UOmvKhJ/dawwiX0cCOeetN5LcaaiqQbVOWYK62SGHw==",
       "dependencies": {
         "fast-deep-equal": "^3.1.1",
         "json-schema-traverse": "^1.0.0",
@@ -9794,7 +9794,7 @@
       "version": "1.0.0-beta.109",
       "license": "MIT",
       "dependencies": {
-        "@redocly/ajv": "^8.6.5",
+        "@redocly/ajv": "^8.11.0",
         "@types/node": "^14.11.8",
         "colorette": "^1.2.0",
         "js-levenshtein": "^1.1.6",
@@ -10769,9 +10769,9 @@
       }
     },
     "@redocly/ajv": {
-      "version": "8.6.5",
-      "resolved": "https://registry.npmjs.org/@redocly/ajv/-/ajv-8.6.5.tgz",
-      "integrity": "sha512-3P2TY/u4c6OBqkP+1cTH1iGAEv0O34PV3vV2Wnos/nNHu62OTrtC4zcaxttG0pHtPtn42StrhGq7SsiFgP4Bfw==",
+      "version": "8.11.0",
+      "resolved": "https://registry.npmjs.org/@redocly/ajv/-/ajv-8.11.0.tgz",
+      "integrity": "sha512-9GWx27t7xWhDIR02PA18nzBdLcKQRgc46xNQvjFkrYk4UOmvKhJ/dawwiX0cCOeetN5LcaaiqQbVOWYK62SGHw==",
       "requires": {
         "fast-deep-equal": "^3.1.1",
         "json-schema-traverse": "^1.0.0",
@@ -10819,7 +10819,7 @@
     "@redocly/openapi-core": {
       "version": "file:packages/core",
       "requires": {
-        "@redocly/ajv": "^8.6.5",
+        "@redocly/ajv": "^8.11.0",
         "@types/js-levenshtein": "^1.1.0",
         "@types/js-yaml": "^4.0.3",
         "@types/lodash.isequal": "^4.5.5",
diff --git a/packages/core/package.json b/packages/core/package.json
index 8250a923..ba9f737f 100644
--- a/packages/core/package.json
+++ b/packages/core/package.json
@@ -32,7 +32,7 @@
     "Roman Hotsiy <roman@redoc.ly> (https://redoc.ly/)"
   ],
   "dependencies": {
-    "@redocly/ajv": "^8.6.5",
+    "@redocly/ajv": "^8.11.0",
     "@types/node": "^14.11.8",
     "colorette": "^1.2.0",
     "js-levenshtein": "^1.1.6",
diff --git a/packages/core/src/rules/ajv.ts b/packages/core/src/rules/ajv.ts
index 42eda361..b7790b48 100644
--- a/packages/core/src/rules/ajv.ts
+++ b/packages/core/src/rules/ajv.ts
@@ -1,4 +1,4 @@
-import Ajv, { ValidateFunction, ErrorObject } from '@redocly/ajv';
+import Ajv, { ValidateFunction, ErrorObject } from '@redocly/ajv/dist/2020';
 import { Location, escapePointer } from '../ref-utils';
 import { ResolveFn } from '../walk';
 
@@ -20,7 +20,7 @@ function getAjv(resolve: ResolveFn, allowAdditionalProperties: boolean) {
       discriminator: true,
       allowUnionTypes: true,
       validateFormats: false, // TODO: fix it
-      defaultAdditionalProperties: allowAdditionalProperties,
+      defaultUnevaluatedProperties: allowAdditionalProperties,
       loadSchemaSync(base: string, $ref: string) {
         const resolvedRef = resolve({ $ref }, base.split('#')[0]);
         if (!resolvedRef || !resolvedRef.location) return false;
@@ -87,8 +87,8 @@ export function validateJsonSchema(
     if (propName) {
       message = `\`${propName}\` property ${message}`;
     }
-    if (error.keyword === 'additionalProperties') {
-      const property = error.params.additionalProperty;
+    if (error.keyword === 'additionalProperties' || error.keyword === 'unevaluatedProperties') {
+      const property = error.params.additionalProperty || error.params.unevaluatedProperty;
       message = `${message} \`${property}\``;
       error.instancePath += '/' + escapePointer(property);
     }
diff --git a/packages/core/src/rules/utils.ts b/packages/core/src/rules/utils.ts
index d235e2c4..b7407b3d 100644
--- a/packages/core/src/rules/utils.ts
+++ b/packages/core/src/rules/utils.ts
@@ -109,7 +109,8 @@ export function validateExample(
           message: `Example value must conform to the schema: ${error.message}.`,
           location: {
             ...new Location(dataLoc.source, error.instancePath),
-            reportOnKey: error.keyword === 'additionalProperties',
+            reportOnKey:
+              error.keyword === 'unevaluatedProperties' || error.keyword === 'additionalProperties',
           },
           from: location,
           suggest: error.suggest,


Test patch:
diff --git a/packages/core/src/rules/oas3/__tests__/no-invalid-media-type-examples.test.ts b/packages/core/src/rules/oas3/__tests__/no-invalid-media-type-examples.test.ts
index a1b6f8e1..2d85840b 100644
--- a/packages/core/src/rules/oas3/__tests__/no-invalid-media-type-examples.test.ts
+++ b/packages/core/src/rules/oas3/__tests__/no-invalid-media-type-examples.test.ts
@@ -128,7 +128,7 @@ describe('no-invalid-media-type-examples', () => {
               "source": "foobar.yaml",
             },
           ],
-          "message": "Example value must conform to the schema: must NOT have additional properties \`c\`.",
+          "message": "Example value must conform to the schema: must NOT have unevaluated properties \`c\`.",
           "ruleId": "no-invalid-media-type-examples",
           "severity": "error",
           "suggest": Array [],
@@ -137,7 +137,7 @@ describe('no-invalid-media-type-examples', () => {
     `);
   });
 
-  it('should not on invalid example with allowAdditionalProperties', async () => {
+  it('should not report on valid example with allowAdditionalProperties', async () => {
     const document = parseYamlToDocument(
       outdent`
         openapi: 3.0.0
@@ -177,6 +177,55 @@ describe('no-invalid-media-type-examples', () => {
     expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`Array []`);
   });
 
+  it('should not report on valid example with allowAdditionalProperties and allOf and $ref', async () => {
+    const document = parseYamlToDocument(
+      outdent`
+        openapi: 3.0.0
+        components:
+          schemas:
+            C:
+              properties:
+                c:
+                  type: string
+        paths:
+          /pet:
+            get:
+              responses:
+                200:
+                  content:
+                    application/json:
+                      example:
+                        a: "string"
+                        b: 13
+                        c: "string"
+                      schema:
+                        type: object
+                        allOf:
+                          - $ref: '#/components/schemas/C'
+                        properties:
+                          a:
+                            type: string
+                          b:
+                            type: number
+
+      `,
+      'foobar.yaml'
+    );
+
+    const results = await lintDocument({
+      externalRefResolver: new BaseResolver(),
+      document,
+      config: await makeConfig({
+        'no-invalid-media-type-examples': {
+          severity: 'error',
+          allowAdditionalProperties: false,
+        },
+      }),
+    });
+
+    expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`Array []`);
+  });
+
   it('should not on invalid examples', async () => {
     const document = parseYamlToDocument(
       outdent`