Repository: sasstools/sass-lint
Pull request #459 (2015-12-21 11:20:06)

Return parse errors as lint errors
In an effort to reduce the amount of unhandled errors thrown by `sass-lint` I've converted the parse errors and undefined tree errors that can be thrown while the AST is being constructed into lint errors.

Firstly this prevents AST bugs from crashing peoples linting builds altogether and only prevents the file with the parse error from being linted rather than the whole project.

Secondly as the line is reported directly as a parse error it should allow people to provide a more useful bug report in the event of something un expected happening rather than them having to be confronted with a meaningless stack trace at the moment. 

I've gone with the Eslint report style in the sense that the ruleId reported for these errors is `Fatal`.

There's not a specific issue that this relates to for `sass-lint` other than my desire to remove all thrown errors as best as possible but it does relate to many issues encountered in `gulp-sass-lint` and `grunt-sass-lint` etc

[#18](https://github.com/sasstools/gulp-sass-lint/issues/18) for example.

Errors thrown by failing rules will still be thrown as errors for now as they do break the lint cycle.

Not the cleanest implementation right now but I figure this will be a big consideration once we move to v2.0.

Also added a few tests to the CLI to ensure these errors are reported correctly.

`DCO 1.1 Signed-off-by: Dan Purdy danjpurdy@gmail.com`


Patch:
diff --git a/index.js b/index.js
index 9888eb8..f9a194f 100644
--- a/index.js
+++ b/index.js
@@ -33,13 +33,29 @@ sassLint.resultCount = function (results) {
 
 sassLint.lintText = function (file, options, configPath) {
   var rules = slRules(this.getConfig(options, configPath)),
-      ast = groot(file.text, file.format, file.filename),
+      ast = {},
       detects,
       results = [],
       errors = 0,
       warnings = 0;
 
-  if (ast.content.length > 0) {
+  try {
+    ast = groot(file.text, file.format, file.filename);
+  }
+  catch (e) {
+    var line = e.line || 1;
+    errors++;
+
+    results = [{
+      ruleId: 'Fatal',
+      line: line,
+      column: 1,
+      message: e.message,
+      severity: 2
+    }];
+  }
+
+  if (ast.content && ast.content.length > 0) {
     rules.forEach(function (rule) {
       detects = rule.rule.detect(ast, rule);
       results = results.concat(detects);
@@ -54,7 +70,6 @@ sassLint.lintText = function (file, options, configPath) {
     });
   }
 
-
   results.sort(helpers.sortDetects);
 
   return {
diff --git a/lib/groot.js b/lib/groot.js
index 4a3e7de..f3bad09 100644
--- a/lib/groot.js
+++ b/lib/groot.js
@@ -6,8 +6,7 @@
 var gonzales = require('gonzales-pe');
 
 module.exports = function (text, syntax, filename) {
-  var fileInfo = filename ? ' at ' + filename : '',
-      tree;
+  var tree;
 
   // Run `.toString()` to allow Buffers to be passed in
   text = text.toString();
@@ -18,11 +17,20 @@ module.exports = function (text, syntax, filename) {
     });
   }
   catch (e) {
-    throw new Error('Parsing error' + fileInfo + ': ' + e.message);
+    throw {
+      message: e.message,
+      file: filename,
+      line: e.line
+    };
   }
 
   if (typeof tree === 'undefined') {
-    throw new Error('Undefined tree' + fileInfo + ': ' + text.toString() + ' => ' + tree.toString());
+    throw {
+      message: 'Undefined tree',
+      file: filename,
+      text: text.toString(),
+      tree: tree.toString()
+    };
   }
 
   return tree;


Test patch:
diff --git a/tests/cli.js b/tests/cli.js
index 5775ce1..68bb893 100644
--- a/tests/cli.js
+++ b/tests/cli.js
@@ -307,4 +307,70 @@ describe('cli', function () {
     });
 
   });
+
+  it('parse errors should report as a lint error', function (done) {
+    var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
+
+    childProcess.exec(command, function (err, stdout) {
+      var result = JSON.parse(stdout)[0];
+
+      if (err !== null) {
+        return done(new Error('Parse error failure'));
+      }
+
+      assert.equal(1, result.errorCount);
+      done();
+    });
+  });
+
+  it('parse errors should report as severity 2', function (done) {
+    var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
+
+    childProcess.exec(command, function (err, stdout) {
+      var result = JSON.parse(stdout)[0],
+          messages = result.messages[0],
+          severity = 2;
+
+      if (err !== null) {
+        return done(new Error('Parse error failure'));
+      }
+
+      assert.equal(severity, messages.severity);
+      done();
+    });
+  });
+
+  it('parse errors should report the correct message', function (done) {
+    var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
+
+    childProcess.exec(command, function (err, stdout) {
+      var result = JSON.parse(stdout)[0],
+          message = result.messages[0].message,
+          expected = 'Please check validity of the block starting from line #5';
+
+      if (err !== null) {
+        return done(new Error('Parse error failure'));
+      }
+
+      assert.equal(expected, message);
+      done();
+    });
+  });
+
+  it('parse errors rule Id should be \'Fatal\'', function (done) {
+    var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json';
+
+    childProcess.exec(command, function (err, stdout) {
+      var result = JSON.parse(stdout)[0],
+          messages = result.messages[0],
+          ruleId = 'Fatal';
+
+      if (err !== null) {
+        return done(new Error('Parse error failure'));
+      }
+
+      assert.equal(ruleId, messages.ruleId);
+      done();
+    });
+  });
 });
diff --git a/tests/main.js b/tests/main.js
index 137494b..02a7af7 100644
--- a/tests/main.js
+++ b/tests/main.js
@@ -24,4 +24,50 @@ describe('sass lint', function () {
       done();
     });
   });
+
+  //////////////////////////////
+  // Parse Errors should return as lint errors
+  //////////////////////////////
+  it('Parse Errors should return as lint errors', function (done) {
+    lintFile('parse.scss', function (data) {
+      assert.equal(1, data.errorCount);
+      done();
+    });
+  });
+
+  it('Parse Errors should not include warnings too', function (done) {
+    lintFile('parse.scss', function (data) {
+      assert.equal(0, data.warningCount);
+      done();
+    });
+  });
+
+  it('Parse Errors should return as severity 2', function (done) {
+    lintFile('parse.scss', function (data) {
+      var severity = data.messages[0].severity;
+
+      assert.equal(2, severity);
+      done();
+    });
+  });
+
+  it('Parse Errors should return the correct error message', function (done) {
+    lintFile('parse.scss', function (data) {
+      var message = data.messages[0].message,
+          expected = 'Please check validity of the block starting from line #5';
+
+      assert.equal(expected, message);
+      done();
+    });
+  });
+
+  it('Parse Errors should return the rule ID \'Fatal\'', function (done) {
+    lintFile('parse.scss', function (data) {
+      var ruleId = data.messages[0].ruleId,
+          expected = 'Fatal';
+
+      assert.equal(expected, ruleId);
+      done();
+    });
+  });
 });
diff --git a/tests/sass/parse.scss b/tests/sass/parse.scss
new file mode 100644
index 0000000..837044d
--- /dev/null
+++ b/tests/sass/parse.scss
@@ -0,0 +1,7 @@
+.one {
+  color: red;
+}
+
+#test
+  color: blue;
+}