Skip to content

Commit

Permalink
feat(core): improve validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@P0lip
P0lip committed Apr 14, 2022
1 parent aeb7d5b commit 39708e9
Show file tree
Hide file tree
Showing 36 changed files with 859 additions and 429 deletions.
4 changes: 1 addition & 3 deletions jest.config.js
View file
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
View file
@@ -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
View file
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
View file
@@ -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
7 changes: 4 additions & 3 deletions packages/core/package.json
View file
Expand Up @@ -40,12 +40,14 @@
"@stoplight/spectral-parsers": "^1.0.0",
"@stoplight/spectral-ref-resolver": "^1.0.0",
"@stoplight/spectral-runtime": "^1.0.0",
"@stoplight/types": "12.3.0",
"@stoplight/types": "12.5.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",
"json-schema": "0.4.0",
"es-aggregate-error": "^1.0.7",
"jsonpath-plus": "6.0.1",
"lodash": "~4.17.21",
"lodash.topath": "^4.5.2",
Expand All @@ -60,7 +62,6 @@
"@stoplight/spectral-functions": "*",
"@stoplight/spectral-parsers": "*",
"@stoplight/yaml": "^4.2.2",
"@types/json-schema": "^7.0.7",
"@types/minimatch": "^3.0.5",
"@types/treeify": "^1.0.0",
"nock": "^13.1.0",
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/ruleset/__tests__/__fixtures__/overrides/formats.ts
View file
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
28 changes: 13 additions & 15 deletions packages/core/src/ruleset/__tests__/ruleset.test.ts
View file
Expand Up @@ -9,8 +9,8 @@ import { print } from './__helpers__/print';
import { RulesetValidationError } from '../validation/index';
import { isPlainObject } from '@stoplight/json';
import { Format } from '../format';
import { JSONSchema4, JSONSchema6, JSONSchema7 } from 'json-schema';
import { FormatsSet } from '../utils/formatsSet';
import type { JSONSchema4, JSONSchema6, JSONSchema7 } from 'json-schema';
import { Formats } from '../formats';

async function loadRuleset(mod: Promise<{ default: RulesetDefinition }>, source?: string): Promise<Ruleset> {
return new Ruleset((await mod).default, { source });
Expand Down Expand Up @@ -247,7 +247,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 Expand Up @@ -1378,24 +1378,22 @@ describe('Ruleset', () => {
},
});

expect(ruleset.rules['valid-id'].getGivenForFormats(new FormatsSet([draft4]))).toStrictEqual(['$..id']);
expect(ruleset.rules['valid-id'].getGivenForFormats(new FormatsSet([draft6]))).toStrictEqual(['$..$id']);
expect(ruleset.rules['valid-id'].getGivenForFormats(new FormatsSet([draft7]))).toStrictEqual(['$..$id']);
expect(ruleset.rules['valid-id'].getGivenForFormats(new FormatsSet([draft6, draft7]))).toStrictEqual([
'$..$id',
]);
expect(ruleset.rules['valid-id'].getGivenForFormats(new Formats([draft4]))).toStrictEqual(['$..id']);
expect(ruleset.rules['valid-id'].getGivenForFormats(new Formats([draft6]))).toStrictEqual(['$..$id']);
expect(ruleset.rules['valid-id'].getGivenForFormats(new Formats([draft7]))).toStrictEqual(['$..$id']);
expect(ruleset.rules['valid-id'].getGivenForFormats(new Formats([draft6, draft7]))).toStrictEqual(['$..$id']);

expect(ruleset.rules['valid-parameter'].getGivenForFormats(new FormatsSet([oas2]))).toStrictEqual([
expect(ruleset.rules['valid-parameter'].getGivenForFormats(new Formats([oas2]))).toStrictEqual([
'$.parameters[*]',
'$.paths[*].parameters[?(@ && !@.$ref)]',
'$.paths[*][get,put,post,delete,options,head,patch,trace].parameters[?(@ && !@.$ref)]',
]);
expect(ruleset.rules['valid-parameter'].getGivenForFormats(new FormatsSet([oas3]))).toStrictEqual([
expect(ruleset.rules['valid-parameter'].getGivenForFormats(new Formats([oas3]))).toStrictEqual([
'$.components.parameters[*]',
'$.paths[*].parameters[?(@ && !@.$ref)]',
'$.paths[*][get,put,post,delete,options,head,patch,trace].parameters[?(@ && !@.$ref)]',
]);
expect(ruleset.rules['valid-parameter'].getGivenForFormats(new FormatsSet([oas2, oas3]))).toStrictEqual([
expect(ruleset.rules['valid-parameter'].getGivenForFormats(new Formats([oas2, oas3]))).toStrictEqual([
'$.components.parameters[*]',
'$.paths[*].parameters[?(@ && !@.$ref)]',
'$.paths[*][get,put,post,delete,options,head,patch,trace].parameters[?(@ && !@.$ref)]',
Expand Down Expand Up @@ -1442,7 +1440,7 @@ describe('Ruleset', () => {
},
});

expect(() => ruleset.rules['valid-header'].getGivenForFormats(new FormatsSet([oas3]))).toThrowError(
expect(() => ruleset.rules['valid-header'].getGivenForFormats(new Formats([oas3]))).toThrowError(
ReferenceError(
'Alias "HeaderObject" is circular. Resolution stack: HeaderObject -> HeaderObjects -> Components -> HeaderObject',
),
Expand Down Expand Up @@ -1479,8 +1477,8 @@ describe('Ruleset', () => {
},
});

expect(ruleset.rules['valid-id'].getGivenForFormats(new FormatsSet([draft7]))).toStrictEqual([]);
expect(ruleset.rules['valid-id'].getGivenForFormats(new FormatsSet([]))).toStrictEqual([]);
expect(ruleset.rules['valid-id'].getGivenForFormats(new Formats([draft7]))).toStrictEqual([]);
expect(ruleset.rules['valid-id'].getGivenForFormats(new Formats([]))).toStrictEqual([]);
});

it('should be serializable', () => {
Expand Down
4 changes: 2 additions & 2 deletions ...ages/core/src/ruleset/utils/formatsSet.ts → packages/core/src/ruleset/formats.ts
View file
@@ -1,10 +1,10 @@
import type { Format } from '../format';
import type { Format } from './format';

function printFormat(format: Format): string {
return format.displayName ?? format.name;
}

export class FormatsSet<T extends Format = Format> extends Set<T> {
export class Formats<T extends Format = Format> extends Set<T> {
public toJSON(): string[] {
return Array.from(this).map(printFormat);
}
Expand Down
98 changes: 51 additions & 47 deletions packages/core/src/ruleset/rulesetFunction.ts → packages/core/src/ruleset/function.ts
View file
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

0 comments on commit 39708e9

Please sign in to comment.