Permalink
Browse files

Fix: Report inherited errors at `extends` location

Fix #1717
Close #1808
  • Loading branch information...
molant committed Feb 5, 2019
1 parent 7655590 commit 36b564624c0f899987d1b9dd84185899e592dfb9
@@ -5,7 +5,7 @@ import { debug as d } from 'hint/dist/src/lib/utils/debug';
import { IHint } from 'hint/dist/src/lib/types';
import { HintContext } from 'hint/dist/src/lib/hint-context';

import { BabelConfigEvents, BabelConfigInvalidJSON, BabelConfigInvalidSchema } from '@hint/parser-babel-config';
import { BabelConfigEvents, BabelConfigExtendsError, BabelConfigInvalidJSON, BabelConfigInvalidSchema } from '@hint/parser-babel-config';

import meta from './meta/is-valid';

@@ -28,6 +28,14 @@ export default class BabelConfigIsValidHint implements IHint {
await context.report(resource, error.message);
};

const invalidExtends = async (babelConfigInvalid: BabelConfigExtendsError, event: string) => {
const { error, resource, getLocation } = babelConfigInvalid;

debug(`${event} received`);

await context.report(resource, error.message, { location: getLocation('extends') });
};

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

@@ -41,8 +49,7 @@ export default class BabelConfigIsValidHint implements IHint {
};

context.on('parse::error::babel-config::json', invalidJSONFile);
context.on('parse::error::babel-config::circular', invalidJSONFile);
context.on('parse::error::babel-config::extends', invalidJSONFile);
context.on('parse::error::babel-config::extends', invalidExtends);
context.on('parse::error::babel-config::schema', invalidSchema);
}
}
@@ -72,14 +72,26 @@ const tests: HintLocalTest[] = [
name: 'If .babelrc contains a circular reference, it should fail',
path: path.join(__dirname, 'fixtures', 'circular'),
reports: [
{ message: `Circular reference found in file ${path.join(__dirname, 'fixtures', 'circular-2', '.babelrc')}` }
{
message: `Circular reference found in file ${path.join(__dirname, 'fixtures', 'circular-2', '.babelrc')}`,
position: {
column: 3,
line: 1
}
}
]
},
{
name: 'If .babelrc contains an invalid extends, it should fail',
path: path.join(__dirname, 'fixtures', 'invalid-extends'),
reports: [
{ message: `Unexpected token ' in JSON at position 191` }
{
message: `Unexpected token ' in JSON at position 191`,
position: {
column: 3,
line: 1
}
}
]
}
];
@@ -7,6 +7,7 @@ import { debug as d } from 'hint/dist/src/lib/utils/debug';

import {
TypeScriptConfigEvents,
TypeScriptConfigExtendsError,
TypeScriptConfigInvalidJSON,
TypeScriptConfigInvalidSchema
} from '@hint/parser-typescript-config';
@@ -34,6 +35,14 @@ export default class TypeScriptConfigIsValid implements IHint {
await context.report(resource, error.message);
};

const invalidExtends = async (typeScriptConfigInvalid: TypeScriptConfigExtendsError, event: string) => {
const { error, resource, getLocation } = typeScriptConfigInvalid;

debug(`${event} received`);

await context.report(resource, error.message, { location: getLocation('extends') });
};

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

@@ -42,13 +51,12 @@ export default class TypeScriptConfigIsValid implements IHint {
for (let i = 0; i < groupedErrors.length; i++) {
const groupedError = groupedErrors[i];

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

context.on('parse::error::typescript-config::json', invalidJSONFile);
context.on('parse::error::typescript-config::circular', invalidJSONFile);
context.on('parse::error::typescript-config::extends', invalidJSONFile);
context.on('parse::error::typescript-config::extends', invalidExtends);
context.on('parse::error::typescript-config::schema', invalidSchema);
}
}
@@ -0,0 +1,6 @@
{
"compilerOptions": {
"module": "esnext"
},
"extends": "../no-comments/invalid/tsconfig.json"
}
@@ -48,14 +48,26 @@ const tests: HintLocalTest[] = [
name: 'If the configuration has a circular reference, it should fail',
path: path.join(__dirname, 'fixtures', 'circular'),
reports: [
{ message: `Circular reference found in file ${path.join(__dirname, 'fixtures', 'circular-2', 'tsconfig.circular.json')}` }
{
message: `Circular reference found in file ${path.join(__dirname, 'fixtures', 'circular-2', 'tsconfig.circular.json')}`,
position: {
column: 5,
line: 4
}
}
]
},
{
name: 'If the configuration has an invalid extends, it should fail',
path: path.join(__dirname, 'fixtures', 'invalid-extends'),
reports: [
{ message: `Unexpected token i in JSON at position 0` }
{
message: `Unexpected token i in JSON at position 0`,
position: {
column: 5,
line: 4
}
}
]
}
];
@@ -15,6 +15,17 @@ const tests: HintLocalTest[] = [
name: 'Configuration with "compilerOptions.removeComments = false" should fail',
path: path.join(__dirname, 'fixtures', 'no-comments', 'invalid'),
reports: [{ message: 'The compiler option "removeComments" should be enabled to reduce the output size.' }]
},
{
name: 'Configuration with "compilerOptions.removeComments = false" in extends should fail',
path: path.join(__dirname, 'fixtures', 'extends-with-error'),
reports: [{
message: 'The compiler option "removeComments" should be enabled to reduce the output size.',
position: {
column: 5,
line: 4
}
}]
}
];

