Fix: Report inherited errors at extends location

Fix webhintio#1717

packages/hint-babel-config/src/is-valid.ts

@@ -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);
     }
 }

packages/hint-babel-config/tests/is-valid.ts

@@ -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
+                }
+            }
         ]
     }
 ];

packages/hint-typescript-config/src/is-valid.ts

@@ -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);
     }
 }

packages/hint-typescript-config/tests/fixtures/extends-with-error/tsconfig.json

@@ -0,0 +1,6 @@
+{
+    "compilerOptions": {
+        "module": "esnext"
+    },
+    "extends": "../no-comments/invalid/tsconfig.json"
+}

packages/hint-typescript-config/tests/is-valid.ts

@@ -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
+                }
+            }
         ]
     }
 ];

packages/hint-typescript-config/tests/no-comments.ts

@@ -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
+            }
+        }]
     }
 ];
 

packages/hint/src/lib/types/parser.ts

@@ -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;
             }
         }
 

packages/hint/src/lib/utils/json-parser.ts

@@ -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);
 };

packages/hint/tests/lib/types/parser.ts

@@ -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) => {

packages/parser-babel-config/src/parser.ts

@@ -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;
             }

packages/parser-babel-config/src/types.ts

@@ -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;

packages/parser-babel-config/tests/tests.ts

@@ -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();
 });