diff --git a/src/language/CodeInspection.js b/src/language/CodeInspection.js index 5d6460d21ce..ffc035ba0ea 100644 --- a/src/language/CodeInspection.js +++ b/src/language/CodeInspection.js @@ -383,7 +383,7 @@ define(function (require, exports, module) { aborted = true; } - if (inspectionResult.result.errors) { + if (inspectionResult.result.errors.length) { allErrors.push({ providerName: provider.name, results: inspectionResult.result.errors @@ -436,18 +436,18 @@ define(function (require, exports, module) { * 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}}. - * + * Providers implement scanFile() if results are available synchronously, or scanFileAsync() if results + * may require an async wait (if both are implemented, scanFile() is ignored). scanFileAsync() returns + * a {$.Promise} object resolved with the same type of value as scanFile() is expected to return. + * Rejecting the promise is treated as an internal error in the provider. + * * @param {string} languageId - * @param {{name:string, scanFileAsync:?function(string, string):!{$.Promise}, 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. + * If no errors found, return either null or an object with a zero-length `errors` array. */ function register(languageId, provider) { if (!_providers[languageId]) { diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index f1b9bdab555..bd37e199dd4 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -179,7 +179,6 @@ define({ "FILE_FILTER_INSTRUCTIONS" : "Exclude files and folders matching any of the following strings / substrings or wildcards. 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}", diff --git a/test/spec/CodeInspection-test.js b/test/spec/CodeInspection-test.js index 05e9cf8bd18..be8652c48fd 100644 --- a/test/spec/CodeInspection-test.js +++ b/test/spec/CodeInspection-test.js @@ -42,6 +42,7 @@ define(function (require, exports, module) { CodeInspection, CommandManager, Commands = require("command/Commands"), + DocumentManager, EditorManager, prefs; @@ -111,6 +112,7 @@ define(function (require, exports, module) { brackets = testWindow.brackets; Strings = testWindow.require("strings"); CommandManager = brackets.test.CommandManager; + DocumentManager = brackets.test.DocumentManager; EditorManager = brackets.test.EditorManager; prefs = brackets.test.PreferencesManager.getExtensionPrefs("linting"); CodeInspection = brackets.test.CodeInspection; @@ -137,6 +139,7 @@ define(function (require, exports, module) { $ = null; brackets = null; CommandManager = null; + DocumentManager = null; EditorManager = null; SpecRunnerUtils.closeTestWindow(); }); @@ -413,8 +416,29 @@ define(function (require, exports, module) { describe("Code Inspection UI", function () { beforeEach(function () { CodeInspection._unregisterAll(); + CodeInspection.toggleEnabled(true); }); + // Utility to create an async provider where the testcase can control when each async result resolves + function makeAsyncLinter() { + return { + name: "Test Async Linter", + scanFileAsync: function (text, fullPath) { + if (!this.futures[fullPath]) { + this.futures[fullPath] = []; + this.filesCalledOn.push(fullPath); + } + + var result = new $.Deferred(); + this.futures[fullPath].push(result); + return result.promise(); + }, + futures: {}, // map from full path to array of Deferreds (in call order) + filesCalledOn: [] // in order of first call for each path + }; + } + + it("should run test linter when a JavaScript document opens and indicate errors in the panel", function () { var codeInspector = createCodeInspector("javascript linter", failLintResult()); CodeInspection.register("javascript", codeInspector); @@ -427,45 +451,124 @@ define(function (require, exports, module) { expect($statusBar.is(":visible")).toBe(true); }); }); - - it("should show only warnings for the current file", function () { + + it("should ignore async results from previous file", function () { CodeInspection.toggleEnabled(false); - var firstTime = true, - deferred1 = new $.Deferred(), - deferred2 = new $.Deferred(); - - var asyncProvider = { - name: "Test Async Linter", - scanFileAsync: function () { - if (firstTime) { - firstTime = false; - return deferred1.promise(); - } else { - return deferred2.promise(); - } - } - }; - + var asyncProvider = makeAsyncLinter(); CodeInspection.register("javascript", asyncProvider); - waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js", "errors.js"], "open test files")); + waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js", "errors.js"]), "open test files"); + + var errorsJS = SpecRunnerUtils.makeAbsolute("errors.js"), + noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js"); runs(function () { + // Start linting the first file CodeInspection.toggleEnabled(true); - CommandManager.execute(Commands.FILE_CLOSE); + expect(asyncProvider.filesCalledOn).toEqual([errorsJS]); + + // Close that file, switching to the 2nd one + waitsForDone(CommandManager.execute(Commands.FILE_CLOSE)); }); - // Close the file which was started to lint runs(function () { - // let the linter finish - deferred1.resolve(failLintResult()); + // Verify that we started linting the 2nd file + expect(DocumentManager.getCurrentDocument().file.fullPath).toBe(noErrorsJS); + expect(asyncProvider.filesCalledOn).toEqual([errorsJS, noErrorsJS]); + + // Finish old (stale) linting session - verify results not shown + asyncProvider.futures[errorsJS][0].resolve(failLintResult()); expect($("#problems-panel").is(":visible")).toBe(false); - deferred2.resolve(successfulLintResult()); + + // Finish new (current) linting session + asyncProvider.futures[noErrorsJS][0].resolve(successfulLintResult()); expect($("#problems-panel").is(":visible")).toBe(false); }); }); + + it("should ignore async results from previous run in same file - finishing in order", function () { + CodeInspection.toggleEnabled(false); + + var asyncProvider = makeAsyncLinter(); + CodeInspection.register("javascript", asyncProvider); + + waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js"]), "open test files"); + + var noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js"); + + runs(function () { + // Start linting the file + CodeInspection.toggleEnabled(true); + expect(asyncProvider.filesCalledOn).toEqual([noErrorsJS]); + + // "Modify" the file + $(DocumentManager).triggerHandler("documentSaved", DocumentManager.getCurrentDocument()); + expect(asyncProvider.futures[noErrorsJS].length).toBe(2); + + // Finish old (stale) linting session - verify results not shown + asyncProvider.futures[noErrorsJS][0].resolve(failLintResult()); + expect($("#problems-panel").is(":visible")).toBe(false); + + // Finish new (current) linting session - verify results are shown + asyncProvider.futures[noErrorsJS][1].resolve(failLintResult()); + expect($("#problems-panel").is(":visible")).toBe(true); + }); + }); + + it("should ignore async results from previous run in same file - finishing reverse order", function () { + CodeInspection.toggleEnabled(false); + + var asyncProvider = makeAsyncLinter(); + CodeInspection.register("javascript", asyncProvider); + + waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js"]), "open test files"); + + var noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js"); + + runs(function () { + // Start linting the file + CodeInspection.toggleEnabled(true); + expect(asyncProvider.filesCalledOn).toEqual([noErrorsJS]); + + // "Modify" the file + $(DocumentManager).triggerHandler("documentSaved", DocumentManager.getCurrentDocument()); + expect(asyncProvider.futures[noErrorsJS].length).toBe(2); + + // Finish new (current) linting session - verify results are shown + asyncProvider.futures[noErrorsJS][1].resolve(failLintResult()); + expect($("#problems-panel").is(":visible")).toBe(true); + + // Finish old (stale) linting session - verify results don't replace current results + asyncProvider.futures[noErrorsJS][0].resolve(successfulLintResult()); + expect($("#problems-panel").is(":visible")).toBe(true); + }); + }); + + it("should ignore async results after linting disabled", function () { + CodeInspection.toggleEnabled(false); + + var asyncProvider = makeAsyncLinter(); + CodeInspection.register("javascript", asyncProvider); + waitsForDone(SpecRunnerUtils.openProjectFiles(["no-errors.js"]), "open test files"); + + var noErrorsJS = SpecRunnerUtils.makeAbsolute("no-errors.js"); + + runs(function () { + // Start linting the file + CodeInspection.toggleEnabled(true); + expect(asyncProvider.filesCalledOn).toEqual([noErrorsJS]); + + // Disable linting + CodeInspection.toggleEnabled(false); + + // Finish old (stale) linting session - verify results not shown + asyncProvider.futures[noErrorsJS][0].resolve(failLintResult()); + expect($("#problems-panel").is(":visible")).toBe(false); + }); + }); + it("should show problems panel after too many errors", function () { var lintResult = { errors: [ @@ -517,8 +620,8 @@ define(function (require, exports, module) { }); }); - it("should not show the problems panel when there is no linting error", function () { - var codeInspector = createCodeInspector("javascript linter", successfulLintResult()); + it("should not show the problems panel when there is no linting error - empty errors array", function () { + var codeInspector = createCodeInspector("javascript linter", {errors: []}); CodeInspection.register("javascript", codeInspector); waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000); @@ -530,19 +633,20 @@ define(function (require, exports, module) { }); }); - it("status icon should toggle Errors panel when errors present", function () { - var codeInspector = createCodeInspector("javascript linter", failLintResult()); + it("should not show the problems panel when there is no linting error - null result", function () { + var codeInspector = createCodeInspector("javascript linter", null); CodeInspection.register("javascript", codeInspector); - waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file"); + waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000); runs(function () { - toggleJSLintResults(false); - toggleJSLintResults(true); + expect($("#problems-panel").is(":visible")).toBe(false); + var $statusBar = $("#status-inspection"); + expect($statusBar.is(":visible")).toBe(true); }); }); - it("should run two linters and display two expanded collapsible sections in the errors panel", function () { + it("should display two expanded, collapsible sections in the errors panel when two linters have errors", function () { var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult()); var codeInspector2 = createCodeInspector("javascript linter 2", failLintResult()); CodeInspection.register("javascript", codeInspector1); @@ -561,17 +665,56 @@ define(function (require, exports, module) { }); }); - it("should run the linter and display no collapsible header section in the errors panel", function () { - var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult()); + it("should display no header section when only one linter has errors", function () { + var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult()), + codeInspector2 = createCodeInspector("javascript linter 2", {errors: []}), // 1st way of reporting 0 errors + codeInspector3 = createCodeInspector("javascript linter 3", null); // 2nd way of reporting 0 errors CodeInspection.register("javascript", codeInspector1); + CodeInspection.register("javascript", codeInspector2); + CodeInspection.register("javascript", codeInspector3); waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000); runs(function () { + expect($("#problems-panel").is(":visible")).toBe(true); expect($(".inspector-section").is(":visible")).toBeFalsy(); }); }); + it("should only display header sections for linters with errors", function () { + var codeInspector1 = createCodeInspector("javascript linter 1", failLintResult()), + codeInspector2 = createCodeInspector("javascript linter 2", {errors: []}), // 1st way of reporting 0 errors + codeInspector3 = createCodeInspector("javascript linter 3", null), // 2nd way of reporting 0 errors + codeInspector4 = createCodeInspector("javascript linter 4", failLintResult()); + CodeInspection.register("javascript", codeInspector1); + CodeInspection.register("javascript", codeInspector2); + CodeInspection.register("javascript", codeInspector3); + CodeInspection.register("javascript", codeInspector4); + + waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file", 5000); + + runs(function () { + expect($("#problems-panel").is(":visible")).toBe(true); + + var $inspectorSections = $(".inspector-section td"); + expect($inspectorSections.length).toEqual(2); + expect($inspectorSections[0].innerHTML.indexOf("javascript linter 1 (1)")).not.toBe(-1); + expect($inspectorSections[1].innerHTML.indexOf("javascript linter 4 (1)")).not.toBe(-1); + }); + }); + + it("status icon should toggle Errors panel when errors present", function () { + var codeInspector = createCodeInspector("javascript linter", failLintResult()); + CodeInspection.register("javascript", codeInspector); + + waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file"); + + runs(function () { + toggleJSLintResults(false); + toggleJSLintResults(true); + }); + }); + it("status icon should not toggle Errors panel when no errors present", function () { var codeInspector = createCodeInspector("javascript linter", successfulLintResult()); CodeInspection.register("javascript", codeInspector); @@ -810,7 +953,7 @@ define(function (require, exports, module) { }); }); - it("should report an async linter which throws an exception", function () { + it("should report an async linter which rejects", function () { var errorMessage = "I'm full of bugs on purpose", providerName = "Buggy Async Linter", buggyAsyncProvider = {