Repository: sasstools/sass-lint (js)

Issue:
UTF-8 w/ BOM + @mixin causes fatal parsing error
Demo repo [here](https://github.com/mattdistefano/sass-lint-bom-issue-demo)

Short version is that when you have a file encoded as UTF-8 w/ BOM that starts with `@mixin`, the CLI linter fails with a `1:1  error  Please check validity of the block starting from line #1  Fatal`.

UTF-8 w/out BOM works fine, as does UTF-8 w/ BOM without `@mixin` at the start. 

Happens on OS X w/ node 6.2.0 and Windows 10 w/ node 5.8.0.

The gulp-sass-lint plugin actually works fine in all scenarios.

Pull request description:
Fix BOM markers causing parse issues
As discussed in #880 BOM markers in files were causing issues with the way gonzales is parsing the files we send it. This change strips any BOM markers from the files text before it is parsed.

Fixes #880 

`<DCO 1.1 Signed-off-by: Dan Purdy dan@danpurdy.co.uk>`



Patch:
diff --git a/lib/groot.js b/lib/groot.js
index 986ce8b..1d2ee52 100644
--- a/lib/groot.js
+++ b/lib/groot.js
@@ -3,14 +3,15 @@
 //////////////////////////////
 'use strict';
 
-var gonzales = require('gonzales-pe'),
-    fm = require('front-matter');
+var gonzales = require('gonzales-pe');
+var fm = require('front-matter');
+var helpers = require('./helpers');
 
 module.exports = function (text, syntax, filename) {
   var tree;
 
   // Run `.toString()` to allow Buffers to be passed in
-  text = text.toString();
+  text = helpers.stripBom(text.toString());
 
   // if we're skipping front matter do it here, fall back to just our text in case it fails
   if (fm.test(text)) {
diff --git a/lib/helpers.js b/lib/helpers.js
index d2cb330..0b9af3c 100644
--- a/lib/helpers.js
+++ b/lib/helpers.js
@@ -394,4 +394,24 @@ helpers.isPartialStringMatch = function (needle, haystack) {
   return false;
 };
 
+/**
+ *  A copy of the the stripBom module from https://github.com/sindresorhus/strip-bom/blob/master/index.js
+ *  The module requires node > 4 whereas we support earlier versions.
+ *  This function strips the BOM marker from the beginning of a file
+ *
+ * @param {string} str - The string we wish to strip the BOM marker from
+ * @returns {string} The string without a BOM marker
+ */
+helpers.stripBom = function (str) {
+  if (typeof str !== 'string') {
+    throw new TypeError('Expected a string, got ' + typeof str);
+  }
+
+  if (str.charCodeAt(0) === 0xFEFF) {
+    return str.slice(1);
+  }
+
+  return str;
+};
+
 module.exports = helpers;


Test patch:
diff --git a/tests/bom-utf8/starts-with-mixin-utf8-bom.scss b/tests/bom-utf8/starts-with-mixin-utf8-bom.scss
new file mode 100644
index 0000000..4f12e18
--- /dev/null
+++ b/tests/bom-utf8/starts-with-mixin-utf8-bom.scss
@@ -0,0 +1,5 @@
+﻿@mixin foo(
+  $param: 'def'
+) {
+  color: red;
+}
diff --git a/tests/bom-utf8/var-utf8-bom.scss b/tests/bom-utf8/var-utf8-bom.scss
new file mode 100644
index 0000000..bc5580d
--- /dev/null
+++ b/tests/bom-utf8/var-utf8-bom.scss
@@ -0,0 +1,5 @@
+﻿$my-color: red;
+
+body {
+  color: $my-color;
+}
\ No newline at end of file
diff --git a/tests/cli.js b/tests/cli.js
index 8df2996..df7c7fe 100644
--- a/tests/cli.js
+++ b/tests/cli.js
@@ -472,3 +472,43 @@ describe('cli', function () {
     });
   });
 });
+
+// ==============================================================================
+//  UTF-8 BOM
+// ==============================================================================
+
+describe('Reading files with UTF-8 BOM', function () {
+  before(function () {
+    var testText = fs.readFileSync('tests/bom-utf8/starts-with-mixin-utf8-bom.scss').toString();
+    if (testText.charCodeAt(0) !== 0xFEFF) {
+      throw new Error('BOM test files have no BOM markers');
+    }
+  });
+
+  it('should not throw a parse error from file containing a BOM', function (done) {
+    var command = 'node bin/sass-lint -v tests/bom-utf8/starts-with-mixin-utf8-bom.scss --format json';
+
+    exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err
+      var result = JSON.parse(stdout)[0];
+
+      // Files with BOM markers that start with a mixin throw a fatal error
+      // https://github.com/sasstools/sass-lint/issues/880
+      assert.equal(result.errorCount, 0);
+      done();
+    });
+  });
+
+  it('should return the correct amount of warnings from a file containing BOM markers', function (done) {
+    var command = 'node bin/sass-lint -v tests/bom-utf8/var-utf8-bom.scss --format json';
+
+    exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err
+      var result = JSON.parse(stdout)[0];
+
+      // Files starting with variables threw extra errors
+      // see https://github.com/sasstools/sass-lint/issues/880
+      assert.equal(result.errorCount, 0);
+      assert.equal(result.warningCount, 2);
+      done();
+    });
+  });
+});
diff --git a/tests/helpers/stripBom.js b/tests/helpers/stripBom.js
new file mode 100644
index 0000000..523e026
--- /dev/null
+++ b/tests/helpers/stripBom.js
@@ -0,0 +1,28 @@
+'use strict';
+
+var assert = require('assert'),
+    helpers = require('../../lib/helpers'),
+    fs = require('fs');
+
+describe('helpers - stripBom', function () {
+
+  //////////////////////////////
+  // Strip BOM
+  //////////////////////////////
+
+  it('should remove the BOM marker', function (done) {
+    var file = fs.readFileSync('tests/bom-utf8/starts-with-mixin-utf8-bom.scss').toString();
+    assert.equal(file.charCodeAt(0), 0xFEFF);
+    assert.notEqual(helpers.stripBom(file).charCodeAt(0), 0xFEFF);
+    done();
+  });
+
+  it('should throw an error if not passed a string', function (done) {
+    assert.throws(
+      function () {
+        helpers.stripBom(8);
+      }, Error
+    );
+    done();
+  });
+});