chore: improve OAS functions (#1362)

* chore: improve OAS functions

* fix: check for presence of operationId

src/__tests__/__fixtures__/custom-oas-ruleset.json

@@ -18,8 +18,8 @@
   "except": {
     "/test/file.json#/info": ["info-contact", "info-description"],
     "/test/file.json#": [ "oas3-api-servers"],
-    "/test/file.json#/paths/~1a.two/get": ["operation-2xx-response"],
-    "/test/file.json#/paths/~1b.three/get": ["operation-2xx-response"],
+    "/test/file.json#/paths/~1a.two/get/responses": ["operation-2xx-response"],
+    "/test/file.json#/paths/~1b.three/get/responses": ["operation-2xx-response"],
     "another.yaml#": ["dummy-rule", "info-contact"]
   }
 }

src/__tests__/linter.jest.test.ts

@@ -599,12 +599,12 @@ console.log(this.cache.get('test') || this.cache.set('test', []).get('test'));
       const document = {
         openapi: '3.0.2',
         paths: {
-          '/a.one': { get: 17 },
-          '/a.two': { get: 18 },
-          '/a.three': { get: 19 },
-          '/b.one': { get: 17 },
-          '/b.two': { get: 18 },
-          '/b.three': { get: 19 },
+          '/a.one': { get: { responses: { 17: {} } } },
+          '/a.two': { get: { responses: { 18: {} } } },
+          '/a.three': { get: { responses: { 19: {} } } },
+          '/b.one': { get: { responses: { 17: {} } } },
+          '/b.two': { get: { responses: { 18: {} } } },
+          '/b.three': { get: { responses: { 19: {} } } },
         },
       };
 
@@ -622,25 +622,25 @@ console.log(this.cache.get('test') || this.cache.set('test', []).get('test'));
           expect.objectContaining({
             code: 'operation-2xx-response',
             message: 'Operation must have at least one `2xx` response.',
-            path: ['paths', '/a.one', 'get'],
+            path: ['paths', '/a.one', 'get', 'responses'],
             source: '/test/file.json',
           }),
           expect.objectContaining({
             code: 'operation-2xx-response',
             message: 'Operation must have at least one `2xx` response.',
-            path: ['paths', '/a.three', 'get'],
+            path: ['paths', '/a.three', 'get', 'responses'],
             source: '/test/file.json',
           }),
           expect.objectContaining({
             code: 'operation-2xx-response',
             message: 'Operation must have at least one `2xx` response.',
-            path: ['paths', '/b.one', 'get'],
+            path: ['paths', '/b.one', 'get', 'responses'],
             source: '/test/file.json',
           }),
           expect.objectContaining({
             code: 'operation-2xx-response',
             message: 'Operation must have at least one `2xx` response.',
-            path: ['paths', '/b.two', 'get'],
+            path: ['paths', '/b.two', 'get', 'responses'],
             source: '/test/file.json',
           }),
         ]),
@@ -651,7 +651,7 @@ console.log(this.cache.get('test') || this.cache.set('test', []).get('test'));
           expect.objectContaining({
             code: 'operation-2xx-response',
             message: 'Operation must have at least one `2xx` response.',
-            path: ['paths', '/a.two', 'get'],
+            path: ['paths', '/a.two', 'get', 'responses'],
             source: '/test/file.json',
           }),
         ]),
@@ -662,7 +662,7 @@ console.log(this.cache.get('test') || this.cache.set('test', []).get('test'));
           expect.objectContaining({
             code: 'operation-2xx-response',
             message: 'Operation must have at least one `2xx` response.',
-            path: ['paths', '/b.three', 'get'],
+            path: ['paths', '/b.three', 'get', 'responses'],
             source: '/test/file.json',
           }),
         ]),

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

