Skip to content

Commit

Permalink
fix: use :line:col format in webpack errors (#489)
Browse files Browse the repository at this point in the history
As discussed in #481, changes the format of TypeScript error file locations to use file:line:col. This means files can be opened from the terminal at the location the error occurred (e.g. cmd+click on Mac).

Closes: #481
  • Loading branch information
robcrocombe committed Jul 28, 2020
1 parent cf52452 commit f3a0c98
Show file tree
Hide file tree
Showing 12 changed files with 39 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/formatter/WebpackFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function createWebpackFormatter(formatter: Formatter, context: string): Formatte
if (issue.file) {
let location = forwardSlash(path.relative(context, issue.file));
if (issue.location) {
location += ` ${formatIssueLocation(issue.location)}`;
location += `:${formatIssueLocation(issue.location)}`;
}

return [color(`${severity} in ${location}`), formatter(issue), ''].join(os.EOL);
Expand Down
7 changes: 1 addition & 6 deletions src/issue/IssueLocation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,7 @@ function compareIssueLocations(locationA?: IssueLocation, locationB?: IssueLocat
}

function formatIssueLocation(location: IssueLocation) {
return [
`${location.start.line}:${location.start.column}`,
location.start.line !== location.end.line
? `${location.end.line}:${location.end.column}`
: `${location.end.column}`,
].join('-');
return `${location.start.line}:${location.start.column}`;
}

export { IssueLocation, compareIssueLocations, formatIssueLocation };
2 changes: 1 addition & 1 deletion src/issue/IssueWebpackError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class IssueWebpackError extends Error {
this.file = forwardSlash(relative(context, issue.file));

if (issue.location) {
this.file += ` ${formatIssueLocation(issue.location)}`;
this.file += `:${formatIssueLocation(issue.location)}`;
}
}

Expand Down
8 changes: 4 additions & 4 deletions test/e2e/EsLint.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('EsLint', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'WARNING in src/authenticate.ts 14:34-37',
'WARNING in src/authenticate.ts:14:34',
'@typescript-eslint/no-explicit-any: Unexpected any. Specify a different type.',
' 12 | }',
' 13 | ',
Expand All @@ -70,7 +70,7 @@ describe('EsLint', () => {
' 17 | {',
].join('\n'),
[
'WARNING in src/index.ts 31:44-49',
'WARNING in src/index.ts:31:44',
"@typescript-eslint/no-unused-vars: 'event' is defined but never used.",
' 29 | }',
' 30 | });',
Expand Down Expand Up @@ -126,7 +126,7 @@ describe('EsLint', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'WARNING in src/model/User.ts 11:5-19',
'WARNING in src/model/User.ts:11:5',
"@typescript-eslint/no-unused-vars: 'temporary' is defined but never used.",
' 9 | }',
' 10 | ',
Expand All @@ -137,7 +137,7 @@ describe('EsLint', () => {
' 14 | function getUserName(user: User): string {',
].join('\n'),
[
'WARNING in src/model/User.ts 11:16-19',
'WARNING in src/model/User.ts:11:16',
'@typescript-eslint/no-explicit-any: Unexpected any. Specify a different type.',
' 9 | }',
' 10 | ',
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/TypeScriptContextOption.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ describe('TypeScript Context Option', () => {
const errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/model/User.ts 11:16-25',
'ERROR in src/model/User.ts:11:16',
"TS2339: Property 'firstName' does not exist on type 'User'.",
' 9 | ',
' 10 | function getUserName(user: User): string {',
Expand All @@ -113,7 +113,7 @@ describe('TypeScript Context Option', () => {
' 14 | }',
].join('\n'),
[
'ERROR in src/model/User.ts 11:32-40',
'ERROR in src/model/User.ts:11:32',
"TS2339: Property 'lastName' does not exist on type 'User'.",
' 9 | ',
' 10 | function getUserName(user: User): string {',
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/TypeScriptPnpSupport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('TypeScript PnP Support', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/model/User.ts 11:16-25',
'ERROR in src/model/User.ts:11:16',
"TS2339: Property 'firstName' does not exist on type 'User'.",
' 9 | ',
' 10 | function getUserName(user: User): string {',
Expand All @@ -78,7 +78,7 @@ describe('TypeScript PnP Support', () => {
' 14 | }',
].join('\n'),
[
'ERROR in src/model/User.ts 11:32-40',
'ERROR in src/model/User.ts:11:32',
"TS2339: Property 'lastName' does not exist on type 'User'.",
' 9 | ',
' 10 | function getUserName(user: User): string {',
Expand Down Expand Up @@ -109,7 +109,7 @@ describe('TypeScript PnP Support', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 1:23-39',
'ERROR in src/index.ts:1:23',
"TS2307: Cannot find module './authenticate'.",
" > 1 | import { login } from './authenticate';",
' | ^^^^^^^^^^^^^^^^',
Expand Down Expand Up @@ -153,7 +153,7 @@ describe('TypeScript PnP Support', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 34:12-16',
'ERROR in src/index.ts:34:12',
"TS2339: Property 'role' does not exist on type 'void'.",
' 32 | const user = await login(email, password);',
' 33 | ',
Expand All @@ -164,7 +164,7 @@ describe('TypeScript PnP Support', () => {
' 37 | console.log(`Logged in as ${getUserName(user)}`);',
].join('\n'),
[
'ERROR in src/index.ts 35:45-49',
'ERROR in src/index.ts:35:45',
"TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
' 33 | ',
" 34 | if (user.role === 'admin') {",
Expand All @@ -175,7 +175,7 @@ describe('TypeScript PnP Support', () => {
' 38 | }',
].join('\n'),
[
'ERROR in src/index.ts 37:45-49',
'ERROR in src/index.ts:37:45',
"TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
' 35 | console.log(`Logged in as ${getUserName(user)} [admin].`);',
' 36 | } else {',
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/TypeScriptSolutionBuilderApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('TypeScript SolutionBuilder API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in packages/shared/src/intersect.ts 2:41-49',
'ERROR in packages/shared/src/intersect.ts:2:41',
"TS2339: Property 'includes' does not exist on type 'T'.",
' 1 | function intersect<T>(arrayA: T[] = [], arrayB: T): T[] {',
' > 2 | return arrayA.filter((item) => arrayB.includes(item));',
Expand All @@ -78,7 +78,7 @@ describe('TypeScript SolutionBuilder API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in packages/client/src/index.ts 4:42-48',
'ERROR in packages/client/src/index.ts:4:42',
"TS2345: Argument of type 'T[]' is not assignable to parameter of type 'T'.",
" 'T[]' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint '{}'.",
' 2 | ',
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/TypeScriptVueExtension.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('TypeScript Vue Extension', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/component/LoggedIn.vue 27:21-32',
'ERROR in src/component/LoggedIn.vue:27:21',
"TS2304: Cannot find name 'getUserName'.",
' 25 | const user: User = this.user;',
' 26 | ',
Expand Down Expand Up @@ -122,7 +122,7 @@ describe('TypeScript Vue Extension', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/component/LoggedIn.vue 27:29-38',
'ERROR in src/component/LoggedIn.vue:27:29',
"TS2339: Property 'firstName' does not exist on type 'User'.",
' 25 | const user: User = this.user;',
' 26 | ',
Expand All @@ -133,7 +133,7 @@ describe('TypeScript Vue Extension', () => {
' 30 | async logout() {',
].join('\n'),
[
'ERROR in src/model/User.ts 11:16-25',
'ERROR in src/model/User.ts:11:16',
"TS2339: Property 'firstName' does not exist on type 'User'.",
' 9 | ',
' 10 | function getUserName(user: User): string {',
Expand Down
24 changes: 12 additions & 12 deletions test/e2e/TypeScriptWatchApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('TypeScript Watch API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 34:7-28',
'ERROR in src/index.ts:34:7',
`TS2367: This condition will always return 'false' since the types 'Role' and '"admin"' have no overlap.`,
' 32 | const user = await login(email, password);',
' 33 | ',
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('TypeScript Watch API', () => {
);
expect(errors).toEqual([
[
'ERROR in src/model/User.ts 1:22-30',
'ERROR in src/model/User.ts:1:22',
"TS2307: Cannot find module './Role'.",
" > 1 | import { Role } from './Role';",
' | ^^^^^^^^',
Expand All @@ -135,7 +135,7 @@ describe('TypeScript Watch API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 34:7-31',
'ERROR in src/index.ts:34:7',
"TS2367: This condition will always return 'false' since the types 'Role' and '\"provider\"' have no overlap.",
' 32 | const user = await login(email, password);',
' 33 | ',
Expand Down Expand Up @@ -194,7 +194,7 @@ describe('TypeScript Watch API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 34:7-28',
'ERROR in src/index.ts:34:7',
`TS2367: This condition will always return 'false' since the types 'Role' and '"admin"' have no overlap.`,
' 32 | const user = await login(email, password);',
' 33 | ',
Expand Down Expand Up @@ -236,7 +236,7 @@ describe('TypeScript Watch API', () => {
);
expect(errors).toEqual([
[
'ERROR in src/model/User.ts 1:22-30',
'ERROR in src/model/User.ts:1:22',
"TS2307: Cannot find module './Role'.",
" > 1 | import { Role } from './Role';",
' | ^^^^^^^^',
Expand All @@ -256,7 +256,7 @@ describe('TypeScript Watch API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 34:7-31',
'ERROR in src/index.ts:34:7',
"TS2367: This condition will always return 'false' since the types 'Role' and '\"provider\"' have no overlap.",
' 32 | const user = await login(email, password);',
' 33 | ',
Expand Down Expand Up @@ -308,7 +308,7 @@ describe('TypeScript Watch API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/model/User.ts 11:16-25',
'ERROR in src/model/User.ts:11:16',
"TS2339: Property 'firstName' does not exist on type 'User'.",
' 9 | ',
' 10 | function getUserName(user: User): string {',
Expand All @@ -319,7 +319,7 @@ describe('TypeScript Watch API', () => {
' 14 | }',
].join('\n'),
[
'ERROR in src/model/User.ts 11:32-40',
'ERROR in src/model/User.ts:11:32',
"TS2339: Property 'lastName' does not exist on type 'User'.",
' 9 | ',
' 10 | function getUserName(user: User): string {',
Expand Down Expand Up @@ -350,7 +350,7 @@ describe('TypeScript Watch API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 1:23-39',
'ERROR in src/index.ts:1:23',
"TS2307: Cannot find module './authenticate'.",
" > 1 | import { login } from './authenticate';",
' | ^^^^^^^^^^^^^^^^',
Expand Down Expand Up @@ -394,7 +394,7 @@ describe('TypeScript Watch API', () => {
errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/index.ts 34:12-16',
'ERROR in src/index.ts:34:12',
"TS2339: Property 'role' does not exist on type 'void'.",
' 32 | const user = await login(email, password);',
' 33 | ',
Expand All @@ -405,7 +405,7 @@ describe('TypeScript Watch API', () => {
' 37 | console.log(`Logged in as ${getUserName(user)}`);',
].join('\n'),
[
'ERROR in src/index.ts 35:45-49',
'ERROR in src/index.ts:35:45',
"TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
' 33 | ',
" 34 | if (user.role === 'admin') {",
Expand All @@ -416,7 +416,7 @@ describe('TypeScript Watch API', () => {
' 38 | }',
].join('\n'),
[
'ERROR in src/index.ts 37:45-49',
'ERROR in src/index.ts:37:45',
"TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
' 35 | console.log(`Logged in as ${getUserName(user)} [admin].`);',
' 36 | } else {',
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/WebpackIssueScope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe('Webpack Issue Scope', () => {
const errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/notUsedFile.ts 1:7-8',
'ERROR in src/notUsedFile.ts:1:7',
"TS2322: Type '\"1\"' is not assignable to type 'number'.",
' > 1 | const x: number = "1";',
' | ^',
Expand All @@ -96,7 +96,7 @@ describe('Webpack Issue Scope', () => {
const errors = await driver.waitForErrors();
expect(errors).toEqual([
[
'ERROR in src/notUsedFile.ts 1:7-8',
'ERROR in src/notUsedFile.ts:1:7',
"TS2322: Type '\"1\"' is not assignable to type 'number'.",
' > 1 | const x: number = "1";',
' | ^',
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/WebpackProductionBuild.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe('Webpack Production Build', () => {
// first error is from the webpack module resolution
expect.anything(),
[
'ERROR in src/authenticate.ts 1:22-36',
'ERROR in src/authenticate.ts:1:22',
"TS2307: Cannot find module './model/User'.",
" > 1 | import { User } from './model/User';",
' | ^^^^^^^^^^^^^^',
Expand All @@ -129,7 +129,7 @@ describe('Webpack Production Build', () => {
' 4 | const response = await fetch(',
].join('\n'),
[
'ERROR in src/index.ts 2:29-43',
'ERROR in src/index.ts:2:29',
"TS2307: Cannot find module './model/User'.",
" 1 | import { login } from './authenticate';",
" > 2 | import { getUserName } from './model/User';",
Expand Down
6 changes: 3 additions & 3 deletions test/unit/formatter/WebpackFormatter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,18 @@ describe('formatter/WebpackFormatter', () => {
});

it('formats location', () => {
expect(formatter(issue)).toContain('1:7-16');
expect(formatter(issue)).toContain(':1:7');
expect(
formatter({
...issue,
location: { start: { line: 1, column: 7 }, end: { line: 10, column: 16 } },
})
).toContain('1:7-10:16');
).toContain(':1:7');
});

it('formats issue header like webpack', () => {
expect(formatter(issue)).toEqual(
[`ERROR in some/file.ts 1:7-16`, 'TS123: Some issue content', ''].join(os.EOL)
[`ERROR in some/file.ts:1:7`, 'TS123: Some issue content', ''].join(os.EOL)
);
});
});

0 comments on commit f3a0c98

Please sign in to comment.