Repository: sasstools/sass-lint (js)

Issue:
Cache contents of config file
Placing a console.log inside config.js shows that we're loading the config from scratch once per rule. We should be loading the config once per invocation of sass-lint.

I'm referring to using sass-lint from the CLI, and specifying a config file. e.g. `sass-lint -c .sass-lint.yml`

Pull request:
Add cacheable config
By default the config is now cached on each invocation of sass-lint with an option to override this behaviour. 

You can override this behaviour if you want, an example would be someone tweaking their `sass-lint` config within a IDE plugin.

Updated the tests to work with this new implementation.

Some of the crude benchmarks I ran for loading the config when cached showed an average of a 26% speed increase loading each rule as opposed to always loading from scratch though i'm sure with tweaks we could improve this and definitely an opportunity with 2.0

Looking for feedback here about if everyone is happy for this to move forward. It follows our usual pattern of preference i.e. `options > user config > defaults`

closes #279 

`DCO 1.1 Signed-off-by: Dan Purdy <danjpurdy@gmail.com>`

Patch:
diff --git a/docs/options/cache-config.md b/docs/options/cache-config.md
new file mode 100644
index 0000000..bf56153
--- /dev/null
+++ b/docs/options/cache-config.md
@@ -0,0 +1,7 @@
+# Cache Config
+
+Option `cache-config` will tell Sass Lint to cache the current configuration for each invocation of sass-lint. This option is disabled by default but you may consider enabling it if you're not planning on modifying your `.sass-lint.yml` config file as there will be a small performance gain.
+
+## Options
+
+* `true`/`false` (defaults to `false`)
diff --git a/lib/config.js b/lib/config.js
index d8c9e44..c424b81 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -5,7 +5,12 @@ var yaml = require('js-yaml'),
     path = require('path'),
     merge = require('merge');
 
