feat: improve formatting of error messages

Author: jhnns

src/formatLessError.js

@@ -1,7 +1,36 @@
+const os = require('os');
+
+/**
+ * Tries to get an excerpt of the file where the error happened.
+ * Uses err.line and err.column.
+ *
+ * Returns an empty string if the excerpt could not be retrieved.
+ *
+ * @param {LessError} err
+ * @returns {Array<string>}
+ */
+function getFileExcerptIfPossible(lessErr) {
+  try {
+    const excerpt = lessErr.extract.slice(0, 2);
+    const column = Math.max(lessErr.column - 1, 0);
+
+    if (typeof excerpt[0] === 'undefined') {
+      excerpt.shift();
+    }
+
+    excerpt.push(`${new Array(column).join(' ')}^`);
+
+    return excerpt;
+  } catch (unexpectedErr) {
+    // If anything goes wrong here, we don't want any errors to be reported to the user
+    return [];
+  }
+}
+
 /**
  * Beautifies the error message from Less.
  *
- * @param {Error} lessErr
+ * @param {LessError} lessErr
  * @param {string} lessErr.type - e.g. 'Name'
  * @param {string} lessErr.message - e.g. '.undefined-mixin is undefined'
  * @param {string} lessErr.filename - e.g. '/path/to/style.less'
@@ -11,18 +40,23 @@
  * @param {string} lessErr.callExtract - e.g. undefined
  * @param {number} lessErr.column - e.g. 6
  * @param {Array<string>} lessErr.extract - e.g. ['    .my-style {', '      .undefined-mixin;', '      display: block;']
+ * @returns {LessError}
  */
-function formatLessError(lessErr) {
-  const extract = lessErr.extract ? `\n near lines:\n   ${lessErr.extract.join('\n   ')}` : '';
-  const err = new Error(
-      `${lessErr.message}\n @ ${lessErr.filename
-      } (line ${lessErr.line}, column ${lessErr.column})${
-      extract}`,
-  );
+function formatLessError(err) { /* eslint-disable no-param-reassign */
+  const msg = err.message;
 
+  // Instruct webpack to hide the JS stack from the console
+  // Usually you're only interested in the SASS stack in this case.
   err.hideStack = true;
 
+  err.message = [
+    os.EOL,
+    ...getFileExcerptIfPossible(err),
+    msg.charAt(0).toUpperCase() + msg.slice(1),
+    `      in ${err.filename} (line ${err.line}, column ${err.column})`,
+  ].join(os.EOL);
+
   return err;
-}
+} /* eslint-enable no-param-reassign */
 
 module.exports = formatLessError;

test/__snapshots__/index.test.js.snap

@@ -0,0 +1,30 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`should fail if a file is tried to be loaded from include paths and with webpack's resolver simultaneously 1`] = `
+"Module build failed: 
+
+@import \\"some/module.less\\";
+@import \\"~some/module.less\\";
+^
+'~some/module.less' wasn't found. Tried - /test/fixtures/less/~some/module.less,/test/fixtures/node_modules/~some/module.less,~some/module.less
+      in /test/fixtures/less/error-mixed-resolvers.less (line 3, column 0)"
+`;
+
+exports[`should provide a useful error message if the import could not be found 1`] = `
+"Module build failed: 
+
+@import \\"not-existing\\";
+^
+Can't resolve './not-existing.less' in '/test/fixtures/less'
+      in /test/fixtures/less/error-import-not-existing.less (line 1, column 0)"
+`;
+
+exports[`should provide a useful error message if there was a syntax error 1`] = `
+"Module build failed: 
+
+but this is a syntax error
+
+^
+Unrecognised input. Possibly missing something
+      in /test/fixtures/less/error-syntax.less (line 6, column 0)"
+`;

test/fixtures/less/error-import-not-existing.less

@@ -1 +1 @@
-@import "not-existing";
+@import "not-existing";

test/fixtures/less/error-syntax.less

@@ -0,0 +1,5 @@
+.this-less-code-works {
+  background: hotpink;
+}
+
+but this is a syntax error

test/helpers/compareErrorMessage.js

@@ -0,0 +1,15 @@
+const path = require('path');
+
+const projectPath = path.resolve(__dirname, '..', '..');
+const projectPathPattern = new RegExp(projectPath, 'g');
+
+// We need to remove all environment dependent features
+function compareErrorMessage(msg) {
+  const envIndependentMsg = msg
+    .replace(projectPathPattern, '')
+    .replace(/\\/g, '/');
+
+  expect(envIndependentMsg).toMatchSnapshot();
+}
+
+module.exports = compareErrorMessage;

test/helpers/createSpec.js

@@ -12,6 +12,7 @@ const ignore = [
   'import-non-less',
   'error-import-not-existing',
   'error-mixed-resolvers',
+  'error-syntax',
 ];
 const lessReplacements = [
   [/~some\//g, '../node_modules/some/'],

test/index.test.js

@@ -2,6 +2,7 @@ const path = require('path');
 const compile = require('./helpers/compile');
 const moduleRules = require('./helpers/moduleRules');
 const { readCssFixture, readSourceMap } = require('./helpers/readFixture');
+const compareErrorMessage = require('./helpers/compareErrorMessage');
 
 const nodeModulesPath = path.resolve(__dirname, 'fixtures', 'node_modules');
 
@@ -16,7 +17,7 @@ async function compileAndCompare(fixture, { lessLoaderOptions, lessLoaderContext
   ]);
   const [actualCss] = inspect.arguments;
 
-  return expect(actualCss).toBe(expectedCss);
+  expect(actualCss).toBe(expectedCss);
 }
 
 test('should compile simple less without errors', async () => {
@@ -116,18 +117,26 @@ test('should not alter the original options object', async () => {
   expect(copiedOptions).toEqual(options);
 });
 
-test('should report error correctly', async () => {
-  const err = await compile('error-import-not-existing')
+test('should fail if a file is tried to be loaded from include paths and with webpack\'s resolver simultaneously', async () => {
+  const err = await compile('error-mixed-resolvers', moduleRules.basic({ paths: [nodeModulesPath] }))
     .catch(e => e);
 
   expect(err).toBeInstanceOf(Error);
-  expect(err.message).toMatch(/not-existing/);
+  compareErrorMessage(err.message);
 });
 
-test('should fail if a file is tried to be loaded from include paths and with webpack\'s resolver simultaneously', async () => {
-  const err = await compile('error-mixed-resolvers', moduleRules.basic({ paths: [nodeModulesPath] }))
+test('should provide a useful error message if the import could not be found', async () => {
+  const err = await compile('error-import-not-existing', moduleRules.basic())
+    .catch(e => e);
+
+  expect(err).toBeInstanceOf(Error);
+  compareErrorMessage(err.message);
+});
+
+test('should provide a useful error message if there was a syntax error', async () => {
+  const err = await compile('error-syntax', moduleRules.basic())
     .catch(e => e);
 
   expect(err).toBeInstanceOf(Error);
-  expect(err.message).toMatch(/'~some\/module\.less' wasn't found/);
+  compareErrorMessage(err.message);
 });