Merge pull request #7362 from adobe/kai/fix-6661-multi-linters

ingorichter · ingorichter · commit b12f8eda9cdd · 2014-12-11T14:14:21.000-08:00
Fix #6661. Configurable multiple linters.
diff --git a/.brackets.json b/.brackets.json
@@ -8,6 +8,12 @@
         "es5": true
     },
     "defaultExtension": "js",
+    "language": {
+        "javascript": {
+            "linting.prefer": ["JSLint", "JSHint"],
+            "linting.usePreferredOnly": true
+        }
+    },
     "path": {
         "src/thirdparty/CodeMirror2/**/*.js": {
             "spaceUnits": 2,
@@ -20,4 +26,4 @@
     },
     "spaceUnits": 4,
     "useTabChar": false
-}
+}
diff --git a/src/language/CodeInspection.js b/src/language/CodeInspection.js
@@ -75,9 +75,11 @@ define(function (require, exports, module) {
     /**
      * Constants for the preferences defined in this file.
      */
-    var PREF_ENABLED = "enabled",
-        PREF_COLLAPSED = "collapsed",
-        PREF_ASYNC_TIMEOUT = "asyncTimeout";
+    var PREF_ENABLED            = "enabled",
+        PREF_COLLAPSED          = "collapsed",
+        PREF_ASYNC_TIMEOUT      = "asyncTimeout",
+        PREF_PREFER_PROVIDERS   = "prefer",
+        PREF_PREFERRED_ONLY     = "usePreferredOnly";
     
     var prefs = PreferencesManager.getExtensionPrefs("linting");
     
@@ -158,11 +160,42 @@ define(function (require, exports, module) {
      * Decision is made depending on the file extension.
      *
      * @param {!string} filePath
-     * @return ?{Array.<{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:?function(string, string):?{errors:!Array, aborted:boolean}}>} provider
+     * @return {Array.<{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:?function(string, string):?{errors:!Array, aborted:boolean}}>}
      */
     function getProvidersForPath(filePath) {
-        var providers = _providers[LanguageManager.getLanguageForPath(filePath).getId()];
-        return (providers && providers.slice(0)) || [];
+        var language            = LanguageManager.getLanguageForPath(filePath).getId(),
+            context             = PreferencesManager._buildContext(filePath, language),
+            installedProviders  = _providers[language],
+            preferredProviders,
+
+            prefPreferredProviderNames  = prefs.get(PREF_PREFER_PROVIDERS, context),
+            prefPreferredOnly           = prefs.get(PREF_PREFERRED_ONLY, context),
+        
+            providers;
+        
+        // ensure there is an instance and that a copy is returned, always
+        installedProviders = (installedProviders && installedProviders.slice(0)) || [];
+        
+        if (prefPreferredProviderNames && prefPreferredProviderNames.length) {
+            if (typeof prefPreferredProviderNames === "string") {
+                prefPreferredProviderNames = [prefPreferredProviderNames];
+            }
+            preferredProviders = prefPreferredProviderNames.reduce(function (result, key) {
+                var provider = _.find(installedProviders, {name: key});
+                if (provider) {
+                    result.push(provider);
+                }
+                return result;
+            }, []);
+            if (prefPreferredOnly) {
+                providers = preferredProviders;
+            } else {
+                providers = _.union(preferredProviders, installedProviders);
+            }
+        } else {
+            providers = installedProviders;
+        }
+        return providers;
     }
 
     /**
@@ -184,7 +217,7 @@ define(function (require, exports, module) {
         var response = new $.Deferred(),
             results = [];
 
-        providerList = (providerList || getProvidersForPath(file.fullPath)) || [];
+        providerList = providerList || getProvidersForPath(file.fullPath);
 
         if (!providerList.length) {
             response.resolve(null);
@@ -458,14 +491,6 @@ define(function (require, exports, module) {
             _providers[languageId] = [];
         }
         
-        if (languageId === "javascript") {
-            // This is a special case to enable extension provider to replace the JSLint provider
-            // in favor of their own implementation
-            _.remove(_providers[languageId], function (registeredProvider) {
-                return registeredProvider.name === "JSLint";
-            });
-        }
-        
         _providers[languageId].push(provider);
         
         run();  // in case a file of this type is open currently
@@ -580,7 +605,7 @@ define(function (require, exports, module) {
         });
     
     prefs.definePreference(PREF_ASYNC_TIMEOUT, "number", 10000);
-    
+        
     // Initialize items dependent on HTML DOM
     AppInit.htmlReady(function () {
         // Create bottom panel to list error details
@@ -644,6 +669,8 @@ define(function (require, exports, module) {
     // Testing
     exports._unregisterAll          = _unregisterAll;
     exports._PREF_ASYNC_TIMEOUT     = PREF_ASYNC_TIMEOUT;
+    exports._PREF_PREFER_PROVIDERS  = PREF_PREFER_PROVIDERS;
+    exports._PREF_PREFERRED_ONLY    = PREF_PREFERRED_ONLY;
 
     // Public API
     exports.register            = register;
diff --git a/test/spec/CodeInspection-test.js b/test/spec/CodeInspection-test.js
@@ -30,7 +30,8 @@ define(function (require, exports, module) {
     var SpecRunnerUtils  = require("spec/SpecRunnerUtils"),
         FileSystem       = require("filesystem/FileSystem"),
         StringUtils      = require("utils/StringUtils"),
-        Strings          = require("strings");
+        Strings          = require("strings"),
+        _                = require("thirdparty/lodash");
 
     describe("Code Inspection", function () {
         this.category = "integration";
@@ -42,8 +43,9 @@ define(function (require, exports, module) {
             CodeInspection,
             CommandManager,
             Commands  = require("command/Commands"),
-            DocumentManager,
             EditorManager,
+            DocumentManager,
+            PreferencesManager,
             prefs;
 
         var toggleJSLintResults = function (visible) {
@@ -115,6 +117,7 @@ define(function (require, exports, module) {
                     EditorManager = brackets.test.EditorManager;
                     prefs = brackets.test.PreferencesManager.getExtensionPrefs("linting");
                     CodeInspection = brackets.test.CodeInspection;
+                    PreferencesManager = brackets.test.PreferencesManager;
                     CodeInspection.toggleEnabled(true);
                 });
             });
@@ -300,6 +303,60 @@ define(function (require, exports, module) {
                 });
             });
             
+            it("should use preferences for providers lookup", function () {
+                var pm = PreferencesManager.getExtensionPrefs("linting"),
+                    codeInspector1 = createCodeInspector("html1", failLintResult),
+                    codeInspector2 = createCodeInspector("html2", successfulLintResult),
+                    codeInspector3 = createCodeInspector("html3", successfulLintResult),
+                    codeInspector4 = createCodeInspector("html4", successfulLintResult),
+                    codeInspector5 = createCodeInspector("html5", failLintResult);
+                
+                CodeInspection.register("html", codeInspector1);
+                CodeInspection.register("html", codeInspector2);
+                CodeInspection.register("html", codeInspector3);
+                CodeInspection.register("html", codeInspector4);
+                CodeInspection.register("html", codeInspector5);
+                
+                function setAtLocation(name, value) {
+                    pm.set(name, value, {location: {layer: "language", layerID: "html", scope: "user"}});
+                }
+                
+                runs(function () {
+                    var providers;
+                    
+                    setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html3", "html4"]);
+                    providers = CodeInspection.getProvidersForPath("my/index.html");
+                    expect(providers).toNotBe(null);
+                    expect(_.pluck(providers, "name")).toEqual(["html3", "html4", "html1", "html2", "html5"]);
+
+                    setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html5", "html6"]);
+                    providers = CodeInspection.getProvidersForPath("index.html");
+                    expect(providers).toNotBe(null);
+                    expect(_.pluck(providers, "name")).toEqual(["html5", "html1", "html2", "html3", "html4"]);
+
+                    setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html19", "html100"]);
+                    providers = CodeInspection.getProvidersForPath("index.html");
+                    expect(providers).toNotBe(null);
+                    expect(_.pluck(providers, "name")).toEqual(["html1", "html2", "html3", "html4", "html5"]);
+                    
+                    setAtLocation(CodeInspection._PREF_PREFERRED_ONLY, true);
+                    providers = CodeInspection.getProvidersForPath("test.html");
+                    expect(providers).toEqual([]);
+
+                    setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html2", "html1"]);
+                    setAtLocation(CodeInspection._PREF_PREFERRED_ONLY, true);
+                    providers = CodeInspection.getProvidersForPath("c:/temp/another.html");
+                    expect(providers).toNotBe(null);
+                    expect(_.pluck(providers, "name")).toEqual(["html2", "html1"]);
+                    
+                    setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, undefined);
+                    setAtLocation(CodeInspection._PREF_PREFERRED_ONLY, undefined);
+                    providers = CodeInspection.getProvidersForPath("index.html");
+                    expect(providers).toNotBe(null);
+                    expect(_.pluck(providers, "name")).toEqual(["html1", "html2", "html3", "html4", "html5"]);
+                });
+            });
+
             it("should run asynchoronous implementation when both available in the provider", function () {
                 var provider = createAsyncCodeInspector("javascript async linter with sync impl", failLintResult(), 200, true);
                 CodeInspection.register("javascript", provider);
@@ -1071,20 +1128,6 @@ define(function (require, exports, module) {
                 CodeInspection._unregisterAll();
             });
 
-            it("should unregister JSLint linter if a new javascript linter is registered", function () {
-                var codeInspector1 = createCodeInspector("JSLint", successfulLintResult());
-                CodeInspection.register("javascript", codeInspector1);
-                var codeInspector2 = createCodeInspector("javascript inspector", successfulLintResult());
-                CodeInspection.register("javascript", codeInspector2, true);
-
-                waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js"]), "open test file", 5000);
-
-                runs(function () {
-                    expect(codeInspector1.scanFile).not.toHaveBeenCalled();
-                    expect(codeInspector2.scanFile).toHaveBeenCalled();
-                });
-            });
-
             it("should call inspector 1 and inspector 2", function () {
                 var codeInspector1 = createCodeInspector("javascript inspector 1", successfulLintResult());
                 CodeInspection.register("javascript", codeInspector1);
