Skip to content

Commit

Permalink
Fix: Error returned when the config is invalid.
Browse files Browse the repository at this point in the history 
Fix webhintio#1526
  • Loading branch information
@sarvaje
sarvaje committed Jan 5, 2019
1 parent 5ddaa75 commit ddb87ef
Show file tree
Hide file tree
Showing 8 changed files with 238 additions and 58 deletions.
7 changes: 3 additions & 4 deletions packages/hint-babel-config/src/is-valid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ export default class BabelConfigIsValidHint implements IHint {
};

const invalidSchema = async (fetchEnd: BabelConfigInvalidSchema) => {
const { errors, prettifiedErrors, resource } = fetchEnd;
const { prettifiedErrors, resource } = fetchEnd;

debug(`parse::error::babel-config::schema received`);

for (let i = 0; i < errors.length; i++) {
for (let i = 0; i < prettifiedErrors.length; i++) {
const message = prettifiedErrors[i];
const location = errors[i].location;

await context.report(resource, message, { location });
await context.report(resource, message);
}
};

Expand Down
8 changes: 4 additions & 4 deletions packages/hint-babel-config/tests/is-valid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const tests: HintLocalTest[] = [
{
name: `Invalid .babelrc configuration should fail`,
path: path.join(__dirname, 'fixtures', 'invalid-schema', '.babelrc'),
reports: [{ message: `'moduleId' should be string.` }]
reports: [{ message: `moduleId should be 'string'.` }]
},
{
name: `Invalid json file should fail`,
Expand All @@ -32,17 +32,17 @@ const tests: HintLocalTest[] = [
{
name: `If package.json contains invalid "babel" property, it should fail`,
path: path.join(__dirname, 'fixtures', 'has-invalid-babel-package-json', 'package.json'),
reports: [{ message: `'moduleId' should be string.` }]
reports: [{ message: `moduleId should be 'string'.` }]
},
{
name: `If .babelrc contains an additional property, it should fail`,
path: path.join(__dirname, 'fixtures', 'has-additional-property', '.babelrc'),
reports: [{ message: `Should NOT have additional properties. Additional property found 'additional'.` }]
reports: [{ message: `root should NOT have additional properties. Additional property found 'additional'.` }]
},
{
name: `If .babelrc contains an invalid value, it should fail`,
path: path.join(__dirname, 'fixtures', 'has-invalid-enum-property', '.babelrc'),
reports: [{ message: `'sourceMaps' should be equal to one of the allowed values 'both, inline, true, false'. Value found 'invalidValue'` }]
reports: [{ message: `sourceMaps should be equal to one of the allowed values 'both, inline, true, false'. Value found 'invalidValue'` }]
},
{
name: 'If .babelrc contains a circular reference, it should fail',
Expand Down
7 changes: 3 additions & 4 deletions packages/hint-typescript-config/src/is-valid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,14 @@ export default class TypeScriptConfigIsValid implements IHint {
};

const invalidSchema = async (fetchEnd: TypeScriptConfigInvalidSchema) => {
const { errors, prettifiedErrors, resource } = fetchEnd;
const { prettifiedErrors, resource } = fetchEnd;

debug(`parse::error::typescript-config::schema received`);

for (let i = 0; i < errors.length; i++) {
for (let i = 0; i < prettifiedErrors.length; i++) {
const message = prettifiedErrors[i];
const location = errors[i].location;

await context.report(resource, message, { location });
await context.report(resource, message);
}
};

Expand Down
9 changes: 2 additions & 7 deletions packages/hint-typescript-config/tests/is-valid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,13 @@ const tests: HintLocalTest[] = [
{
name: 'Invalid configuration should fail',
path: path.join(__dirname, 'fixtures', 'invalidschemaenum'),
reports: [{
message: `'compilerOptions.lib[3]' should be equal to one of the allowed values 'es5, es6, es2015, es7, es2016, es2017, es2018, esnext, dom, dom.iterable, webworker, scripthost, es2015.core, es2015.collection, es2015.generator, es2015.iterable, es2015.promise, es2015.proxy, es2015.reflect, es2015.symbol, es2015.symbol.wellknown, es2016.array.include, es2017.object, es2017.intl, es2017.sharedmemory, es2017.string, es2017.typedarrays, es2018.intl, es2018.promise, es2018.regexp, esnext.asynciterable, esnext.array, esnext.intl, esnext.symbol'. Value found 'invalidlib'`,
position: { column: 12, line: 9 }
}]
reports: [{ message: `compilerOptions.lib[3] should be equal to one of the allowed values 'es5, es6, es2015, es7, es2016, es2017, es2018, esnext, dom, dom.iterable, webworker, scripthost, es2015.core, es2015.collection, es2015.generator, es2015.iterable, es2015.promise, es2015.proxy, es2015.reflect, es2015.symbol, es2015.symbol.wellknown, es2016.array.include, es2017.object, es2017.intl, es2017.sharedmemory, es2017.string, es2017.typedarrays, es2018.intl, es2018.promise, es2018.regexp, esnext.asynciterable, esnext.array, esnext.intl, esnext.symbol'. Value found 'invalidlib'` }]
},
{
name: 'If schema has an invalid pattern, it should fail',
path: path.join(__dirname, 'fixtures', 'invalidschemapattern'),
reports: [
{ message: `'compilerOptions.target' should be equal to one of the allowed values 'es3, es5, es6, es2015, es2016, es2017, es2018, esnext'. Value found 'invalid'` },
{ message: `'compilerOptions.target' should match pattern '^([eE][sS]([356]|(201[5678])|[nN][eE][xX][tT]))$'. Value found 'invalid'` },
{ message: `'compilerOptions.target' should be equal to one of the allowed values 'es3, es5, es6, es2015, es2016, es2017, es2018, esnext'. Value found 'invalid' or 'compilerOptions.target' should match pattern '^([eE][sS]([356]|(201[5678])|[nN][eE][xX][tT]))$'. Value found 'invalid'` }
{ message: `compilerOptions.target should be equal to one of the allowed values 'es3, es5, es6, es2015, es2016, es2017, es2018, esnext'. Value found '"invalid"'. Or compilerOptions.target should match pattern '^([eE][sS]([356]|(201[5678])|[nN][eE][xX][tT]))$'. Value found 'invalid'` }
]
},
{
Expand Down
12 changes: 9 additions & 3 deletions packages/hint/src/lib/config/config-validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { validate } from '../utils/schema-validator';
import { debug as d } from '../utils/debug';
import { UserConfig } from '../types';
import * as logger from '../utils/logging';
import { SchemaValidationResult } from '../types/schema-validation-result';

const debug = d(__filename);
const schema = require('./config-schema.json');
Expand All @@ -25,10 +26,15 @@ const schema = require('./config-schema.json');

/** Validates that a given config object is valid */
export const validateConfig = (config: UserConfig): boolean => {

debug('Validating configuration');
if (!validate(schema, config).valid) {
logger.error('Configuration schema is not valid');
const validateInfo: SchemaValidationResult = validate(schema, config);

if (!validateInfo.valid) {
logger.error('Configuration schema is not valid:');

validateInfo.prettifiedErrors.forEach((error: string) => {
logger.error(` - ${error}`);
});

return false;
}
Expand Down
150 changes: 128 additions & 22 deletions packages/hint/src/lib/utils/schema-validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import {
cloneDeep,
groupBy,
reduce,
without
without,
Dictionary
} from 'lodash';

import { IJSONLocationFunction, ISchemaValidationError, SchemaValidationResult } from '../types';
Expand All @@ -28,8 +29,11 @@ enum ErrorKeyword {
additionalProperties = 'additionalProperties',
anyOf = 'anyOf',
enum = 'enum',
oneOf = 'oneOf',
pattern = 'pattern',
type = 'type'
required = 'required',
type = 'type',
uniqueItems = 'uniqueItems'
}

const generateError = (type: string, action: ((error: ajv.ErrorObject, property: string, errors?: ajv.ErrorObject[]) => string)): ((error: ajv.ErrorObject, errors?: ajv.ErrorObject[]) => string | null) => {
Expand All @@ -50,7 +54,7 @@ const generateError = (type: string, action: ((error: ajv.ErrorObject, property:
const generateAdditionalPropertiesError = generateError(ErrorKeyword.additionalProperties, (error: ajv.ErrorObject, property: string): string => {
const additionalProperty = (error.params as ajv.AdditionalPropertiesParams).additionalProperty;

return `${property ? `'${property}' ` : ''}${property ? error.message : `${error.message && error.message[0].toLocaleUpperCase()}${error.message && error.message.substr(1)}`}. Additional property found '${additionalProperty}'.`;
return `${property ? `${property}` : 'root'} ${property ? error.message : `${error.message}`}. Additional property found '${additionalProperty}'.`;
});

/**
Expand All @@ -59,38 +63,62 @@ const generateAdditionalPropertiesError = generateError(ErrorKeyword.additionalP
const generateEnumError = generateError(ErrorKeyword.enum, (error: ajv.ErrorObject, property: string): string => {
const allowedValues = (error.params as ajv.EnumParams).allowedValues;

return `'${property}' ${error.message} '${allowedValues.join(', ')}'. Value found '${error.data}'`;
return `${property} ${error.message} '${allowedValues.join(', ')}'. Value found '${error.data}'`;
});

/**
* Returns a readable error for 'pattern' errors.
*/
const generatePatternError = generateError(ErrorKeyword.pattern, (error: ajv.ErrorObject, property: string) => {
return `'${property}' ${error.message && error.message.replace(/"/g, '\'')}. Value found '${error.data}'`;
return `${property} ${error.message && error.message.replace(/"/g, '\'')}. Value found '${error.data}'`;
});

/**
* Returns a readable error for 'type' errors.
*/
const generateTypeError = generateError(ErrorKeyword.type, (error: ajv.ErrorObject, property: string) => {
return `'${property}' ${error.message && error.message.replace(/"/g, '\'')}.`;
return `${property} should be '${(error.params as ajv.TypeParams).type}'.`;
});

/**
* Returns a readable error for 'anyOf' errors.
*/
const generateAnyOfError = generateError(ErrorKeyword.anyOf, (error: ajv.ErrorObject, property: string, errors?: ajv.ErrorObject[]): string => {
const otherErrors = without(errors, error);
const generateUniqueItemError = generateError(ErrorKeyword.uniqueItems, (error: ajv.ErrorObject, property: string) => {
return `${property} ${error.message && error.message.replace(/"/g, '\'')}.`;
});

const results = otherErrors.map((otherError) => {
// eslint-disable-next-line typescript/no-use-before-define, no-use-before-define
return generate(otherError);
});
const getRequiredProperty = (error: ajv.ErrorObject): string => {
return `'${(error.params as ajv.RequiredParams).missingProperty}'`;
};

return results.join(' or ');
});
const getTypeProperty = (error: ajv.ErrorObject): string => {
return `'${(error.params as ajv.TypeParams).type}'`;
};

const errorGenerators: Array<((error: ajv.ErrorObject, errors?: ajv.ErrorObject[]) => string | null)> = [generateAdditionalPropertiesError, generateEnumError, generatePatternError, generateTypeError, generateAnyOfError];
const getEnumValues = (error: ajv.ErrorObject): string => {
return `'${(error.params as ajv.EnumParams).allowedValues.join(', ')}'`;
};

const generateAnyOfMessageRequired = (errrors: ajv.ErrorObject[]): string => {
return `should have required properties ${errrors.map(getRequiredProperty).join(' or ')}`;
};

const generateAnyOfMessageType = (errors: ajv.ErrorObject[]): string => {
return `should be ${errors.map(getTypeProperty).join(' or ')}.`;
};

const generateAnyOfMessageEnum = (errors: ajv.ErrorObject[]): string => {
return `should be equal to one of the allowed values ${errors.map(getEnumValues).join(' or ')}. Value found '${JSON.stringify(errors[0].data)}'.`;
};

type GenerateAnyOfMessage = {
[index: string]: (errors: ajv.ErrorObject[]) => string;
};

const generateAnyOfMessage: GenerateAnyOfMessage = {
[ErrorKeyword.required]: generateAnyOfMessageRequired,
[ErrorKeyword.type]: generateAnyOfMessageType,
[ErrorKeyword.enum]: generateAnyOfMessageEnum
};

const errorGenerators: Array<((error: ajv.ErrorObject, errors?: ajv.ErrorObject[]) => string | null)> = [generateAdditionalPropertiesError, generateEnumError, generatePatternError, generateTypeError, generateUniqueItemError];

/**
* Returns a readable error message.
Expand All @@ -107,16 +135,94 @@ const generate = (error: ajv.ErrorObject, errors?: ajv.ErrorObject[]): string |
}, error.message || '');
};

/**
* Returns a readable error for 'anyOf' and 'oneOf' errors.
*/
const generateAnyOfError = (error: ajv.ErrorObject, errors?: ajv.ErrorObject[]): string => {
const otherErrors = without(errors, error);
const grouped = groupBy(otherErrors, 'keyword');

const results = reduce(grouped, (allMessages, groupedErrors, keyword) => {
const dataPath = error.dataPath;

const messageGenerator = generateAnyOfMessage[keyword];

if (messageGenerator) {
allMessages.push(`${dataPath ? dataPath.substr(1) : 'root'} ${messageGenerator(groupedErrors)}`);

return allMessages;
}

groupedErrors.forEach((error) => {
const errorGenerated = generate(error, groupedErrors) || '';

if (errorGenerated) {
allMessages.push(`${errorGenerated}`);
}
});

return allMessages;
}, [] as string[]);

return results.join(' Or ');
};

const generateErrorsMessage = (errors: ajv.ErrorObject[]): string[] => {
const grouped = groupBy(errors, 'keyword');

const result = reduce(grouped, (allMessages, groupedErrors, keyword) => {
if (keyword === ErrorKeyword.required) {
const dataPath = groupedErrors[0].dataPath;

allMessages.push(`${dataPath ? dataPath.substr(1) : 'root'} should have required properties ${groupedErrors.map(getRequiredProperty).join(' and ')}`);

return allMessages;
}

groupedErrors.forEach((error) => {
allMessages.push(generate(error, groupedErrors) || '');
});

return allMessages;
}, [] as string[]);

return result;
};

const prettify = (errors: ajv.ErrorObject[]) => {
const grouped = groupBy(errors, 'dataPath');
const grouped: Dictionary<ajv.ErrorObject[]> = groupBy(errors, 'dataPath');

const result = reduce(grouped, (allMessages, groupErrors: ajv.ErrorObject[]) => {
groupErrors.forEach((error) => {
allMessages.push(generate(error, groupErrors) || '');
let errors = groupErrors;

const anyOf = groupErrors.find((error) => {
return error.keyword === ErrorKeyword.anyOf || error.keyword === ErrorKeyword.oneOf;
});

return allMessages;
if (anyOf) {
const anyOfErrors = groupErrors.filter((error) => {
return error.schemaPath.includes(anyOf.schemaPath) || anyOf.schema.some((schema: any) => {
return error.schemaPath.includes(schema.$ref);
});
});

errors = without(groupErrors, ...anyOfErrors);

allMessages.push(generateAnyOfError(anyOf, anyOfErrors));


if (errors.length === 0) {
return allMessages;
}
}

/*
* If there is no 'anyOf' error, then join with 'and' the rest of the messages.
* if they have the same keyword.
*/
return allMessages.concat(generateErrorsMessage(errors));

// return allMessages;
}, [] as string[]);

return result;
Expand Down
Loading

0 comments on commit ddb87ef

Please sign in to comment.