Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Merge pull request #6530 from busykai/fix-5137-async-linting
Browse files Browse the repository at this point in the history 
Support linting providers that return results asynchronously
  • Loading branch information
@peterflynn
peterflynn committed Apr 8, 2014
2 parents cf50d9c + c3411bb commit 9111c08
Show file tree
Hide file tree
Showing 3 changed files with 416 additions and 33 deletions.
125 changes: 94 additions & 31 deletions src/language/CodeInspection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ define(function (require, exports, module) {
* Constants for the preferences defined in this file.
*/
var PREF_ENABLED = "enabled",
PREF_COLLAPSED = "collapsed";
PREF_COLLAPSED = "collapsed",
PREF_ASYNC_TIMEOUT = "asyncTimeout";

var prefs = PreferencesManager.getExtensionPrefs("linting");

Expand Down Expand Up @@ -120,7 +121,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 = {};

Expand All @@ -129,6 +130,15 @@ define(function (require, exports, module) {
* @type {boolean}
*/
var _hasErrors;

/**
* Promise of the returned by the last call to inspectFile or null if linting is disabled. Used to prevent any stale promises
* to cause updates of the UI.
*
* @private
* @type {$.Promise}
*/
var _currentPromise = null;

/**
* Enable or disable the "Go to First Error" command
Expand All @@ -148,7 +158,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()];
Expand All @@ -166,7 +176,7 @@ define(function (require, exports, module) {
* 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) {
Expand All @@ -179,29 +189,72 @@ 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.done(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) {
window.setTimeout(function () {
// timeout error
var errTimeout = {
pos: { line: -1, col: 0},
message: StringUtils.format(Strings.LINTER_TIMED_OUT, provider.name, prefs.get(PREF_ASYNC_TIMEOUT)),
type: Type.ERROR
};
runPromise.resolve({errors: [errTimeout]});
}, prefs.get(PREF_ASYNC_TIMEOUT));
provider.scanFileAsync(fileText, file.fullPath)
.done(function (scanResult) {
PerfUtils.addMeasurement(perfTimerProvider);
runPromise.resolve(scanResult);
})
.fail(function (err) {
var errError = {
pos: {line: -1, col: 0},
message: StringUtils.format(Strings.LINTER_FAILED, provider.name, err),
type: Type.ERROR
};
console.error("[CodeInspection] Provider " + provider.name + " (async) failed: " + err);
runPromise.resolve({errors: [errError]});
});
} else {
try {
var scanResult = provider.scanFile(fileText, file.fullPath);
PerfUtils.addMeasurement(perfTimerProvider);
runPromise.resolve(scanResult);
} catch (err) {
var errError = {
pos: {line: -1, col: 0},
message: StringUtils.format(Strings.LINTER_FAILED, provider.name, err),
type: Type.ERROR
};
console.error("[CodeInspection] Provider " + provider.name + " (sync) threw an error: " + err);
runPromise.resolve({errors: [errError]});
}
}
return runPromise.promise();

PerfUtils.addMeasurement(perfTimerProvider);
}, false);

masterPromise.then(function () {
// sync async may have pushed results in different order, restore the original order
results.sort(function (a, b) {
return providerList.indexOf(a.provider) - providerList.indexOf(b.provider);
});
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);
Expand All @@ -216,7 +269,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) {
Expand Down Expand Up @@ -261,6 +314,7 @@ define(function (require, exports, module) {
function run() {
if (!_enabled) {
_hasErrors = false;
_currentPromise = null;
Resizer.hide($problemsPanel);
StatusBar.updateIndicator(INDICATOR_ID, true, "inspection-disabled", Strings.LINT_DISABLED);
setGotoEnabled(false);
Expand All @@ -278,12 +332,12 @@ define(function (require, exports, module) {
var providersReportingProblems = [];

// run all the providers registered for this file type
inspectFile(currentDoc.file, providerList).then(function (results) {
// check if current document wasn't changed while inspectFile was running
if (currentDoc !== DocumentManager.getCurrentDocument()) {
(_currentPromise = inspectFile(currentDoc.file, providerList)).then(function (results) {
// check if promise has not changed while inspectFile was running
if (this !== _currentPromise) {
return;
}

// how many errors in total?
var errors = results.reduce(function (a, item) { return a + (item.result ? item.result.errors.length : 0); }, 0);

Expand Down Expand Up @@ -361,6 +415,7 @@ define(function (require, exports, module) {
} else {
// No provider for current file
_hasErrors = false;
_currentPromise = null;
Resizer.hide($problemsPanel);
var language = currentDoc && LanguageManager.getLanguageForPath(currentDoc.file.fullPath);
if (language) {
Expand All @@ -380,9 +435,16 @@ define(function (require, exports, module) {
* Registering any provider for the "javascript" language automatically unregisters the built-in
* Brackets JSLint provider. This is a temporary convenience until UI exists for disabling
* registered providers.
*
* If provider implements both scanFileAsync and scanFile functions for asynchronous and synchronous
* code inspection, respectively, the asynchronous version will take precedence and will be used to
* perform code inspection.
*
* A code inspection provider's scanFileAsync must return a {$.Promise} object which should be
* resolved with ?{errors:!Array, aborted:boolean}}.
*
* @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.
Expand All @@ -391,15 +453,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
Expand Down Expand Up @@ -508,7 +570,7 @@ define(function (require, exports, module) {
toggleCollapsed(prefs.get(PREF_COLLAPSED), true);
});


prefs.definePreference(PREF_ASYNC_TIMEOUT, "number", 10000);

// Initialize items dependent on HTML DOM
AppInit.htmlReady(function () {
Expand Down Expand Up @@ -571,12 +633,13 @@ define(function (require, exports, module) {
});

// Testing
exports._unregisterAll = _unregisterAll;
exports._unregisterAll = _unregisterAll;
exports._PREF_ASYNC_TIMEOUT = PREF_ASYNC_TIMEOUT;

// Public API
exports.register = register;
exports.Type = Type;
exports.toggleEnabled = toggleEnabled;
exports.inspectFile = inspectFile;
exports.requestRun = run;
exports.requestRun = run;
});
3 changes: 3 additions & 0 deletions src/nls/root/strings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ define({
"FILE_FILTER_INSTRUCTIONS" : "Exclude files and folders matching any of the following strings / substrings or <a href='{0}' title='{0}'>wildcards</a>. Enter each string on a new line.",
"FILE_FILTER_LIST_PREFIX" : "except",
"FILE_FILTER_CLIPPED_SUFFIX" : "and {0} more",

"FILTER_COUNTING_FILES" : "Counting files\u2026",
"FILTER_FILE_COUNT" : "Allows {0} of {1} files {2}",
"FILTER_FILE_COUNT_ALL" : "Allows all {0} files {1}",
Expand Down Expand Up @@ -239,6 +240,8 @@ define({
"LINT_DISABLED" : "Linting is disabled",
"NO_LINT_AVAILABLE" : "No linter available for {0}",
"NOTHING_TO_LINT" : "Nothing to lint",
"LINTER_TIMED_OUT" : "{0} has timed out after waiting for {1} ms",
"LINTER_FAILED" : "{0} terminated with error: {1}",


/**
Expand Down
Loading

0 comments on commit 9111c08

Please sign in to comment.