Fixup DiagnosticCodeHighlight and SourceLocation columns (#8965)

parcel-bundler · May 16, 2023 · be85a8c · be85a8c
1 parent e53e824
commit be85a8c
Show file tree

Hide file tree

Showing 37 changed files with 1,104 additions and 996 deletions.
diff --git a/packages/core/core/src/SymbolPropagation.js b/packages/core/core/src/SymbolPropagation.js
@@ -15,7 +15,7 @@ import invariant from 'assert';
 import nullthrows from 'nullthrows';
 import {setEqual} from '@parcel/utils';
 import logger from '@parcel/logger';
-import {md} from '@parcel/diagnostic';
+import {md, convertSourceLocationToHighlight} from '@parcel/diagnostic';
 import {BundleBehavior} from './types';
 import {fromProjectPathRelative, fromProjectPath} from './projectPath';
 
@@ -444,12 +444,7 @@ export function propagateSymbols({
                         fromProjectPath(options.projectRoot, loc?.filePath) ??
                         undefined,
                       language: incomingDep.value.sourceAssetType ?? undefined,
-                      codeHighlights: [
-                        {
-                          start: loc.start,
-                          end: loc.end,
-                        },
-                      ],
+                      codeHighlights: [convertSourceLocationToHighlight(loc)],
                     },
                   ]
                 : undefined,

diff --git a/packages/core/core/src/requests/PathRequest.js b/packages/core/core/src/requests/PathRequest.js
@@ -16,7 +16,11 @@ import type {
 } from '../types';
 import type {ConfigAndCachePath} from './ParcelConfigRequest';
 
-import ThrowableDiagnostic, {errorToDiagnostic, md} from '@parcel/diagnostic';
+import ThrowableDiagnostic, {
+  convertSourceLocationToHighlight,
+  errorToDiagnostic,
+  md,
+} from '@parcel/diagnostic';
 import {PluginLogger} from '@parcel/logger';
 import nullthrows from 'nullthrows';
 import path from 'path';
@@ -188,7 +192,7 @@ export class ResolverRunner {
           filePath,
           code: await this.options.inputFS.readFile(filePath, 'utf8'),
           codeHighlights: dependency.loc
-            ? [{start: dependency.loc.start, end: dependency.loc.end}]
+            ? [convertSourceLocationToHighlight(dependency.loc)]
             : [],
         },
       ];