-var defaults = yaml.safeLoad(fs.readFileSync(path.join(__dirname, 'config', 'sass-lint.yml'), 'utf8'));
+var cacheConfig = {},
+    cacheEnabled = false;
+
+var loadDefaults = function loadDefaults () {
+  return yaml.safeLoad(fs.readFileSync(path.join(__dirname, 'config', 'sass-lint.yml'), 'utf8'));
+};
 
 var findFile = function findFile (configPath, filename) {
   var HOME = process.env.HOME || process.env.HOMEPATH || process.env.USERPROFILE,
@@ -37,13 +42,28 @@ module.exports = function (options, configPath) {
       configMergeExists = false,
       optionsMerge = false,
       optionsMergeExists = false,
+      finalCacheExists = false,
       config = {},
-      finalConfig = {};
+      finalConfig = {},
+      defaults;
 
   // ensure our inline options and rules are not undefined
   options = options ? options : {};
   options.rules = options.rules ? options.rules : {};
 
+  // ensure our user defined cache option is respected
+  if (options.options && options.options.hasOwnProperty('cache-config')) {
+    if (options.options['cache-config'] && Object.keys(cacheConfig).length) {
+      return cacheConfig;
+    }
+  }
+  else {
+    // check to see if the config cache already exists and is enabled
+    if (cacheEnabled && Object.keys(cacheConfig).length) {
+      return cacheConfig;
+    }
+  }
+
   if (options.options && options.options['config-file']) {
     configPath = options.options['config-file'];
   }
@@ -87,7 +107,7 @@ module.exports = function (options, configPath) {
   // order of preference is inline options > user config > default config
   // merge-default-rules defaults to true so each step above should merge with the previous. If at any step merge-default-rules is set to
   // false it should skip that steps merge.
-
+  defaults = loadDefaults();
   finalConfig = merge.recursive(defaults, config, options);
 
   // if merge-default-rules is set to false in user config file then we essentially skip the merging with default rules by overwriting our
@@ -106,5 +126,14 @@ module.exports = function (options, configPath) {
     finalConfig.rules = merge.recursive(finalConfig.rules, options.rules);
   }
 
+  // check to see if our final config contains a cache-config value
+  finalCacheExists = (finalConfig.options && typeof finalConfig.options['cache-config'] !== 'undefined');
+
+  // set our global cache enabled flag here, it will be false by default
+  cacheEnabled = finalCacheExists ? finalConfig.options['cache-config'] : false;
+
+  // set our cached config to our final config
+  cacheConfig = cacheEnabled ? finalConfig : {};
+
   return finalConfig;
 };


Test patch:
diff --git a/tests/config.js b/tests/config.js
index 70e3b9d..0ac61fa 100644
--- a/tests/config.js
+++ b/tests/config.js
@@ -1,102 +1,192 @@
 'use strict';
 
 var equal = require('deep-equal'),
+    assert = require('assert'),
     yaml = require('js-yaml'),
     fs = require('fs'),
     path = require('path'),
     merge = require('merge'),
     config = require('../lib/config');
 
-var defaults = yaml.safeLoad(fs.readFileSync(path.join(__dirname, '..', 'lib', 'config', 'sass-lint.yml'), 'utf8'));
-
-var options = {
-  'options': {
-    'formatter': 'stylish'
-  },
-  'files': {
-    'ignore': [
-      'foo',
-      'bar'
-    ]
-  },
-  'rules': {
-    'no-duplicate-property': 0,
-    'indentation': [
-      2,
-        {
-          'size': 4
-        }
-    ]
-  }
+var custOptions = function () {
+  return {
+    'options': {
+      'formatter': 'stylish',
+      'cache-config': false
+    },
+    'files': {
+      'ignore': [
+        'foo',
+        'bar'
+      ]
+    },
+    'rules': {
+      'no-duplicate-properties': 0,
+      'indentation': [
+        2,
+          {
+            'size': 4
+          }
+      ]
+    }
+  };
 };
 
 describe('config', function () {
   it('should return the defaults if no config is passed in', function (done) {
-    var conf = config();
-
-    equal(
-      conf,
-      defaults,
-      {
-        'strict': true
-      }
+    var defaultConfig = yaml.safeLoad(fs.readFileSync(path.join(__dirname, '..', 'lib', 'config', 'sass-lint.yml'), 'utf8')),
+        conf = config();
+
+    assert(
+      equal(
+        conf,
+        defaultConfig,
+        {
+          'strict': true
+        }
+      )
     );
 
     done();
   });
 
   it('should merge options when passed in', function (done) {
-    var conf = config(options),
-        merged = merge.recursive(defaults, options);
-
-    equal(
-      conf,
-      merged,
-      {
-        'strict': true
-      }
+
+    var defaultConfig = yaml.safeLoad(fs.readFileSync(path.join(__dirname, '..', 'lib', 'config', 'sass-lint.yml'), 'utf8')),
+        tempOptions = custOptions(),
+        conf = config(tempOptions),
+        merged = merge.recursive(defaultConfig, tempOptions);
+
+    assert(
+      equal(
+        conf,
+        merged,
+        {
+          'strict': true
+        }
+      )
     );
 
     done();
   });
 
   it('should use a config file if one is provided', function (done) {
-    var conf = config({
+    var defaultConfig = yaml.safeLoad(fs.readFileSync(path.join(__dirname, '..', 'lib', 'config', 'sass-lint.yml'), 'utf8')),
+        tempOptions = {
           'options': {
-            'config-file': 'tests/yml/.stylish-output.yml'
+            'config-file': 'tests/yml/.stylish-output.yml',
+            'cache-config': false
           }
-        }),
-        merged = merge.recursive(defaults, fs.readFileSync(path.join(__dirname, 'yml', '.stylish-output.yml')));
-
-    equal(
-      conf,
-      merged,
-      {
-        'strict': true
-      }
+        },
+        conf = config(tempOptions),
+        merged = merge.recursive(defaultConfig, tempOptions, yaml.safeLoad(fs.readFileSync(path.join(__dirname, 'yml', '.stylish-output.yml'), 'utf8')));
+
+    assert(
+      equal(
+        conf,
+        merged,
+        {
+          'strict': true
+        }
+      )
     );
 
     done();
   });
 
-  it('should not merge rules when `merge-default-rules` is false', function (done) {
-    var opts = options,
+  it('should not merge default rules when `merge-default-rules` is false in config file [check unique]', function (done) {
+    var defaultConfig = yaml.safeLoad(fs.readFileSync(path.join(__dirname, '..', 'lib', 'config', 'sass-lint.yml'), 'utf8')),
+        tempOptions = custOptions(),
         conf,
         merged;
 
-    opts.options['merge-default-rules'] = false;
+    conf = config(tempOptions, path.join(__dirname, 'yml', '.no-merge-default.yml'));
+    merged = merge.recursive(defaultConfig, tempOptions);
+
+    assert(
+      !equal(
+        conf,
+        merged,
+        {
+          'strict': true
+        }
+      )
+    );
+
+    done();
+  });
+
+  it('should not merge default rules when `merge-default-rules` is false in config file [check equal]', function (done) {
+    var customConfig = yaml.safeLoad(fs.readFileSync(path.join(__dirname, 'yml', '.no-merge-default.yml'), 'utf8')),
+        tempOptions = custOptions(),
+        conf,
+        merged;
 
-    merged = merge.recursive(defaults, opts);
-    merged.rules = opts.rules;
 
-    conf = config(opts);
+    conf = config(tempOptions, path.join(__dirname, 'yml', '.no-merge-default.yml'));
+    merged = merge.recursive(customConfig, tempOptions);
 
-    equal(
-      conf,
-      merged,
-      {
-        'strict': true
-      }
+    assert(
+      equal(
+        conf,
+        merged,
+        {
+          'strict': true
+        }
+      )
+    );
+
+    done();
+  });
+
+  it('should not merge custom option rules when `merge-default-rules` is false', function (done) {
+    var defaultConfig = yaml.safeLoad(fs.readFileSync(path.join(__dirname, '..', 'lib', 'config', 'sass-lint.yml'), 'utf8')),
+        tempOptions = custOptions(),
+        conf,
+        merged;
+
+    tempOptions.options['merge-default-rules'] = false;
+    conf = config(tempOptions);
+    merged = merge.recursive(tempOptions, defaultConfig);
+    merged.rules = tempOptions.rules;
+
+    assert(
+      equal(
+        conf,
+        merged,
+        {
+          'strict': true
+        }
+      )
+    );
+
+    done();
+  });
+
+  it('should load a cached version of the config', function (done) {
+    var tempOptions = {
+          'options': {}
+        },
+        confFirst,
+        confSecond;
+
+    tempOptions.options['cache-config'] = true;
+
+    // first pass at config to set the cache
+    confFirst = config(tempOptions);
+
+    // second pass to check if cache loads adding in a config file that shouldn't be loaded
+    tempOptions.options['config-file'] = 'tests/yml/.stylish-output.yml';
+    confSecond = config(tempOptions);
+
+    assert(
+      equal(
+        confFirst,
+        confSecond,
+        {
+          'strict': true
+        }
+      )
     );
 
     done();
diff --git a/tests/output.js b/tests/output.js
index ef76a70..0268970 100644
--- a/tests/output.js
+++ b/tests/output.js
@@ -46,7 +46,8 @@ describe('output', function () {
   it('should have configurable formatters', function (done) {
     var formatted = lint.format(results, {
       'options': {
-        'formatter': 'json'
+        'formatter': 'json',
+        'cache-config': false
       }
     });
 
@@ -59,7 +60,8 @@ describe('output', function () {
     var options = {
       'options': {
         'formatter': 'json',
-        'output-file': 'tests/output.json'
+        'output-file': 'tests/output.json',
+        'cache-config': false
       }
     };
 
diff --git a/tests/rules/_lint.js b/tests/rules/_lint.js
index a9cb160..1d34d94 100644
--- a/tests/rules/_lint.js
+++ b/tests/rules/_lint.js
@@ -23,7 +23,8 @@ module.exports.test = function (text, options, cb) {
 
   options = {
     'options': {
-      'merge-default-rules': false
+      'merge-default-rules': false,
+      'cache-config': false
     },
     'rules': options
   };
diff --git a/tests/yml/.color-keyword-errors.yml b/tests/yml/.color-keyword-errors.yml
index 0943855..fc4fb96 100644
--- a/tests/yml/.color-keyword-errors.yml
+++ b/tests/yml/.color-keyword-errors.yml
@@ -1,5 +1,6 @@
 options:
   formatter: json
+  cache-config: false
 files:
   include: '**/*.s+(a|c)ss'
 rules:
diff --git a/tests/yml/.json-lint.yml b/tests/yml/.json-lint.yml
index 90c3410..b5ccbfb 100644
--- a/tests/yml/.json-lint.yml
+++ b/tests/yml/.json-lint.yml
@@ -1,5 +1,6 @@
 options:
   formatter: json
+  cache-config: false
 files:
   include: '**/*.s+(a|c)ss'
 rules:
diff --git a/tests/yml/.no-merge-default.yml b/tests/yml/.no-merge-default.yml
new file mode 100644
index 0000000..a8a4706
--- /dev/null
+++ b/tests/yml/.no-merge-default.yml
@@ -0,0 +1,59 @@
+options:
+  formatter: stylish
+  cache-config: false
+  merge-default-rules: false
+files:
+  include: '**/*.s+(a|c)ss'
+rules:
+  # Extends
+  extends-before-mixins: 0
+  extends-before-declarations: 0
+  placeholder-in-extend: 0
+
+  # Mixins
+  mixins-before-declarations: 0
+
+  # Line Spacing
+  one-declaration-per-line: 0
+  empty-line-between-blocks: 0
+  single-line-per-selector: 0
+
+  # Disallows
+  no-debug: 0
+  no-duplicate-properties: 0
+  no-empty-rulesets: 0
+  no-extends: 0
+  no-ids: 0
+  no-important: 0
+  no-warn: 0
+  no-color-keywords: 0
+  no-invalid-hex: 0
+  no-css-comments: 0
+  no-color-literals: 1
+
+  # Style Guide
+  border-zero: 0
+  clean-import-paths: 0
+  empty-args: 0
+  hex-length: 0
+  hex-notation: 0
+  indentation: 0
+  leading-zero: 0
+  nesting-depth: 0
+  property-sort-order: 0
+  quotes: 0
+  variable-for-property: 0
+  zero-unit: 0
+
+  # Inner Spacing
+  space-after-comma: 0
+  space-before-colon: 0
+  space-after-colon: 0
+  space-before-brace: 0
+  space-before-bang: 0
+  space-after-bang: 0
+  space-between-parens: 0
+
+  # Final Items
+  trailing-semicolon: 0
+  final-newline: 0
diff --git a/tests/yml/.stylish-errors.yml b/tests/yml/.stylish-errors.yml
index 6fa3e58..d36e45b 100644
--- a/tests/yml/.stylish-errors.yml
+++ b/tests/yml/.stylish-errors.yml
@@ -1,5 +1,6 @@
 options:
   formatter: stylish
+  cache-config: false
 files:
   include: '**/*.s+(a|c)ss'
 rules:
diff --git a/tests/yml/.stylish-output.yml b/tests/yml/.stylish-output.yml
index 9f38c10..48329e1 100644
--- a/tests/yml/.stylish-output.yml
+++ b/tests/yml/.stylish-output.yml
@@ -1,5 +1,6 @@
 options:
   formatter: stylish
+  cache-config: false
 files:
   include: '**/*.s+(a|c)ss'
 rules: