Skip to content

Commit

Permalink
feat: use @stoplight/better-ajv-errors (#1236)
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip authored Jun 18, 2020
1 parent c44e00a commit ec986c9
Show file tree
Hide file tree
Showing 19 changed files with 70 additions and 153 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"test": "jest --silent"
},
"dependencies": {
"@stoplight/better-ajv-errors": "https://github.com/stoplightio/better-ajv-errors.git#bundled",
"@stoplight/json": "3.6.0",
"@stoplight/json-ref-readers": "1.1.1",
"@stoplight/json-ref-resolver": "3.0.9",
Expand All @@ -70,7 +71,6 @@
"abort-controller": "3.0.0",
"ajv": "6.12.2",
"ajv-oai": "1.2.0",
"better-ajv-errors": "0.6.7",
"blueimp-md5": "2.13.0",
"chalk": "4.0.0",
"eol": "0.9.1",
Expand All @@ -83,7 +83,7 @@
"proxy-agent": "3.1.1",
"strip-ansi": "6.0",
"text-table": "0.2",
"tslib": "1.11.1",
"tslib": "1.13.0",
"yargs": "15.3.1"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/services/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ describe('Linter service', () => {
}),
expect.objectContaining({
code: 'oas2-schema',
message: `Object should have required property \`title\`.`,
message: '`info` property should have required property `title`.',
path: [],
range: {
end: {
Expand Down
14 changes: 7 additions & 7 deletions src/functions/__tests__/schema-path.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('schema-path', () => {
expect(runSchemaPath(target, fieldToCheck, path)).toEqual([
{
path: ['example'],
message: 'Object should have required property `url`',
message: '`example` property should have required property `url`',
},
]);
});
Expand All @@ -79,7 +79,7 @@ describe('schema-path', () => {
expect(runSchemaPath(target, fieldToCheck, path)).toEqual([
{
path: ['example'],
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be string',
message: '`example` property type should be string',
},
]);
});
Expand Down Expand Up @@ -109,11 +109,11 @@ describe('schema-path', () => {
};
expect(runSchemaPath(target, '$.examples.*', path)).toEqual([
{
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be string',
message: '`id` property type should be string',
path: ['examples', 'application/json', 'id'],
},
{
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be string',
message: '`id` property type should be string',
path: ['examples', 'application/yaml', 'id'],
},
]);
Expand All @@ -138,11 +138,11 @@ describe('schema-path', () => {
};
expect(runSchemaPath(target, '$.schema.examples.*', path)).toEqual([
{
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be string',
message: '`0` property type should be string',
path: ['schema', 'examples', '0'],
},
{
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be string',
message: '`2` property type should be string',
path: ['schema', 'examples', '2'],
},
]);
Expand All @@ -159,7 +159,7 @@ describe('schema-path', () => {

expect(runSchemaPath(target, fieldToCheck, path)).toEqual([
{
message: '{{property|gravis|append-property|optional-typeof|capitalize}}format should match format `url`',
message: '`example` property should match format `url`',
path: ['example'],
},
]);
Expand Down
25 changes: 12 additions & 13 deletions src/functions/__tests__/schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('schema', () => {

expect(runSchema('', testSchema)).toEqual([
{
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be number',
message: 'Value type should be number',
path: [],
},
]);
Expand All @@ -38,7 +38,7 @@ describe('schema', () => {

expect(runSchema(0, testSchema)).toEqual([
{
message: `{{property|gravis|append-property|optional-typeof|capitalize}}type should be string`,
message: `Value type should be string`,
path: [],
},
]);
Expand All @@ -51,7 +51,7 @@ describe('schema', () => {

expect(runSchema(false, testSchema)).toEqual([
{
message: `{{property|gravis|append-property|optional-typeof|capitalize}}type should be string`,
message: `Value type should be string`,
path: [],
},
]);
Expand All @@ -64,7 +64,7 @@ describe('schema', () => {

expect(runSchema(null, testSchema)).toEqual([
{
message: `{{property|gravis|append-property|optional-typeof|capitalize}}type should be string`,
message: `Value type should be string`,
path: [],
},
]);
Expand Down Expand Up @@ -105,7 +105,7 @@ describe('schema', () => {
const input = { foo: 'bar' };
expect(runSchema(input, testSchema)).toEqual([
expect.objectContaining({
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be array',
message: 'Value type should be array',
path: [],
}),
]);
Expand All @@ -115,8 +115,7 @@ describe('schema', () => {
const input = ['1', '2'];
expect(runSchema(input, testSchema)).toEqual([
expect.objectContaining({
message:
'{{property|gravis|append-property|optional-typeof|capitalize}}maxItems should NOT have more than 1 items',
message: 'Object should not have more than 1 items',
path: [],
}),
]);
Expand Down Expand Up @@ -152,7 +151,7 @@ describe('schema', () => {
),
).toEqual([
{
message: `{{property|gravis|append-property|optional-typeof|capitalize}}type should be string`,
message: '`bar` property type should be string',
path: ['foo', 'bar'],
},
]);
Expand Down Expand Up @@ -186,7 +185,7 @@ describe('schema', () => {
const input = 'not an email';
expect(runSchema(input, testSchema)).toEqual([
expect.objectContaining({
message: '{{property|gravis|append-property|optional-typeof|capitalize}}format should match format `email`',
message: 'String should match format `email`',
path: [],
}),
]);
Expand Down Expand Up @@ -224,7 +223,7 @@ describe('schema', () => {
expect(runSchema(2, testSchema)).toEqual([
{
path: [],
message: `{{property|gravis|append-property|optional-typeof|capitalize}}type should be string`,
message: `Value type should be string`,
},
]);
expect(runSchema('a', testSchema2)).toEqual([]);
Expand All @@ -244,7 +243,7 @@ describe('schema', () => {
expect(runSchema(2, testSchema)).toEqual([
{
path: [],
message: `{{property|gravis|append-property|optional-typeof|capitalize}}type should be string`,
message: `Value type should be string`,
},
]);
expect(runSchema('a', testSchema2)).toEqual([]);
Expand Down Expand Up @@ -279,7 +278,7 @@ describe('schema', () => {
it('reports pretty enum errors for a number', () => {
expect(runSchema(2, testSchema)).toEqual([
{
message: `{{property|gravis|append-property|optional-typeof|capitalize}}type should be string`,
message: 'Value type should be string',
path: [],
},
]);
Expand All @@ -296,7 +295,7 @@ describe('schema', () => {
it('reports pretty enum errors for a string', () => {
expect(runSchema('baz', testSchema)).toEqual([
{
message: '{{property|gravis|append-property|optional-typeof|capitalize}}type should be integer',
message: 'Value type should be integer',
path: [],
},
]);
Expand Down
78 changes: 15 additions & 63 deletions src/functions/schema.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { decodePointerFragment } from '@stoplight/json';
import { Optional } from '@stoplight/types';
import * as AJV from 'ajv';
import { ValidateFunction } from 'ajv';
import * as jsonSpecV4 from 'ajv/lib/refs/json-schema-draft-04.json';
import * as jsonSpecV6 from 'ajv/lib/refs/json-schema-draft-06.json';
import * as jsonSpecV7 from 'ajv/lib/refs/json-schema-draft-07.json';
import { IOutputError } from 'better-ajv-errors';
import { capitalize, escapeRegExp } from 'lodash';
import * as betterAjvErrors from '@stoplight/better-ajv-errors';
import { IFunction, IFunctionResult, JSONSchema } from '../types';
const oasFormatValidator = require('ajv-oai/lib/format-validator');
const betterAjvErrors = require('better-ajv-errors/lib/modern');

export interface ISchemaFunction extends IFunction<ISchemaOptions> {
Ajv: typeof AJV;
Expand All @@ -32,12 +29,8 @@ export interface ISchemaOptions {
prepareResults?(errors: AJV.ErrorObject[]): void;
}

interface IAJVOutputError extends IOutputError {
path?: string;
}

const logger = {
warn(...args: any[]) {
warn(...args: unknown[]): void {
const firstArg = args[0];
if (typeof firstArg === 'string') {
if (firstArg.startsWith('unknown format')) return;
Expand All @@ -51,7 +44,7 @@ const logger = {
const ajvInstances = {};

function getAjv(oasVersion: Optional<number>, allErrors: Optional<boolean>): AJV.Ajv {
const type: string = oasVersion && oasVersion >= 2 ? 'oas' + oasVersion : 'jsonschema';
const type: string = oasVersion !== void 0 && oasVersion >= 2 ? 'oas' + oasVersion : 'jsonschema';
if (typeof ajvInstances[type] !== 'undefined') {
return ajvInstances[type];
}
Expand Down Expand Up @@ -92,7 +85,7 @@ function getSchemaId(schemaObj: JSONSchema): void | string {
}

const validators = new (class extends WeakMap<JSONSchema, ValidateFunction> {
public get({ schema: schemaObj, oasVersion, allErrors }: ISchemaOptions) {
public get({ schema: schemaObj, oasVersion, allErrors }: ISchemaOptions): ValidateFunction {
const ajv = getAjv(oasVersion, allErrors);
const schemaId = getSchemaId(schemaObj);
let validator = schemaId !== void 0 ? ajv.getSchema(schemaId) : void 0;
Expand All @@ -111,39 +104,6 @@ const validators = new (class extends WeakMap<JSONSchema, ValidateFunction> {
}
})();

const replaceProperty = (
substring: string,
potentialProperty: Optional<number | string>,
propertyName: Optional<string>,
) => {
if (typeof potentialProperty === 'string' && propertyName !== void 0) {
return `Property \`${propertyName}\``;
}

return '{{property|gravis|append-property|optional-typeof|capitalize}}';
};

const cleanAJVErrorMessage = (message: string, path: Optional<string>, suggestion: Optional<string>, type: string) => {
let cleanMessage = message.trim();

if (path) {
cleanMessage = message.replace(
new RegExp(`^${escapeRegExp(decodePointerFragment(path))}:?\\s*(?:(Property\\s+)([^\\s]+))?`),
replaceProperty,
);
} else if (cleanMessage.startsWith(':')) {
cleanMessage = cleanMessage.replace(/:\s*/, replaceProperty);
} else if (cleanMessage.startsWith('Property ')) {
cleanMessage = cleanMessage.replace(/(Property\s+)([^\s]+)/, replaceProperty);
} else {
cleanMessage = `${capitalize(type)} ${cleanMessage}`;
}

return `${cleanMessage.replace(/['"]/g, '`')}${
typeof suggestion === 'string' && suggestion.length > 0 ? `. ${suggestion}` : ''
}`;
};

export const schema: ISchemaFunction = (targetVal, opts, paths, { rule }) => {
const path = paths.target ?? paths.given;

Expand All @@ -164,26 +124,18 @@ export const schema: ISchemaFunction = (targetVal, opts, paths, { rule }) => {
try {
// we used the compiled validation now, hence this lookup here (see the logic above for more info)
const validator = opts.ajv ?? validators.get(opts);
if (!validator(targetVal) && validator.errors) {
if (validator(targetVal) === false && Array.isArray(validator.errors)) {
opts.prepareResults?.(validator.errors);

try {
results.push(
...(betterAjvErrors(schemaObj, targetVal, validator.errors, { format: 'js' }) as IAJVOutputError[]).map(
({ suggestion, error, path: errorPath }) => ({
message: cleanAJVErrorMessage(error, errorPath, suggestion, typeof targetVal),
path: [...path, ...(errorPath ? errorPath.replace(/^\//, '').split('/') : [])],
}),
),
);
} catch {
results.push(
...validator.errors.map(({ message, dataPath }) => ({
message: message ? cleanAJVErrorMessage(message, dataPath, void 0, typeof targetVal) : '',
path: [...path, ...dataPath.split('/').slice(1).map(decodePointerFragment)],
})),
);
}
results.push(
...betterAjvErrors(schemaObj, validator.errors, {
propertyPath: path,
targetValue: targetVal,
}).map(({ suggestion, error, path: errorPath }) => ({
message: suggestion !== void 0 ? `${error}. ${suggestion}` : error,
path: [...path, ...(errorPath !== '' ? errorPath.replace(/^\//, '').split('/') : [])],
})),
);
}
} catch (ex) {
if (!(ex instanceof AJV.MissingRefError)) {
Expand All @@ -204,7 +156,7 @@ export const schema: ISchemaFunction = (targetVal, opts, paths, { rule }) => {

schema.Ajv = AJV;
// eslint-disable-next-line @typescript-eslint/unbound-method
schema.createAJVInstance = (opts: AJV.Options) => {
schema.createAJVInstance = (opts: AJV.Options): AJV.Ajv => {
const ajv = new AJV(opts);

ajv.addFormat('int32', { type: 'number', validate: oasFormatValidator.int32 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe(`Rule '${ruleName}'`, () => {
expect(results).toEqual([
expect.objectContaining({
code: ruleName,
message: 'Headers schema type should be `object` (Object should have required property `type`).',
message: 'Headers schema type should be `object` (`headers` property should have required property `type`).',
path: ['components', 'messages', 'aMessage', 'headers'],
severity: rule.severity,
}),
Expand Down Expand Up @@ -115,7 +115,7 @@ describe(`Rule '${ruleName}'`, () => {
expect(results).toEqual([
expect.objectContaining({
code: ruleName,
message: 'Headers schema type should be `object` (Object should have required property `type`).',
message: 'Headers schema type should be `object` (`headers` property should have required property `type`).',
path: ['components', 'messageTraits', 'aTrait', 'headers'],
severity: rule.severity,
}),
Expand All @@ -134,7 +134,7 @@ describe(`Rule '${ruleName}'`, () => {
expect(results).toEqual([
expect.objectContaining({
code: ruleName,
message: 'Headers schema type should be `object` (Object should have required property `type`).',
message: 'Headers schema type should be `object` (`headers` property should have required property `type`).',
path: ['channels', 'users/{userId}/signedUp', property, 'message', 'headers'],
severity: rule.severity,
}),
Expand Down
6 changes: 3 additions & 3 deletions src/rulesets/asyncapi/__tests__/asyncapi-payload-default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe(`Rule '${ruleName}'`, () => {
expect(results).toEqual([
expect.objectContaining({
code: ruleName,
message: 'Object should have required property `value`',
message: '`default` property should have required property `value`',
path: ['components', 'messages', 'aMessage', 'payload', 'default'],
severity: rule.severity,
}),
Expand All @@ -87,7 +87,7 @@ describe(`Rule '${ruleName}'`, () => {
expect(results).toEqual([
expect.objectContaining({
code: ruleName,
message: 'Object should have required property `value`',
message: '`default` property should have required property `value`',
path: ['components', 'messageTraits', 'aTrait', 'payload', 'default'],
severity: rule.severity,
}),
Expand All @@ -106,7 +106,7 @@ describe(`Rule '${ruleName}'`, () => {
expect(results).toEqual([
expect.objectContaining({
code: ruleName,
message: 'Object should have required property `value`',
message: '`default` property should have required property `value`',
path: ['channels', 'users/{userId}/signedUp', property, 'message', 'payload', 'default'],
severity: rule.severity,
}),
Expand Down
Loading

0 comments on commit ec986c9

Please sign in to comment.