@@ -53,22 +53,6 @@ describe('oasOpIdUnique', () => {
     });
 
     expect(results).toEqual([
-      {
-        code: 'operation-operationId-unique',
-        message: 'Every operation must have a unique `operationId`.',
-        path: ['paths', '/path1', 'get', 'operationId'],
-        range: {
-          end: {
-            character: 28,
-            line: 4,
-          },
-          start: {
-            character: 23,
-            line: 4,
-          },
-        },
-        severity: DiagnosticSeverity.Error,
-      },
       {
         code: 'operation-operationId-unique',
         message: 'Every operation must have a unique `operationId`.',
@@ -103,22 +87,6 @@ describe('oasOpIdUnique', () => {
     });
 
     expect(results).toEqual([
-      {
-        code: 'operation-operationId-unique',
-        message: 'Every operation must have a unique `operationId`.',
-        path: ['paths', '/path1', 'get', 'operationId'],
-        range: {
-          end: {
-            character: 28,
-            line: 4,
-          },
-          start: {
-            character: 23,
-            line: 4,
-          },
-        },
-        severity: DiagnosticSeverity.Error,
-      },
       {
         code: 'operation-operationId-unique',
         message: 'Every operation must have a unique `operationId`.',

src/rulesets/oas/functions/oasExample.ts

@@ -1,10 +1,6 @@
-import type { IFunction, IFunctionContext, IFunctionResult } from '../../../types';
-import type { Dictionary } from '@stoplight/types';
+import type { IFunction, IFunctionContext, IFunctionResult, JSONSchema } from '../../../types';
 import type { ISchemaOptions } from '../../../functions/schema';
-
-function isObject(value: unknown): value is Dictionary<any> {
-  return value !== null && typeof value === 'object';
-}
+import { isObject } from './utils/isObject';
 
 interface IOasExampleOptions {
   oasVersion: 2 | 3;
@@ -69,7 +65,7 @@ export const oasExample: IFunction<IOasExampleOptions> = function (
   }
 
   const schemaOpts: ISchemaOptions = {
-    schema: opts.schemaField === '$' ? targetVal : targetVal[opts.schemaField],
+    schema: opts.schemaField === '$' ? targetVal : (targetVal[opts.schemaField] as JSONSchema),
     oasVersion: opts.oasVersion,
   };
 
@@ -104,7 +100,7 @@ export const oasExample: IFunction<IOasExampleOptions> = function (
 
         const result = this.functions.schema.call(
           this,
-          keyed ? exampleValue.value : exampleValue,
+          keyed && isObject(exampleValue) ? exampleValue.value : exampleValue,
           schemaOpts,
           {
             given: paths.given,

src/rulesets/oas/functions/oasOp2xxResponse.ts

@@ -1,18 +1,22 @@
-import type { IFunction, IFunctionResult } from '../../../types';
+import type { IFunction } from '../../../types';
+import { isObject } from './utils/isObject';
 
 export const oasOp2xxResponse: IFunction = targetVal => {
-  if (!targetVal) {
+  if (!isObject(targetVal)) {
     return;
   }
 
-  const results: IFunctionResult[] = [];
-  const responses = Object.keys(targetVal);
-  if (responses.filter(response => Number(response) >= 200 && Number(response) < 300).length === 0) {
-    results.push({
-      message: 'operations must define at least one 2xx response',
-    });
+  for (const response of Object.keys(targetVal)) {
+    if (Number(response) >= 200 && Number(response) < 300) {
+      return;
+    }
   }
-  return results;
+
+  return [
+    {
+      message: 'operations must define at least one 2xx or 3xx response',
+    },
+  ];
 };
 
 export default oasOp2xxResponse;

src/rulesets/oas/functions/oasOpFormDataConsumeCheck.ts

@@ -1,20 +1,24 @@
-import type { IFunction, IFunctionResult } from '../../../types';
+import type { IFunction } from '../../../types';
+
+const validConsumeValue = /(application\/x-www-form-urlencoded|multipart\/form-data)/;
 
 export const oasOpFormDataConsumeCheck: IFunction = targetVal => {
-  const results: IFunctionResult[] = [];
+  const parameters: unknown = targetVal.parameters;
+  const consumes: unknown = targetVal.consumes;
 
-  const parameters = targetVal.parameters;
-  const consumes = targetVal.consumes || [];
+  if (!Array.isArray(parameters) || !Array.isArray(consumes)) {
+    return;
+  }
 
-  if (parameters?.find((p: any) => p.in === 'formData')) {
-    if (!consumes.join(',').match(/(application\/x-www-form-urlencoded|multipart\/form-data)/)) {
-      results.push({
-        message: 'consumes must include urlencoded, multipart, or formdata media type when using formData parameter',
-      });
-    }
+  if (parameters.some(p => p?.in === 'formData') && !validConsumeValue.test(consumes?.join(','))) {
+    return [
+      {
+        message: 'Consumes must include urlencoded, multipart, or form-data media type when using formData parameter.',
+      },
+    ];
   }
 
-  return results;
+  return;
 };
 
 export default oasOpFormDataConsumeCheck;

src/rulesets/oas/functions/oasOpIdUnique.ts

@@ -1,37 +1,29 @@
 import type { IFunction, IFunctionResult } from '../../../types';
+import { getAllOperations } from './utils/getAllOperations';
 
-export const oasOpIdUnique: IFunction = (targetVal, _options, functionPaths) => {
+export const oasOpIdUnique: IFunction = targetVal => {
   const results: IFunctionResult[] = [];
 
-  const { paths = {} } = targetVal;
+  const { paths } = targetVal;
 
-  const ids: any[] = [];
+  const seenIds: unknown[] = [];
 
-  for (const path in paths) {
-    if (Object.keys(paths[path]).length > 0) {
-      for (const operation in paths[path]) {
-        if (operation !== 'parameters') {
-          const { operationId } = paths[path][operation];
-
-          if (operationId) {
-            ids.push({
-              path: ['paths', path, operation, 'operationId'],
-              operationId,
-            });
-          }
-        }
-      }
+  for (const { path, operation } of getAllOperations(paths)) {
+    if (!('operationId' in paths[path][operation])) {
+      continue;
     }
-  }
 
-  ids.forEach(operationId => {
-    if (ids.filter(id => id.operationId === operationId.operationId).length > 1) {
+    const { operationId } = paths[path][operation];
+
+    if (seenIds.includes(operationId)) {
       results.push({
-        message: 'operationId must be unique',
-        path: operationId.path || functionPaths.given,
+        message: 'operationId must be unique.',
+        path: ['paths', path, operation, 'operationId'],
       });
+    } else {
+      seenIds.push(operationId);
     }
-  });
+  }
 
   return results;
 };

src/rulesets/oas/functions/oasOpSecurityDefined.ts

@@ -1,8 +1,20 @@
 import type { JsonPath } from '@stoplight/types';
 
-const _get = require('lodash/get');
+import type { IFunction, IFunctionResult } from '../../../types';
+import { getAllOperations } from './utils/getAllOperations';
+import { isObject } from './utils/isObject';
 
-import { IFunction, IFunctionResult } from '../../../types';
+function _get(value: unknown, path: JsonPath): unknown {
+  for (const segment of path) {
+    if (!isObject(value)) {
+      break;
+    }
+
+    value = value[segment];
+  }
+
+  return value;
+}
 
 export const oasOpSecurityDefined: IFunction<{
   schemesPath: JsonPath;
@@ -11,30 +23,32 @@ export const oasOpSecurityDefined: IFunction<{
 
   const { schemesPath } = options;
 
-  const { paths = {} } = targetVal;
-  const schemes = _get(targetVal, schemesPath) || {};
-  const allDefs = Object.keys(schemes);
-
-  for (const path in paths) {
-    if (Object.keys(paths[path]).length > 0)
-      for (const operation in paths[path]) {
-        if (operation !== 'parameters') {
-          const { security = [] } = paths[path][operation];
-
-          for (const index in security) {
-            if (security[index]) {
-              const securityKeys = Object.keys(security[index]);
-
-              if (securityKeys.length > 0 && !allDefs.includes(securityKeys[0])) {
-                results.push({
-                  message: 'operation referencing undefined security scheme',
-                  path: ['paths', path, operation, 'security', index],
-                });
-              }
-            }
-          }
-        }
+  const { paths } = targetVal;
+  const schemes = _get(targetVal, schemesPath);
+
+  const allDefs = isObject(schemes) ? Object.keys(schemes) : [];
+
+  for (const { path, operation } of getAllOperations(paths)) {
+    const { security } = paths[path][operation];
+
+    if (!Array.isArray(security)) {
+      continue;
+    }
+
+    for (const [index, value] of security.entries()) {
+      if (!isObject(value)) {
+        continue;
+      }
+
+      const securityKeys = Object.keys(value);
+
+      if (securityKeys.length > 0 && !allDefs.includes(securityKeys[0])) {
+        results.push({
+          message: 'Operation referencing undefined security scheme.',
+          path: ['paths', path, operation, 'security', index],
+        });
       }
+    }
   }
 
   return results;