From b28a68c9a5278029f220c5d6072d69fbbaa32b8f Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Thu, 28 Jan 2021 19:31:01 -0500 Subject: [PATCH 1/7] Ignore mypy lint errors for other files mypy sometimes returns errors for files that aren't the same as the file it was called against. This PR adds some filtering based on the current file name. fixes https://github.com/microsoft/vscode-python/issues/10190 --- src/client/linters/baseLinter.ts | 2 ++ src/client/linters/mypy.ts | 12 +++++++++++- src/client/linters/types.ts | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index 565bdd5a9103..cce2a0d3b232 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -20,6 +20,7 @@ const namedRegexp = require('named-js-regexp'); const REGEX = '(?\\d+),(?-?\\d+),(?\\w+),(?\\w+\\d+):(?.*)\\r?(\\n|$)'; export interface IRegexGroup { + file: string; line: number; column: number; code: string; @@ -53,6 +54,7 @@ export function parseLine( match.column = Number(match.column); return { + file: match.file, code: match.code, message: match.message, column: isNaN(match.column) || match.column <= 0 ? 0 : match.column - colOffset, diff --git a/src/client/linters/mypy.ts b/src/client/linters/mypy.ts index eff5c71be37a..fc29c15704d7 100644 --- a/src/client/linters/mypy.ts +++ b/src/client/linters/mypy.ts @@ -1,5 +1,6 @@ import { CancellationToken, OutputChannel, TextDocument } from 'vscode'; import '../common/extensions'; +import * as path from 'path'; import { Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { BaseLinter } from './baseLinter'; @@ -14,10 +15,19 @@ export class MyPy extends BaseLinter { protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX); + const filteredMessages: ILintMessage[] = []; messages.forEach((msg) => { + if (msg.file == null) { + return; + } + const messageFilePath = path.join(this.getWorkspaceRootPath(document), msg.file); + if (messageFilePath !== document.uri.fsPath) { + return; + } msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity); msg.code = msg.type; + filteredMessages.push(msg); }); - return messages; + return filteredMessages; } } diff --git a/src/client/linters/types.ts b/src/client/linters/types.ts index fb6d1d44e1ca..1c4c742f5937 100644 --- a/src/client/linters/types.ts +++ b/src/client/linters/types.ts @@ -63,6 +63,7 @@ export interface ILinterManager { } export interface ILintMessage { + file?: string; line: number; column: number; code: string | undefined; From 9c999bc36fba0731bc62f995df58f5b315489ef4 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Thu, 28 Jan 2021 19:43:44 -0500 Subject: [PATCH 2/7] use regex instead --- src/client/linters/baseLinter.ts | 2 -- src/client/linters/mypy.ts | 21 ++++++++------------- src/client/linters/types.ts | 1 - src/test/linters/mypy.unit.test.ts | 4 ++-- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index cce2a0d3b232..565bdd5a9103 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -20,7 +20,6 @@ const namedRegexp = require('named-js-regexp'); const REGEX = '(?\\d+),(?-?\\d+),(?\\w+),(?\\w+\\d+):(?.*)\\r?(\\n|$)'; export interface IRegexGroup { - file: string; line: number; column: number; code: string; @@ -54,7 +53,6 @@ export function parseLine( match.column = Number(match.column); return { - file: match.file, code: match.code, message: match.message, column: isNaN(match.column) || match.column <= 0 ? 0 : match.column - colOffset, diff --git a/src/client/linters/mypy.ts b/src/client/linters/mypy.ts index fc29c15704d7..b876b688448f 100644 --- a/src/client/linters/mypy.ts +++ b/src/client/linters/mypy.ts @@ -1,12 +1,14 @@ import { CancellationToken, OutputChannel, TextDocument } from 'vscode'; import '../common/extensions'; -import * as path from 'path'; +import { escapeRegExp } from 'lodash'; import { Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { BaseLinter } from './baseLinter'; import { ILintMessage } from './types'; -export const REGEX = '(?[^:]+):(?\\d+)(:(?\\d+))?: (?\\w+): (?.*)\\r?(\\n|$)'; +export function getRegex(filepath: string): string { + return `${escapeRegExp(filepath)}:(?\\d+)(:(?\\d+))?: (?\\w+): (?.*)\\r?(\\n|$)`; +} export class MyPy extends BaseLinter { constructor(outputChannel: OutputChannel, serviceContainer: IServiceContainer) { @@ -14,20 +16,13 @@ export class MyPy extends BaseLinter { } protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { - const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX); - const filteredMessages: ILintMessage[] = []; + const fileName = document.uri.fsPath.slice(this.getWorkspaceRootPath(document).length + 1); + const regex = getRegex(fileName); + const messages = await this.run([document.uri.fsPath], document, cancellation, regex); messages.forEach((msg) => { - if (msg.file == null) { - return; - } - const messageFilePath = path.join(this.getWorkspaceRootPath(document), msg.file); - if (messageFilePath !== document.uri.fsPath) { - return; - } msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity); msg.code = msg.type; - filteredMessages.push(msg); }); - return filteredMessages; + return messages; } } diff --git a/src/client/linters/types.ts b/src/client/linters/types.ts index 1c4c742f5937..fb6d1d44e1ca 100644 --- a/src/client/linters/types.ts +++ b/src/client/linters/types.ts @@ -63,7 +63,6 @@ export interface ILinterManager { } export interface ILintMessage { - file?: string; line: number; column: number; code: string | undefined; diff --git a/src/test/linters/mypy.unit.test.ts b/src/test/linters/mypy.unit.test.ts index 5da759cb7108..956f1a0e448b 100644 --- a/src/test/linters/mypy.unit.test.ts +++ b/src/test/linters/mypy.unit.test.ts @@ -5,7 +5,7 @@ import { expect } from 'chai'; import { parseLine } from '../../client/linters/baseLinter'; -import { REGEX } from '../../client/linters/mypy'; +import { getRegex } from '../../client/linters/mypy'; import { ILintMessage, LinterId } from '../../client/linters/types'; // This following is a real-world example. See gh=2380. @@ -55,7 +55,7 @@ suite('Linting - MyPy', () => { ], ]; for (const [line, expected] of tests) { - const msg = parseLine(line, REGEX, LinterId.MyPy); + const msg = parseLine(line, getRegex('provider.pyi'), LinterId.MyPy); expect(msg).to.deep.equal(expected); } From a84044351de3f354edb68a87a8019343bb859e27 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Thu, 28 Jan 2021 19:52:33 -0500 Subject: [PATCH 3/7] add news --- news/2 Fixes/10190.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 news/2 Fixes/10190.md diff --git a/news/2 Fixes/10190.md b/news/2 Fixes/10190.md new file mode 100644 index 000000000000..c5b2bb7306ed --- /dev/null +++ b/news/2 Fixes/10190.md @@ -0,0 +1,2 @@ +Prevent mypy errors for other files showing in current file. +(thanks [Steve Dignam](https://github.com/sbdchd)) From 434eded74e6c839aa86ae3a9e78e39d0a45abcb6 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Wed, 24 Feb 2021 22:48:53 -0500 Subject: [PATCH 4/7] code review fixes --- src/client/linters/mypy.ts | 4 ++-- src/test/linters/mypy.unit.test.ts | 37 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/client/linters/mypy.ts b/src/client/linters/mypy.ts index b876b688448f..fbe88ffd6e61 100644 --- a/src/client/linters/mypy.ts +++ b/src/client/linters/mypy.ts @@ -16,8 +16,8 @@ export class MyPy extends BaseLinter { } protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { - const fileName = document.uri.fsPath.slice(this.getWorkspaceRootPath(document).length + 1); - const regex = getRegex(fileName); + const relativeFilePath = document.uri.fsPath.slice(this.getWorkspaceRootPath(document).length + 1); + const regex = getRegex(relativeFilePath); const messages = await this.run([document.uri.fsPath], document, cancellation, regex); messages.forEach((msg) => { msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.mypyCategorySeverity); diff --git a/src/test/linters/mypy.unit.test.ts b/src/test/linters/mypy.unit.test.ts index 956f1a0e448b..6eebd9a84840 100644 --- a/src/test/linters/mypy.unit.test.ts +++ b/src/test/linters/mypy.unit.test.ts @@ -60,4 +60,41 @@ suite('Linting - MyPy', () => { expect(msg).to.deep.equal(expected); } }); + test('regex excludes unexpected files', () => { + // mypy run against `foo/bar.py` returning errors for foo/__init__.py + const outputWithUnexpectedFile = ` +foo/__init__.py:4:5: error: Statement is unreachable [unreachable] +foo/bar.py:2:14: error: Incompatible types in assignment (expression has type "str", variable has type "int") [assignment] +Found 2 errors in 2 files (checked 1 source file) +`; + + const lines = outputWithUnexpectedFile.split('\n'); + const tests: [string, ILintMessage | undefined][] = [ + [lines[0], undefined], + [lines[1], undefined], + [ + lines[2], + { + code: undefined, + message: + 'Incompatible types in assignment (expression has type "str", variable has type "int") [assignment]', + column: 14, + line: 2, + type: 'error', + provider: 'mypy', + }, + ], + [lines[3], undefined], + ]; + for (const [line, expected] of tests) { + const msg = parseLine(line, getRegex('foo/bar.py'), LinterId.MyPy); + + expect(msg).to.deep.equal(expected); + } + }); + test('getRegex escapes filename correctly', () => { + expect(getRegex('foo/bar.py')).to.eql( + String.raw`foo/bar\.py:(?\d+)(:(?\d+))?: (?\w+): (?.*)\r?(\n|$)`, + ); + }); }); From fd614e1b758dde2e4316493b066c15d21fe5f689 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Wed, 24 Feb 2021 23:00:16 -0500 Subject: [PATCH 5/7] cut new line to better match actual output ``` ##########Linting Output - mypy########## foo/__init__.py:4:5: error: Statement is unreachable [unreachable] foo/bar.py:2:14: error: Incompatible types in assignment (expression has type "str", variable has type "int") [assignment] Found 2 errors in 2 files (checked 1 source file) ``` --- src/test/linters/mypy.unit.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/linters/mypy.unit.test.ts b/src/test/linters/mypy.unit.test.ts index 6eebd9a84840..9513ec1e8156 100644 --- a/src/test/linters/mypy.unit.test.ts +++ b/src/test/linters/mypy.unit.test.ts @@ -62,7 +62,7 @@ suite('Linting - MyPy', () => { }); test('regex excludes unexpected files', () => { // mypy run against `foo/bar.py` returning errors for foo/__init__.py - const outputWithUnexpectedFile = ` + const outputWithUnexpectedFile = `\ foo/__init__.py:4:5: error: Statement is unreachable [unreachable] foo/bar.py:2:14: error: Incompatible types in assignment (expression has type "str", variable has type "int") [assignment] Found 2 errors in 2 files (checked 1 source file) @@ -71,9 +71,8 @@ Found 2 errors in 2 files (checked 1 source file) const lines = outputWithUnexpectedFile.split('\n'); const tests: [string, ILintMessage | undefined][] = [ [lines[0], undefined], - [lines[1], undefined], [ - lines[2], + lines[1], { code: undefined, message: @@ -84,7 +83,7 @@ Found 2 errors in 2 files (checked 1 source file) provider: 'mypy', }, ], - [lines[3], undefined], + [lines[2], undefined], ]; for (const [line, expected] of tests) { const msg = parseLine(line, getRegex('foo/bar.py'), LinterId.MyPy); From 9c7b95126e9d0f67b1043e8ff5c82293b985289c Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Fri, 5 Mar 2021 17:49:49 -0500 Subject: [PATCH 6/7] fix offset --- src/test/linters/mypy.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/mypy.unit.test.ts b/src/test/linters/mypy.unit.test.ts index e98cdd0c3bf9..e91c6edabec7 100644 --- a/src/test/linters/mypy.unit.test.ts +++ b/src/test/linters/mypy.unit.test.ts @@ -86,7 +86,7 @@ Found 2 errors in 2 files (checked 1 source file) [lines[2], undefined], ]; for (const [line, expected] of tests) { - const msg = parseLine(line, getRegex('foo/bar.py'), LinterId.MyPy); + const msg = parseLine(line, getRegex('foo/bar.py'), LinterId.MyPy, 1); expect(msg).to.deep.equal(expected); } From 75830b266ce8689815abf72b79e3be6098e2db0f Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Tue, 9 Mar 2021 17:51:30 -0500 Subject: [PATCH 7/7] fix test? --- src/test/linters/mypy.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/linters/mypy.unit.test.ts b/src/test/linters/mypy.unit.test.ts index e91c6edabec7..b697a719a475 100644 --- a/src/test/linters/mypy.unit.test.ts +++ b/src/test/linters/mypy.unit.test.ts @@ -77,7 +77,7 @@ Found 2 errors in 2 files (checked 1 source file) code: undefined, message: 'Incompatible types in assignment (expression has type "str", variable has type "int") [assignment]', - column: 14, + column: 13, line: 2, type: 'error', provider: 'mypy',