feat: improve error formatting to match webpack 4 convention (#641)

TypeStrong · Aug 5, 2021 · acc7d12 · acc7d12
1 parent cdb531f
commit acc7d12
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 46 deletions.
diff --git a/src/formatter/WebpackFormatter.ts b/src/formatter/WebpackFormatter.ts
@@ -1,9 +1,8 @@
 import os from 'os';
 import chalk from 'chalk';
-import path from 'path';
 import { Formatter } from './Formatter';
 import { formatIssueLocation } from '../issue';
-import forwardSlash from '../utils/path/forwardSlash';
+import { relativeToContext } from '../utils/path/relativeToContext';
 
 function createWebpackFormatter(formatter: Formatter): Formatter {
   // mimics webpack error formatter
@@ -13,14 +12,14 @@ function createWebpackFormatter(formatter: Formatter): Formatter {
     const severity = issue.severity.toUpperCase();
 
     if (issue.file) {
-      let location = forwardSlash(path.relative(process.cwd(), issue.file));
+      let location = chalk.whiteBright.bold(relativeToContext(issue.file, process.cwd()));
       if (issue.location) {
-        location += `:${formatIssueLocation(issue.location)}`;
+        location += ` ${chalk.green.bold(formatIssueLocation(issue.location))}`;
       }
 
-      return [color(`${severity} in ${location}`), formatter(issue), ''].join(os.EOL);
+      return [`${color(severity)} in ${location}`, formatter(issue), ''].join(os.EOL);
     } else {
-      return [color(`${severity} in `) + formatter(issue), ''].join(os.EOL);
+      return [`${color(severity)} in ` + formatter(issue), ''].join(os.EOL);
     }
   };
 }

diff --git a/src/issue/IssueLocation.ts b/src/issue/IssueLocation.ts
@@ -24,8 +24,20 @@ function compareIssueLocations(locationA?: IssueLocation, locationB?: IssueLocat
   );
 }
 
