Add support for pylint error ranges

news/1 Enhancements/18068.md

@@ -0,0 +1 @@
+Add support for pylint error ranges. Requires Python 3.8 and pylint 2.12.2 or higher. (thanks [Marc Mueller](https://github.com/cdce8p))

src/client/linters/lintingEngine.ts

@@ -143,7 +143,11 @@ export class LintingEngine implements ILintingEngine {
 
     private createDiagnostics(message: ILintMessage, _document: vscode.TextDocument): vscode.Diagnostic {
         const position = new vscode.Position(message.line - 1, message.column);
-        const range = new vscode.Range(position, position);
+        let endPosition: vscode.Position = position;
+        if (message.endLine && message.endColumn) {
+            endPosition = new vscode.Position(message.endLine - 1, message.endColumn);
+        }
+        const range = new vscode.Range(position, endPosition);
 
         const severity = lintSeverityToVSSeverity.get(message.severity!)!;
         const diagnostic = new vscode.Diagnostic(range, message.message, severity);

src/client/linters/pylint.ts

@@ -5,10 +5,19 @@ import { CancellationToken, TextDocument } from 'vscode';
 import '../common/extensions';
 import { Product } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
+import { traceError } from '../logging';
 import { BaseLinter } from './baseLinter';
 import { ILintMessage } from './types';
 
-const REGEX = '(?<line>\\d+),(?<column>-?\\d+),(?<type>\\w+),(?<code>[\\w-]+):(?<message>.*)\\r?(\\n|$)';
+interface IJsonMessage {
+    column: number | null;
+    line: number;
+    message: string;
+    symbol: string;
+    type: string;
+    endLine?: number | null;
+    endColumn?: number | null;
+}
 
 export class Pylint extends BaseLinter {
     constructor(serviceContainer: IServiceContainer) {
@@ -18,17 +27,59 @@ export class Pylint extends BaseLinter {
     protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
         const uri = document.uri;
         const settings = this.configService.getSettings(uri);
-        const args = [
-            "--msg-template='{line},{column},{category},{symbol}:{msg}'",
-            '--reports=n',
-            '--output-format=text',
-            uri.fsPath,
-        ];
-        const messages = await this.run(args, document, cancellation, REGEX);
+        const args = ['--reports=n', '--output-format=json', uri.fsPath];
+        const messages = await this.run(args, document, cancellation);
         messages.forEach((msg) => {
             msg.severity = this.parseMessagesSeverity(msg.type, settings.linting.pylintCategorySeverity);
         });
 
         return messages;
     }
+
+    private parseOutputMessage(outputMsg: IJsonMessage, colOffset: number = 0): ILintMessage | undefined {
+        // Both 'endLine' and 'endColumn' are only present on pylint 2.12.2+
+        // If present, both can still be 'null' if AST node didn't have endLine and / or endColumn information.
+        // If 'endColumn' is 'null' or not preset, set it to 'undefined' to
+        // prevent the lintingEngine from inferring an error range.
+        if (outputMsg.endColumn) {
+            outputMsg.endColumn = outputMsg.endColumn <= 0 ? 0 : outputMsg.endColumn - colOffset;
+        } else {
+            outputMsg.endColumn = undefined;
+        }
+
+        return {
+            code: outputMsg.symbol,
+            message: outputMsg.message,
+            column: outputMsg.column === null || outputMsg.column <= 0 ? 0 : outputMsg.column - colOffset,
+            line: outputMsg.line,
+            type: outputMsg.type,
+            provider: this.info.id,
+            endLine: outputMsg.endLine === null ? undefined : outputMsg.endLine,
+            endColumn: outputMsg.endColumn,
+        };
+    }
+
+    protected async parseMessages(
+        output: string,
+        _document: TextDocument,
+        _token: CancellationToken,
+        _: string,
+    ): Promise<ILintMessage[]> {
+        const messages: ILintMessage[] = [];
+        try {
+            const parsedOutput: IJsonMessage[] = JSON.parse(output);
+            for (const outputMsg of parsedOutput) {
+                const msg = this.parseOutputMessage(outputMsg, this.columnOffset);
+                if (msg) {
+                    messages.push(msg);
+                    if (messages.length >= this.pythonSettings.linting.maxNumberOfProblems) {
+                        break;
+                    }
+                }
+            }
+        } catch (ex) {
+            traceError(`Linter '${this.info.id}' failed to parse the output '${output}.`, ex);
+        }
+        return messages;
+    }
 }

src/client/linters/types.ts

@@ -55,6 +55,8 @@ export interface ILinterManager {
 export interface ILintMessage {
     line: number;
     column: number;
+    endLine?: number;
+    endColumn?: number;
     code: string | undefined;
     message: string;
     type: string;

src/test/linters/common.ts

@@ -44,6 +44,30 @@ export function linterMessageAsLine(msg: ILintMessage): string {
     }
 }
 
+function pylintMessageAsString(msg: ILintMessage, trailingComma: boolean = true): string {
+    return `    {
+        "type": "${msg.type}",
+        "line": ${msg.line},
+        "column": ${msg.column},
+        "symbol": "${msg.code}",
+        "message": "${msg.message}",
+        "endLine": ${msg.endLine ?? null},
+        "endColumn": ${msg.endColumn ?? null}
+    }${trailingComma ? ',' : ''}`;
+}
+
+export function pylintLinterMessagesAsOutput(messages: ILintMessage[]): string {
+    const lines: string[] = ['['];
+    if (messages) {
+        const pylintMessages = messages.slice(0, -1).map((msg) => pylintMessageAsString(msg, true));
+        const lastMessage = pylintMessageAsString(messages[messages.length - 1], false);
+
+        lines.push(...pylintMessages, lastMessage);
+    }
+    lines.push(']');
+    return lines.join(os.EOL);
+}
+
 export function getLinterID(product: Product): LinterId {
     const linterID = LINTERID_BY_PRODUCT.get(product);
     if (!linterID) {

src/test/linters/lint.multilinter.test.ts

@@ -25,8 +25,19 @@ class MockPythonToolExecService extends PythonToolExecutionService {
     public flake8Msg =
         '1,1,W,W391:blank line at end of file\ns:142:13), <anonymous>:1\n1,7,E,E999:SyntaxError: invalid syntax\n';
 
-    public pylintMsg =
-        "************* Module print\ns:142:13), <anonymous>:1\n1,0,error,syntax-error:Missing parentheses in call to 'print'. Did you mean print(x)? (<unknown>, line 1)\n";
+    public pylintMsg = `[
+    {
+        "type": "error",
+        "module": "print",
+        "obj": "",
+        "line": 1,
+        "column": 0,
+        "path": "print.py",
+        "symbol": "syntax-error",
+        "message": "Missing parentheses in call to 'print'. Did you mean print(x)? (<unknown>, line 1)",
+        "message-id": "E0001"
+    }
+]`;
 
     // Depending on moduleName being exec'd, return the appropriate sample.
     public async exec(

src/test/linters/lint.unit.test.ts

@@ -18,7 +18,14 @@ import {
 import { ProductType } from '../../client/common/types';
 import { LINTERID_BY_PRODUCT } from '../../client/linters/constants';
 import { ILintMessage, LintMessageSeverity } from '../../client/linters/types';
-import { BaseTestFixture, getLinterID, getProductName, linterMessageAsLine, throwUnknownProduct } from './common';
+import {
+    BaseTestFixture,
+    getLinterID,
+    getProductName,
+    linterMessageAsLine,
+    pylintLinterMessagesAsOutput,
+    throwUnknownProduct,
+} from './common';
 
 const pylintMessagesToBeReturned: ILintMessage[] = [
     {
@@ -155,6 +162,8 @@ const pylintMessagesToBeReturned: ILintMessage[] = [
         message: "Instance of 'Foo' has no 'blip' member",
         provider: '',
         type: 'warning',
+        endLine: undefined,
+        endColumn: undefined,
     },
     {
         line: 61,
@@ -164,6 +173,8 @@ const pylintMessagesToBeReturned: ILintMessage[] = [
         message: "Instance of 'Foo' has no 'blip' member",
         provider: '',
         type: 'warning',
+        endLine: 61,
+        endColumn: undefined,
     },
     {
         line: 72,
@@ -173,6 +184,8 @@ const pylintMessagesToBeReturned: ILintMessage[] = [
         message: "Instance of 'Foo' has no 'blip' member",
         provider: '',
         type: 'warning',
+        endLine: 72,
+        endColumn: 28,
     },
     {
         line: 75,
@@ -182,6 +195,8 @@ const pylintMessagesToBeReturned: ILintMessage[] = [
         message: "Instance of 'Foo' has no 'blip' member",
         provider: '',
         type: 'warning',
+        endLine: 75,
+        endColumn: 28,
     },
     {
         line: 77,
@@ -191,6 +206,8 @@ const pylintMessagesToBeReturned: ILintMessage[] = [
         message: "Instance of 'Foo' has no 'blip' member",
         provider: '',
         type: 'warning',
+        endLine: 77,
+        endColumn: 24,
     },
     {
         line: 83,
@@ -200,6 +217,8 @@ const pylintMessagesToBeReturned: ILintMessage[] = [
         message: "Instance of 'Foo' has no 'blip' member",
         provider: '',
         type: 'warning',
+        endLine: 83,
+        endColumn: 24,
     },
 ];
 const flake8MessagesToBeReturned: ILintMessage[] = [
@@ -604,6 +623,10 @@ class TestFixture extends BaseTestFixture {
             return;
         }
 
+        if (product && getLinterID(product) == 'pylint') {
+            this.setStdout(pylintLinterMessagesAsOutput(messages));
+            return;
+        }
         const lines: string[] = [];
         for (const msg of messages) {
             if (msg.provider === '' && product) {

src/test/linters/pylint.test.ts

@@ -102,10 +102,32 @@ suite('Linting - Pylint', () => {
         workspace.setup((x) => x.getWorkspaceFolder(TypeMoq.It.isAny())).returns(() => wsf.object);
 
         const linterOutput = [
-            'No config file found, using default configuration',
-            '************* Module test',
-            '1,1,convention,C0111:Missing module docstring',
-            '3,-1,error,E1305:Too many arguments for format string',
+            '[',
+            '    {',
+            '        "type": "convention",',
+            '        "module": "test",',
+            '        "obj": "",',
+            '        "line": 1,',
+            '        "column": 1,',
+            `        "path": "${fileFolder}/test.py",`,
+            '        "symbol": "missing-module-docstring",',
+            '        "message": "Missing module docstring",',
+            '        "message-id": "C0114",',
+            '        "endLine": null,',
+            '        "endColumn": null',
+            '    },',
+            '    {',
+            '        "type": "error",',
+            '        "module": "test",',
+            '        "obj": "",',
+            '        "line": 3,',
+            '        "column": -1,',
+            `        "path": "${fileFolder}/test.py",`,
+            '        "symbol": "too-many-format-args",',
+            '        "message": "Too many arguments for format string",',
+            '        "message-id": "E1305"',
+            '     }',
+            ']',
         ].join(os.EOL);
         execService
             .setup((x) => x.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))