Fix #5137 async linting

src/language/CodeInspection.js

@@ -23,7 +23,7 @@
 
 
 /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4 */
-/*global define, $, Mustache, brackets */
+/*global define, $, Mustache, brackets, setTimeout */
 
 /**
  * Manages linters and other code inspections on a per-language basis. Provides a UI and status indicator for
@@ -120,7 +120,7 @@ define(function (require, exports, module) {
 
     /**
      * @private
-     * @type {Object.<languageId:string, Array.<{name:string, scanFile:function(string, string):Object}>>}
+     * @type {{languageId:string, Array.<{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:function(string, string):Object}>}}
      */
     var _providers = {};
 
@@ -148,7 +148,7 @@ define(function (require, exports, module) {
      * Decision is made depending on the file extension.
      *
      * @param {!string} filePath
-     * @return ?{Array.<{name:string, 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}}>} provider
      */
     function getProvidersForPath(filePath) {
         return _providers[LanguageManager.getLanguageForPath(filePath).getId()];
@@ -160,13 +160,20 @@ define(function (require, exports, module) {
      * This method doesn't update the Brackets UI, just provides inspection results.
      * These results will reflect any unsaved changes present in the file if currently open.
      * 
+     * If a provider implements scanFileAsync, then it will be used to scan the file. Otherwise, scanFile,
+     * a synchronous version of the function will be used. Provider must never reject a promise and resolve it
+     * with null in case the results cannot be retrieved for whatever reason.
+     * 
+     * A code inspection provider's scanFileAsync must return a {$.Promise} object which must be resolved with
+     * ?{errors:!Array, aborted:boolean}}.
+     * 
      * The Promise yields an array of provider-result pair objects (the result is the return value of the
      * provider's scanFile() - see register() for details). The result object may be null if there were no
      * errors from that provider.
      * If there are no providers registered for this file, the Promise yields null instead.
      *
      * @param {!File} file File that will be inspected for errors.
-     * @param {?Array.<{name:string, scanFile:function(string, string):?{errors:!Array, aborted:boolean}}>} providerList
+     * @param {?Array.<{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:function(string, string):?{errors:!Array, aborted:boolean}}>} providerList
      * @return {$.Promise} a jQuery promise that will be resolved with ?Array.<{provider:Object, result: ?{errors:!Array, aborted:boolean}}>
      */
     function inspectFile(file, providerList) {
@@ -179,29 +186,50 @@ define(function (require, exports, module) {
             response.resolve(null);
             return response.promise();
         }
-
+        
         DocumentManager.getDocumentText(file)
             .done(function (fileText) {
-                var perfTimerInspector = PerfUtils.markStart("CodeInspection:\t" + file.fullPath);
-
-                providerList.forEach(function (provider) {
-                    var perfTimerProvider = PerfUtils.markStart("CodeInspection '" + provider.name + "':\t" + file.fullPath);
-
-                    try {
-                        var scanResult = provider.scanFile(fileText, file.fullPath);
+                var perfTimerInspector = PerfUtils.markStart("CodeInspection:\t" + file.fullPath),
+                    masterPromise;
+
+                masterPromise = Async.doInParallel(providerList, function (provider) {
+                    var perfTimerProvider = PerfUtils.markStart("CodeInspection '" + provider.name + "':\t" + file.fullPath),
+                        runPromise = new $.Deferred();
+
+                    runPromise.then(function (scanResult) {
                         results.push({provider: provider, result: scanResult});
-                    } catch (err) {
-                        console.error("[CodeInspection] Provider " + provider.name + " threw an error: " + err);
-                        response.reject(err);
-                        return;
+                    });
+
+                    if (provider.scanFileAsync) {
+                        setTimeout(function () { runPromise.resolve(null); }, 500);
+                        provider.scanFileAsync(fileText, file.fullPath)
+                            .then(function (scanResult) {
+                                PerfUtils.addMeasurement(perfTimerProvider);
+                                runPromise.resolve(scanResult);
+                            })
+                            .fail(function (err) {
+                                console.error("[CodeInspection] Provider " + provider.name + " (async) failed: " + err);
+                                runPromise.resolve(null);
+                            });
+                    } else {
+                        try {
+                            var scanResult = provider.scanFile(fileText, file.fullPath);
+                            PerfUtils.addMeasurement(perfTimerProvider);
+                            runPromise.resolve(scanResult);
+                        } catch (err) {
+                            console.error("[CodeInspection] Provider " + provider.name + " (sync) threw an error: " + err);
+                            runPromise.resolve(null);
+                        }
                     }
+                    return runPromise.promise();
 
-                    PerfUtils.addMeasurement(perfTimerProvider);
+                }, false);
+
+                masterPromise.then(function () {
+                    PerfUtils.addMeasurement(perfTimerInspector);
+                    response.resolve(results);
                 });
 
-                PerfUtils.addMeasurement(perfTimerInspector);
-
-                response.resolve(results);
             })
             .fail(function (err) {
                 console.error("[CodeInspection] Could not read file for inspection: " + file.fullPath);
@@ -216,7 +244,7 @@ define(function (require, exports, module) {
      * change based on the number of problems reported and how many provider reported problems.
      * 
      * @param {Number} numProblems - total number of problems across all providers
-     * @param {Array.<{name:string, scanFile:function(string, string):Object}>} providersReportingProblems - providers that reported problems
+     * @param {Array.<{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:function(string, string):Object}>} providersReportingProblems - providers that reported problems
      * @param {boolean} aborted - true if any provider returned a result with the 'aborted' flag set
      */
     function updatePanelTitleAndStatusBar(numProblems, providersReportingProblems, aborted) {
@@ -382,7 +410,7 @@ define(function (require, exports, module) {
      * registered providers.
      *
      * @param {string} languageId
-     * @param {{name:string, scanFile:function(string, string):?{errors:!Array, aborted:boolean}} provider
+     * @param {{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:function(string, string):?{errors:!Array, aborted:boolean}}} provider
      *
      * Each error is: { pos:{line,ch}, endPos:?{line,ch}, message:string, type:?Type }
      * If type is unspecified, Type.WARNING is assumed.
@@ -391,15 +419,15 @@ define(function (require, exports, module) {
         if (!_providers[languageId]) {
             _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

test/spec/CodeInspection-test.js

@@ -61,6 +61,29 @@ define(function (require, exports, module) {
 
             return provider;
         }
+
+        function createAsyncCodeInspector(name, result, scanTime, syncImpl) {
+            var provider = {
+                name: name,
+                scanFileAsync: function () {
+                    var deferred = new $.Deferred();
+                    setTimeout(function () {
+                        deferred.resolve(result);
+                    }, scanTime);
+                    return deferred.promise();
+                }
+            };
+            spyOn(provider, "scanFileAsync").andCallThrough();
+
+            if (syncImpl === true) {
+                provider.scanFile = function () {
+                    return result;
+                };
+                spyOn(provider, "scanFile").andCallThrough();
+            }
+
+            return provider;
+        }
 
         function successfulLintResult() {
             return {errors: []};
@@ -250,6 +273,131 @@ define(function (require, exports, module) {
                     expect(expectedResult).toBeNull();
                 });
             });
+
+            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);
+
+                runs(function () {
+                    var promise = CodeInspection.inspectFile(simpleJavascriptFileEntry);
+
+                    waitsForDone(promise, "file linting", 5000);
+                });
+
+                runs(function () {
+                    expect(provider.scanFileAsync).toHaveBeenCalled();
+                    expect(provider.scanFile).not.toHaveBeenCalled();
+                });
+
+            });
+
+            it("should timeout on a provider that takes too long", function () {
+                var provider = createAsyncCodeInspector("javascript async linter with sync impl", failLintResult(), 1000, true),
+                    result;
+                CodeInspection.register("javascript", provider);
+
+                runs(function () {
+                    var promise = CodeInspection.inspectFile(simpleJavascriptFileEntry);
+                    promise.done(function (r) {
+                        result = r;
+                    });
+
+                    waitsForDone(promise, "file linting", 550);
+                });
+
+                runs(function () {
+                    expect(provider.scanFileAsync).toHaveBeenCalled();
+                    expect(result).toBeDefined();
+                    expect(result[0].provider).toEqual(provider);
+                    expect(result[0].errors).toBeFalsy();
+                });
+
+            });
+
+            it("should run two asynchronous providers and a synchronous one", function () {
+                var asyncProvider1 = createAsyncCodeInspector("javascript async linter 1", failLintResult(), 200, true),
+                    asyncProvider2 = createAsyncCodeInspector("javascript async linter 2", successfulLintResult(), 300, false),
+                    syncProvider3 = createCodeInspector("javascript sync linter 3", failLintResult()),
+                    result;
+                CodeInspection.register("javascript", asyncProvider1);
+                CodeInspection.register("javascript", asyncProvider2);
+                CodeInspection.register("javascript", syncProvider3);
+
+                runs(function () {
+                    var promise = CodeInspection.inspectFile(simpleJavascriptFileEntry);
+                    promise.done(function (r) {
+                        result = r;
+                    });
+
+                    waitsForDone(promise, "file linting", 5000);
+                });
+
+                runs(function () {
+                    var i;
+                    expect(result.length).toEqual(3);
+
+                    for (i = 0; i < result.length; i++) {
+                        switch (result[i].provider.name) {
+                        case asyncProvider1.name:
+                            expect(asyncProvider1.scanFile).not.toHaveBeenCalled();
+                            expect(asyncProvider2.scanFileAsync).toHaveBeenCalled();
+                            break;
+                        case asyncProvider2.name:
+                            expect(asyncProvider2.scanFileAsync).toHaveBeenCalled();
+                            break;
+                        case syncProvider3.name:
+                            expect(syncProvider3.scanFile).toHaveBeenCalled();
+                            break;
+                        default:
+                            expect(true).toBe(false);
+                            break;
+                        }
+                    }
+                });
+
+            });
+
+            it("should return results for 3 providers when 2 completes and 1 times out", function () {
+                var asyncProvider1 = createAsyncCodeInspector("javascript async linter 1", failLintResult(), 200, true),
+                    asyncProvider2 = createAsyncCodeInspector("javascript async linter 2", failLintResult(), 1000, false),
+                    syncProvider3 = createCodeInspector("javascript sync linter 3", failLintResult()),
+                    result;
+                CodeInspection.register("javascript", asyncProvider1);
+                CodeInspection.register("javascript", asyncProvider2);
+                CodeInspection.register("javascript", syncProvider3);
+
+                runs(function () {
+                    var promise = CodeInspection.inspectFile(simpleJavascriptFileEntry);
+                    promise.done(function (r) {
+                        result = r;
+                    });
+
+                    waitsForDone(promise, "file linting", 5000);
+                });
+
+                runs(function () {
+                    var i;
+                    expect(result.length).toEqual(3);
+
+                    for (i = 0; i < result.length; i++) {
+                        switch (result[i].provider.name) {
+                        case asyncProvider1.name:
+                        case syncProvider3.name:
+                            expect(result[i].result).toBeDefined();
+                            expect(result[i].result).not.toBeNull();
+                            break;
+                        case asyncProvider2.name:
+                            expect(result[i].result).toBeDefined();
+                            expect(result[i].result).toBeNull();
+                            break;
+                        default:
+                            expect(true).toBe(false);
+                            break;
+                        }
+                    }
+                });
+            });
+
         });
 
         describe("Code Inspection UI", function () {