Skip to content

Commit

Permalink
fix(compiler): include start and length with diagnostics (#856)
Browse files Browse the repository at this point in the history 
* fix(compiler): include start and length with diagnostics

* fix(compiler): cleanup & solidify Location type

* fix(compiler): update new tests

* fix(compiler): resolve conflicts with platform compiler
  • Loading branch information
@jye-sf
jye-sf committed Nov 28, 2018
1 parent 6a20ae0 commit b4fad21
Show file tree
Hide file tree
Showing 59 changed files with 406 additions and 234 deletions.
10 changes: 7 additions & 3 deletions packages/@lwc/compiler/src/bundler/bundler.ts
View file
Expand Up @@ -39,18 +39,22 @@ interface RollupWarning {
line: number;
column: number;
};
pos?: number;
}

const DEFAULT_FORMAT = "amd";

function handleRollupWarning(diagnostics: CompilerDiagnostic[]) {
return function onwarn({ message, loc }: RollupWarning) {
const origin = loc
return function onwarn({ message, loc, pos }: RollupWarning) {
// loc and pos are bundled together
const origin = loc && pos
? {
filename: loc.file,
location: {
line: loc.line,
column: loc.column
column: loc.column,
start: pos,
length: 0,
}
} : {};

Expand Down
20 changes: 2 additions & 18 deletions packages/@lwc/compiler/src/compiler/compiler.ts
View file
@@ -1,5 +1,4 @@
import { CompilerDiagnostic, DiagnosticLevel } from "@lwc/errors";
import { Diagnostic } from "../diagnostics/diagnostic";

import { bundle } from "../bundler/bundler";
import { BundleMetadata } from "../bundler/meta-collector";
Expand All @@ -8,24 +7,9 @@ import { version } from '../index';

export { default as templateCompiler } from "@lwc/template-compiler";

/**
* Transforms a CompilerDiagnostic object so that it's compatible with previous Diagnostic type
* @param diagnostic
*/
function temporaryAdapterForTypesafety(diagnostic: CompilerDiagnostic): Diagnostic {
const diag = diagnostic as any;

if (diagnostic.location) {
const { line, column } = diagnostic.location;
diag.location = { line, column, start: -1, length: -1 };
}

return diag as Diagnostic;
}

export interface CompilerOutput {
success: boolean;
diagnostics: Diagnostic[];
diagnostics: CompilerDiagnostic[];
result?: BundleResult;
version: string;
}
Expand Down Expand Up @@ -68,7 +52,7 @@ export async function compile(
return {
version,
success: !hasError(diagnostics),
diagnostics: diagnostics.map(temporaryAdapterForTypesafety),
diagnostics,
result
};
}
Expand Down
35 changes: 0 additions & 35 deletions packages/@lwc/compiler/src/diagnostics/diagnostic.ts
View file

This file was deleted.

100 changes: 64 additions & 36 deletions packages/@lwc/errors/src/compiler/__tests__/errors.spec.ts
View file
Expand Up @@ -10,6 +10,13 @@ import {

import { Location, DiagnosticLevel } from "../../shared/types";

const DEFAULT_LOCATION = {
line: 1,
column: 22,
start: 22,
length: 8
};

const ERROR_INFO = {
code: 4,
message: "Test Error {0} with message {1}",
Expand All @@ -23,22 +30,41 @@ const GENERIC_ERROR = {
};

class CustomError extends Error {
public lwcCode?: number;
public filename?: string;
public line?: number;
public column?: number;
public lwcCode?: number;

constructor(message: string, filename?: string, line?: number, column?: number) {
public start?: number;
public length?: number;

constructor(
message: string,
filename?: string,
line?: number,
column?: number,
start?: number,
length?: number
) {
super(message);

this.name = 'CustomError';

this.filename = filename;
this.line = line;
this.column = column;
this.start = start;
this.length = length;
}
}

function checkErrorEquality(actual: CompilerError, expected: CompilerError) {
expect(actual).toEqual(expected);
expect(actual.code).toEqual(expected.code);
expect(actual.level).toEqual(expected.level);
expect(actual.filename).toEqual(expected.filename);
expect(actual.location).toEqual(expected.location);
}

describe('error handling', () => {
describe('generate compiler diagnostic', () => {
it('generates a compiler diagnostic when config is null', () => {
Expand All @@ -56,14 +82,14 @@ describe('error handling', () => {
message: 'LWC4: Test Error arg1 with message 500',
level: DiagnosticLevel.Error,
filename: 'test.js',
location: { line: 1, column: 22 }
location: DEFAULT_LOCATION
};

expect(generateCompilerDiagnostic(ERROR_INFO, {
messageArgs: ['arg1', 500],
origin: {
filename: 'test.js',
location: { line: 1, column: 22 }
location: DEFAULT_LOCATION
}
})).toEqual(target);
});
Expand All @@ -73,15 +99,15 @@ describe('error handling', () => {
it('generates a compiler error when config is null', () => {
const target = new CompilerError(4, 'LWC4: Test Error {0} with message {1}');

expect(generateCompilerError(ERROR_INFO)).toEqual(target);
checkErrorEquality(generateCompilerError(ERROR_INFO), target);
});
it('generates a compiler error based on the provided error info', () => {
const args = ['arg1', 10];
const target = new CompilerError(4, 'LWC4: Test Error arg1 with message 10');

expect(generateCompilerError(ERROR_INFO, {
checkErrorEquality(generateCompilerError(ERROR_INFO, {
messageArgs: args
})).toEqual(target);
}), target);
});

it('formats an error string properly', () => {
Expand All @@ -106,47 +132,44 @@ describe('error handling', () => {

it('adds the location to the compiler error if it exists as context', () => {
const args = ['arg1', 10];
const location = { line: 4, column: 27 };

const error = generateCompilerError(ERROR_INFO, {
messageArgs: args,
origin: { location }
origin: { location: DEFAULT_LOCATION }
});
expect(error.location).toEqual(location);
expect(error.location).toEqual(DEFAULT_LOCATION);
});
});

describe('normalizeToCompilerError', () => {
it('preserves existing compiler error', () => {
const error = new CompilerError(100, 'LWC100: test err');
expect(normalizeToCompilerError(GENERIC_ERROR, error)).toEqual(error);
checkErrorEquality(normalizeToCompilerError(GENERIC_ERROR, error), error);
});

it('adds origin info to an existing compiler error', () => {
const filename = 'test.js';
const location = { line: 1, column: 1 };

const oldError = new CompilerError(100, 'LWC100: test error', 'old.js', { line: 1, column: 7 });
const newError = new CompilerError(100, 'LWC100: test error', filename, location);
const oldError = new CompilerError(100, 'LWC100: test error', 'old.js', { line: 1, column: 7, start: 7, length: 3 });
const target = new CompilerError(100, 'LWC100: test error', 'test.js', DEFAULT_LOCATION);

expect(normalizeToCompilerError(GENERIC_ERROR, oldError, { filename, location })).toEqual(newError);
checkErrorEquality(normalizeToCompilerError(GENERIC_ERROR, oldError, { filename: 'test.js', location: DEFAULT_LOCATION }), target);
});

it('normalizes a given error into a compiler error', () => {
const error = new CustomError('test error', 'test.js', 2, 5);
const target = new CompilerError(100, 'CustomError: LWC100: Unexpected error: test error', 'test.js', { line: 2, column: 5});
const error = new CustomError('test error', 'test.js', 3, 5, 16, 10);
const target = new CompilerError(100, 'CustomError: LWC100: Unexpected error: test error', 'test.js', { line: 3, column: 5, start: 16, length: 10 });

expect(normalizeToCompilerError(GENERIC_ERROR, error)).toEqual(target);
const normalized = normalizeToCompilerError(GENERIC_ERROR, error);
checkErrorEquality(normalized, target);
});

it('adds additional origin info into the normalized error if provided', () => {
const error = new CustomError('test error');
const target = new CompilerError(100, 'CustomError: LWC100: Unexpected error: test error', 'test.js', { line: 2, column: 5});
const target = new CompilerError(100, 'CustomError: LWC100: Unexpected error: test error', 'test.js', DEFAULT_LOCATION);

expect(normalizeToCompilerError(GENERIC_ERROR, error, {
checkErrorEquality(normalizeToCompilerError(GENERIC_ERROR, error, {
filename: 'test.js',
location: { line: 2, column: 5 }
})).toEqual(target);
location: DEFAULT_LOCATION
}), target);
});

it('ignores the fallback errorInfo when an error code already exists on the error', () => {
Expand All @@ -158,15 +181,15 @@ describe('error handling', () => {
messageArgs: ['arg1', 10],
origin: {
filename: 'test.js',
location: { line: 1, column: 1}
location: DEFAULT_LOCATION
}
});
target.message = `CustomError: ${target.message}`;

expect(normalizeToCompilerError(GENERIC_ERROR, error, {
checkErrorEquality(normalizeToCompilerError(GENERIC_ERROR, error, {
filename: 'test.js',
location: { line: 1, column: 1 }
})).toEqual(target);
location: DEFAULT_LOCATION
}), target);
});
});

Expand All @@ -177,7 +200,7 @@ describe('error handling', () => {
message: 'LWC100: test error',
level: DiagnosticLevel.Error,
filename: 'test.js',
location: { line: 1, column: 1 }
location: DEFAULT_LOCATION
};
const error = new CompilerError(
target.code, target.message, target.filename, target.location
Expand All @@ -192,10 +215,10 @@ describe('error handling', () => {
message: 'LWC100: test error',
level: DiagnosticLevel.Error,
filename: 'test.js',
location: { line: 1, column: 1 }
location: DEFAULT_LOCATION
};
const error = new CompilerError(
target.code, target.message, 'old.js', { line: 1, column: 7 }
target.code, target.message, 'old.js', DEFAULT_LOCATION
);

expect(normalizeToDiagnostic(GENERIC_ERROR, error, {
Expand All @@ -205,13 +228,18 @@ describe('error handling', () => {
});

it('normalizes a given error into a compiler diagnostic', () => {
const error = new CustomError('test error', 'test.js', 2, 5);
const error = new CustomError('test error', 'test.js', 2, 5, 10, 5);
const target = {
code: 100,
message: 'LWC100: Unexpected error: test error',
level: DiagnosticLevel.Error,
filename: 'test.js',
location: { line: 2, column: 5}
location: {
line: 2,
column: 5,
start: 10,
length: 5
}
};

expect(normalizeToDiagnostic(GENERIC_ERROR, error)).toEqual(target);
Expand All @@ -224,12 +252,12 @@ describe('error handling', () => {
message: 'LWC100: Unexpected error: test error',
level: DiagnosticLevel.Error,
filename: 'test.js',
location: { line: 2, column: 5}
location: DEFAULT_LOCATION
};

expect(normalizeToDiagnostic(GENERIC_ERROR, error, {
filename: 'test.js',
location: { line: 2, column: 5 }
location: DEFAULT_LOCATION
})).toEqual(target);
});
});
Expand Down
7 changes: 5 additions & 2 deletions packages/@lwc/errors/src/compiler/utils.ts
View file
Expand Up @@ -80,8 +80,11 @@ function getLocationFromObject(obj: any): Location | undefined {
return obj.location;
} else if (obj.loc) {
return obj.loc;
} else if (obj.line && obj.column) {
return { line: obj.line, column: obj.column };
} else if (
Number.isInteger(obj.line) &&
Number.isInteger(obj.column)
) {
return { line: obj.line, column: obj.column, start: obj.start, length: obj.length };
}
}

Expand Down
16 changes: 12 additions & 4 deletions ...ages/@lwc/template-compiler/src/__tests__/fixtures/attributes/error-boolean/metadata.json
View file
Expand Up @@ -6,7 +6,9 @@
"level": 1,
"location": {
"line": 2,
"column": 8
"column": 8,
"start": 18,
"length": 11
}
},
{
Expand All @@ -15,7 +17,9 @@
"level": 1,
"location": {
"line": 3,
"column": 8
"column": 8,
"start": 43,
"length": 12
}
},
{
Expand All @@ -24,7 +28,9 @@
"level": 1,
"location": {
"line": 4,
"column": 8
"column": 8,
"start": 69,
"length": 13
}
},
{
Expand All @@ -33,7 +39,9 @@
"level": 1,
"location": {
"line": 5,
"column": 8
"column": 8,
"start": 96,
"length": 14
}
}
],
Expand Down

0 comments on commit b4fad21

Please sign in to comment.