Skip to content

Commit 4758b35

Browse files
karthiknadigkarthiknadig
authored
Fix linting under conda run (#18390)
* Fix linting under conda run. * Fix tests * Fix for 2.7
1 parent f7ab5b5 commit 4758b35

23 files changed

+142
-64
lines changed

‎news/2 Fixes/18364.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure linting works under `conda run` (work-around for https://github.com/conda/conda/issues/10972).

‎pythonFiles/linter.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import subprocess
2+
import sys
3+
4+
5+
linter_settings = {
6+
"pylint": {
7+
"args": ["--reports=n", "--output-format=json"],
8+
},
9+
"flake8": {
10+
"args": ["--format", "%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s"],
11+
},
12+
"bandit": {
13+
"args": [
14+
"-f",
15+
"custom",
16+
"--msg-template",
17+
"{line},{col},{severity},{test_id}:{msg}",
18+
"-n",
19+
"-1",
20+
],
21+
},
22+
"mypy": {"args": []},
23+
"prospector": {
24+
"args": ["--absolute-paths", "--output-format=json"],
25+
},
26+
"pycodestyle": {
27+
"args": ["--format", "%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s"],
28+
},
29+
"pydocstyle": {
30+
"args": [],
31+
},
32+
"pylama": {"args": ["--format=parsable"]},
33+
}
34+
35+
36+
def main():
37+
invoke = sys.argv[1]
38+
if invoke == "-m":
39+
linter = sys.argv[2]
40+
args = (
41+
[sys.executable, "-m", linter]
42+
+ linter_settings[linter]["args"]
43+
+ sys.argv[3:]
44+
)
45+
else:
46+
linter = sys.argv[2]
47+
args = [sys.argv[3]] + linter_settings[linter]["args"] + sys.argv[4:]
48+
49+
if hasattr(subprocess, "run"):
50+
subprocess.run(args, encoding="utf-8", stdout=sys.stdout, stderr=sys.stderr)
51+
else:
52+
subprocess.call(args, stdout=sys.stdout, stderr=sys.stderr)
53+
54+
55+
if __name__ == "__main__":
56+
main()

‎src/client/common/process/internal/scripts/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,3 +144,10 @@ export function tensorboardLauncher(args: string[]): string[] {
144144
const script = path.join(SCRIPTS_DIR, 'tensorboard_launcher.py');
145145
return [script, ...args];
146146
}
147+
148+
// linter.py
149+
150+
export function linterScript(): string {
151+
const script = path.join(SCRIPTS_DIR, 'linter.py');
152+
return script;
153+
}

‎src/client/common/process/pythonExecutionFactory.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,5 +191,6 @@ function createPythonService(
191191
execModuleObservable: (m, a, o) => procs.execModuleObservable(m, a, o),
192192
exec: (a, o) => procs.exec(a, o),
193193
execModule: (m, a, o) => procs.execModule(m, a, o),
194+
execForLinter: (m, a, o) => procs.execForLinter(m, a, o),
194195
};
195196
}

‎src/client/common/process/pythonProcess.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,26 @@ class PythonProcessService {
6464

6565
return result;
6666
}
67+
68+
public async execForLinter(
69+
moduleName: string,
70+
args: string[],
71+
options: SpawnOptions,
72+
): Promise<ExecutionResult<string>> {
73+
const opts: SpawnOptions = { ...options };
74+
const executable = this.deps.getExecutionInfo(args);
75+
const result = await this.deps.exec(executable.command, executable.args, opts);
76+
77+
// If a module is not installed we'll have something in stderr.
78+
if (moduleName && ErrorUtils.outputHasModuleNotInstalledError(moduleName, result.stderr)) {
79+
const isInstalled = await this.deps.isModuleInstalled(moduleName);
80+
if (!isInstalled) {
81+
throw new ModuleNotInstalledError(moduleName);
82+
}
83+
}
84+
85+
return result;
86+
}
6787
}
6888

