fix(client/linters/mypy): call mypy incorrectly (#5834)

* fix(client/linters/mypy): call mypy incorrectly
It's supposed to call `mypy` in the root package directory with the relative path to the python file being checked. This commit enforces this behavior.
* fix(client/linters/mypy): syntax error
* Fixes and tests
* Add news item.
* House keeping on tests

Author: yxliang01

news/2 Fixes/5326.md

@@ -0,0 +1,2 @@
+Use relative paths when invoking mypy.
+(thanks to [yxliang01](https://github.com/yxliang01))

src/client/linters/mypy.ts

@@ -1,3 +1,4 @@
+import * as path from 'path';
 import { CancellationToken, OutputChannel, TextDocument } from 'vscode';
 import '../common/extensions';
 import { Product } from '../common/types';
@@ -13,7 +14,9 @@ export class MyPy extends BaseLinter {
     }
 
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
-        const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX);
+        const cwd = this.getWorkspaceRootPath(document);
+        const relativePath = path.relative(cwd, document.uri.fsPath);
+        const messages = await this.run([relativePath], document, cancellation, REGEX);
         messages.forEach(msg => {
             msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity);
             msg.code = msg.type;

src/test/linters/mypy.unit.test.ts

@@ -6,12 +6,20 @@
 // tslint:disable:no-object-literal-type-assertion
 
 import { expect } from 'chai';
+import * as path from 'path';
+import * as sinon from 'sinon';
+import { anything, instance, mock, when } from 'ts-mockito';
+import { CancellationToken, CancellationTokenSource, TextDocument, Uri } from 'vscode';
+import { Product } from '../../client/common/types';
+import { ServiceContainer } from '../../client/ioc/container';
 import { parseLine } from '../../client/linters/baseLinter';
-import { REGEX } from '../../client/linters/mypy';
-import { ILintMessage } from '../../client/linters/types';
+import { LinterManager } from '../../client/linters/linterManager';
+import { MyPy, REGEX } from '../../client/linters/mypy';
+import { ILinterManager, ILintMessage, LintMessageSeverity } from '../../client/linters/types';
+import { MockOutputChannel } from '../mockClasses';
 
 // This following is a real-world example. See gh=2380.
-// tslint:disable-next-line:no-multiline-string
+// tslint:disable:no-multiline-string no-any max-func-body-length
 const output = `
 provider.pyi:10: error: Incompatible types in assignment (expression has type "str", variable has type "int")
 provider.pyi:11: error: Name 'not_declared_var' is not defined
@@ -29,23 +37,23 @@ suite('Linting - MyPy', () => {
                 line: 10,
                 type: 'error',
                 provider: 'mypy'
-             } as ILintMessage],
+            } as ILintMessage],
             [lines[2], {
                 code: undefined,
                 message: 'Name \'not_declared_var\' is not defined',
                 column: 0,
                 line: 11,
                 type: 'error',
                 provider: 'mypy'
-             } as ILintMessage],
+            } as ILintMessage],
             [lines[3], {
                 code: undefined,
                 message: 'Expression has type "Any"',
                 column: 21,
                 line: 12,
                 type: 'error',
                 provider: 'mypy'
-             } as ILintMessage]
+            } as ILintMessage]
         ];
         for (const [line, expected] of tests) {
             const msg = parseLine(line, REGEX, 'mypy');
@@ -54,3 +62,72 @@ suite('Linting - MyPy', () => {
         }
     });
 });
+
+suite('Test Linter', () => {
+    class TestMyPyLinter extends MyPy {
+        // tslint:disable: no-unnecessary-override
+        public async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
+            return super.runLinter(document, cancellation);
+        }
+        public getWorkspaceRootPath(document: TextDocument): string {
+            return super.getWorkspaceRootPath(document);
+        }
+        public async run(args: string[], document: TextDocument, cancellation: CancellationToken, regEx: string = REGEX): Promise<ILintMessage[]> {
+            return super.run(args, document, cancellation, regEx);
+        }
+        public parseMessagesSeverity(error: string, severity: any): LintMessageSeverity {
+            return super.parseMessagesSeverity(error, severity);
+        }
+    }
+
+    let linter: TestMyPyLinter;
+    let getWorkspaceRootPathStub: sinon.SinonStub<[TextDocument], string>;
+    let runStub: sinon.SinonStub<[string[], TextDocument, CancellationToken, (string | undefined)?], Promise<ILintMessage[]>>;
+    const token = new CancellationTokenSource().token;
+    teardown(() => sinon.restore());
+    setup(() => {
+        const linterManager = mock(LinterManager);
+        when(linterManager.getLinterInfo(anything())).thenReturn({ product: Product.mypy } as any);
+        const serviceContainer = mock(ServiceContainer);
+        when(serviceContainer.get<ILinterManager>(ILinterManager)).thenReturn(instance(linterManager));
+        getWorkspaceRootPathStub = sinon.stub(TestMyPyLinter.prototype, 'getWorkspaceRootPath');
+        runStub = sinon.stub(TestMyPyLinter.prototype, 'run');
+        linter = new TestMyPyLinter(instance(mock(MockOutputChannel)), instance(serviceContainer));
+    });
+
+    test('Get cwd based on document', async () => {
+        const fileUri = Uri.file(path.join('a', 'b', 'c', 'd', 'e', 'filename.py'));
+        const cwd = path.join('a', 'b', 'c');
+        const doc = { uri: fileUri } as any as TextDocument;
+        getWorkspaceRootPathStub.callsFake(() => cwd);
+        runStub.callsFake(() => Promise.resolve([]));
+
+        await linter.runLinter(doc, token);
+
+        expect(getWorkspaceRootPathStub.callCount).to.equal(1);
+        expect(getWorkspaceRootPathStub.args[0]).to.deep.equal([doc]);
+    });
+    test('Pass relative path of document to linter', async () => {
+        const fileUri = Uri.file(path.join('a', 'b', 'c', 'd', 'e', 'filename.py'));
+        const cwd = path.join('a', 'b', 'c');
+        const doc = { uri: fileUri } as any as TextDocument;
+        getWorkspaceRootPathStub.callsFake(() => cwd);
+        runStub.callsFake(() => Promise.resolve([]));
+
+        await linter.runLinter(doc, token);
+
+        expect(runStub.callCount).to.equal(1);
+        expect(runStub.args[0]).to.deep.equal([[path.relative(cwd, fileUri.fsPath)], doc, token, REGEX]);
+    });
+    test('Return empty messages', async () => {
+        const fileUri = Uri.file(path.join('a', 'b', 'c', 'd', 'e', 'filename.py'));
+        const cwd = path.join('a', 'b', 'c');
+        const doc = { uri: fileUri } as any as TextDocument;
+        getWorkspaceRootPathStub.callsFake(() => cwd);
+        runStub.callsFake(() => Promise.resolve([]));
+
+        const messages = await linter.runLinter(doc, token);
+
+        expect(messages).to.be.deep.equal([]);
+    });
+});