Fix code to match expectations of other tests

- ensure only valid linters can be set (we can't default to pylint anymore) - ensure await is called for isLintingEnabled.
microsoft · Oct 24, 2018 · fac640a · fac640a
1 parent 4f545bb
commit fac640a
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 13 deletions.
diff --git a/package.json b/package.json
@@ -1245,7 +1245,7 @@
                 },
                 "python.linting.pylintEnabled": {
                     "type": "boolean",
-                    "default": false,
+                    "default": true,
                     "description": "Whether to lint Python files using pylint.",
                     "scope": "resource"
                 },

diff --git a/src/client/linters/linterManager.ts b/src/client/linters/linterManager.ts
@@ -81,20 +81,36 @@ export class LinterManager implements ILinterManager {
         if (!silent) {
             await this.enableUnconfiguredLinters(resource);
         }
-        return this.linters.filter(x => x.isEnabled(resource));
+        return this.linters.filter(x => {
+            let enabled = x.isEnabled(resource);
+            if (x.id === 'pylint') {
+                enabled = enabled === true;
+            }
+            return enabled === true;
+        });
     }
 
     public async setActiveLintersAsync(products: Product[], resource?: Uri): Promise<void> {
-        const active = await this.getActiveLinters(true, resource);
-        for (const x of active) {
-            await x.enableAsync(false, resource);
-        }
-        if (products.length > 0) {
-            const toActivate = this.linters.filter(x => products.findIndex(p => x.product === p) >= 0);
-            for (const x of toActivate) {
-                await x.enableAsync(true, resource);
+        // ensure we only allow valid linters to be set, otherwise leave things alone.
+        // filter out any invalid products:
+        const validProducts = products.filter(product => {
+            const foundIndex = this.linters.findIndex(validLinter => validLinter.product === product);
+            return foundIndex !== -1;
+        });
+
+        // if we have valid linter product(s), enable only those
+        if (validProducts.length > 0) {
+            const active = await this.getActiveLinters(true, resource);
+            for (const x of active) {
+                await x.enableAsync(false, resource);
+            }
+            if (products.length > 0) {
+                const toActivate = this.linters.filter(x => products.findIndex(p => x.product === p) >= 0);
+                for (const x of toActivate) {
+                    await x.enableAsync(true, resource);
+                }
+                await this.enableLintingAsync(true, resource);
             }
-            await this.enableLintingAsync(true, resource);
         }
     }
 

diff --git a/src/test/linters/lint.manager.test.ts b/src/test/linters/lint.manager.test.ts
@@ -81,9 +81,9 @@ suite('Linting - Manager', () => {
 
     test('Enable/disable linting', async () => {
         await lm.enableLintingAsync(false);
-        assert.equal(lm.isLintingEnabled(true), false, 'Linting not disabled');
+        assert.equal(await lm.isLintingEnabled(true), false, 'Linting not disabled');
         await lm.enableLintingAsync(true);
-        assert.equal(lm.isLintingEnabled(true), true, 'Linting not enabled');
+        assert.equal(await lm.isLintingEnabled(true), true, 'Linting not enabled');
     });
 
     test('Set single linter', async () => {