6989
export function createPythonProcessService(

‎src/client/common/process/pythonToolService.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,23 @@ export class PythonToolExecutionService implements IPythonToolExecutionService {
5757
return processService.exec(executionInfo.execPath!, executionInfo.args, { ...options });
5858
}
5959
}
60+
61+
public async execForLinter(
62+
executionInfo: ExecutionInfo,
63+
options: SpawnOptions,
64+
resource: Uri,
65+
): Promise<ExecutionResult<string>> {
66+
if (options.env) {
67+
throw new Error('Environment variables are not supported');
68+
}
69+
const pythonExecutionService = await this.serviceContainer
70+
.get<IPythonExecutionFactory>(IPythonExecutionFactory)
71+
.create({ resource });
72+
73+
if (executionInfo.execPath) {
74+
return pythonExecutionService.exec(executionInfo.args, options);
75+
}
76+
77+
return pythonExecutionService.execForLinter(executionInfo.moduleName!, executionInfo.args, options);
78+
}
6079
}

‎src/client/common/process/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ export interface IPythonExecutionService {
100100

101101
exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
102102
execModule(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
103+
execForLinter(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
103104
}
104105

105106
export class StdErrError extends Error {
@@ -117,4 +118,5 @@ export interface IPythonToolExecutionService {
117118
resource: Uri,
118119
): Promise<ObservableExecutionResult<string>>;
119120
exec(executionInfo: ExecutionInfo, options: SpawnOptions, resource: Uri): Promise<ExecutionResult<string>>;
121+
execForLinter(executionInfo: ExecutionInfo, options: SpawnOptions, resource: Uri): Promise<ExecutionResult<string>>;
120122
}

‎src/client/linters/bandit.ts

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,7 @@ export class Bandit extends BaseLinter {
2626

2727
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
2828
// View all errors in bandit <= 1.5.1 (https://github.com/PyCQA/bandit/issues/371)
29-
const messages = await this.run(
30-
[
31-
'-f',
32-
'custom',
33-
'--msg-template',
34-
'{line},{col},{severity},{test_id}:{msg}',
35-
'-n',
36-
'-1',
37-
document.uri.fsPath,
38-
],
39-
document,
40-
cancellation,
41-
BANDIT_REGEX,
42-
);
29+
const messages = await this.run([document.uri.fsPath], document, cancellation, BANDIT_REGEX);
4330

4431
messages.forEach((msg) => {
4532
msg.severity = severityMapping[msg.type];

‎src/client/linters/baseLinter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ export abstract class BaseLinter implements ILinter {
165165
IPythonToolExecutionService,
166166
);
167167
try {
168-
const result = await pythonToolsExecutionService.exec(
168+
const result = await pythonToolsExecutionService.execForLinter(
169169
executionInfo,
170170
{ cwd, token: cancellation, mergeStdOutErr: false },
171171
document.uri,

‎src/client/linters/flake8.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,7 @@ export class Flake8 extends BaseLinter {
1313
}
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
16-
const messages = await this.run(
17-
['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
18-
document,
19-
cancellation,
20-
);
16+
const messages = await this.run([document.uri.fsPath], document, cancellation);
2117
messages.forEach((msg) => {
2218
msg.severity = this.parseMessagesSeverity(msg.type, this.pythonSettings.linting.flake8CategorySeverity);
2319
// flake8 uses 0th line for some file-wide problems

‎src/client/linters/linterInfo.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import * as path from 'path';
55
import { Uri } from 'vscode';
6+
import { linterScript } from '../common/process/internal/scripts';
67
import { ExecutionInfo, IConfigurationService, ILintingSettings, Product } from '../common/types';
78
import { ILinterInfo, LinterId } from './types';
89

@@ -74,13 +75,20 @@ export class LinterInfo implements ILinterInfo {
7475
public getExecutionInfo(customArgs: string[], resource?: Uri): ExecutionInfo {
7576
const execPath = this.pathName(resource);
7677
const args = this.linterArgs(resource).concat(customArgs);
77-
let moduleName: string | undefined;
78-
79-
// If path information is not available, then treat it as a module,
78+
const script = linterScript();
8079
if (path.basename(execPath) === execPath) {
81-
moduleName = execPath;
80+
return {
81+
execPath: undefined,
82+
args: [script, '-m', this.id, ...args],
83+
product: this.product,
84+
moduleName: execPath,
85+
};
8286
}
83-
84-
return { execPath, moduleName, args, product: this.product };
87+
return {
88+
execPath,
89+
moduleName: this.id,
90+
args: [script, '-p', this.id, execPath, ...args],
91+
product: this.product,
92+
};
8593
}
8694
}

‎src/client/linters/prospector.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class Prospector extends BaseLinter {
3232
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
3333
const cwd = this.getWorkingDirectoryPath(document);
3434
const relativePath = path.relative(cwd, document.uri.fsPath);
35-
return this.run(['--absolute-paths', '--output-format=json', relativePath], document, cancellation);
35+
return this.run([relativePath], document, cancellation);
3636
}
3737

3838
protected async parseMessages(

‎src/client/linters/pycodestyle.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,7 @@ export class Pycodestyle extends BaseLinter {
1313
}
1414

1515
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
16-
const messages = await this.run(
17-
['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath],
18-
document,
19-
cancellation,
20-
);
16+
const messages = await this.run([document.uri.fsPath], document, cancellation);
2117
messages.forEach((msg) => {
2218
msg.severity = this.parseMessagesSeverity(
2319
msg.type,

‎src/client/linters/pylama.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export class PyLama extends BaseLinter {
1515
}
1616

1717
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
18-
const messages = await this.run(['--format=parsable', document.uri.fsPath], document, cancellation, REGEX);
18+
const messages = await this.run([document.uri.fsPath], document, cancellation, REGEX);
1919
// All messages in pylama are treated as warnings for now.
2020
messages.forEach((msg) => {
2121
msg.severity = LintMessageSeverity.Warning;

‎src/client/linters/pylint.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,11 @@ export class Pylint extends BaseLinter {
2727
protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise<ILintMessage[]> {
2828
const { uri } = document;
2929
const settings = this.configService.getSettings(uri);
30-
const args = ['--reports=n', '--output-format=json', uri.fsPath];
30+
const args = [uri.fsPath];
3131
const messages = await this.run(args, document, cancellation);
3232
messages.forEach((msg) => {
3333
msg.severity = this.parseMessagesSeverity(msg.type, settings.linting.pylintCategorySeverity);
3434
});
35-
3635
return messages;
3736
}
3837

‎src/test/linters/lint.args.test.ts

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -142,25 +142,25 @@ suite('Linting - Arguments', () => {
142142
}
143143
test('Flake8', async () => {
144144
const linter = new Flake8(serviceContainer);
145-
const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
145+
const expectedArgs = [fileUri.fsPath];
146146
await testLinter(linter, expectedArgs);
147147
});
148148
test('Pycodestyle', async () => {
149149
const linter = new Pycodestyle(serviceContainer);
150-
const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath];
150+
const expectedArgs = [fileUri.fsPath];
151151
await testLinter(linter, expectedArgs);
152152
});
153153
test('Prospector', async () => {
154154
const linter = new Prospector(serviceContainer);
155155
const expectedPath = workspaceUri
156156
? fileUri.fsPath.substring(workspaceUri.length + 2)
157157
: path.basename(fileUri.fsPath);
158-
const expectedArgs = ['--absolute-paths', '--output-format=json', expectedPath];
158+
const expectedArgs = [expectedPath];
159159
await testLinter(linter, expectedArgs);
160160
});
161161
test('Pylama', async () => {
162162
const linter = new PyLama(serviceContainer);
163-
const expectedArgs = ['--format=parsable', fileUri.fsPath];
163+
const expectedArgs = [fileUri.fsPath];
164164
await testLinter(linter, expectedArgs);
165165
});
166166
test('MyPy', async () => {
@@ -175,29 +175,12 @@ suite('Linting - Arguments', () => {
175175
});
176176
test('Pylint', async () => {
177177
const linter = new Pylint(serviceContainer);
178-
document.setup((d) => d.uri).returns(() => fileUri);
179-
180-
let invoked = false;
181-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
182-
(linter as any).run = (args: any[]) => {
183-
expect(args[args.length - 1]).to.equal(fileUri.fsPath);
184-
invoked = true;
185-
return Promise.resolve([]);
186-
};
187-
await linter.lint(document.object, cancellationToken);
188-
expect(invoked).to.be.equal(true, 'method not invoked');
178+
const expectedArgs = [fileUri.fsPath];
179+
await testLinter(linter, expectedArgs);
189180
});
190181
test('Bandit', async () => {
191182
const linter = new Bandit(serviceContainer);
192-
const expectedArgs = [
193-
'-f',
194-
'custom',
195-
'--msg-template',
196-
'{line},{col},{severity},{test_id}:{msg}',
197-
'-n',
198-
'-1',
199-
fileUri.fsPath,
200-
];
183+
const expectedArgs = [fileUri.fsPath];
201184
await testLinter(linter, expectedArgs);
202185
});
203186
},

‎src/test/linters/lint.functional.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ suite('Linting Functional Tests', () => {
759759
sinon.restore();
760760
});
761761

762-
const pythonPath = childProcess.execSync(`${PYTHON_PATH} -c "import sys;print(sys.executable)"`);
762+
const pythonPath = childProcess.execSync(`"${PYTHON_PATH}" -c "import sys;print(sys.executable)"`);
763763

764764
console.log(`Testing linter with python ${pythonPath}`);
765765

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class MockPythonToolExecService extends PythonToolExecutionService {
4040
]`;
4141

4242
// Depending on moduleName being exec'd, return the appropriate sample.
43-
public async exec(
43+
public async execForLinter(
4444
executionInfo: ExecutionInfo,
4545
_options: SpawnOptions,
4646
_resource: Uri,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ class TestFixture extends BaseTestFixture {
645645

646646
public setStdout(stdout: string) {
647647
this.pythonToolExecService
648-
.setup((s) => s.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
648+
.setup((s) => s.execForLinter(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
649649
.returns(() => Promise.resolve({ stdout }));
650650
}
651651
}

‎src/test/linters/pylint.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ suite('Linting - Pylint', () => {
130130
']',
131131
].join(os.EOL);
132132
execService
133-
.setup((x) => x.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
133+
.setup((x) => x.execForLinter(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
134134
.returns(() => Promise.resolve({ stdout: linterOutput, stderr: '' }));
135135

136136
const lintSettings = new MockLintingSettings();

‎src/test/linters/pylint.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ suite('Pylint - Function runLinter()', () => {
2626
const doc = {
2727
uri: vscode.Uri.file('path/to/doc'),
2828
};
29-
const args = ['--reports=n', '--output-format=json', doc.uri.fsPath];
29+
const args = [doc.uri.fsPath];
3030
class PylintTest extends Pylint {
3131
// eslint-disable-next-line class-methods-use-this
3232
public async run(

‎src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ suite('Terminal - Django Shell Code Execution', () => {
216216
execModuleObservable: procs.execModuleObservable,
217217
exec: procs.exec,
218218
execModule: procs.execModule,
219+
execForLinter: procs.execForLinter,
219220
};
220221
const expectedTerminalArgs = [...terminalArgs, 'manage.py', 'shell'];
221222
pythonExecutionFactory

‎src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,7 @@ suite('Terminal - Code Execution', () => {
367367
execModuleObservable: procs.execModuleObservable,
368368
exec: procs.exec,
369369
execModule: procs.execModule,
370+
execForLinter: procs.execForLinter,
370371
};
371372
pythonExecutionFactory
372373
.setup((p) =>
@@ -479,6 +480,7 @@ suite('Terminal - Code Execution', () => {
479480
execModuleObservable: procs.execModuleObservable,
480481
exec: procs.exec,
481482
execModule: procs.execModule,
483+
execForLinter: procs.execForLinter,
482484
};
483485
pythonExecutionFactory
484486
.setup((p) =>

0 commit comments

Comments
 (0)