-function formatIssueLocation(location: IssueLocation) {
-  return `${location.start.line}:${location.start.column}`;
+function formatIssueLocation({ start, end }: IssueLocation) {
+  if (!end.line || start.line === end.line) {
+    // the same line
+    if (!end.column || start.column === end.column) {
+      // the same column
+      return `${start.line}:${start.column}`;
+    } else {
+      // different column
+      return `${start.line}:${start.column}-${end.column}`;
+    }
+  } else {
+    // different lines
+    return `${start.line}:${start.column}-${end.line}:${end.column}`;
+  }
 }
 
 export { IssueLocation, compareIssueLocations, formatIssueLocation };
diff --git a/src/issue/IssueWebpackError.ts b/src/issue/IssueWebpackError.ts
@@ -1,12 +1,11 @@
 import webpack from 'webpack';
-import { relative } from 'path';
 import { Issue } from './Issue';
 import { formatIssueLocation } from './IssueLocation';
-import forwardSlash from '../utils/path/forwardSlash';
+import { relativeToContext } from '../utils/path/relativeToContext';
+import chalk from 'chalk';
 
 class IssueWebpackError extends webpack.WebpackError {
   readonly hideStack = true;
-  readonly file: string = '';
 
   constructor(message: string, readonly issue: Issue) {
     super(message);
@@ -15,10 +14,10 @@ class IssueWebpackError extends webpack.WebpackError {
     // should be a NormalModule instance.
     // to avoid such a dependency, we do a workaround - error.file will contain formatted location instead
     if (issue.file) {
-      this.file = forwardSlash(relative(process.cwd(), issue.file));
+      this.file = relativeToContext(issue.file, process.cwd());
 
       if (issue.location) {
-        this.file += `:${formatIssueLocation(issue.location)}`;
+        this.file += ` ${chalk.green.bold(formatIssueLocation(issue.location))}`;
       }
     }
 

diff --git a/src/utils/path/relativeToContext.ts b/src/utils/path/relativeToContext.ts
@@ -0,0 +1,13 @@
+import forwardSlash from './forwardSlash';
+import path from 'path';
+
+function relativeToContext(file: string, context: string) {
+  let fileInContext = forwardSlash(path.relative(context, file));
+  if (!fileInContext.startsWith('../')) {
+    fileInContext = './' + fileInContext;
+  }
+
+  return fileInContext;
+}
+
+export { relativeToContext };
diff --git a/test/e2e/TypeScriptContextOption.spec.ts b/test/e2e/TypeScriptContextOption.spec.ts
@@ -88,7 +88,7 @@ describe('TypeScript Context Option', () => {
     const errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in ../src/model/User.ts:11:16',
+        'ERROR in ../src/model/User.ts 11:16-25',
         "TS2339: Property 'firstName' does not exist on type 'User'.",
         '     9 |',
         '    10 | function getUserName(user: User): string {',
@@ -99,7 +99,7 @@ describe('TypeScript Context Option', () => {
         '    14 | export { User, getUserName };',
       ].join('\n'),
       [
-        'ERROR in ../src/model/User.ts:11:32',
+        'ERROR in ../src/model/User.ts 11:32-40',
         "TS2339: Property 'lastName' does not exist on type 'User'.",
         '     9 |',
         '    10 | function getUserName(user: User): string {',

diff --git a/test/e2e/TypeScriptFormatterOption.spec.ts b/test/e2e/TypeScriptFormatterOption.spec.ts
@@ -40,11 +40,11 @@ describe('TypeScript Formatter Option', () => {
       const errors = await driver.waitForErrors();
       expect(errors).toEqual([
         [
-          'ERROR in src/model/User.ts:11:16',
+          'ERROR in ./src/model/User.ts 11:16-25',
           "It is the custom issue statement - TS2339: Property 'firstName' does not exist on type 'User'.",
         ].join('\n'),
         [
-          'ERROR in src/model/User.ts:11:32',
+          'ERROR in ./src/model/User.ts 11:32-40',
           "It is the custom issue statement - TS2339: Property 'lastName' does not exist on type 'User'.",
         ].join('\n'),
       ]);

diff --git a/test/e2e/TypeScriptPnpSupport.spec.ts b/test/e2e/TypeScriptPnpSupport.spec.ts
@@ -31,7 +31,7 @@ describe('TypeScript PnP Support', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in src/model/User.ts:11:16',
+        'ERROR in ./src/model/User.ts 11:16-25',
         "TS2339: Property 'firstName' does not exist on type 'User'.",
         '     9 |',
         '    10 | function getUserName(user: User): string {',
@@ -42,7 +42,7 @@ describe('TypeScript PnP Support', () => {
         '    14 | export { User, getUserName };',
       ].join('\n'),
       [
-        'ERROR in src/model/User.ts:11:32',
+        'ERROR in ./src/model/User.ts 11:32-40',
         "TS2339: Property 'lastName' does not exist on type 'User'.",
         '     9 |',
         '    10 | function getUserName(user: User): string {',
@@ -69,7 +69,7 @@ describe('TypeScript PnP Support', () => {
     errors = await driver.waitForErrors();
     expect(errors).toContain(
       [
-        'ERROR in src/index.ts:1:23',
+        'ERROR in ./src/index.ts 1:23-39',
         "TS2307: Cannot find module './authenticate'.",
         "  > 1 | import { login } from './authenticate';",
         '      |                       ^^^^^^^^^^^^^^^^',
@@ -113,7 +113,7 @@ describe('TypeScript PnP Support', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in src/index.ts:34:12',
+        'ERROR in ./src/index.ts 34:12-16',
         "TS2339: Property 'role' does not exist on type 'void'.",
         '    32 |   const user = await login(email, password);',
         '    33 |',
@@ -124,7 +124,7 @@ describe('TypeScript PnP Support', () => {
         '    37 |     console.log(`Logged in as ${getUserName(user)}`);',
       ].join('\n'),
       [
-        'ERROR in src/index.ts:35:45',
+        'ERROR in ./src/index.ts 35:45-49',
         "TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
         '    33 |',
         "    34 |   if (user.role === 'admin') {",
@@ -135,7 +135,7 @@ describe('TypeScript PnP Support', () => {
         '    38 |   }',
       ].join('\n'),
       [
-        'ERROR in src/index.ts:37:45',
+        'ERROR in ./src/index.ts 37:45-49',
         "TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
         '    35 |     console.log(`Logged in as ${getUserName(user)} [admin].`);',
         '    36 |   } else {',

diff --git a/test/e2e/TypeScriptSolutionBuilderApi.spec.ts b/test/e2e/TypeScriptSolutionBuilderApi.spec.ts
@@ -29,7 +29,7 @@ describe('TypeScript SolutionBuilder API', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in packages/shared/src/intersect.ts:2:41',
+        'ERROR in ./packages/shared/src/intersect.ts 2:41-49',
         "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));',
@@ -51,7 +51,7 @@ describe('TypeScript SolutionBuilder API', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in packages/client/src/index.ts:4:42',
+        'ERROR in ./packages/client/src/index.ts 4:42-48',
         "TS2345: Argument of type 'T[]' is not assignable to parameter of type 'T'.",
         typescript === '~4.0.0'
           ? "  'T' could be instantiated with an arbitrary type which could be unrelated to 'T[]'."

diff --git a/test/e2e/TypeScriptVueExtension.spec.ts b/test/e2e/TypeScriptVueExtension.spec.ts
@@ -62,7 +62,7 @@ describe('TypeScript Vue Extension', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in src/component/LoggedIn.vue:27:23',
+        'ERROR in ./src/component/LoggedIn.vue 27:23-34',
         "TS2304: Cannot find name 'getUserName'.",
         '    25 |         const user: User = this.user;',
         '    26 |',
@@ -90,7 +90,7 @@ describe('TypeScript Vue Extension', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in src/component/LoggedIn.vue:27:31',
+        'ERROR in ./src/component/LoggedIn.vue 27:31-40',
         "TS2339: Property 'firstName' does not exist on type 'User'.",
         '    25 |         const user: User = this.user;',
         '    26 |',
@@ -101,7 +101,7 @@ describe('TypeScript Vue Extension', () => {
         '    30 |     async logout() {',
       ].join('\n'),
       [
-        'ERROR in src/model/User.ts:11:16',
+        'ERROR in ./src/model/User.ts 11:16-25',
         "TS2339: Property 'firstName' does not exist on type 'User'.",
         '     9 |',
         '    10 | function getUserName(user: User): string {',

diff --git a/test/e2e/TypeScriptWatchApi.spec.ts b/test/e2e/TypeScriptWatchApi.spec.ts
@@ -29,7 +29,7 @@ describe('TypeScript Watch API', () => {
       errors = await driver.waitForErrors();
       expect(errors).toEqual([
         [
-          'ERROR in src/index.ts:34:7',
+          'ERROR in ./src/index.ts 34:7-28',
           `TS2367: This condition will always return 'false' since the types 'Role' and '"admin"' have no overlap.`,
           '    32 |   const user = await login(email, password);',
           '    33 |',
@@ -71,7 +71,7 @@ describe('TypeScript Watch API', () => {
       );
       expect(errors).toEqual([
         [
-          'ERROR in src/model/User.ts:1:22',
+          'ERROR in ./src/model/User.ts 1:22-30',
           "TS2307: Cannot find module './Role' or its corresponding type declarations.",
           "  > 1 | import { Role } from './Role';",
           '      |                      ^^^^^^^^',
@@ -91,7 +91,7 @@ describe('TypeScript Watch API', () => {
       errors = await driver.waitForErrors();
       expect(errors).toEqual([
         [
-          'ERROR in src/index.ts:34:7',
+          'ERROR in ./src/index.ts 34:7-31',
           "TS2367: This condition will always return 'false' since the types 'Role' and '\"provider\"' have no overlap.",
           '    32 |   const user = await login(email, password);',
           '    33 |',
@@ -132,7 +132,7 @@ describe('TypeScript Watch API', () => {
       errors = await driver.waitForErrors();
       expect(errors).toEqual([
         [
-          'ERROR in src/index.ts:34:7',
+          'ERROR in ./src/index.ts 34:7-28',
           `TS2367: This condition will always return 'false' since the types 'Role' and '"admin"' have no overlap.`,
           '    32 |   const user = await login(email, password);',
           '    33 |',
@@ -174,7 +174,7 @@ describe('TypeScript Watch API', () => {
       );
       expect(errors).toEqual([
         [
-          'ERROR in src/model/User.ts:1:22',
+          'ERROR in ./src/model/User.ts 1:22-30',
           "TS2307: Cannot find module './Role' or its corresponding type declarations.",
           "  > 1 | import { Role } from './Role';",
           '      |                      ^^^^^^^^',
@@ -194,7 +194,7 @@ describe('TypeScript Watch API', () => {
       errors = await driver.waitForErrors();
       expect(errors).toEqual([
         [
-          'ERROR in src/index.ts:34:7',
+          'ERROR in ./src/index.ts 34:7-31',
           "TS2367: This condition will always return 'false' since the types 'Role' and '\"provider\"' have no overlap.",
           '    32 |   const user = await login(email, password);',
           '    33 |',
@@ -238,7 +238,7 @@ describe('TypeScript Watch API', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in src/model/User.ts:11:16',
+        'ERROR in ./src/model/User.ts 11:16-25',
         "TS2339: Property 'firstName' does not exist on type 'User'.",
         '     9 |',
         '    10 | function getUserName(user: User): string {',
@@ -249,7 +249,7 @@ describe('TypeScript Watch API', () => {
         '    14 | export { User, getUserName };',
       ].join('\n'),
       [
-        'ERROR in src/model/User.ts:11:32',
+        'ERROR in ./src/model/User.ts 11:32-40',
         "TS2339: Property 'lastName' does not exist on type 'User'.",
         '     9 |',
         '    10 | function getUserName(user: User): string {',
@@ -276,7 +276,7 @@ describe('TypeScript Watch API', () => {
     errors = await driver.waitForErrors();
     expect(errors).toContain(
       [
-        'ERROR in src/index.ts:1:23',
+        'ERROR in ./src/index.ts 1:23-39',
         "TS2307: Cannot find module './authenticate'.",
         "  > 1 | import { login } from './authenticate';",
         '      |                       ^^^^^^^^^^^^^^^^',
@@ -320,7 +320,7 @@ describe('TypeScript Watch API', () => {
     errors = await driver.waitForErrors();
     expect(errors).toEqual([
       [
-        'ERROR in src/index.ts:34:12',
+        'ERROR in ./src/index.ts 34:12-16',
         "TS2339: Property 'role' does not exist on type 'void'.",
         '    32 |   const user = await login(email, password);',
         '    33 |',
@@ -331,7 +331,7 @@ describe('TypeScript Watch API', () => {
         '    37 |     console.log(`Logged in as ${getUserName(user)}`);',
       ].join('\n'),
       [
-        'ERROR in src/index.ts:35:45',
+        'ERROR in ./src/index.ts 35:45-49',
         "TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
         '    33 |',
         "    34 |   if (user.role === 'admin') {",
@@ -342,7 +342,7 @@ describe('TypeScript Watch API', () => {
         '    38 |   }',
       ].join('\n'),
       [
-        'ERROR in src/index.ts:37:45',
+        'ERROR in ./src/index.ts 37:45-49',
         "TS2345: Argument of type 'void' is not assignable to parameter of type 'User'.",
         '    35 |     console.log(`Logged in as ${getUserName(user)} [admin].`);',
         '    36 |   } else {',

diff --git a/test/e2e/WebpackProductionBuild.spec.ts b/test/e2e/WebpackProductionBuild.spec.ts
@@ -49,7 +49,7 @@ describe('Webpack Production Build', () => {
         // first error is from the webpack module resolution
         expect.anything(),
         [
-          'ERROR in src/authenticate.ts:1:22',
+          'ERROR in ./src/authenticate.ts 1:22-36',
           "TS2307: Cannot find module './model/User' or its corresponding type declarations.",
           "  > 1 | import { User } from './model/User';",
           '      |                      ^^^^^^^^^^^^^^',
@@ -58,7 +58,7 @@ describe('Webpack Production Build', () => {
           "    4 |   const response = await fetch('/login', {",
         ].join('\n'),
         [
-          'ERROR in src/index.ts:2:29',
+          'ERROR in ./src/index.ts 2:29-43',
           "TS2307: Cannot find module './model/User' or its corresponding type declarations.",
           "    1 | import { login } from './authenticate';",
           "  > 2 | import { getUserName } from './model/User';",

diff --git a/test/unit/formatter/WebpackFormatter.spec.ts b/test/unit/formatter/WebpackFormatter.spec.ts
@@ -37,22 +37,22 @@ describe('formatter/WebpackFormatter', () => {
   });
 
   it('formats issue file', () => {
-    expect(formatter(issue)).toContain(`some/file.ts`);
+    expect(formatter(issue)).toContain(`./some/file.ts`);
   });
 
   it('formats location', () => {
-    expect(formatter(issue)).toContain(':1:7');
+    expect(formatter(issue)).toContain(' 1:7-16');
     expect(
       formatter({
         ...issue,
         location: { start: { line: 1, column: 7 }, end: { line: 10, column: 16 } },
       })
-    ).toContain(':1:7');
+    ).toContain(' 1:7-10:16');
   });
 
   it('formats issue header like webpack', () => {
     expect(formatter(issue)).toEqual(
-      [`ERROR in some/file.ts:1:7`, 'TS123: Some issue content', ''].join(os.EOL)
+      [`ERROR in ./some/file.ts 1:7-16`, 'TS123: Some issue content', ''].join(os.EOL)
     );
   });
 });