diff --git a/packages/core/core/src/requests/TargetRequest.js b/packages/core/core/src/requests/TargetRequest.js
@@ -16,6 +16,7 @@ import type {Entry, ParcelOptions, Target} from '../types';
 import type {ConfigAndCachePath} from './ParcelConfigRequest';
 
 import ThrowableDiagnostic, {
+  convertSourceLocationToHighlight,
   generateJSONCodeHighlights,
   getJSONSourceLocation,
   encodeJSONKeyComponent,
@@ -1417,21 +1418,16 @@ function assertTargetsAreNotEntries(
         codeFrames.push({
           filePath: fromProjectPath(options.projectRoot, loc.filePath),
           codeHighlights: [
-            {
-              start: loc.start,
-              end: loc.end,
-              message: 'Target defined here',
-            },
+            convertSourceLocationToHighlight(loc, 'Target defined here'),
           ],
         });
 
         let inputLoc = input.loc;
         if (inputLoc) {
-          let highlight = {
-            start: inputLoc.start,
-            end: inputLoc.end,
-            message: 'Entry defined here',
-          };
+          let highlight = convertSourceLocationToHighlight(
+            inputLoc,
+            'Entry defined here',
+          );
 
           if (inputLoc.filePath === loc.filePath) {
             codeFrames[0].codeHighlights.push(highlight);
@@ -1497,11 +1493,9 @@ async function debugResolvedTargets(input, targets, targetInfo, options) {
 
     let highlights = [];
     if (input.loc) {
-      highlights.push({
-        start: input.loc.start,
-        end: input.loc.end,
-        message: 'entry defined here',
-      });
+      highlights.push(
+        convertSourceLocationToHighlight(input.loc, 'entry defined here'),
+      );
     }
 
     // Read package.json where target is defined.

diff --git a/packages/core/core/test/TargetRequest.test.js b/packages/core/core/test/TargetRequest.test.js
@@ -199,7 +199,7 @@ describe('TargetResolver', () => {
               line: 2,
             },
             end: {
-              column: 30,
+              column: 31,
               line: 2,
             },
           },
@@ -235,7 +235,7 @@ describe('TargetResolver', () => {
               line: 3,
             },
             end: {
-              column: 34,
+              column: 35,
               line: 3,
             },
           },
@@ -269,7 +269,7 @@ describe('TargetResolver', () => {
               line: 4,
             },
             end: {
-              column: 36,
+              column: 37,
               line: 4,
             },
           },
@@ -315,7 +315,7 @@ describe('TargetResolver', () => {
               line: 3,
             },
             end: {
-              column: 24,
+              column: 25,
               line: 3,
             },
           },
@@ -358,7 +358,7 @@ describe('TargetResolver', () => {
               line: 2,
             },
             end: {
-              column: 30,
+              column: 31,
               line: 2,
             },
           },
@@ -392,7 +392,7 @@ describe('TargetResolver', () => {
               line: 3,
             },
             end: {
-              column: 48,
+              column: 49,
               line: 3,
             },
           },
@@ -426,7 +426,7 @@ describe('TargetResolver', () => {
               line: 4,
             },
             end: {
-              column: 48,
+              column: 49,
               line: 4,
             },
           },
@@ -477,7 +477,7 @@ describe('TargetResolver', () => {
               line: 2,
             },
             end: {
-              column: 30,
+              column: 31,
               line: 2,
             },
           },
@@ -511,7 +511,7 @@ describe('TargetResolver', () => {
               line: 3,
             },
             end: {
-              column: 48,
+              column: 49,
               line: 3,
             },
           },
@@ -545,7 +545,7 @@ describe('TargetResolver', () => {
               line: 4,
             },
             end: {
-              column: 48,
+              column: 49,
               line: 4,
             },
           },
@@ -705,7 +705,7 @@ describe('TargetResolver', () => {
             line: 2,
           },
           end: {
-            column: 30,
+            column: 31,
             line: 2,
           },
         },
@@ -1083,7 +1083,7 @@ describe('TargetResolver', () => {
             line: 2,
           },
           end: {
-            column: 26,
+            column: 27,
             line: 2,
           },
         },
@@ -1128,7 +1128,7 @@ describe('TargetResolver', () => {
             line: 3,
           },
           end: {
-            column: 25,
+            column: 26,
             line: 3,
           },
         },
@@ -1174,7 +1174,7 @@ describe('TargetResolver', () => {
               line: 2,
             },
             end: {
-              column: 30,
+              column: 31,
               line: 2,
             },
           },
@@ -1208,7 +1208,7 @@ describe('TargetResolver', () => {
               line: 4,
             },
             end: {
-              column: 36,
+              column: 37,
               line: 4,
             },
           },

diff --git a/packages/core/diagnostic/src/diagnostic.js b/packages/core/diagnostic/src/diagnostic.js
@@ -238,7 +238,7 @@ export function generateJSONCodeHighlights(
   return ids.map(({key, type, message}) => {
     let pos = nullthrows(map.pointers[key]);
     return {
-      ...getJSONSourceLocation(pos, type),
+      ...getJSONHighlightLocation(pos, type),
       message,
     };
   });
@@ -248,7 +248,7 @@ export function generateJSONCodeHighlights(
  * Converts entries in <a href="/~https://github.com/mischnic/json-sourcemap">@mischnic/json-sourcemap</a>'s
  * <code>result.pointers</code> array.
  */
-export function getJSONSourceLocation(
+export function getJSONHighlightLocation(
   pos: Mapping,
   type?: ?'key' | 'value',
 ): {|
@@ -277,6 +277,42 @@ export function getJSONSourceLocation(
   }
 }
 
+/** Result is 1-based, but end is exclusive */
+export function getJSONSourceLocation(
+  pos: Mapping,
+  type?: ?'key' | 'value',
+): {|
+  start: {|
+    +line: number,
+    +column: number,
+  |},
+  end: {|
+    +line: number,
+    +column: number,
+  |},
+|} {
+  let v = getJSONHighlightLocation(pos, type);
+  return {start: v.start, end: {line: v.end.line, column: v.end.column + 1}};
+}
+
+export function convertSourceLocationToHighlight<
+  Location: {
+    /** 1-based, inclusive */
+    +start: {|
+      +line: number,
+      +column: number,
+    |},
+    /** 1-based, exclusive */
+    +end: {|
+      +line: number,
+      +column: number,
+    |},
+    ...
+  },
+>({start, end}: Location, message?: string): DiagnosticCodeHighlight {
+  return {message, start, end: {line: end.line, column: end.column - 1}};
+}
+
 /** Sanitizes object keys before using them as <code>key</code> in generateJSONCodeHighlights */
 export function encodeJSONKeyComponent(component: string): string {
   return component.replace(/~/g, '~0').replace(/\//g, '~1');

diff --git a/packages/core/diagnostic/test/JSONCodeHighlights.test.js b/packages/core/diagnostic/test/JSONCodeHighlights.test.js
@@ -0,0 +1,36 @@
+// @flow strict-local
+import assert from 'assert';
+
+import {generateJSONCodeHighlights} from '../src/diagnostic';
+
+describe('generateJSONCodeHighlights', () => {
+  it('returns an escaped string 01', () => {
+    let result = generateJSONCodeHighlights(
+      `{
+  "a": 1
+}`,
+      [
+        {key: '/a', type: 'key', message: 'foo1'},
+        {key: '/a', type: 'value', message: 'foo2'},
+        {key: '/a', message: 'foo3'},
+      ],
+    );
+    assert.deepEqual(result, [
+      {
+        start: {line: 2, column: 3},
+        end: {line: 2, column: 5},
+        message: 'foo1',
+      },
+      {
+        start: {line: 2, column: 8},
+        end: {line: 2, column: 8},
+        message: 'foo2',
+      },
+      {
+        start: {line: 2, column: 3},
+        end: {line: 2, column: 8},
+        message: 'foo3',
+      },
+    ]);
+  });
+});
diff --git a/packages/core/integration-tests/test/css-modules.js b/packages/core/integration-tests/test/css-modules.js
@@ -410,6 +410,7 @@ describe('css modules', () => {
                 language: 'js',
                 codeHighlights: [
                   {
+                    message: undefined,
                     end: {
                       column: 45,
                       line: 7,

diff --git a/packages/core/integration-tests/test/css.js b/packages/core/integration-tests/test/css.js
@@ -423,6 +423,7 @@ describe('css', () => {
                 code,
                 codeHighlights: [
                   {
+                    message: undefined,
                     start: {
                       line: 5,
                       column: 3,

diff --git a/packages/core/integration-tests/test/glob.js b/packages/core/integration-tests/test/glob.js
@@ -135,7 +135,7 @@ describe('glob', function () {
 
   it('should error when an unsupported asset type imports a glob', async function () {
     let filePath = path.join(__dirname, '/integration/glob-error/index.html');
-    // $FlowFixMe
+    // $FlowFixMe[prop-missing]
     await assert.rejects(() => bundle(filePath), {
       name: 'BuildError',
       diagnostics: [
@@ -154,7 +154,7 @@ describe('glob', function () {
 
   it('should error when a URL dependency imports a glob', async function () {
     let filePath = path.join(__dirname, '/integration/glob-error/index.css');
-    // $FlowFixMe
+    // $FlowFixMe[prop-missing]
     await assert.rejects(() => bundle(filePath), {
       name: 'BuildError',
       diagnostics: [
@@ -167,6 +167,7 @@ describe('glob', function () {
               code: await inputFS.readFile(filePath, 'utf8'),
               codeHighlights: [
                 {
+                  message: undefined,
                   start: {
                     column: 19,
                     line: 2,
@@ -187,6 +188,7 @@ describe('glob', function () {
             {
               codeHighlights: [
                 {
+                  message: undefined,
                   start: {
                     column: 19,
                     line: 2,

diff --git a/packages/core/integration-tests/test/html.js b/packages/core/integration-tests/test/html.js
@@ -1440,7 +1440,7 @@ describe('html', function () {
               ),
               codeHighlights: [
                 {
-                  message: null,
+                  message: undefined,
                   start: {
                     line: 5,
                     column: 7,
@@ -1729,7 +1729,7 @@ describe('html', function () {
               filePath: path.join(__dirname, '/integration/html-js/index.js'),
               codeHighlights: [
                 {
-                  message: null,
+                  message: undefined,
                   start: {
                     line: 1,
                     column: 1,