Repository: Redocly/redocly-cli
Pull request #1152 (2023-07-05 10:29:57)

fix: correctly validate schema `type: null`
## What/Why/How?

This fixes wrong type validation (broken in **beta.111**).

Since then, we didn't recognise `type: null` being an error:

```yaml

WrongType:
     type: null

```

Whilst the `null` should be a string:

```yaml

CorrectType:
     type: "null"
```

## Reference

The original MR: https://github.com/Redocly/redocly-cli/pull/891

## Testing

## Screenshots (optional)

## Check yourself

- [x] Code is linted
- [ ] 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]: | Command | Mean [s] | Min [s] | Max [s] | Relative |
|:---|---:|---:|---:|---:|
| `REDOCLY_TELEMETRY=off redocly lint packages/core/src/benchmark/benches/rebilly.yaml` | 1.280 ± 0.026 | 1.247 | 1.334 | 1.00 |
| `REDOCLY_TELEMETRY=off redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml` | 1.285 ± 0.026 | 1.232 | 1.324 | 1.00 ± 0.03 |

<!-- thollander/actions-comment-pull-request "hyperfine-comparison" -->
github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <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 | 74.43%     |    3708/4982    |
|                                                                           🟡                                                                           | Branches   | 65.41%     |    2059/3148    |
|                                                                           🟡                                                                           | Functions  | 66.67%     |     616/924     |
|                                                                           🟡                                                                           | Lines      | 74.49%     |    3458/4642    |


<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/utils.ts">`...` / utils.ts</div> | 94.67%     | 89.47%   | 100%      | 94.29% |

</details>

## Test suite run success
613 tests passing in 88 suites.

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

tatomyr: @lornajane sure. Just thinking whether I should change the definition or the snapshot. 

Patch:
diff --git a/packages/core/src/rules/utils.ts b/packages/core/src/rules/utils.ts
index 6d51a9e0..57f11cb0 100644
--- a/packages/core/src/rules/utils.ts
+++ b/packages/core/src/rules/utils.ts
@@ -70,6 +70,8 @@ export function validateDefinedAndNonEmpty(fieldName: string, value: any, ctx: U
 }
 
 export function getSuggest(given: string, variants: string[]): string[] {
+  if (given === null) return variants;
+
   if (typeof given !== 'string' || !variants.length) return [];
 
   const distances: Array<{ variant: string; distance: number }> = [];
@@ -157,7 +159,7 @@ export function validateSchemaEnumType(
   if (!schemaEnum) {
     return;
   }
-  if (!schemaEnum.includes(propertyValue === null ? 'null' : propertyValue)) {
+  if (!schemaEnum.includes(propertyValue)) {
     report({
       location,
       message: `\`${propName}\` can be one of the following only: ${schemaEnum


Test patch:
diff --git a/__tests__/lint/oas3.1/openapi.yaml b/__tests__/lint/oas3.1/openapi.yaml
index 621260ca..b86c49f6 100644
--- a/__tests__/lint/oas3.1/openapi.yaml
+++ b/__tests__/lint/oas3.1/openapi.yaml
@@ -228,7 +228,7 @@ components:
           type: 'null'
         tag:
           type:
-            - null
+            - 'null'
             - string
             - integer
           examples:
@@ -287,11 +287,6 @@ components:
             - string
             - 'null'
         eleven:
-          description: x-nullable string
-          type:
-            - string
-            - null
-        twelve:
           description: file/binary
     person:
       type: object
@@ -322,7 +317,7 @@ components:
           description: location can be null, set using `nullable` property thats supported by OpenAPI `3.0.x`
           type:
             - string
-            - null
+            - 'null'
         age:
           description: Age of Person
           type: integer
diff --git a/packages/core/src/rules/common/__tests__/spec.test.ts b/packages/core/src/rules/common/__tests__/spec.test.ts
index f921af9d..7b39f5c3 100644
--- a/packages/core/src/rules/common/__tests__/spec.test.ts
+++ b/packages/core/src/rules/common/__tests__/spec.test.ts
@@ -455,7 +455,7 @@ describe('Oas3.1 spec', () => {
     `);
   });
 
-  it('should not report about unknown type', async () => {
+  it('should report about `type: null` and not report `type: "null"`', async () => {
     const document = parseYamlToDocument(
       outdent`
       openapi: 3.1.0
@@ -468,10 +468,18 @@ describe('Oas3.1 spec', () => {
           url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
       components:
         schemas:
-          TestSchema:
-            title: TestSchema
-            description: Property name's description
+          WrongType:
             type: null
+          CorrectType:
+            type: "null"
+          WrongArrayType:
+            type:
+              - null
+              - string
+          CorrectArrayType:
+            type:
+              - 'null'
+              - string
         `
     );
 
@@ -481,7 +489,54 @@ describe('Oas3.1 spec', () => {
       config: await makeConfig({ spec: 'error' }),
     });
 
-    expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`Array []`);
+    expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "from": undefined,
+          "location": Array [
+            Object {
+              "pointer": "#/components/schemas/WrongType/type",
+              "reportOnKey": false,
+              "source": "",
+            },
+          ],
+          "message": "\`type\` can be one of the following only: \\"object\\", \\"array\\", \\"string\\", \\"number\\", \\"integer\\", \\"boolean\\", \\"null\\".",
+          "ruleId": "spec",
+          "severity": "error",
+          "suggest": Array [
+            "object",
+            "array",
+            "string",
+            "number",
+            "integer",
+            "boolean",
+            "null",
+          ],
+        },
+        Object {
+          "from": undefined,
+          "location": Array [
+            Object {
+              "pointer": "#/components/schemas/WrongArrayType/type/0",
+              "reportOnKey": false,
+              "source": "",
+            },
+          ],
+          "message": "\`type\` can be one of the following only: \\"object\\", \\"array\\", \\"string\\", \\"number\\", \\"integer\\", \\"boolean\\", \\"null\\".",
+          "ruleId": "spec",
+          "severity": "error",
+          "suggest": Array [
+            "object",
+            "array",
+            "string",
+            "number",
+            "integer",
+            "boolean",
+            "null",
+          ],
+        },
+      ]
+    `);
   });
 
   it('should not report on SpecExtension with additionalProperties', async () => {