@@ -8,6 +8,10 @@ import getAsPathString from '../utils/network/as-path-string';
import loadJSONFile from '../utils/fs/load-json-file';
import { Events } from './events';

export interface IParsingError extends Error {
resource: string;
}

export type ExtendableConfiguration = {
extends: string;
};
@@ -21,7 +25,7 @@ export abstract class Parser<E extends Events = Events> {
protected engine: Engine<E>;
protected name: string;

protected async finalConfig<T extends ExtendableConfiguration>(config: T, resource: string): Promise<T | null> {
protected finalConfig<T extends ExtendableConfiguration>(config: T, resource: string): T | IParsingError {
if (!config.extends) {
return config;
}
@@ -49,12 +53,12 @@ export abstract class Parser<E extends Events = Events> {

if (configIncludes.has(configPath)) {

await (this.engine as Engine<Events>).emitAsync(`parse::error::${this.name}::circular` as 'parse::error::*', {
error: new Error(`Circular reference found in file ${lastPath}`),
resource
});
const error = new Error(`Circular reference found in file ${lastPath}`) as IParsingError;
const lastPathUri = getAsUri(lastPath);

error.resource = lastPathUri && lastPathUri.toString() || lastPath;

return null;
return error;
}

delete finalConfigJSON.extends;
@@ -67,13 +71,11 @@ export abstract class Parser<E extends Events = Events> {

finalConfigJSON = merge({}, extendedConfig, finalConfigJSON);
} catch (err) {
const lastPathUri = getAsUri(lastPath);

await (this.engine as Engine<Events>).emitAsync(`parse::error::${this.name}::extends` as 'parse::error::*', {
error: err,
resource
});
err.resource = lastPathUri && lastPathUri.toString() || lastPath;

return null;
return err;
}
}

@@ -8,11 +8,13 @@ class JSONResult implements IJSONResult {
private _data: any;
private _lines: string[];
private _root: Node;
private _alternatePath: string | undefined;

public constructor(data: any, root: Node, lines: string[]) {
public constructor(data: any, root: Node, lines: string[], alternatePath?: string) {
this._data = data;
this._lines = lines;
this._root = root;
this._alternatePath = alternatePath;

// Ensure `getLocation` can be passed around without losing context
this.getLocation = this.getLocation.bind(this);
@@ -26,6 +28,15 @@ class JSONResult implements IJSONResult {
const segments = this.pathToSegments(path);
const node = findNodeAtLocation(this._root, segments) || null;

/**
* The node isn't in the current file. Use alternative path if provided. This happens
* when extending configurations.
*/
if (!node && this._alternatePath && path !== this._alternatePath) {

return this.getLocation(this._alternatePath, options);
}

return this.offsetToLocation(this.getAdjustedOffset(node, path, options));
}

@@ -129,8 +140,9 @@ class JSONResult implements IJSONResult {
/**
* Parse the provided JSON returning a `JSONResult` with location information.
* @param json The JSON string to parse
* @param alternatePath The alternative `path` to use when one is not found
*/
export const parseJSON = (json: string): IJSONResult => {
export const parseJSON = (json: string, alternatePath?: string): IJSONResult => {
const lines = json.split('\n');
const data = parse(json);
const root = parseTree(json);
@@ -141,5 +153,5 @@ export const parseJSON = (json: string): IJSONResult => {
JSON.parse(json);
}

return new JSONResult(data, root, lines);
return new JSONResult(data, root, lines, alternatePath);
};
@@ -4,7 +4,7 @@ import * as proxyquire from 'proxyquire';
import { EventEmitter2 } from 'eventemitter2';

import { ExtendableConfiguration } from '../../../src/lib/types/parser';
import { Events, ErrorEvent } from '../../../src/lib/types';
import { Events } from '../../../src/lib/types';
import { Engine } from '../../../src/lib/engine';

type FileModule = {
@@ -109,7 +109,7 @@ test(`If config doesn't have an extends property, it should return the same obje
t.true(config === result);
});

test('If there is a circular reference, it should throw an exception', async (t) => {
test('If there is a circular reference, it should return an instance of an Error', async (t) => {
const sandbox = t.context.sandbox;
const Parser = loadScript(t.context);

@@ -127,18 +127,15 @@ test('If there is a circular reference, it should throw an exception', async (t)

sandbox.stub(t.context.asPathString, 'default').returns('circularReference');
sandbox.stub(t.context.path, 'resolve').returns('circularReference');
const engineEmitAsyncSpy = sandbox.spy(t.context.engine, 'emitAsync');

const testParser = new TestParser(t.context.engine);

const result = await testParser.config(config, 'circularReference');

t.is(result, null);
t.true(engineEmitAsyncSpy.calledOnce);
t.is((engineEmitAsyncSpy.args[0][1] as ErrorEvent).error.message, 'Circular reference found in file circularReference');
t.true(result instanceof Error);
t.is(result.message, 'Circular reference found in file circularReference');
});

test('If one of the extended files is no a valid JSON, it should throw an exception', async (t) => {
test('If one of the extended files is no a valid JSON, it should return an instance of an Error', async (t) => {
const sandbox = t.context.sandbox;
const Parser = loadScript(t.context);

@@ -157,14 +154,11 @@ test('If one of the extended files is no a valid JSON, it should throw an except
sandbox.stub(t.context.asPathString, 'default').returns('valid-with-invalid-extends');
sandbox.stub(t.context.path, 'resolve').returns('invalid-extends');
sandbox.stub(t.context.loadJSONFileModule, 'default').throws(new Error('InvalidJSON'));
const engineEmitAsyncSpy = sandbox.spy(t.context.engine, 'emitAsync');

const testParser = new TestParser(t.context.engine);

const result = await testParser.config(config, 'valid-with-invalid-extends');

t.true(engineEmitAsyncSpy.calledOnce);
t.is(result, null);
t.true(result instanceof Error);
});

test('If everything is ok, it should merge all the extended configurations', async (t) => {
@@ -58,7 +58,7 @@ export default class BabelConfigParser extends Parser<BabelConfigEvents> {
try {
const response = fetchEnd.response;
// When using local connector to read local files, 'content' is empty.
let result = parseJSON(response.body.content);
let result = parseJSON(response.body.content, 'extends');

if (isPackageJson && !result.data.babel) {
return;
@@ -74,6 +74,17 @@ export default class BabelConfigParser extends Parser<BabelConfigEvents> {

const finalConfig = await this.finalConfig<BabelConfig>(config, resource);

if (finalConfig instanceof Error) {
await this.engine.emitAsync(`parse::error::babel-config::extends`,
{
error: finalConfig,
getLocation: result.getLocation,
resource
});

return;
}

if (!finalConfig) {
return;
}
@@ -50,10 +50,13 @@ export type BabelConfigInvalidSchema = ErrorEvent & {
groupedErrors: GroupedError[];
};

export type BabelConfigExtendsError = ErrorEvent & {
getLocation: IJSONLocationFunction;
}

export type BabelConfigEvents = Events & {
'parse::end::babel-config': BabelConfigParsed;
'parse::error::babel-config::circular': ErrorEvent;
'parse::error::babel-config::extends': ErrorEvent;
'parse::error::babel-config::extends': BabelConfigExtendsError;
'parse::error::babel-config::json': BabelConfigInvalidJSON;
'parse::error::babel-config::schema': BabelConfigInvalidSchema;
'parse::start::babel-config': BabelConfigParseStart;
@@ -243,7 +243,7 @@ test('If we receive a valid json with an extends, it should emit the event parse
sandbox.restore();
});

test('If we receive a json with an extends with a loop, it should emit the event parse::error::babel-config::circular', async (t) => {
test('If we receive a json with an extends with a loop, it should emit the event parse::error::babel-config::extends', async (t) => {
const sandbox = sinon.createSandbox();
const engine: Engine<BabelConfigEvents> = new EventEmitter2({
delimiter: '::',
@@ -265,7 +265,7 @@ test('If we receive a json with an extends with a loop, it should emit the event

// 3 times, the previous call, the start parse and the error
t.is(engineEmitASyncSpy.callCount, 3);
t.is(engineEmitASyncSpy.args[2][0], 'parse::error::babel-config::circular');
t.is(engineEmitASyncSpy.args[2][0], 'parse::error::babel-config::extends');

sandbox.restore();
});
Oops, something went wrong.

0 comments on commit 36b5646

Please sign in to comment.