chore: improve OAS functions (#1362)

* chore: improve OAS functions

* fix: check for presence of operationId

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/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;

src/rulesets/oas/functions/oasOpSuccessResponse.ts

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

src/rulesets/oas/functions/oasTagDefined.ts

@@ -2,30 +2,37 @@
 // an operation is also present in the global tags array.
 
 import type { IFunction, IFunctionResult } from '../../../types';
+import { getAllOperations } from './utils/getAllOperations';
+import { isObject } from './utils/isObject';
 
 export const oasTagDefined: IFunction = targetVal => {
   const results: IFunctionResult[] = [];
 
-  const globalTags = (targetVal.tags || []).map(({ name }: { name: string }) => name);
-
-  const { paths = {} } = targetVal;
-
-  const validOperationKeys = ['get', 'head', 'post', 'put', 'patch', 'delete', 'options', 'trace'];
-
-  for (const path in paths) {
-    if (Object.keys(paths[path]).length > 0) {
-      for (const operation in paths[path]) {
-        if (validOperationKeys.includes(operation)) {
-          const { tags = [] } = paths[path][operation];
-          tags.forEach((tag: string, index: number) => {
-            if (globalTags.indexOf(tag) === -1) {
-              results.push({
-                message: 'Operation tags should be defined in global tags.',
-                path: ['paths', path, operation, 'tags', index],
-              });
-            }
-          });
-        }
+  const globalTags: string[] = [];
+
+  if (Array.isArray(targetVal.tags)) {
+    for (const tag of targetVal.tags) {
+      if (isObject(tag) && typeof tag.name === 'string') {
+        globalTags.push(tag.name);
+      }
+    }
+  }
+
+  const { paths } = targetVal;
+
+  for (const { path, operation } of getAllOperations(paths)) {
+    const { tags } = paths[path][operation];
+
+    if (!Array.isArray(tags)) {
+      continue;
+    }
+
+    for (const [i, tag] of tags.entries()) {
+      if (!globalTags.includes(tag)) {
+        results.push({
+          message: 'Operation tags should be defined in global tags.',
+          path: ['paths', path, operation, 'tags', i],
+        });
       }
     }
   }

src/rulesets/oas/functions/utils/getAllOperations.ts

@@ -0,0 +1,33 @@
+import { isObject } from './isObject';
+
+const validOperationKeys = ['get', 'head', 'post', 'put', 'patch', 'delete', 'options', 'trace'];
+
+export function* getAllOperations(paths: unknown): IterableIterator<{ path: string; operation: string }> {
+  if (!isObject(paths)) {
+    return;
+  }
+
+  const item = {
+    path: '',
+    operation: '',
+  };
+
+  for (const path of Object.keys(paths)) {
+    const operations = paths[path];
+    if (!isObject(operations)) {
+      continue;
+    }
+
+    item.path = path;
+
+    for (const operation of Object.keys(operations)) {
+      if (!isObject(operations[operation]) || !validOperationKeys.includes(operation)) {
+        continue;
+      }
+
+      item.operation = operation;
+
+      yield item;
+    }
+  }
+}

src/rulesets/oas/functions/utils/isObject.ts

@@ -0,0 +1,5 @@
+import type { Dictionary } from '@stoplight/types';
+
+export function isObject(value: unknown): value is Dictionary<unknown> {
+  return value !== null && typeof value === 'object';
+}

test-harness/scenarios/severity/fail-on-error.oas3.scenario

@@ -30,11 +30,10 @@ OpenAPI 3.x detected
   2:6   warning  info-description              OpenAPI object info `description` must be present and non-empty string.  info
   7:9   warning  operation-description         Operation `description` must be present and non-empty string.            paths./test.get
   7:9   warning  operation-tags                Operation should have non-empty `tags` array.                            paths./test.get
-  8:20    error  operation-operationId-unique  Every operation must have a unique `operationId`.                        paths./test.get.operationId
   10:9    error  parser                        Mapping key must be a string scalar rather than number                   paths./test.get.responses[200]
  12:10  warning  operation-description         Operation `description` must be present and non-empty string.            paths./test.post
  12:10  warning  operation-tags                Operation should have non-empty `tags` array.                            paths./test.post
  13:20    error  operation-operationId-unique  Every operation must have a unique `operationId`.                        paths./test.post.operationId
   15:9    error  parser                        Mapping key must be a string scalar rather than number                   paths./test.post.responses[200]
 
-✖ 11 problems (4 errors, 7 warnings, 0 infos, 0 hints)
+✖ 10 problems (3 errors, 7 warnings, 0 infos, 0 hints)