Repository: Redocly/openapi-cli
Pull request #310 (2021-06-14 06:58:12)

fix: crash when node is null
## What/Why/How?

fix: #195 

## Check yourself

- [x] Code is linted
- [x] Tested with redoc/reference-docs



Patch:
diff --git a/packages/core/src/rules/common/spec.ts b/packages/core/src/rules/common/spec.ts
index b50921c1..0d677f02 100644
--- a/packages/core/src/rules/common/spec.ts
+++ b/packages/core/src/rules/common/spec.ts
@@ -5,19 +5,22 @@ import { isRef } from '../../ref-utils';
 
 export const OasSpec: Oas3Rule | Oas2Rule = () => {
   return {
-    any(node: any, { report, type, location, key, resolve }) {
+    any(node: any, { report, type, location, key, resolve, ignoreNextVisitorsOnNode } ) {
       const nodeType = oasTypeOf(node);
+
       if (type.items) {
         if (nodeType !== 'array') {
           report({
             message: `Expected type \`${type.name}\` (array) but got \`${nodeType}\``,
           });
+          ignoreNextVisitorsOnNode();
         }
         return;
       } else if (nodeType !== 'object') {
         report({
           message: `Expected type \`${type.name}\` (object) but got \`${nodeType}\``,
         });
+        ignoreNextVisitorsOnNode();
         return;
       }
 
diff --git a/packages/core/src/rules/oas2/index.ts b/packages/core/src/rules/oas2/index.ts
index 8d532730..ba3ae1ce 100644
--- a/packages/core/src/rules/oas2/index.ts
+++ b/packages/core/src/rules/oas2/index.ts
@@ -3,7 +3,6 @@ import { InfoDescription } from '../common/info-description';
 import { InfoContact } from '../common/info-contact';
 import { InfoLicense } from '../common/info-license-url';
 import { InfoLicenseUrl } from '../common/license-url';
-
 import { BooleanParameterPrefixes } from './boolean-parameter-prefixes';
 import { TagDescription } from '../common/tag-description';
 import { TagsAlphabetical } from '../common/tags-alphabetical';
@@ -32,31 +31,26 @@ import { OperationSummary } from '../common/operation-summary';
 import { NoAmbiguousPaths } from '../common/no-ambiguous-paths';
 
 export const rules = {
+  spec: OasSpec as Oas2Rule,
   'info-description': InfoDescription as Oas2Rule,
   'info-contact': InfoContact as Oas2Rule,
   'info-license': InfoLicense as Oas2Rule,
   'info-license-url': InfoLicenseUrl as Oas2Rule,
-
   'tag-description': TagDescription as Oas2Rule,
   'tags-alphabetical': TagsAlphabetical as Oas2Rule,
-
   'paths-kebab-case': PathsKebabCase as Oas2Rule,
   'no-enum-type-mismatch': NoEnumTypeMismatch as Oas2Rule,
-
   'boolean-parameter-prefixes': BooleanParameterPrefixes as Oas2Rule,
   'no-path-trailing-slash': NoPathTrailingSlash as Oas2Rule,
   'operation-2xx-response': Operation2xxResponse as Oas2Rule,
-
   'operation-operationId-unique': OperationIdUnique as Oas2Rule,
   'operation-parameters-unique': OperationParametersUnique as Oas2Rule,
   'path-parameters-defined': PathParamsDefined as Oas2Rule,
   'operation-tag-defined': OperationTagDefined as Oas2Rule,
-
   'path-declaration-must-exist': PathDeclarationMustExist as Oas2Rule,
   'operation-operationId-url-safe': OperationIdUrlSafe as Oas2Rule,
   'operation-operationId': OperationOperationId as Oas2Rule,
   'operation-summary': OperationSummary as Oas2Rule,
-
   'operation-description': OperationDescription as Oas2Rule,
   'path-not-include-query': PathNotIncludeQuery as Oas2Rule,
   'path-params-defined': PathParamsDefined as Oas2Rule,
@@ -66,10 +60,7 @@ export const rules = {
   'no-unresolved-refs': NoUnresolvedRefs as Oas2Rule,
   'no-identical-paths': NoIdenticalPaths as Oas2Rule,
   'no-ambiguous-paths': NoAmbiguousPaths as Oas2Rule,
-
   'path-http-verbs-order': PathHttpVerbsOrder as Oas2Rule,
-
-  spec: OasSpec as Oas2Rule,
 };
 
 export const preprocessors = {};
diff --git a/packages/core/src/rules/oas3/index.ts b/packages/core/src/rules/oas3/index.ts
index 589ec862..4653345f 100644
--- a/packages/core/src/rules/oas3/index.ts
+++ b/packages/core/src/rules/oas3/index.ts
@@ -1,5 +1,4 @@
 import { Oas3RuleSet } from '../../oas-types';
-
 import { OasSpec } from '../common/spec';
 import { Operation2xxResponse } from '../common/operation-2xx-response';
 import { OperationIdUnique } from '../common/operation-operationId-unique';
@@ -42,6 +41,7 @@ import { NoEmptyEnumServers } from './no-servers-empty-enum';
 import { Oas3Decorator } from '../../visitors';
 
 export const rules = {
+  spec: OasSpec,
   'info-description': InfoDescription,
   'info-contact': InfoContact,
   'info-license': InfoLicense,
@@ -78,8 +78,7 @@ export const rules = {
   'no-identical-paths': NoIdenticalPaths,
   'no-ambiguous-paths': NoAmbiguousPaths,
   'no-undefined-server-variable': NoUndefinedServerVariable,
-  'no-servers-empty-enum': NoEmptyEnumServers,
-  spec: OasSpec,
+  'no-servers-empty-enum': NoEmptyEnumServers
 } as Oas3RuleSet;
 
 export const preprocessors = {};
diff --git a/packages/core/src/visitors.ts b/packages/core/src/visitors.ts
index c9a6fcf7..591836d7 100644
--- a/packages/core/src/visitors.ts
+++ b/packages/core/src/visitors.ts
@@ -49,7 +49,12 @@ import { NormalizedNodeType } from './types';
 import { Stack } from './utils';
 import { UserContext, ResolveResult, ProblemSeverity } from './walk';
 import { Location } from './ref-utils';
-export type VisitFunction<T> = (node: T, ctx: UserContext, parents?: any) => void;
+export type VisitFunction<T> = (
+  node: T,
+  ctx: UserContext & { ignoreNextVisitorsOnNode: () => void },
+  parents?: any,
+  context?: any,
+) => void;
 
 type VisitRefFunction = (node: OasRef, ctx: UserContext, resolved: ResolveResult<any>) => void;
 
@@ -86,7 +91,6 @@ export type VisitorLevelContext = {
   isSkippedLevel: false;
   type: NormalizedNodeType;
   parent: VisitorLevelContext | null;
-
   activatedOn: Stack<{
     node?: any;
     withParentNode?: any;
@@ -264,6 +268,11 @@ export function normalizeVisitors<T extends BaseVisitor>(
 ): NormalizedOasVisitors<T> {
   const normalizedVisitors: NormalizedOasVisitors<T> = {} as any;
 
+  normalizedVisitors.any = {
+    enter: [],
+    leave: [],
+  };
+
   for (const typeName of Object.keys(types) as Array<keyof T>) {
     normalizedVisitors[typeName] = {
       enter: [],
@@ -271,11 +280,6 @@ export function normalizeVisitors<T extends BaseVisitor>(
     } as any;
   }
 
-  normalizedVisitors.any = {
-    enter: [],
-    leave: [],
-  };
-
   normalizedVisitors.ref = {
     enter: [],
     leave: [],
@@ -376,7 +380,7 @@ export function normalizeVisitors<T extends BaseVisitor>(
     }
 
     for (const typeName of visitorKeys as Array<keyof T>) {
-      const typeVisitor = (visitor[typeName] as any) as NestedVisitObject<any, T>;
+      const typeVisitor = visitor[typeName] as any as NestedVisitObject<any, T>;
       const normalizedTypeVisitor = normalizedVisitors[typeName]!;
 
       if (!typeVisitor) continue;
diff --git a/packages/core/src/walk.ts b/packages/core/src/walk.ts
index 82088799..d778a2d9 100644
--- a/packages/core/src/walk.ts
+++ b/packages/core/src/walk.ts
@@ -1,6 +1,5 @@
 import { Referenced } from './typings/openapi';
 import { Location, isRef } from './ref-utils';
-
 import {
   VisitorLevelContext,
   NormalizedOasVisitors,
@@ -12,7 +11,6 @@ import { ResolvedRefMap, Document, ResolveError, YamlParseError, Source } from '
 import { pushStack, popStack } from './utils';
 import { OasVersion } from './oas-types';
 import { NormalizedNodeType, isNamedType } from './types';
-
 type NonUndefined = string | number | boolean | symbol | bigint | object | Record<string, any>;
 
 export type ResolveResult<T extends NonUndefined> =
@@ -109,7 +107,6 @@ export function walkDocument<T>(opts: {
   ctx: WalkContext;
 }) {
   const { document, rootType, normalizedVisitors, resolvedRefMap, ctx } = opts;
-
   const seenNodesPerType: Record<string, Set<any>> = {};
   const seenRefs = new Set<any>();
 
@@ -124,7 +121,6 @@ export function walkDocument<T>(opts: {
   ) {
     let currentLocation = location;
     const { node: resolvedNode, location: resolvedLocation, error } = resolve(node);
-
     const enteredContexts: Set<VisitorLevelContext> = new Set();
 
     if (isRef(node)) {
@@ -209,7 +205,16 @@ export function walkDocument<T>(opts: {
             if (!activatedOn.skipped) {
               visitedBySome = true;
               enteredContexts.add(context);
-              visitWithContext(visit, resolvedNode, context, ruleId, severity);
+              const ignoreNextVisitorsOnNode = visitWithContext(
+                visit,
+                resolvedNode,
+                context,
+                ruleId,
+                severity,
+              );
+              if (ignoreNextVisitorsOnNode) {
+                break;
+              }
             }
           }
         }
@@ -234,15 +239,16 @@ export function walkDocument<T>(opts: {
           }
 
           if (isRef(node)) {
-            props.push(...Object.keys(node).filter((k) => k!== '$ref' && !props.includes(k))); // properties on the same level as $ref
+            props.push(...Object.keys(node).filter((k) => k !== '$ref' && !props.includes(k))); // properties on the same level as $ref
           }
 
           for (const propName of props) {
             let value = resolvedNode[propName];
+
             let loc = resolvedLocation;
 
             if (value === undefined) {
-              value =  node[propName];
+              value = node[propName];
               loc = location; // properties on the same level as $ref should resolve against original location, not target
             }
 
@@ -322,6 +328,7 @@ export function walkDocument<T>(opts: {
       }
     }
 
+    // returns true ignores all the next visitors on the specific node
     function visitWithContext(
       visit: VisitFunction<any>,
       node: any,
@@ -330,6 +337,8 @@ export function walkDocument<T>(opts: {
       severity: ProblemSeverity,
     ) {
       const report = reportFn.bind(undefined, ruleId, severity);
+      let ignoreNextVisitorsOnNode = false;
+
       visit(
         node,
         {
@@ -341,9 +350,15 @@ export function walkDocument<T>(opts: {
           key,
           parentLocations: collectParentsLocations(context),
           oasVersion: ctx.oasVersion,
+          ignoreNextVisitorsOnNode: () => {
+            ignoreNextVisitorsOnNode = true;
+          },
         },
         collectParents(context),
+        context,
       );
+
+      return ignoreNextVisitorsOnNode;
     }
 
     function resolve<T>(
@@ -354,6 +369,7 @@ export function walkDocument<T>(opts: {
       const refId = from + '::' + ref.$ref;
 
       const resolvedRef = resolvedRefMap.get(refId);
+
       if (!resolvedRef) {
         return {
           location: undefined,


Test patch:
diff --git a/packages/core/__tests__/walk.test.ts b/packages/core/__tests__/walk.test.ts
index 19aea9e7..be407fd2 100644
--- a/packages/core/__tests__/walk.test.ts
+++ b/packages/core/__tests__/walk.test.ts
@@ -1327,3 +1327,71 @@ describe('type extensions', () => {
     `);
   });
 });
+
+describe('ignoreNextRules', () => {
+  it('should correctly skip top level', async () => {
+    const calls: string[] = [];
+
+    const testRuleSet: Oas3RuleSet = {
+      skip: jest.fn(() => {
+        return {
+          Operation: {
+            enter: jest.fn((op, ctx) => {
+              if (op.operationId === 'get') {
+                ctx.ignoreNextVisitorsOnNode();
+                calls.push(`enter and skip operation ${op.operationId}`);
+              } else {
+                calls.push(`enter and not skip operation ${op.operationId}`);
+              }
+            }),
+            leave: jest.fn((op) => {
+              if (op.operationId === 'get') {
+                calls.push(`leave skipped operation ${op.operationId}`);
+              } else {
+                calls.push(`leave not skipped operation ${op.operationId}`);
+              }
+            }),
+          },
+        };
+      }),
+      test: jest.fn(() => {
+        return {
+          Operation: {
+            enter: jest.fn((op) => calls.push(`enter operation ${op.operationId}`)),
+            leave: jest.fn((op) => calls.push(`leave operation ${op.operationId}`)),
+          },
+        };
+      }),
+    };
+
+    const document = parseYamlToDocument(
+      outdent`
+        openapi: 3.0.0
+        paths:
+          /pet:
+            get:
+              operationId: get
+            put:
+              operationId: put
+      `,
+      '',
+    );
+
+    await lintDocument({
+      externalRefResolver: new BaseResolver(),
+      document,
+      config: makeConfigForRuleset(testRuleSet),
+    });
+
+    expect(calls).toMatchInlineSnapshot(`
+      Array [
+        "enter and skip operation get",
+        "leave skipped operation get",
+        "enter and not skip operation put",
+        "enter operation put",
+        "leave not skipped operation put",
+        "leave operation put",
+      ]
+    `);
+  });
+});
diff --git a/packages/core/src/rules/common/__tests__/no-enum-type-mismatch.test.ts b/packages/core/src/rules/common/__tests__/no-enum-type-mismatch.test.ts
index dc863610..db36f07a 100644
--- a/packages/core/src/rules/common/__tests__/no-enum-type-mismatch.test.ts
+++ b/packages/core/src/rules/common/__tests__/no-enum-type-mismatch.test.ts
@@ -162,4 +162,49 @@ describe('Oas3 typed enum', () => {
       ]
     `);
   });
+
+  it('should not crash on null schema when there is spec rule', async () => {
+    const document = parseYamlToDocument(
+      outdent`
+          openapi: 3.0.0
+          info:
+            title: test
+            version: 1.2.3
+          paths:
+            /some:
+              get:
+                responses:
+                  '200':
+                    description: test
+                    content:
+                      application/json:
+                        schema: null
+        `,
+      'foobar.yaml',
+    );
+
+    const results = await lintDocument({
+      externalRefResolver: new BaseResolver(),
+      document,
+      config: makeConfig({ 'spec': 'error', 'no-enum-type-mismatch': 'error' }),
+    });
+
+    expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`
+      Array [
+        Object {
+          "location": Array [
+            Object {
+              "pointer": "#/paths/~1some/get/responses/200/content/application~1json/schema",
+              "reportOnKey": false,
+              "source": "foobar.yaml",
+            },
+          ],
+          "message": "Expected type \`Schema\` (object) but got \`null\`",
+          "ruleId": "spec",
+          "severity": "error",
+          "suggest": Array [],
+        },
+      ]
+    `);
+  });
 });