feat(core): improve validation (#2026)

Author: P0lip

jest.config.js

@@ -8,9 +8,7 @@ const projectDefault = {
   preset: 'ts-jest',
   moduleNameMapper: {
     ...mapValues(pathsToModuleNameMapper(compilerOptions.paths), v => path.join(__dirname, v)),
-    '^@stoplight/spectral-test-utils$': '<rootDir>/test-utils/node/index.ts',
-    '^nimma/fallbacks$': '<rootDir>/node_modules/nimma/dist/cjs/fallbacks/index.js',
-    '^nimma/legacy$': '<rootDir>/node_modules/nimma/dist/legacy/cjs/index.js',
+    '^@stoplight/spectral\\-test\\-utils$': '<rootDir>/test-utils/node/index.ts',
   },
   testEnvironment: 'node',
   globals: {

karma.conf.ts

@@ -1,6 +1,7 @@
 // Karma configuration
 // Generated on Tue Jul 02 2019 17:18:30 GMT+0200 (Central European Summer Time)
 
+import * as path from 'path';
 import type { TransformCallback, TransformContext } from 'karma-typescript';
 import type { Config } from 'karma';
 
@@ -14,7 +15,7 @@ module.exports = (config: Config): void => {
     frameworks: ['jasmine', 'karma-typescript'],
 
     // list of files / patterns to load in the browser
-    files: ['./__karma__/jest.ts', 'packages/*/src/**/*.ts'],
+    files: ['./__karma__/jest.ts', './test-utils/*.ts', 'packages/*/src/**/*.ts'],
 
     // list of files / patterns to exclude
     exclude: ['packages/cli/**', 'packages/ruleset-bundler/src/plugins/commonjs.ts', '**/*.jest.test.ts'],
@@ -24,6 +25,7 @@ module.exports = (config: Config): void => {
     preprocessors: {
       'packages/*/src/**/*.ts': ['karma-typescript'],
       './__karma__/**/*.ts': ['karma-typescript'],
+      './test-utils/*.ts': ['karma-typescript'],
     },
 
     // @ts-expect-error: non-standard - karmaTypeScriptConfig
@@ -35,6 +37,7 @@ module.exports = (config: Config): void => {
         resolve: {
           alias: {
             '@stoplight/spectral-test-utils': require.resolve('./test-utils/browser/index.js'),
+            '@stoplight/spectral-test-utils/matchers': path.join(__dirname, './test-utils/matchers.ts'),
             nimma: require.resolve('./node_modules/nimma/dist/legacy/cjs/index.js'),
             'nimma/fallbacks': require.resolve('./node_modules/nimma/dist/legacy/cjs/fallbacks/index.js'),
             'nimma/legacy': require.resolve('./node_modules/nimma/dist/legacy/cjs/index.js'),

package.json

@@ -89,7 +89,6 @@
     "jest": "^27.4.3",
     "jest-mock": "^27.4.2",
     "jest-when": "^3.4.2",
-    "json-schema": "^0.4.0",
     "karma": "^6.1.1",
     "karma-chrome-launcher": "^3.1.0",
     "karma-jasmine": "^3.3.1",

packages/cli/src/services/__tests__/linter.test.ts

@@ -1,8 +1,12 @@
+import '@stoplight/spectral-test-utils/matchers';
+
 import { join, resolve } from '@stoplight/path';
 import nock from 'nock';
 import * as yargs from 'yargs';
 import { DiagnosticSeverity } from '@stoplight/types';
 import { RulesetValidationError } from '@stoplight/spectral-core';
+import '@stoplight/spectral-test-utils/matchers';
+import AggregateError = require('es-aggregate-error');
 import * as process from 'process';
 
 import lintCommand from '../../commands/lint';
@@ -221,8 +225,32 @@ describe('Linter service', () => {
       });
 
       it('fails trying to extend an invalid relative ruleset', () => {
-        return expect(run(`lint ${validCustomOas3SpecPath} -r ${invalidNestedRulesetPath}`)).rejects.toThrowError(
-          RulesetValidationError,
+        return expect(
+          run(`lint ${validCustomOas3SpecPath} -r ${invalidNestedRulesetPath}`),
+        ).rejects.toThrowAggregateError(
+          new AggregateError([
+            new RulesetValidationError('must be equal to one of the allowed values', [
+              'rules',
+              'rule-with-invalid-enum',
+            ]),
+            new RulesetValidationError('the rule must have at least "given" and "then" properties', [
+              'rules',
+              'rule-without-given-nor-them',
+            ]),
+            new RulesetValidationError('allowed types are "style" and "validation"', [
+              'rules',
+              'rule-with-invalid-enum',
+              'type',
+            ]),
+            new RulesetValidationError('must be equal to one of the allowed values', [
+              'rules',
+              'rule-without-given-nor-them',
+            ]),
+            new RulesetValidationError(
+              'the value has to be one of: 0, 1, 2, 3 or "error", "warn", "info", "hint", "off"',
+              ['rules', 'rule-with-invalid-enum', 'severity'],
+            ),
+          ]),
         );
       });
     });
@@ -235,8 +263,30 @@ describe('Linter service', () => {
       });
 
       it('outputs "invalid ruleset" error', () => {
-        return expect(run(`lint ${validOas3SpecPath} -r ${invalidRulesetPath}`)).rejects.toThrowError(
-          RulesetValidationError,
+        return expect(run(`lint ${validOas3SpecPath} -r ${invalidRulesetPath}`)).rejects.toThrowAggregateError(
+          new AggregateError([
+            new RulesetValidationError('must be equal to one of the allowed values', [
+              'rules',
+              'rule-with-invalid-enum',
+            ]),
+            new RulesetValidationError('the rule must have at least "given" and "then" properties', [
+              'rules',
+              'rule-without-given-nor-them',
+            ]),
+            new RulesetValidationError('allowed types are "style" and "validation"', [
+              'rules',
+              'rule-with-invalid-enum',
+              'type',
+            ]),
+            new RulesetValidationError('must be equal to one of the allowed values', [
+              'rules',
+              'rule-without-given-nor-them',
+            ]),
+            new RulesetValidationError(
+              'the value has to be one of: 0, 1, 2, 3 or "error", "warn", "info", "hint", "off"',
+              ['rules', 'rule-with-invalid-enum', 'severity'],
+            ),
+          ]),
         );
       });
 

packages/core/package.json

@@ -45,11 +45,13 @@
     "@stoplight/spectral-ref-resolver": "^1.0.0",
     "@stoplight/spectral-runtime": "^1.0.0",
     "@stoplight/types": "~13.2.0",
+    "@types/es-aggregate-error": "^1.0.2",
     "@types/json-schema": "^7.0.11",
     "ajv": "^8.6.0",
     "ajv-errors": "~3.0.0",
     "ajv-formats": "~2.1.0",
     "blueimp-md5": "2.18.0",
+    "es-aggregate-error": "^1.0.7",
     "jsonpath-plus": "6.0.1",
     "lodash": "~4.17.21",
     "lodash.topath": "^4.5.2",

packages/core/src/ruleset/__tests__/__fixtures__/overrides/formats.ts

@@ -22,7 +22,9 @@ const ruleset: RulesetDefinition = {
           then: {
             function: schema,
             functionOptions: {
-              type: 'number',
+              schema: {
+                type: 'number',
+              },
             },
           },
         },
@@ -39,7 +41,9 @@ const ruleset: RulesetDefinition = {
           then: {
             function: schema,
             functionOptions: {
-              type: 'boolean',
+              schema: {
+                type: 'boolean',
+              },
             },
           },
         },

packages/core/src/ruleset/__tests__/ruleset.test.ts

@@ -259,7 +259,7 @@ describe('Ruleset', () => {
   describe('error handling', () => {
     it('given empty ruleset, should throw a user friendly error', () => {
       expect(() => new Ruleset({})).toThrowError(
-        new RulesetValidationError('Ruleset must have rules or extends or overrides defined'),
+        new RulesetValidationError('Ruleset must have rules or extends or overrides defined', []),
       );
     });
   });

packages/core/src/ruleset/function.ts

@@ -8,56 +8,58 @@ import type { JSONSchema7 } from 'json-schema';
 
 import { printPath, PrintStyle, printValue } from '@stoplight/spectral-runtime';
 
-import { RulesetValidationError } from './validation';
+import { RulesetValidationError } from './validation/index';
 import { IFunctionResult, JSONSchema, RulesetFunction, RulesetFunctionWithValidator } from '../types';
+import { isObject } from 'lodash';
+import AggregateError = require('es-aggregate-error');
 
 const ajv = new Ajv({ allErrors: true, allowUnionTypes: true, strict: true, keywords: ['x-internal'] });
 ajvErrors(ajv);
 addFormats(ajv);
 
 export class RulesetFunctionValidationError extends RulesetValidationError {
-  constructor(fn: string, errors: ErrorObject[]) {
-    const messages = errors.map(error => {
-      switch (error.keyword) {
-        case 'type': {
-          const path = printPath(error.instancePath.slice(1).split('/'), PrintStyle.Dot);
-          const values = Array.isArray(error.params.type) ? error.params.type.join(', ') : String(error.params.type);
-
-          return `"${fn}" function and its "${path}" option accepts only the following types: ${values}`;
-        }
-
-        case 'required': {
-          const missingProperty = (error as RequiredError).params.missingProperty;
-          const missingPropertyPath =
-            error.instancePath === ''
-              ? missingProperty
-              : printPath([...error.instancePath.slice(1).split('/'), missingProperty], PrintStyle.Dot);
-
-          return `"${fn}" function is missing "${missingPropertyPath}" option`;
-        }
-
-        case 'additionalProperties': {
-          const additionalProperty = (error as AdditionalPropertiesError).params.additionalProperty;
-          const additionalPropertyPath =
-            error.instancePath === ''
-              ? additionalProperty
-              : printPath([...error.instancePath.slice(1).split('/'), additionalProperty], PrintStyle.Dot);
-
-          return `"${fn}" function does not support "${additionalPropertyPath}" option`;
-        }
-
-        case 'enum': {
-          const path = printPath(error.instancePath.slice(1).split('/'), PrintStyle.Dot);
-          const values = (error as EnumError).params.allowedValues.map(printValue).join(', ');
-
-          return `"${fn}" function and its "${path}" option accepts only the following values: ${values}`;
-        }
-        default:
-          return error.message;
+  constructor(fn: string, error: ErrorObject) {
+    super(RulesetFunctionValidationError.printMessage(fn, error), error.instancePath.slice(1).split('/'));
+  }
+
+  private static printMessage(fn: string, error: ErrorObject): string {
+    switch (error.keyword) {
+      case 'type': {
+        const path = printPath(error.instancePath.slice(1).split('/'), PrintStyle.Dot);
+        const values = Array.isArray(error.params.type) ? error.params.type.join(', ') : String(error.params.type);
+
+        return `"${fn}" function and its "${path}" option accepts only the following types: ${values}`;
+      }
+
+      case 'required': {
+        const missingProperty = (error as RequiredError).params.missingProperty;
+        const missingPropertyPath =
+          error.instancePath === ''
+            ? missingProperty
+            : printPath([...error.instancePath.slice(1).split('/'), missingProperty], PrintStyle.Dot);
+
+        return `"${fn}" function is missing "${missingPropertyPath}" option`;
       }
-    });
 
-    super(messages.join('\n'));
+      case 'additionalProperties': {
+        const additionalProperty = (error as AdditionalPropertiesError).params.additionalProperty;
+        const additionalPropertyPath =
+          error.instancePath === ''
+            ? additionalProperty
+            : printPath([...error.instancePath.slice(1).split('/'), additionalProperty], PrintStyle.Dot);
+
+        return `"${fn}" function does not support "${additionalPropertyPath}" option`;
+      }
+
+      case 'enum': {
+        const path = printPath(error.instancePath.slice(1).split('/'), PrintStyle.Dot);
+        const values = (error as EnumError).params.allowedValues.map(printValue).join(', ');
+
+        return `"${fn}" function and its "${path}" option accepts only the following values: ${values}`;
+      }
+      default:
+        return error.message ?? 'unknown error';
+    }
   }
 }
 
@@ -138,25 +140,27 @@ export function createRulesetFunction<I extends unknown, O extends unknown>(
 
   Reflect.defineProperty(wrappedFn, 'name', { value: fn.name });
 
-  const validOpts = new Set<unknown>();
+  const validOpts = new WeakSet();
   wrappedFn.validator = function (o: unknown): asserts o is O {
-    if (validOpts.has(o)) return; // I don't like this.
+    if (isObject(o) && validOpts.has(o)) return; // I don't like this.
 
     if (validateOptions(o)) {
-      validOpts.add(o);
+      if (isObject(o)) validOpts.add(o);
       return;
     }
 
     if (options === null) {
-      throw new TypeError(`"${fn.name || '<unknown>'}" function does not accept any options`);
+      throw new RulesetValidationError(`"${fn.name || '<unknown>'}" function does not accept any options`, []);
     } else if (
       'errors' in validateOptions &&
       Array.isArray(validateOptions.errors) &&
       validateOptions.errors.length > 0
     ) {
-      throw new RulesetFunctionValidationError(fn.name || '<unknown>', validateOptions.errors);
+      throw new AggregateError(
+        validateOptions.errors.map(error => new RulesetFunctionValidationError(fn.name || '<unknown>', error)),
+      );
     } else {
-      throw new Error(`"functionOptions" of "${fn.name || '<unknown>'}" function must be valid`);
+      throw new RulesetValidationError(`"functionOptions" of "${fn.name || '<unknown>'}" function must be valid`, []);
     }
   };
 

packages/core/src/ruleset/mergers/rules.ts

@@ -44,7 +44,7 @@ export function mergeRule(
           owner: existingRule.owner,
         });
       } else {
-        assertValidRule(rule);
+        assertValidRule(rule, name);
         return new Rule(name, rule, ruleset);
       }
 

packages/core/src/ruleset/meta/js-extensions.json

@@ -40,12 +40,12 @@
     },
     "Format": {
       "$anchor": "format",
-      "spectral-runtime": "format",
+      "x-spectral-runtime": "format",
       "errorMessage": "must be a valid format"
     },
     "Function": {
       "$anchor": "function",
-      "spectral-runtime": "function",
+      "x-spectral-runtime": "ruleset-function",
       "type": "object",
       "properties": {
         "function": true