From 15635398db9f099d37aa56d7e02fd610a69d57a5 Mon Sep 17 00:00:00 2001 From: Scott Saponas Date: Thu, 19 Jul 2018 22:48:08 -0700 Subject: [PATCH 1/8] Fixing https://github.com/Microsoft/vscode-python/issues/2198 --- src/client/linters/baseLinter.ts | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index 51319c0f581b..eca875edb132 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -149,21 +149,23 @@ export abstract class BaseLinter implements ILinter { } private parseLines(outputLines: string[], regEx: string): ILintMessage[] { - return outputLines - .filter((value, index) => index <= this.pythonSettings.linting.maxNumberOfProblems) - .map(line => { - try { - const msg = this.parseLine(line, regEx); - if (msg) { - return msg; - } - } catch (ex) { - this.logger.logError(`Linter '${this.info.id}' failed to parse the line '${line}.`, ex); + const messages: ILintMessage[] = []; + let messageCount: number = 0; + let i: number; + for (i = messageCount; i < outputLines.length && messageCount < this.pythonSettings.linting.maxNumberOfProblems; i += 1) + { + const line = outputLines[i]; + try { + const msg = this.parseLine(line, regEx); + if (msg) { + messages.push(msg); + messageCount += 1; } - return; - }) - .filter(item => item !== undefined) - .map(item => item!); + } catch (ex) { + this.logger.logError(`Linter '${this.info.id}' failed to parse the line '${line}.`, ex); + } + } + return messages; } private displayLinterResultHeader(data: string) { From 9c67479f3754c142d3c6d3a348dec917a0b6fc0c Mon Sep 17 00:00:00 2001 From: Scott Saponas Date: Fri, 20 Jul 2018 10:20:56 -0700 Subject: [PATCH 2/8] Added news item entry documenting issue fix. --- news/2 Fixes/2198.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 news/2 Fixes/2198.md diff --git a/news/2 Fixes/2198.md b/news/2 Fixes/2198.md new file mode 100644 index 000000000000..f65027dcc6f7 --- /dev/null +++ b/news/2 Fixes/2198.md @@ -0,0 +1,3 @@ +Changing message parsing in BaseLinter so it respects python.linting.maxNumberOfProblems. + +(thanks [Scott Saponas](https://github.com/saponas/)) From f64a78602fefec580fa160a178226a075476cab8 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 1 Aug 2018 11:06:12 -0700 Subject: [PATCH 3/8] Tweak news entry --- news/2 Fixes/2198.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/news/2 Fixes/2198.md b/news/2 Fixes/2198.md index f65027dcc6f7..5859dc459b08 100644 --- a/news/2 Fixes/2198.md +++ b/news/2 Fixes/2198.md @@ -1,3 +1,2 @@ -Changing message parsing in BaseLinter so it respects python.linting.maxNumberOfProblems. - +Change linter message parsing so it respects `python.linting.maxNumberOfProblems`. (thanks [Scott Saponas](https://github.com/saponas/)) From d3b88b40d945e2a39a08afcbcff7757197f9d61b Mon Sep 17 00:00:00 2001 From: Scott Saponas Date: Thu, 9 Aug 2018 14:41:31 -0700 Subject: [PATCH 4/8] Simplify Max Problems check. --- src/client/linters/baseLinter.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index eca875edb132..a814b1aba9ff 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -150,16 +150,14 @@ export abstract class BaseLinter implements ILinter { private parseLines(outputLines: string[], regEx: string): ILintMessage[] { const messages: ILintMessage[] = []; - let messageCount: number = 0; - let i: number; - for (i = messageCount; i < outputLines.length && messageCount < this.pythonSettings.linting.maxNumberOfProblems; i += 1) - { - const line = outputLines[i]; + for (const line of outputLines) { try { const msg = this.parseLine(line, regEx); if (msg) { messages.push(msg); - messageCount += 1; + if (messages.length >= this.pythonSettings.linting.maxNumberOfProblems) { + break; + } } } catch (ex) { this.logger.logError(`Linter '${this.info.id}' failed to parse the line '${line}.`, ex); From 23809fc233c4521f79ea470d1c3bf213da3909c0 Mon Sep 17 00:00:00 2001 From: Scott Saponas Date: Thu, 9 Aug 2018 14:42:21 -0700 Subject: [PATCH 5/8] Add unittest for multiline lint messages. --- src/test/linters/lint.test.ts | 19 +++++++++++++++++++ .../pythonFiles/linting/threeLineLints.py | 15 +++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 src/test/pythonFiles/linting/threeLineLints.py diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index 737a2686729c..2427a40c192b 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -23,6 +23,7 @@ const pep8ConfigPath = path.join(pythoFilesPath, 'pep8config'); const pydocstyleConfigPath27 = path.join(pythoFilesPath, 'pydocstyleconfig27'); const pylintConfigPath = path.join(pythoFilesPath, 'pylintconfig'); const fileToLint = path.join(pythoFilesPath, 'file.py'); +const threeLinLintsPath = path.join(pythoFilesPath, 'threeLineLints.py'); const pylintMessagesToBeReturned: ILintMessage[] = [ { line: 24, column: 0, severity: LintMessageSeverity.Information, code: 'I0011', message: 'Locally disabling no-member (E1101)', provider: '', type: '' }, @@ -279,4 +280,22 @@ suite('Linting - General Tests', () => { assert.notEqual(messages!.filter(x => x.source === 'pylint').length, 0, 'No pylint messages.'); assert.notEqual(messages!.filter(x => x.source === 'flake8').length, 0, 'No flake8 messages.'); }); + // tslint:disable-next-line:no-any + async function testLinterMessageCount(product: Product, pythonFile: string, messageCountToBeReceived: number): Promise { + const outputChannel = ioc.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); + const cancelToken = new CancellationTokenSource(); + const document = await workspace.openTextDocument(pythonFile); + + await linterManager.setActiveLintersAsync([product], document.uri); + const linter = linterManager.createLinter(product, outputChannel, ioc.serviceContainer); + + const messages = await linter.lint(document, cancelToken.token); + assert.equal(messages.length, messageCountToBeReceived, 'Expected number of lint errors does not match lint error count'); + } + test('Three line output counted as one message', async () => { + const maxErrors = 5; + const target = IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace; + await configService.updateSettingAsync('linting.maxNumberOfProblems', maxErrors, rootWorkspaceUri, target); + await testLinterMessageCount(Product.pylint, threeLinLintsPath, maxErrors); + }); }); diff --git a/src/test/pythonFiles/linting/threeLineLints.py b/src/test/pythonFiles/linting/threeLineLints.py new file mode 100644 index 000000000000..fb9b5322a682 --- /dev/null +++ b/src/test/pythonFiles/linting/threeLineLints.py @@ -0,0 +1,15 @@ +"""pylint messages with three lines of output""" + +__revision__ = None + +class Foo(object): + + def __init__(self): + pass + + def meth1(self,arg): + """missing a space after the comma in args""" + a = (1,2) + b = (1,2) + c = (1,2) + print (self) From 0eec7ece034ddcee39f320fc69976b24d7dd486d Mon Sep 17 00:00:00 2001 From: Scott Saponas Date: Mon, 27 Aug 2018 14:30:17 -0700 Subject: [PATCH 6/8] fix typo in constant name. --- src/test/linters/lint.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/linters/lint.test.ts b/src/test/linters/lint.test.ts index 2427a40c192b..8b9d702228ad 100644 --- a/src/test/linters/lint.test.ts +++ b/src/test/linters/lint.test.ts @@ -23,7 +23,7 @@ const pep8ConfigPath = path.join(pythoFilesPath, 'pep8config'); const pydocstyleConfigPath27 = path.join(pythoFilesPath, 'pydocstyleconfig27'); const pylintConfigPath = path.join(pythoFilesPath, 'pylintconfig'); const fileToLint = path.join(pythoFilesPath, 'file.py'); -const threeLinLintsPath = path.join(pythoFilesPath, 'threeLineLints.py'); +const threeLineLintsPath = path.join(pythoFilesPath, 'threeLineLints.py'); const pylintMessagesToBeReturned: ILintMessage[] = [ { line: 24, column: 0, severity: LintMessageSeverity.Information, code: 'I0011', message: 'Locally disabling no-member (E1101)', provider: '', type: '' }, @@ -296,6 +296,6 @@ suite('Linting - General Tests', () => { const maxErrors = 5; const target = IS_MULTI_ROOT_TEST ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace; await configService.updateSettingAsync('linting.maxNumberOfProblems', maxErrors, rootWorkspaceUri, target); - await testLinterMessageCount(Product.pylint, threeLinLintsPath, maxErrors); + await testLinterMessageCount(Product.pylint, threeLineLintsPath, maxErrors); }); }); From 4bcb4a6458a9b4d96f49c3138143339549217c0e Mon Sep 17 00:00:00 2001 From: Scott Saponas Date: Mon, 27 Aug 2018 14:41:08 -0700 Subject: [PATCH 7/8] Adding comments to document what lint messages should be produced. --- src/test/pythonFiles/linting/threeLineLints.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/pythonFiles/linting/threeLineLints.py b/src/test/pythonFiles/linting/threeLineLints.py index fb9b5322a682..f9dec95af3fc 100644 --- a/src/test/pythonFiles/linting/threeLineLints.py +++ b/src/test/pythonFiles/linting/threeLineLints.py @@ -8,7 +8,16 @@ def __init__(self): pass def meth1(self,arg): - """missing a space after the comma in args""" + """missing a space between 'self' and 'arg'. This should trigger the + following three line lint warning: + + C: 10, 0: Exactly one space required after comma + def meth1(self,arg): + ^ (bad-whitespace) + + The following three lines should also cause three line lint errors + due to "Exactly one space required after comma" + """ a = (1,2) b = (1,2) c = (1,2) From 8db24474f3018e77d2c47edf919724c933dad585 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Tue, 28 Aug 2018 12:10:03 -0700 Subject: [PATCH 8/8] Touch up docstring --- src/test/pythonFiles/linting/threeLineLints.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/pythonFiles/linting/threeLineLints.py b/src/test/pythonFiles/linting/threeLineLints.py index f9dec95af3fc..e8b578d93f11 100644 --- a/src/test/pythonFiles/linting/threeLineLints.py +++ b/src/test/pythonFiles/linting/threeLineLints.py @@ -9,14 +9,14 @@ def __init__(self): def meth1(self,arg): """missing a space between 'self' and 'arg'. This should trigger the - following three line lint warning: + following three line lint warning:: - C: 10, 0: Exactly one space required after comma - def meth1(self,arg): - ^ (bad-whitespace) + C: 10, 0: Exactly one space required after comma + def meth1(self,arg): + ^ (bad-whitespace) - The following three lines should also cause three line lint errors - due to "Exactly one space required after comma" + The following three lines of tuples should also cause three-line lint + errors due to "Exactly one space required after comma" messages. """ a = (1,2) b = (1,2)