Improve elm compiler error output (#7994)

Co-authored-by: ChristophP <deedop@hotmail.de> Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com>
parcel-bundler · May 20, 2022 · 97a7864 · 97a7864
1 parent 8981c88
commit 97a7864
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 7 deletions.
diff --git a/packages/core/integration-tests/test/elm.js b/packages/core/integration-tests/test/elm.js
@@ -81,4 +81,56 @@ describe('elm', function () {
     assert(js.includes('Elm'));
     assert(js.includes('init'));
   });
+
+  it('should produce correct formatting and indentation when compilation fails', async function () {
+    const normalizedPath = path.normalize(
+      'test/integration/elm-compile-error/src/Main.elm',
+    );
+    await assert.rejects(
+      () =>
+        bundle(path.join(__dirname, 'integration/elm-compile-error/index.js'), {
+          mode: 'production',
+        }),
+
+      {
+        name: 'BuildError',
+        diagnostics: [
+          {
+            message:
+              '\n' +
+              `-- TYPE MISMATCH --------------- ${normalizedPath}\n` +
+              '\n' +
+              'The 1st argument to `text` is not what I expect:\n' +
+              '\n' +
+              '7|     Html.text 5 "Hello, world!"\n' +
+              '                 **^**\n' +
+              'This argument is a number of type:\n' +
+              '\n' +
+              '    **number**\n' +
+              '\n' +
+              'But `text` needs the 1st argument to be:\n' +
+              '\n' +
+              '    **String**\n' +
+              '\n' +
+              '__Hint__: Try using **String.fromInt** to convert it to a string?',
+            origin: '@parcel/elm-transformer',
+            stack: '',
+          },
+          {
+            message:
+              '\n' +
+              `-- TOO MANY ARGS --------------- ${normalizedPath}\n` +
+              '\n' +
+              'The `text` function expects 1 argument, but it got 2 instead.\n' +
+              '\n' +
+              '7|     Html.text 5 "Hello, world!"\n' +
+              '       **^^^^^^^^^**\n' +
+              'Are there any missing commas? Or missing parentheses?',
+            origin: '@parcel/elm-transformer',
+            stack: '',
+          },
+        ],
+      },
+    );
+  });
 });
diff --git a/packages/core/integration-tests/test/integration/elm-compile-error/elm.json b/packages/core/integration-tests/test/integration/elm-compile-error/elm.json
@@ -0,0 +1,24 @@
+{
+    "type": "application",
+    "source-directories": [
+        "src"
+    ],
+    "elm-version": "0.19.1",
+    "dependencies": {
+        "direct": {
+            "elm/browser": "1.0.1",
+            "elm/core": "1.0.2",
+            "elm/html": "1.0.0"
+        },
+        "indirect": {
+            "elm/json": "1.1.3",
+            "elm/time": "1.0.0",
+            "elm/url": "1.0.0",
+            "elm/virtual-dom": "1.0.2"
+        }
+    },
+    "test-dependencies": {
+        "direct": {},
+        "indirect": {}
+    }
+}
diff --git a/packages/core/integration-tests/test/integration/elm-compile-error/index.js b/packages/core/integration-tests/test/integration/elm-compile-error/index.js
@@ -0,0 +1,5 @@
+var local = require('./src/Main.elm');
+
+module.exports = function () {
+    return local;
+};
diff --git a/packages/core/integration-tests/test/integration/elm-compile-error/src/Main.elm b/packages/core/integration-tests/test/integration/elm-compile-error/src/Main.elm
@@ -0,0 +1,7 @@
+module Main exposing (main)
+
+import Html
+
+
+main =
+    Html.text 5 "Hello, world!"
diff --git a/packages/reporters/cli/src/CLIReporter.js b/packages/reporters/cli/src/CLIReporter.js
@@ -230,7 +230,11 @@ async function writeDiagnostic(
 
 function wrapWithIndent(string, indent = 0, initialIndent = indent) {
   let width = getTerminalWidth().columns;
-  return indentString(wrapAnsi(string, width - indent), indent, initialIndent);
+  return indentString(
+    wrapAnsi(string.trimEnd(), width - indent, {trim: false}),
+    indent,
+    initialIndent,
+  );
 }
 
 function indentString(string, indent = 0, initialIndent = indent) {

diff --git a/packages/reporters/dev-server/src/templates/500.html b/packages/reporters/dev-server/src/templates/500.html
@@ -29,6 +29,7 @@
         color: salmon;
         margin: 10px;
         font-family: Menlo, Consolas, monospace;
+        white-space: pre;
       }
 
       .error-hints-container {

diff --git a/packages/transformers/elm/src/ElmTransformer.js b/packages/transformers/elm/src/ElmTransformer.js
@@ -6,7 +6,7 @@ import spawn from 'cross-spawn';
 import path from 'path';
 import {minify} from 'terser';
 import nullthrows from 'nullthrows';
-import ThrowableDiagnostic from '@parcel/diagnostic';
+import ThrowableDiagnostic, {md} from '@parcel/diagnostic';
 // $FlowFixMe
 import elm from 'node-elm-compiler';
 // $FlowFixMe
@@ -46,6 +46,7 @@ export default (new Transformer({
       // $FlowFixMe[sketchy-null-string]
       debug: !options.env.PARCEL_ELM_NO_DEBUG && options.mode !== 'production',
       optimize: asset.env.shouldOptimize,
+      report: 'json',
     };
     asset.invalidateOnEnvChange('PARCEL_ELM_NO_DEBUG');
     for (const filePath of await elm.findAllDependencies(asset.filePath)) {
@@ -60,12 +61,13 @@ export default (new Transformer({
     try {
       code = await compileToString(elm, elmBinary, asset, compilerConfig);
     } catch (e) {
+      let compilerJson = e.message.split('\n')[1];
+      let compilerDiagnostics = JSON.parse(compilerJson);
+
       throw new ThrowableDiagnostic({
-        diagnostic: {
-          message: 'Compilation failed',
-          origin: '@parcel/elm-transformer',
-          stack: e.toString(),
-        },
+        diagnostic: compilerDiagnostics.errors.flatMap(
+          elmErrorToParcelDiagnostics,
+        ),
       });
     }
 
@@ -160,3 +162,33 @@ async function minifyElmOutput(source) {
   if (result.code != null) return result.code;
   throw result.error;
 }
+
+function formatMessagePiece(piece) {
+  if (piece.string) {
+    if (piece.underline) {
+      return md`${md.underline(piece.string)}`;
+    }
+    return md`${md.bold(piece.string)}`;
+  }
+  return md`${piece}`;
+}
+
+function elmErrorToParcelDiagnostics(error) {
+  const relativePath = path.relative(process.cwd(), error.path);
+  return error.problems.map(problem => {
+    const padLength = 80 - 5 - problem.title.length - relativePath.length;
+    const dashes = '-'.repeat(padLength);
+    const message = [
+      '',
+      `-- ${problem.title} ${dashes} ${relativePath}`,
+      '',
+      problem.message.map(formatMessagePiece).join(''),
+    ].join('\n');
+
+    return {
+      message,
+      origin: '@parcel/elm-transformer',
+      stack: '', // set stack to empty since it is not useful
+    };
+  });
+}