Skip to content

Commit

Permalink
feat(core): improve validation
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip committed Apr 27, 2022
1 parent ec08efe commit e9fcea5
Show file tree
Hide file tree
Showing 30 changed files with 778 additions and 371 deletions.
4 changes: 1 addition & 3 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
5 changes: 4 additions & 1 deletion karma.conf.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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'],
Expand All @@ -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
Expand All @@ -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'),
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
58 changes: 54 additions & 4 deletions packages/cli/src/services/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -198,8 +202,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'],
),
]),
);
});
});
Expand All @@ -212,8 +240,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'],
),
]),
);
});

Expand Down
2 changes: 2 additions & 0 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@
"@stoplight/spectral-ref-resolver": "^1.0.0",
"@stoplight/spectral-runtime": "^1.0.0",
"@stoplight/types": "13.0.0",
"@types/es-aggregate-error": "^1.0.2",
"@types/json-schema": "^7.0.7",
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ const ruleset: RulesetDefinition = {
then: {
function: schema,
functionOptions: {
type: 'number',
schema: {
type: 'number',
},
},
},
},
Expand All @@ -39,7 +41,9 @@ const ruleset: RulesetDefinition = {
then: {
function: schema,
functionOptions: {
type: 'boolean',
schema: {
type: 'boolean',
},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/ruleset/__tests__/ruleset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', []),
);
});
});
Expand Down
98 changes: 51 additions & 47 deletions packages/core/src/ruleset/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}
}

Expand Down Expand Up @@ -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`, []);
}
};

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/ruleset/mergers/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function mergeRule(
owner: existingRule.owner,
});
} else {
assertValidRule(rule);
assertValidRule(rule, name);
return new Rule(name, rule, ruleset);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/ruleset/meta/js-extensions.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit e9fcea5

Please sign in to comment.