diff --git a/news/2 Fixes/18364.md b/news/2 Fixes/18364.md new file mode 100644 index 000000000000..0a6995d8eb06 --- /dev/null +++ b/news/2 Fixes/18364.md @@ -0,0 +1 @@ +Ensure linting works under `conda run` (work-around for https://github.com/conda/conda/issues/10972). diff --git a/pythonFiles/linter.py b/pythonFiles/linter.py new file mode 100644 index 000000000000..58ad9397f58b --- /dev/null +++ b/pythonFiles/linter.py @@ -0,0 +1,56 @@ +import subprocess +import sys + + +linter_settings = { + "pylint": { + "args": ["--reports=n", "--output-format=json"], + }, + "flake8": { + "args": ["--format", "%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s"], + }, + "bandit": { + "args": [ + "-f", + "custom", + "--msg-template", + "{line},{col},{severity},{test_id}:{msg}", + "-n", + "-1", + ], + }, + "mypy": {"args": []}, + "prospector": { + "args": ["--absolute-paths", "--output-format=json"], + }, + "pycodestyle": { + "args": ["--format", "%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s"], + }, + "pydocstyle": { + "args": [], + }, + "pylama": {"args": ["--format=parsable"]}, +} + + +def main(): + invoke = sys.argv[1] + if invoke == "-m": + linter = sys.argv[2] + args = ( + [sys.executable, "-m", linter] + + linter_settings[linter]["args"] + + sys.argv[3:] + ) + else: + linter = sys.argv[2] + args = [sys.argv[3]] + linter_settings[linter]["args"] + sys.argv[4:] + + if hasattr(subprocess, "run"): + subprocess.run(args, encoding="utf-8", stdout=sys.stdout, stderr=sys.stderr) + else: + subprocess.call(args, stdout=sys.stdout, stderr=sys.stderr) + + +if __name__ == "__main__": + main() diff --git a/src/client/common/process/internal/scripts/index.ts b/src/client/common/process/internal/scripts/index.ts index b0480b4fad13..c8b3570ff2ba 100644 --- a/src/client/common/process/internal/scripts/index.ts +++ b/src/client/common/process/internal/scripts/index.ts @@ -144,3 +144,10 @@ export function tensorboardLauncher(args: string[]): string[] { const script = path.join(SCRIPTS_DIR, 'tensorboard_launcher.py'); return [script, ...args]; } + +// linter.py + +export function linterScript(): string { + const script = path.join(SCRIPTS_DIR, 'linter.py'); + return script; +} diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index 5d88a776faf7..5c724ae39ca7 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -191,5 +191,6 @@ function createPythonService( execModuleObservable: (m, a, o) => procs.execModuleObservable(m, a, o), exec: (a, o) => procs.exec(a, o), execModule: (m, a, o) => procs.execModule(m, a, o), + execForLinter: (m, a, o) => procs.execForLinter(m, a, o), }; } diff --git a/src/client/common/process/pythonProcess.ts b/src/client/common/process/pythonProcess.ts index 6536019e10f7..f4c33592382f 100644 --- a/src/client/common/process/pythonProcess.ts +++ b/src/client/common/process/pythonProcess.ts @@ -64,6 +64,26 @@ class PythonProcessService { return result; } + + public async execForLinter( + moduleName: string, + args: string[], + options: SpawnOptions, + ): Promise> { + const opts: SpawnOptions = { ...options }; + const executable = this.deps.getExecutionInfo(args); + const result = await this.deps.exec(executable.command, executable.args, opts); + + // If a module is not installed we'll have something in stderr. + if (moduleName && ErrorUtils.outputHasModuleNotInstalledError(moduleName, result.stderr)) { + const isInstalled = await this.deps.isModuleInstalled(moduleName); + if (!isInstalled) { + throw new ModuleNotInstalledError(moduleName); + } + } + + return result; + } } export function createPythonProcessService( diff --git a/src/client/common/process/pythonToolService.ts b/src/client/common/process/pythonToolService.ts index 10294854e7b0..136ab56fe0c4 100644 --- a/src/client/common/process/pythonToolService.ts +++ b/src/client/common/process/pythonToolService.ts @@ -57,4 +57,23 @@ export class PythonToolExecutionService implements IPythonToolExecutionService { return processService.exec(executionInfo.execPath!, executionInfo.args, { ...options }); } } + + public async execForLinter( + executionInfo: ExecutionInfo, + options: SpawnOptions, + resource: Uri, + ): Promise> { + if (options.env) { + throw new Error('Environment variables are not supported'); + } + const pythonExecutionService = await this.serviceContainer + .get(IPythonExecutionFactory) + .create({ resource }); + + if (executionInfo.execPath) { + return pythonExecutionService.exec(executionInfo.args, options); + } + + return pythonExecutionService.execForLinter(executionInfo.moduleName!, executionInfo.args, options); + } } diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts index 1defb4901e48..00003f82c58a 100644 --- a/src/client/common/process/types.ts +++ b/src/client/common/process/types.ts @@ -100,6 +100,7 @@ export interface IPythonExecutionService { exec(args: string[], options: SpawnOptions): Promise>; execModule(moduleName: string, args: string[], options: SpawnOptions): Promise>; + execForLinter(moduleName: string, args: string[], options: SpawnOptions): Promise>; } export class StdErrError extends Error { @@ -117,4 +118,5 @@ export interface IPythonToolExecutionService { resource: Uri, ): Promise>; exec(executionInfo: ExecutionInfo, options: SpawnOptions, resource: Uri): Promise>; + execForLinter(executionInfo: ExecutionInfo, options: SpawnOptions, resource: Uri): Promise>; } diff --git a/src/client/linters/bandit.ts b/src/client/linters/bandit.ts index a47f50fcb7fb..bbc8836bfc6b 100644 --- a/src/client/linters/bandit.ts +++ b/src/client/linters/bandit.ts @@ -26,20 +26,7 @@ export class Bandit extends BaseLinter { protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { // View all errors in bandit <= 1.5.1 (https://github.com/PyCQA/bandit/issues/371) - const messages = await this.run( - [ - '-f', - 'custom', - '--msg-template', - '{line},{col},{severity},{test_id}:{msg}', - '-n', - '-1', - document.uri.fsPath, - ], - document, - cancellation, - BANDIT_REGEX, - ); + const messages = await this.run([document.uri.fsPath], document, cancellation, BANDIT_REGEX); messages.forEach((msg) => { msg.severity = severityMapping[msg.type]; diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index d8147089392d..3598d4ef1dfc 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -165,7 +165,7 @@ export abstract class BaseLinter implements ILinter { IPythonToolExecutionService, ); try { - const result = await pythonToolsExecutionService.exec( + const result = await pythonToolsExecutionService.execForLinter( executionInfo, { cwd, token: cancellation, mergeStdOutErr: false }, document.uri, diff --git a/src/client/linters/flake8.ts b/src/client/linters/flake8.ts index c034cfb60639..2a68ec045908 100644 --- a/src/client/linters/flake8.ts +++ b/src/client/linters/flake8.ts @@ -13,11 +13,7 @@ export class Flake8 extends BaseLinter { } protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { - const messages = await this.run( - ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], - document, - cancellation, - ); + const messages = await this.run([document.uri.fsPath], document, cancellation); messages.forEach((msg) => { msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.flake8CategorySeverity); // flake8 uses 0th line for some file-wide problems diff --git a/src/client/linters/linterInfo.ts b/src/client/linters/linterInfo.ts index 32f0312b9e1b..321f23b0f304 100644 --- a/src/client/linters/linterInfo.ts +++ b/src/client/linters/linterInfo.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import { Uri } from 'vscode'; +import { linterScript } from '../common/process/internal/scripts'; import { ExecutionInfo, IConfigurationService, ILintingSettings, Product } from '../common/types'; import { ILinterInfo, LinterId } from './types'; @@ -74,13 +75,20 @@ export class LinterInfo implements ILinterInfo { public getExecutionInfo(customArgs: string[], resource?: Uri): ExecutionInfo { const execPath = this.pathName(resource); const args = this.linterArgs(resource).concat(customArgs); - let moduleName: string | undefined; - - // If path information is not available, then treat it as a module, + const script = linterScript(); if (path.basename(execPath) === execPath) { - moduleName = execPath; + return { + execPath: undefined, + args: [script, '-m', this.id, ...args], + product: this.product, + moduleName: execPath, + }; } - - return { execPath, moduleName, args, product: this.product }; + return { + execPath, + moduleName: this.id, + args: [script, '-p', this.id, execPath, ...args], + product: this.product, + }; } } diff --git a/src/client/linters/prospector.ts b/src/client/linters/prospector.ts index 9b970d880bfc..fa4b3907255b 100644 --- a/src/client/linters/prospector.ts +++ b/src/client/linters/prospector.ts @@ -32,7 +32,7 @@ export class Prospector extends BaseLinter { protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { const cwd = this.getWorkingDirectoryPath(document); const relativePath = path.relative(cwd, document.uri.fsPath); - return this.run(['--absolute-paths', '--output-format=json', relativePath], document, cancellation); + return this.run([relativePath], document, cancellation); } protected async parseMessages( diff --git a/src/client/linters/pycodestyle.ts b/src/client/linters/pycodestyle.ts index c7f83a8ab974..30517980e83c 100644 --- a/src/client/linters/pycodestyle.ts +++ b/src/client/linters/pycodestyle.ts @@ -13,11 +13,7 @@ export class Pycodestyle extends BaseLinter { } protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { - const messages = await this.run( - ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], - document, - cancellation, - ); + const messages = await this.run([document.uri.fsPath], document, cancellation); messages.forEach((msg) => { msg.severity = this.parseMessagesSeverity( msg.type, diff --git a/src/client/linters/pylama.ts b/src/client/linters/pylama.ts index c01bdf80261e..6431d55de348 100644 --- a/src/client/linters/pylama.ts +++ b/src/client/linters/pylama.ts @@ -15,7 +15,7 @@ export class PyLama extends BaseLinter { } protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { - const messages = await this.run(['--format=parsable', document.uri.fsPath], document, cancellation, REGEX); + const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX); // All messages in pylama are treated as warnings for now. messages.forEach((msg) => { msg.severity = LintMessageSeverity.Warning; diff --git a/src/client/linters/pylint.ts b/src/client/linters/pylint.ts index e1b9662f530d..911ea7af9ff3 100644 --- a/src/client/linters/pylint.ts +++ b/src/client/linters/pylint.ts @@ -27,12 +27,11 @@ export class Pylint extends BaseLinter { protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { const { uri } = document; const settings = this.configService.getSettings(uri); - const args = ['--reports=n', '--output-format=json', uri.fsPath]; + const args = [uri.fsPath]; const messages = await this.run(args, document, cancellation); messages.forEach((msg) => { msg.severity = this.parseMessagesSeverity(msg.type, settings.linting.pylintCategorySeverity); }); - return messages; } diff --git a/src/test/linters/lint.args.test.ts b/src/test/linters/lint.args.test.ts index cf2a000720d0..1e4a07f03842 100644 --- a/src/test/linters/lint.args.test.ts +++ b/src/test/linters/lint.args.test.ts @@ -142,12 +142,12 @@ suite('Linting - Arguments', () => { } test('Flake8', async () => { const linter = new Flake8(serviceContainer); - const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath]; + const expectedArgs = [fileUri.fsPath]; await testLinter(linter, expectedArgs); }); test('Pycodestyle', async () => { const linter = new Pycodestyle(serviceContainer); - const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath]; + const expectedArgs = [fileUri.fsPath]; await testLinter(linter, expectedArgs); }); test('Prospector', async () => { @@ -155,12 +155,12 @@ suite('Linting - Arguments', () => { const expectedPath = workspaceUri ? fileUri.fsPath.substring(workspaceUri.length + 2) : path.basename(fileUri.fsPath); - const expectedArgs = ['--absolute-paths', '--output-format=json', expectedPath]; + const expectedArgs = [expectedPath]; await testLinter(linter, expectedArgs); }); test('Pylama', async () => { const linter = new PyLama(serviceContainer); - const expectedArgs = ['--format=parsable', fileUri.fsPath]; + const expectedArgs = [fileUri.fsPath]; await testLinter(linter, expectedArgs); }); test('MyPy', async () => { @@ -175,29 +175,12 @@ suite('Linting - Arguments', () => { }); test('Pylint', async () => { const linter = new Pylint(serviceContainer); - document.setup((d) => d.uri).returns(() => fileUri); - - let invoked = false; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (linter as any).run = (args: any[]) => { - expect(args[args.length - 1]).to.equal(fileUri.fsPath); - invoked = true; - return Promise.resolve([]); - }; - await linter.lint(document.object, cancellationToken); - expect(invoked).to.be.equal(true, 'method not invoked'); + const expectedArgs = [fileUri.fsPath]; + await testLinter(linter, expectedArgs); }); test('Bandit', async () => { const linter = new Bandit(serviceContainer); - const expectedArgs = [ - '-f', - 'custom', - '--msg-template', - '{line},{col},{severity},{test_id}:{msg}', - '-n', - '-1', - fileUri.fsPath, - ]; + const expectedArgs = [fileUri.fsPath]; await testLinter(linter, expectedArgs); }); }, diff --git a/src/test/linters/lint.functional.test.ts b/src/test/linters/lint.functional.test.ts index 01ab1c9bf8db..c33db2d89565 100644 --- a/src/test/linters/lint.functional.test.ts +++ b/src/test/linters/lint.functional.test.ts @@ -759,7 +759,7 @@ suite('Linting Functional Tests', () => { sinon.restore(); }); - const pythonPath = childProcess.execSync(`${PYTHON_PATH} -c "import sys;print(sys.executable)"`); + const pythonPath = childProcess.execSync(`"${PYTHON_PATH}" -c "import sys;print(sys.executable)"`); console.log(`Testing linter with python ${pythonPath}`); diff --git a/src/test/linters/lint.multilinter.test.ts b/src/test/linters/lint.multilinter.test.ts index acecbf35624d..dba263e78479 100644 --- a/src/test/linters/lint.multilinter.test.ts +++ b/src/test/linters/lint.multilinter.test.ts @@ -40,7 +40,7 @@ class MockPythonToolExecService extends PythonToolExecutionService { ]`; // Depending on moduleName being exec'd, return the appropriate sample. - public async exec( + public async execForLinter( executionInfo: ExecutionInfo, _options: SpawnOptions, _resource: Uri, diff --git a/src/test/linters/lint.unit.test.ts b/src/test/linters/lint.unit.test.ts index b5468b9a8b27..6363521b4ded 100644 --- a/src/test/linters/lint.unit.test.ts +++ b/src/test/linters/lint.unit.test.ts @@ -645,7 +645,7 @@ class TestFixture extends BaseTestFixture { public setStdout(stdout: string) { this.pythonToolExecService - .setup((s) => s.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .setup((s) => s.execForLinter(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve({ stdout })); } } diff --git a/src/test/linters/pylint.test.ts b/src/test/linters/pylint.test.ts index 0b2f83438c3e..b89dddc1cd51 100644 --- a/src/test/linters/pylint.test.ts +++ b/src/test/linters/pylint.test.ts @@ -130,7 +130,7 @@ suite('Linting - Pylint', () => { ']', ].join(os.EOL); execService - .setup((x) => x.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .setup((x) => x.execForLinter(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve({ stdout: linterOutput, stderr: '' })); const lintSettings = new MockLintingSettings(); diff --git a/src/test/linters/pylint.unit.test.ts b/src/test/linters/pylint.unit.test.ts index 2648dacd9ab5..60b8f20aec20 100644 --- a/src/test/linters/pylint.unit.test.ts +++ b/src/test/linters/pylint.unit.test.ts @@ -26,7 +26,7 @@ suite('Pylint - Function runLinter()', () => { const doc = { uri: vscode.Uri.file('path/to/doc'), }; - const args = ['--reports=n', '--output-format=json', doc.uri.fsPath]; + const args = [doc.uri.fsPath]; class PylintTest extends Pylint { // eslint-disable-next-line class-methods-use-this public async run( diff --git a/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts b/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts index 65ba2a27f35f..5568006684ba 100644 --- a/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts +++ b/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts @@ -216,6 +216,7 @@ suite('Terminal - Django Shell Code Execution', () => { execModuleObservable: procs.execModuleObservable, exec: procs.exec, execModule: procs.execModule, + execForLinter: procs.execForLinter, }; const expectedTerminalArgs = [...terminalArgs, 'manage.py', 'shell']; pythonExecutionFactory diff --git a/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts b/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts index 8fcc308d99e8..416320c66b61 100644 --- a/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts +++ b/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts @@ -367,6 +367,7 @@ suite('Terminal - Code Execution', () => { execModuleObservable: procs.execModuleObservable, exec: procs.exec, execModule: procs.execModule, + execForLinter: procs.execForLinter, }; pythonExecutionFactory .setup((p) => @@ -479,6 +480,7 @@ suite('Terminal - Code Execution', () => { execModuleObservable: procs.execModuleObservable, exec: procs.exec, execModule: procs.execModule, + execForLinter: procs.execForLinter, }; pythonExecutionFactory .setup((p) =>