Skip to content

Commit b5d59ff

Browse files
author
Mikhail Arkhipov
authored
* Fix pylint search * Handle quote escapes in strings * Escapes in strings * CR feedback * Missing pip * Test * Tests * Tests * Mac python path * Tests * Tests * Test * "Go To Python object" doesn't work * Proper detection and type population in virtual env * Test fixes * Simplify venv check * Remove duplicates * Test * Discover pylintrc better + tests * Undo change * CR feedback * Set interprereter before checking install * Fix typo and path compare on Windows * Rename method * microsoft#815 - 'F' in flake8 means warning * 730 - same folder temp * Properly resolve ~ * Test * Test
1 parent 02dc3ed commit b5d59ff

8 files changed

Lines changed: 106 additions & 33 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1859,4 +1859,4 @@
18591859
"publisherDisplayName": "Microsoft",
18601860
"publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8"
18611861
}
1862-
}
1862+
}

‎src/client/common/editor.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as dmp from 'diff-match-patch';
22
import * as fs from 'fs';
3+
import * as md5 from 'md5';
34
import { EOL } from 'os';
45
import * as path from 'path';
56
import { Position, Range, TextDocument, TextEdit, WorkspaceEdit } from 'vscode';
@@ -220,18 +221,19 @@ function getTextEditsInternal(before: string, diffs: [number, string][], startLi
220221
export function getTempFileWithDocumentContents(document: TextDocument): Promise<string> {
221222
return new Promise<string>((resolve, reject) => {
222223
const ext = path.extname(document.uri.fsPath);
223-
// tslint:disable-next-line:no-shadowed-variable no-require-imports
224-
const tmp = require('tmp');
225-
tmp.file({ postfix: ext }, (err, tmpFilePath, fd) => {
226-
if (err) {
227-
return reject(err);
224+
// Don't create file in temp folder since external utilities
225+
// look into configuration files in the workspace and are not able
226+
// to find custom rules if file is saved in a random disk location.
227+
// This means temp file has to be created in the same folder
228+
// as the original one and then removed.
229+
230+
// tslint:disable-next-line:no-require-imports
231+
const fileName = `${document.uri.fsPath}.${md5(document.uri.fsPath)}${ext}`;
232+
fs.writeFile(fileName, document.getText(), ex => {
233+
if (ex) {
234+
reject(`Failed to create a temporary file, ${ex.message}`);
228235
}
229-
fs.writeFile(tmpFilePath, document.getText(), ex => {
230-
if (ex) {
231-
return reject(`Failed to create a temporary file, ${ex.message}`);
232-
}
233-
resolve(tmpFilePath);
234-
});
236+
resolve(fileName);
235237
});
236238
});
237239
}

‎src/client/formatters/baseFormatter.ts‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as vscode from 'vscode';
44
import { OutputChannel, TextEdit, Uri } from 'vscode';
55
import { IWorkspaceService } from '../common/application/types';
66
import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
7+
import '../common/extensions';
78
import { isNotInstalledError } from '../common/helpers';
89
import { IPythonToolExecutionService } from '../common/process/types';
910
import { IInstaller, IOutputChannel, Product } from '../common/types';
@@ -50,37 +51,32 @@ export abstract class BaseFormatter {
5051
// However they don't support returning the diff of the formatted text when reading data from the input stream.
5152
// Yes getting text formatted that way avoids having to create a temporary file, however the diffing will have
5253
// to be done here in node (extension), i.e. extension cpu, i.e. les responsive solution.
53-
const tmpFileCreated = document.isDirty;
54-
const filePromise = tmpFileCreated ? getTempFileWithDocumentContents(document) : Promise.resolve(document.fileName);
55-
const filePath = await filePromise;
56-
if (token && token.isCancellationRequested) {
54+
const tempFile = await this.createTempFile(document);
55+
if (this.checkCancellation(document.fileName, tempFile, token)) {
5756
return [];
5857
}
5958

6059
const executionInfo = this.helper.getExecutionInfo(this.product, args, document.uri);
61-
executionInfo.args.push(filePath);
60+
executionInfo.args.push(tempFile);
6261
const pythonToolsExecutionService = this.serviceContainer.get<IPythonToolExecutionService>(IPythonToolExecutionService);
6362
const promise = pythonToolsExecutionService.exec(executionInfo, { cwd, throwOnStdErr: true, token }, document.uri)
6463
.then(output => output.stdout)
6564
.then(data => {
66-
if (token && token.isCancellationRequested) {
65+
if (this.checkCancellation(document.fileName, tempFile, token)) {
6766
return [] as TextEdit[];
6867
}
6968
return getTextEditsFromPatch(document.getText(), data);
7069
})
7170
.catch(error => {
72-
if (token && token.isCancellationRequested) {
71+
if (this.checkCancellation(document.fileName, tempFile, token)) {
7372
return [] as TextEdit[];
7473
}
7574
// tslint:disable-next-line:no-empty
7675
this.handleError(this.Id, error, document.uri).catch(() => { });
7776
return [] as TextEdit[];
7877
})
7978
.then(edits => {
80-
// Delete the temporary file created
81-
if (tmpFileCreated) {
82-
fs.unlinkSync(filePath);
83-
}
79+
this.deleteTempFile(document.fileName, tempFile).ignoreErrors();
8480
return edits;
8581
});
8682
vscode.window.setStatusBarMessage(`Formatting with ${this.Id}`, promise);
@@ -101,4 +97,25 @@ export abstract class BaseFormatter {
10197

10298
this.outputChannel.appendLine(`\n${customError}\n${error}`);
10399
}
100+
101+
private async createTempFile(document: vscode.TextDocument): Promise<string> {
102+
return document.isDirty
103+
? await getTempFileWithDocumentContents(document)
104+
: document.fileName;
105+
}
106+
107+
private deleteTempFile(originalFile: string, tempFile: string): Promise<void> {
108+
if (originalFile !== tempFile) {
109+
return fs.unlink(tempFile);
110+
}
111+
return Promise.resolve();
112+
}
113+
114+
private checkCancellation(originalFile: string, tempFile: string, token?: vscode.CancellationToken): boolean {
115+
if (token && token.isCancellationRequested) {
116+
this.deleteTempFile(originalFile, tempFile).ignoreErrors();
117+
return true;
118+
}
119+
return false;
120+
}
104121
}

‎src/client/linters/pylint.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Copyright (c) Microsoft Corporation. All rights reserved.
33
// Licensed under the MIT License.
44

5+
import * as os from 'os';
56
import * as path from 'path';
67
import { OutputChannel } from 'vscode';
78
import { CancellationToken, TextDocument } from 'vscode';
@@ -76,13 +77,12 @@ export class Pylint extends BaseLinter {
7677
return true;
7778
}
7879

79-
let dir = folder;
80-
if (await fs.fileExistsAsync(path.join(dir, pylintrc)) || await fs.fileExistsAsync(path.join(dir, dotPylintrc))) {
80+
if (await fs.fileExistsAsync(path.join(folder, pylintrc)) || await fs.fileExistsAsync(path.join(folder, dotPylintrc))) {
8181
return true;
8282
}
8383

84-
let current = dir;
85-
let above = path.dirname(dir);
84+
let current = folder;
85+
let above = path.dirname(folder);
8686
do {
8787
if (!await fs.fileExistsAsync(path.join(current, '__init__.py'))) {
8888
break;
@@ -94,11 +94,11 @@ export class Pylint extends BaseLinter {
9494
above = path.dirname(above);
9595
} while (!fs.arePathsSame(current, above));
9696

97-
dir = path.resolve('~');
98-
if (await fs.fileExistsAsync(path.join(dir, dotPylintrc))) {
97+
const home = os.homedir();
98+
if (await fs.fileExistsAsync(path.join(home, dotPylintrc))) {
9999
return true;
100100
}
101-
if (await fs.fileExistsAsync(path.join(dir, '.config', pylintrc))) {
101+
if (await fs.fileExistsAsync(path.join(home, '.config', pylintrc))) {
102102
return true;
103103
}
104104

‎src/test/format/extension.format.test.ts‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const yapfFileToAutoFormat = path.join(formatFilesPath, 'yapfFileToAutoFormat.py
2323
let formattedYapf = '';
2424
let formattedAutoPep8 = '';
2525

26+
// tslint:disable-next-line:max-func-body-length
2627
suite('Formatting', () => {
2728
let ioc: UnitTestIocContainer;
2829

@@ -94,7 +95,53 @@ suite('Formatting', () => {
9495
});
9596
compareFiles(formattedContents, textEditor.document.getText());
9697
}
97-
test('AutoPep8', async () => await testFormatting(new AutoPep8Formatter(ioc.serviceContainer), formattedAutoPep8, autoPep8FileToFormat, 'autopep8.output'));
9898

99+
test('AutoPep8', async () => await testFormatting(new AutoPep8Formatter(ioc.serviceContainer), formattedAutoPep8, autoPep8FileToFormat, 'autopep8.output'));
99100
test('Yapf', async () => await testFormatting(new YapfFormatter(ioc.serviceContainer), formattedYapf, yapfFileToFormat, 'yapf.output'));
101+
102+
test('Yapf on dirty file', async () => {
103+
const sourceDir = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'formatting');
104+
const targetDir = path.join(__dirname, '..', 'pythonFiles', 'formatting');
105+
106+
const originalName = 'formatWhenDirty.py';
107+
const resultsName = 'formatWhenDirtyResult.py';
108+
const fileToFormat = path.join(targetDir, originalName);
109+
const formattedFile = path.join(targetDir, resultsName);
110+
111+
if (!fs.pathExistsSync(targetDir)) {
112+
fs.mkdirSync(targetDir);
113+
}
114+
fs.copySync(path.join(sourceDir, originalName), fileToFormat, { overwrite: true });
115+
fs.copySync(path.join(sourceDir, resultsName), formattedFile, { overwrite: true });
116+
117+
const textDocument = await vscode.workspace.openTextDocument(fileToFormat);
118+
const textEditor = await vscode.window.showTextDocument(textDocument);
119+
await textEditor.edit(builder => {
120+
// Make file dirty. Trailing blanks will be removed.
121+
builder.insert(new vscode.Position(0, 0), '\n \n');
122+
});
123+
124+
const dir = path.dirname(fileToFormat);
125+
const configFile = path.join(dir, '.style.yapf');
126+
try {
127+
// Create yapf configuration file
128+
const content = '[style]\nbased_on_style = pep8\nindent_width=5\n';
129+
fs.writeFileSync(configFile, content);
130+
131+
const options = { insertSpaces: textEditor.options.insertSpaces! as boolean, tabSize: 1 };
132+
const formatter = new YapfFormatter(ioc.serviceContainer);
133+
const edits = await formatter.formatDocument(textDocument, options, new CancellationTokenSource().token);
134+
await textEditor.edit(editBuilder => {
135+
edits.forEach(edit => editBuilder.replace(edit.range, edit.newText));
136+
});
137+
138+
const expected = fs.readFileSync(formattedFile).toString();
139+
const actual = textEditor.document.getText();
140+
compareFiles(expected, actual);
141+
} finally {
142+
if (fs.existsSync(configFile)) {
143+
fs.unlinkSync(configFile);
144+
}
145+
}
146+
});
100147
});

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

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

44
import { expect } from 'chai';
55
import { Container } from 'inversify';
6+
import * as os from 'os';
67
import * as path from 'path';
78
import * as TypeMoq from 'typemoq';
89
import { CancellationTokenSource, OutputChannel, TextDocument, Uri, WorkspaceFolder } from 'vscode';
@@ -100,15 +101,15 @@ suite('Linting - Pylintrc search', () => {
100101
expect(result).to.be.equal(true, `'${dotPylintrc}' not detected in the module tree.`);
101102
});
102103
test('.pylintrc up the ~ folder', async () => {
103-
const home = path.resolve('~');
104+
const home = os.homedir();
104105
const rc = path.join(home, dotPylintrc);
105106
fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true));
106107

107108
const result = await Pylint.hasConfigurationFile(fileSystem.object, basePath, platformService.object);
108109
expect(result).to.be.equal(true, `'${dotPylintrc}' not detected in the ~ folder.`);
109110
});
110111
test('pylintrc up the ~/.config folder', async () => {
111-
const home = path.resolve('~');
112+
const home = os.homedir();
112113
const rc = path.join(home, '.config', pylintrc);
113114
fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true));
114115

‎src/test/pythonFiles/formatting/formatWhenDirty.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
x = 0
2+
if x > 0:
3+
x = 1

‎src/test/pythonFiles/formatting/formatWhenDirtyResult.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
x = 0
2+
if x > 0:
3+
x = 1

0 commit comments

Comments
 (0)