Repository: parcel-bundler/parcel (js)

Issue:
`codeHighlights` is sometimes an object and sometimes an array of objects
<!---
Thanks for filing an issue 😄 ! Before you submit, please read the following:

Search open/closed issues before submitting since someone might have asked the same thing before!
-->

# 🐛 bug report

<!--- Provide a general summary of the issue here -->

If I'm reading [the documentation](https://v2.parceljs.org/plugin-system/logger/#diagnosticcodeframe) right, it states that on a code frame, `codeHighlights` should be an array of code highlight objects. This seems to be true some of the time, but at other times, a code frame within a diagnostic within an error will contain `codeHighlights` whose value is a single code highlight object. This inconsistency makes handling code highlights objects on errors more complex than it really needs to be.

## 🎛 Configuration (.babelrc, package.json, cli command)

I have this set up as an isolated repo here: https://github.com/Aaronius/parcel2bug/tree/code-highlights. Just follow the readme.

.babelrc:

```
{
  "bundler": "@parcel/bundler-default",
  "transformers": {
    "*.js": ["@parcel/transformer-babel","@parcel/transformer-js"]
  },
  "runtimes": {
    "browser": ["@parcel/runtime-js"]
  },
  "packagers": {
    "*.js": "@parcel/packager-js"
  },
  "namers": ["@parcel/namer-default"],
  "resolvers": ["@parcel/resolver-default"],
  "reporters": ["@parcel/reporter-cli"]
}
```

package.json:

```
{
  "name": "parcel2bug",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "parcel": "^2.0.0-nightly.385"
  }
}
```

build call:

```
const bundler = new Parcel({
      entries: path.join(__dirname, 'src/index.js'),
      sourceMaps: false,
      patchConsole: false,
      autoinstall: false,
      scopeHoist: true,
      disableCache: true,
      inputFS,
      outputFS,
      workerFarm,
      distDir: VIRTUAL_DIST_DIR,
    });

    await bundler.run();
```
<!--- If describing a bug, tell us what your babel configuration looks like -->


## 🤔 Expected Behavior
`codeHighlights` should always be an array.
<!--- Tell us what should happen -->

## 😯 Current Behavior
`codeHighlights` is sometimes an object.
<!--- Tell us what happens instead of the expected behavior -->

<!--- If you are seeing an error, please include the full error message and stack trace -->

## 💁 Possible Solution
Always make `codeHighlights` an array.
<!--- Not obligatory, but suggest a fix/reason for the bug -->

## 🔦 Context
We're doing some custom error handling and manipulation in order to present a specifically crafted error message to users of our build service.
<!--- How has this issue affected you? What are you trying to accomplish? -->

<!--- Providing context helps us come up with a solution that is most useful in the real world -->

## 💻 Code Sample
https://github.com/Aaronius/parcel2bug/tree/code-highlights. Just follow the readme.
<!-- Please provide a code repository, gist, code snippet or sample files to reproduce the issue -->

## 🌍 Your Environment

<!--- Include as many relevant details about the environment you experienced the bug in -->

| Software         | Version(s) |
| ---------------- | ---------- |
| Parcel           | 2.0.0-nightly.385
| Node             | 12.12.0
| npm/Yarn         | npm 6.13.4
| Operating System | Mac Mojave

<!-- Love parcel? Please consider supporting our collective:
👉  https://opencollective.com/parcel/donate -->


Pull request:
Simplify diagnostics
<!---
Thanks for filing a pull request 😄 ! Before you submit, please read the following:

Search open/closed issues before submitting since someone might have pushed the same thing before!
-->

# ↪️ Pull Request

<!---
Provide a general summary of the pull request here
Please look for any issues that this PR resolves and tag them in the PR.
-->

This PR simplifies diagnostics, so codehighlights in diagnostics are always arrays and a ThrowableDiagnostic always takes in an array of diagnostics.

This is a breaking change, I can add a legacy diagnostic option back to ThrowableDiagnostic but I thought it would be better to just fail the plugins that rely on this option to ensure they update their plugins instead.

Closes #5078

## ✔️ PR Todo

