Repository: rwjblue/ember-template-lint
Pull request #175 (2017-02-09 20:05:17)

Fix config with plugins
This PR adds two commits:
- tests that fail with certain plugin configuration, or after repeated loading of the config
- fixes for the newly added tests

Some more background:

The first problem is more or less cosmetic - with a plugin configuration, there may be some non-default rules added (implemented in the plugin source). Those rules are evaluated as invalid as they are not part of the default set of rules. In this PR, rules are validated against loaded rules (which consist of the default rules and additional rules declared in plugins) rather than the default rules, which fixes the problem. Without the fix, a meaningless warning is reported.

The second problem is more serious - if the configuration is loaded more than once within a single runtime, it can lose important settings, making the whole configuration unusable. For example, in the first pass, ```loadedRules``` and ```loadedConfigurations``` keys are added as root configuration keys. Those keys contain important information, especially when there are some plugins declared. During the second pass, however, those keys are moved under ```rules``` as they are considered as invalid in the configuration root, and then they are replaced with newly constructed keys. At the same time, the ```plugins``` key is restructured during the first pass (from an array to a hash), and totally destroyed in the second pass (the hash is processed like an array, which means no plugins are found). As a result of combining the two problems described above, the newly constructed keys lose essential information (```plugins``` is empty and ```loadedRules``` does not contain rules coming from the plugins). The final consequence of this is that none of the declared plugins are effective.

Note: Even in a situation when the configuration is loaded with the ```require``` function from a file, multiple attempts to load such a configuration results in a broken configuration due to caching of the required files (the cached structures are changed by the configuration loader and then served again).

The second problem is fixed by adding a boolean flag to the configuration (```._processed```), saying that the configuration has already been processed and should simply be returned on all subsequent attempts to get the configuration.


Patch:
diff --git a/lib/get-config.js b/lib/get-config.js
index d72d1d1..8be50b3 100644
--- a/lib/get-config.js
+++ b/lib/get-config.js
@@ -203,7 +203,7 @@ function validateRules(config, options) {
   var invalidKeysFound = [];
 
   for (var key in config.rules) {
-    if (!defaultRules[key]) {
+    if (!config.loadedRules[key]) {
       invalidKeysFound.push(key);
     }
   }
@@ -216,15 +216,19 @@ function validateRules(config, options) {
 module.exports = function(options) {
   var config = options.config || resolveConfigPath(options.configPath);
 
-  ensureRootProperties(config);
-  migrateRulesFromRoot(config, options);
+  if (!config._processed) {
+    ensureRootProperties(config);
+    migrateRulesFromRoot(config, options);
 
-  processPlugins(config, options);
-  processLoadedRules(config);
-  processLoadedConfigurations(config);
-  processExtends(config, options);
+    processPlugins(config, options);
+    processLoadedRules(config);
+    processLoadedConfigurations(config);
+    processExtends(config, options);
 
-  validateRules(config, options);
+    validateRules(config, options);
+
+    config._processed = true;
+  }
 
   return config;
 };


Test patch:
diff --git a/test/unit/get-config-test.js b/test/unit/get-config-test.js
index baaf5ac..a099702 100644
--- a/test/unit/get-config-test.js
+++ b/test/unit/get-config-test.js
@@ -175,5 +175,70 @@ describe('get-config', function() {
 
   });
 
+  it('validates non-default loaded rules', function() {
+    var message;
+    var actual = getConfig({
+      console: { log: function(_message) {
+        message = _message;
+      }},
+
+      config: {
+
+        rules: {
+          'foo-bar': true
+        },
+
+        plugins: [{
+          name: 'myplugin',
+          rules: {
+            'foo-bar': 'plugin-function-placeholder'
+          }
+        }]
+      }
+
+    });
+
+    assert(!message);
+    assert(actual.loadedRules['foo-bar'] === 'plugin-function-placeholder');
+  });
+
+  it('getting config is idempotent', function() {
+    var firstMessage;
+    var secondMessage;
+    var firstPass = getConfig({
+      console: { log: function(_message) {
+        firstMessage = _message;
+      }},
+
+      config: {
+
+        rules: {
+          'foo-bar': true
+        },
+
+        plugins: [{
+          name: 'myplugin',
+          rules: {
+            'foo-bar': 'plugin-function-placeholder'
+          }
+        }]
+      }
+
+    });
+    var firstPassJSON = JSON.stringify(firstPass);
+    var secondPass = getConfig({
+      console: { log: function(_message) {
+        secondMessage = _message;
+      }},
+
+      config: firstPass
+
+    });
+    var secondPassJSON = JSON.stringify(secondPass);
+
+    assert(firstPassJSON === secondPassJSON);
+    assert(!firstMessage);
+    assert(!secondMessage);
+  });
 
 });