Skip to content

Fix linting under conda run #18390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/18364.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure linting works under `conda run` (work-around for https://github.com/conda/conda/issues/10972).
56 changes: 56 additions & 0 deletions pythonFiles/linter.py
Original file line number Diff line number Diff line change
@@ -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()
7 changes: 7 additions & 0 deletions src/client/common/process/internal/scripts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
1 change: 1 addition & 0 deletions src/client/common/process/pythonExecutionFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
}
20 changes: 20 additions & 0 deletions src/client/common/process/pythonProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,26 @@ class PythonProcessService {

return result;
}

public async execForLinter(
moduleName: string,
args: string[],
options: SpawnOptions,
): Promise<ExecutionResult<string>> {
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(
Expand Down
19 changes: 19 additions & 0 deletions src/client/common/process/pythonToolService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ExecutionResult<string>> {
if (options.env) {
throw new Error('Environment variables are not supported');
}
const pythonExecutionService = await this.serviceContainer
.get<IPythonExecutionFactory>(IPythonExecutionFactory)
.create({ resource });

if (executionInfo.execPath) {
return pythonExecutionService.exec(executionInfo.args, options);
}

return pythonExecutionService.execForLinter(executionInfo.moduleName!, executionInfo.args, options);
}
}
2 changes: 2 additions & 0 deletions src/client/common/process/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export interface IPythonExecutionService {

exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
execModule(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
execForLinter(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
}

export class StdErrError extends Error {
Expand All @@ -117,4 +118,5 @@ export interface IPythonToolExecutionService {
resource: Uri,
): Promise<ObservableExecutionResult<string>>;
exec(executionInfo: ExecutionInfo, options: SpawnOptions, resource: Uri): Promise<ExecutionResult<string>>;
execForLinter(executionInfo: ExecutionInfo, options: SpawnOptions, resource: Uri): Promise<ExecutionResult<string>>;
}
15 changes: 1 addition & 14 deletions src/client/linters/bandit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,7 @@ export class Bandit extends BaseLinter {

protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
// 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];
Expand Down
2 changes: 1 addition & 1 deletion src/client/linters/baseLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 1 addition & 5 deletions src/client/linters/flake8.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ export class Flake8 extends BaseLinter {
}

protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
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
Expand Down
20 changes: 14 additions & 6 deletions src/client/linters/linterInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
};
}
}
2 changes: 1 addition & 1 deletion src/client/linters/prospector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class Prospector extends BaseLinter {
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
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(
Expand Down
6 changes: 1 addition & 5 deletions src/client/linters/pycodestyle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ export class Pycodestyle extends BaseLinter {
}

protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
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,
Expand Down
2 changes: 1 addition & 1 deletion src/client/linters/pylama.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class PyLama extends BaseLinter {
}

protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
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;
Expand Down
3 changes: 1 addition & 2 deletions src/client/linters/pylint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@ export class Pylint extends BaseLinter {
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
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;
}

Expand Down
31 changes: 7 additions & 24 deletions src/test/linters/lint.args.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,25 +142,25 @@ 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 () => {
const linter = new Prospector(serviceContainer);
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 () => {
Expand All @@ -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);
});
},
Expand Down
2 changes: 1 addition & 1 deletion src/test/linters/lint.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);

Expand Down
2 changes: 1 addition & 1 deletion src/test/linters/lint.multilinter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/test/linters/lint.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/linters/pylint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/test/linters/pylint.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ suite('Terminal - Code Execution', () => {
execModuleObservable: procs.execModuleObservable,
exec: procs.exec,
execModule: procs.execModule,
execForLinter: procs.execForLinter,
};
pythonExecutionFactory
.setup((p) =>
Expand Down Expand Up @@ -479,6 +480,7 @@ suite('Terminal - Code Execution', () => {
execModuleObservable: procs.execModuleObservable,
exec: procs.exec,
execModule: procs.execModule,
execForLinter: procs.execForLinter,
};
pythonExecutionFactory
.setup((p) =>
Expand Down