- [x] Added/updated unit tests for this change
- [x] Filled out test instructions (In case there aren't any unit tests)
- [x] Included links to related issues/PRs


Patch:
diff --git a/packages/core/core/src/Parcel.js b/packages/core/core/src/Parcel.js
index c03067116..4de85a47c 100644
--- a/packages/core/core/src/Parcel.js
+++ b/packages/core/core/src/Parcel.js
@@ -421,9 +421,8 @@ export default class Parcel {
 }
 
 export class BuildError extends ThrowableDiagnostic {
-  constructor(diagnostics: Array<Diagnostic>) {
-    super({diagnostic: diagnostics});
-
+  constructor(diagnostic: Array<Diagnostic> | Diagnostic) {
+    super({diagnostic});
     this.name = 'BuildError';
   }
 }
diff --git a/packages/core/diagnostic/src/diagnostic.js b/packages/core/diagnostic/src/diagnostic.js
index 6be3ef3ee..ee48bb939 100644
--- a/packages/core/diagnostic/src/diagnostic.js
+++ b/packages/core/diagnostic/src/diagnostic.js
@@ -41,7 +41,7 @@ export type DiagnosticCodeFrame = {|
    *
    */
   code?: string,
-  codeHighlights: DiagnosticCodeHighlight | Array<DiagnosticCodeHighlight>,
+  codeHighlights: Array<DiagnosticCodeHighlight>,
 |};
 
 /**
@@ -103,33 +103,34 @@ export type Diagnostifiable =
   | string;
 
 /** Normalize the given value into a diagnostic. */
-export function anyToDiagnostic(
-  input: Diagnostifiable,
-): Diagnostic | Array<Diagnostic> {
+export function anyToDiagnostic(input: Diagnostifiable): Array<Diagnostic> {
   // $FlowFixMe
-  let diagnostic: Diagnostic | Array<Diagnostic> = input;
+  let diagnostic: Array<Diagnostic> = input;
+
   if (input instanceof ThrowableDiagnostic) {
     diagnostic = input.diagnostics;
   } else if (input instanceof Error) {
     diagnostic = errorToDiagnostic(input);
   }
 
-  return diagnostic;
+  return Array.isArray(diagnostic) ? diagnostic : [diagnostic];
 }
 
 /** Normalize the given error into a diagnostic. */
 export function errorToDiagnostic(
   error: ThrowableDiagnostic | PrintableError | string,
   realOrigin?: string,
-): Diagnostic | Array<Diagnostic> {
+): Array<Diagnostic> {
   let codeFrame: DiagnosticCodeFrame | void = undefined;
 
   if (typeof error === 'string') {
-    return {
-      origin: realOrigin ?? 'Error',
-      message: error,
-      codeFrame,
-    };
+    return [
+      {
+        origin: realOrigin ?? 'Error',
+        message: error,
+        codeFrame,
+      },
+    ];
   }
 
   if (error instanceof ThrowableDiagnostic) {
@@ -144,29 +145,38 @@ export function errorToDiagnostic(
   if (error.loc && error.source != null) {
     codeFrame = {
       code: error.source,
-      codeHighlights: {
-        start: {
-          line: error.loc.line,
-          column: error.loc.column,
-        },
-        end: {
-          line: error.loc.line,
-          column: error.loc.column,
+      codeHighlights: [
+        {
+          start: {
+            line: error.loc.line,
+            column: error.loc.column,
+          },
+          end: {
+            line: error.loc.line,
+            column: error.loc.column,
+          },
         },
-      },
+      ],
     };
   }
 
-  return {
-    origin: realOrigin ?? 'Error',
-    message: error.message,
-    name: error.name,
-    filePath: error.filePath ?? error.fileName,
-    stack: error.highlightedCodeFrame ?? error.codeFrame ?? error.stack,
-    codeFrame,
-  };
+  return [
+    {
+      origin: realOrigin ?? 'Error',
+      message: error.message,
+      name: error.name,
+      filePath: error.filePath ?? error.fileName,
+      stack: error.highlightedCodeFrame ?? error.codeFrame ?? error.stack,
+      codeFrame,
+    },
+  ];
 }
 
+type ThrowableDiagnosticOpts = {
+  diagnostic: Diagnostic | Array<Diagnostic>,
+  ...
+};
+
 /**
  * An error wrapper around a diagnostic that can be <code>throw</code>n (e.g. to signal a
  * build error).
@@ -174,12 +184,12 @@ export function errorToDiagnostic(
 export default class ThrowableDiagnostic extends Error {
   diagnostics: Array<Diagnostic>;
 
-  constructor(opts: {diagnostic: Diagnostic | Array<Diagnostic>, ...}) {
+  constructor(opts: ThrowableDiagnosticOpts) {
     let diagnostics = Array.isArray(opts.diagnostic)
       ? opts.diagnostic
       : [opts.diagnostic];
 
-    // construct error from diagnostics...
+    // Construct error from diagnostics
     super(diagnostics[0].message);
     this.stack = diagnostics[0].stack ?? super.stack;
     this.name = diagnostics[0].name ?? super.name;
diff --git a/packages/core/utils/src/prettyDiagnostic.js b/packages/core/utils/src/prettyDiagnostic.js
index cc0a9f3e3..f4b7fbdbf 100644
--- a/packages/core/utils/src/prettyDiagnostic.js
+++ b/packages/core/utils/src/prettyDiagnostic.js
@@ -45,10 +45,7 @@ export default async function prettyDiagnostic(
   };
 
   if (codeFrame !== undefined) {
-    let highlights = Array.isArray(codeFrame.codeHighlights)
-      ? codeFrame.codeHighlights
-      : [codeFrame.codeHighlights];
-
+    let highlights = codeFrame.codeHighlights;
     let code =
       codeFrame.code ??
       (options &&
diff --git a/packages/namers/default/src/DefaultNamer.js b/packages/namers/default/src/DefaultNamer.js
index 8724b0f0c..daf771e6f 100644
--- a/packages/namers/default/src/DefaultNamer.js
+++ b/packages/namers/default/src/DefaultNamer.js
@@ -61,16 +61,18 @@ export default (new Namer({
             message: `Target "${bundle.target.name}" declares an output file path of "${fullName}" which does not match the compiled bundle type "${bundle.type}".`,
             filePath: loc.filePath,
             codeFrame: {
-              codeHighlights: {
-                start: loc.start,
-                end: loc.end,
-                message: `Did you mean "${fullName.slice(
-                  0,
-                  -path.extname(fullName).length,
-                ) +
-                  '.' +
-                  bundle.type}"?`,
-              },
+              codeHighlights: [
+                {
+                  start: loc.start,
+                  end: loc.end,
+                  message: `Did you mean "${fullName.slice(
+                    0,
+                    -path.extname(fullName).length,
+                  ) +
+                    '.' +
+                    bundle.type}"?`,
+                },
+              ],
             },
             hints: [
               `Try changing the file extension of "${
diff --git a/packages/optimizers/terser/src/TerserOptimizer.js b/packages/optimizers/terser/src/TerserOptimizer.js
index 3f1b8d6b3..93fd33cfe 100644
--- a/packages/optimizers/terser/src/TerserOptimizer.js
+++ b/packages/optimizers/terser/src/TerserOptimizer.js
@@ -46,12 +46,12 @@ export default (new Optimizer({
       // $FlowFixMe
       let {message, line, col} = error;
       if (line != null && col != null) {
-        let diagnostic = [];
+        let diagnostics = [];
         let mapping = map?.findClosestMapping(line, col);
         if (mapping && mapping.original && mapping.source) {
           let {source, original} = mapping;
           let filePath = path.resolve(options.projectRoot, source);
-          diagnostic.push({
+          diagnostics.push({
             message,
             origin: '@parcel/optimizer-terser',
             language: 'js',
@@ -64,12 +64,12 @@ export default (new Optimizer({
           });
         }
 
-        if (diagnostic.length === 0 || options.logLevel === 'verbose') {
+        if (diagnostics.length === 0 || options.logLevel === 'verbose') {
           let loc = {
             line: line,
             column: col,
           };
-          diagnostic.push({
+          diagnostics.push({
             message,
             origin: '@parcel/optimizer-terser',
             language: 'js',
@@ -81,7 +81,7 @@ export default (new Optimizer({
             hints: ["It's likely that Terser doesn't support this syntax yet."],
           });
         }
-        throw new ThrowableDiagnostic({diagnostic});
+        throw new ThrowableDiagnostic({diagnostic: diagnostics});
       } else {
         throw error;
       }
diff --git a/packages/shared/scope-hoisting/src/utils.js b/packages/shared/scope-hoisting/src/utils.js
index a8a5c43e5..e3627738a 100644
--- a/packages/shared/scope-hoisting/src/utils.js
+++ b/packages/shared/scope-hoisting/src/utils.js
@@ -285,10 +285,12 @@ export function getThrowableDiagnosticForNode(
   }
   if (loc) {
     diagnostic.codeFrame = {
-      codeHighlights: {
-        start: loc.start,
-        end: loc.end,
-      },
+      codeHighlights: [
+        {
+          start: loc.start,
+          end: loc.end,
+        },
+      ],
     };
   }
   return new ThrowableDiagnostic({
diff --git a/packages/transformers/js/src/visitors/fs.js b/packages/transformers/js/src/visitors/fs.js
index a37c9dd08..b6cdde779 100644
--- a/packages/transformers/js/src/visitors/fs.js
+++ b/packages/transformers/js/src/visitors/fs.js
@@ -118,7 +118,7 @@ export default ({
             filePath: loc?.filePath ?? asset.filePath,
             codeFrame: loc
               ? {
-                  codeHighlights: {start: loc.start, end: loc.end},
+                  codeHighlights: [{start: loc.start, end: loc.end}],
                 }
               : undefined,
           };
@@ -133,7 +133,7 @@ export default ({
             filePath: loc?.filePath ?? asset.filePath,
             codeFrame: loc
               ? {
-                  codeHighlights: {start: loc.start, end: loc.end},
+                  codeHighlights: [{start: loc.start, end: loc.end}],
                 }
               : undefined,
             hints: ['You might want to enable `inlineFS`?'],
diff --git a/packages/transformers/typescript-types/src/TSTypesTransformer.js b/packages/transformers/typescript-types/src/TSTypesTransformer.js
index 6adca2ce2..c7b7c3af3 100644
--- a/packages/transformers/typescript-types/src/TSTypesTransformer.js
+++ b/packages/transformers/typescript-types/src/TSTypesTransformer.js
@@ -119,11 +119,13 @@ export default (new Transformer({
 
             codeframe = {
               code: source,
-              codeHighlights: {
-                start,
-                end,
-                message: diagnosticMessage,
-              },
+              codeHighlights: [
+                {
+                  start,
+                  end,
+                  message: diagnosticMessage,
+                },
+              ],
             };
           }
         }
diff --git a/packages/transformers/vue/src/VueTransformer.js b/packages/transformers/vue/src/VueTransformer.js
index b6913c88c..07581204d 100644
--- a/packages/transformers/vue/src/VueTransformer.js
+++ b/packages/transformers/vue/src/VueTransformer.js
@@ -167,16 +167,18 @@ function createDiagnostic(err, filePath) {
   };
   if (err.loc) {
     diagnostic.codeFrame = {
-      codeHighlights: {
-        start: {
-          line: err.loc.start.line + err.loc.start.offset,
-          column: err.loc.start.column,
-        },
-        end: {
-          line: err.loc.end.line + err.loc.end.offset,
-          column: err.loc.end.column,
+      codeHighlights: [
+        {
+          start: {
+            line: err.loc.start.line + err.loc.start.offset,
+            column: err.loc.start.column,
+          },
+          end: {
+            line: err.loc.end.line + err.loc.end.offset,
+            column: err.loc.end.column,
+          },
         },
-      },
+      ],
     };
   }
   return diagnostic;
diff --git a/packages/validators/typescript/src/TypeScriptValidator.js b/packages/validators/typescript/src/TypeScriptValidator.js
index 2f4506e39..612f069c0 100644
--- a/packages/validators/typescript/src/TypeScriptValidator.js
+++ b/packages/validators/typescript/src/TypeScriptValidator.js
@@ -189,11 +189,13 @@ function getValidateResultFromDiagnostics(
 
           codeframe = {
             code: source,
-            codeHighlights: {
-              start,
-              end,
-              message: diagnosticMessage,
-            },
+            codeHighlights: [
+              {
+                start,
+                end,
+                message: diagnosticMessage,
+              },
+            ],
           };
         }
       }


Test patch:
diff --git a/packages/core/integration-tests/test/output-formats.js b/packages/core/integration-tests/test/output-formats.js
index 5d55af5c5..ea0167337 100644
--- a/packages/core/integration-tests/test/output-formats.js
+++ b/packages/core/integration-tests/test/output-formats.js
@@ -396,16 +396,18 @@ describe('output formats', function() {
             filePath: source,
             language: 'js',
             codeFrame: {
-              codeHighlights: {
-                start: {
-                  line: 1,
-                  column: 10,
-                },
-                end: {
-                  line: 1,
-                  column: 15,
+              codeHighlights: [
+                {
+                  start: {
+                    line: 1,
+                    column: 10,
+                  },
+                  end: {
+                    line: 1,
+                    column: 15,
+                  },
                 },
-              },
+              ],
             },
           },
         ],
@@ -638,16 +640,18 @@ describe('output formats', function() {
             filePath: source,
             language: 'js',
             codeFrame: {
-              codeHighlights: {
-                start: {
-                  line: 1,
-                  column: 10,
-                },
-                end: {
-                  line: 1,
-                  column: 15,
+              codeHighlights: [
+                {
+                  start: {
+                    line: 1,
+                    column: 10,
+                  },
+                  end: {
+                    line: 1,
+                    column: 15,
+                  },
                 },
-              },
+              ],
             },
           },
         ],
@@ -1002,16 +1006,18 @@ describe('output formats', function() {
             filePath: source,
             language: 'js',
             codeFrame: {
-              codeHighlights: {
-                start: {
-                  line: 1,
-                  column: 16,
-                },
-                end: {
-                  line: 1,
-                  column: 40,
+              codeHighlights: [
+                {
+                  start: {
+                    line: 1,
+                    column: 16,
+                  },
+                  end: {
+                    line: 1,
+                    column: 40,
+                  },
                 },
-              },
+              ],
             },
           },
         ],
@@ -1048,16 +1054,18 @@ describe('output formats', function() {
               filePath: source,
               language: 'js',
               codeFrame: {
-                codeHighlights: {
-                  start: {
-                    line: 1,
-                    column: 1,
+                codeHighlights: [
+                  {
+                    start: {
+                      line: 1,
+                      column: 1,
+                    },
+                    end: {
+                      line: 1,
+                      column: 29,
+                    },
                   },
-                  end: {
-                    line: 1,
-                    column: 29,
-                  },
-                },
+                ],
               },
             },
           ],
diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
index 75064d3e0..3ff78eb0f 100644
--- a/packages/core/integration-tests/test/scope-hoisting.js
+++ b/packages/core/integration-tests/test/scope-hoisting.js
@@ -346,16 +346,18 @@ describe('scope hoisting', function() {
             filePath: source,
             language: 'js',
             codeFrame: {
-              codeHighlights: {
-                start: {
-                  line: 1,
-                  column: 8,
+              codeHighlights: [
+                {
+                  start: {
+                    line: 1,
+                    column: 8,
+                  },
+                  end: {
+                    line: 1,
+                    column: 8,
+                  },
                 },
-                end: {
-                  line: 1,
-                  column: 8,
-                },
-              },
+              ],
             },
           },
         ],
@@ -381,16 +383,18 @@ describe('scope hoisting', function() {
             ),
             language: 'js',
             codeFrame: {
-              codeHighlights: {
-                start: {
-                  line: 1,
-                  column: 9,
-                },
-                end: {
-                  line: 1,
-                  column: 11,
+              codeHighlights: [
+                {
+                  start: {
+                    line: 1,
+                    column: 9,
+                  },
+                  end: {
+                    line: 1,
+                    column: 11,
+                  },
                 },
-              },
+              ],
             },
           },
         ],
@@ -1847,16 +1851,18 @@ describe('scope hoisting', function() {
             filePath: source,
             language: 'js',
             codeFrame: {
-              codeHighlights: {
-                start: {
-                  line: 3,
-                  column: 10,
-                },
-                end: {
-                  line: 3,
-                  column: 31,
+              codeHighlights: [
+                {
+                  start: {
+                    line: 3,
+                    column: 10,
+                  },
+                  end: {
+                    line: 3,
+                    column: 31,
+                  },
                 },
-              },
+              ],
             },
           },
         ],