diff --git a/package.json b/package.json index c43bcf450b5d..bc0889529bdd 100644 --- a/package.json +++ b/package.json @@ -1859,4 +1859,4 @@ "publisherDisplayName": "Microsoft", "publisherId": "998b010b-e2af-44a5-a6cd-0b5fd3b9b6f8" } -} +} \ No newline at end of file diff --git a/src/client/common/editor.ts b/src/client/common/editor.ts index 24be7cb209fc..e839a13e6a8b 100644 --- a/src/client/common/editor.ts +++ b/src/client/common/editor.ts @@ -1,5 +1,6 @@ import * as dmp from 'diff-match-patch'; import * as fs from 'fs'; +import * as md5 from 'md5'; import { EOL } from 'os'; import * as path from 'path'; import { Position, Range, TextDocument, TextEdit, WorkspaceEdit } from 'vscode'; @@ -220,18 +221,19 @@ function getTextEditsInternal(before: string, diffs: [number, string][], startLi export function getTempFileWithDocumentContents(document: TextDocument): Promise { return new Promise((resolve, reject) => { const ext = path.extname(document.uri.fsPath); - // tslint:disable-next-line:no-shadowed-variable no-require-imports - const tmp = require('tmp'); - tmp.file({ postfix: ext }, (err, tmpFilePath, fd) => { - if (err) { - return reject(err); + // Don't create file in temp folder since external utilities + // look into configuration files in the workspace and are not able + // to find custom rules if file is saved in a random disk location. + // This means temp file has to be created in the same folder + // as the original one and then removed. + + // tslint:disable-next-line:no-require-imports + const fileName = `${document.uri.fsPath}.${md5(document.uri.fsPath)}${ext}`; + fs.writeFile(fileName, document.getText(), ex => { + if (ex) { + reject(`Failed to create a temporary file, ${ex.message}`); } - fs.writeFile(tmpFilePath, document.getText(), ex => { - if (ex) { - return reject(`Failed to create a temporary file, ${ex.message}`); - } - resolve(tmpFilePath); - }); + resolve(fileName); }); }); } diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index f678665fc02d..9e91b00a5a19 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -4,6 +4,7 @@ import * as vscode from 'vscode'; import { OutputChannel, TextEdit, Uri } from 'vscode'; import { IWorkspaceService } from '../common/application/types'; import { STANDARD_OUTPUT_CHANNEL } from '../common/constants'; +import '../common/extensions'; import { isNotInstalledError } from '../common/helpers'; import { IPythonToolExecutionService } from '../common/process/types'; import { IInstaller, IOutputChannel, Product } from '../common/types'; @@ -50,26 +51,24 @@ export abstract class BaseFormatter { // However they don't support returning the diff of the formatted text when reading data from the input stream. // Yes getting text formatted that way avoids having to create a temporary file, however the diffing will have // to be done here in node (extension), i.e. extension cpu, i.e. les responsive solution. - const tmpFileCreated = document.isDirty; - const filePromise = tmpFileCreated ? getTempFileWithDocumentContents(document) : Promise.resolve(document.fileName); - const filePath = await filePromise; - if (token && token.isCancellationRequested) { + const tempFile = await this.createTempFile(document); + if (this.checkCancellation(document.fileName, tempFile, token)) { return []; } const executionInfo = this.helper.getExecutionInfo(this.product, args, document.uri); - executionInfo.args.push(filePath); + executionInfo.args.push(tempFile); const pythonToolsExecutionService = this.serviceContainer.get(IPythonToolExecutionService); const promise = pythonToolsExecutionService.exec(executionInfo, { cwd, throwOnStdErr: true, token }, document.uri) .then(output => output.stdout) .then(data => { - if (token && token.isCancellationRequested) { + if (this.checkCancellation(document.fileName, tempFile, token)) { return [] as TextEdit[]; } return getTextEditsFromPatch(document.getText(), data); }) .catch(error => { - if (token && token.isCancellationRequested) { + if (this.checkCancellation(document.fileName, tempFile, token)) { return [] as TextEdit[]; } // tslint:disable-next-line:no-empty @@ -77,10 +76,7 @@ export abstract class BaseFormatter { return [] as TextEdit[]; }) .then(edits => { - // Delete the temporary file created - if (tmpFileCreated) { - fs.unlinkSync(filePath); - } + this.deleteTempFile(document.fileName, tempFile).ignoreErrors(); return edits; }); vscode.window.setStatusBarMessage(`Formatting with ${this.Id}`, promise); @@ -101,4 +97,25 @@ export abstract class BaseFormatter { this.outputChannel.appendLine(`\n${customError}\n${error}`); } + + private async createTempFile(document: vscode.TextDocument): Promise { + return document.isDirty + ? await getTempFileWithDocumentContents(document) + : document.fileName; + } + + private deleteTempFile(originalFile: string, tempFile: string): Promise { + if (originalFile !== tempFile) { + return fs.unlink(tempFile); + } + return Promise.resolve(); + } + + private checkCancellation(originalFile: string, tempFile: string, token?: vscode.CancellationToken): boolean { + if (token && token.isCancellationRequested) { + this.deleteTempFile(originalFile, tempFile).ignoreErrors(); + return true; + } + return false; + } } diff --git a/src/client/linters/pylint.ts b/src/client/linters/pylint.ts index 53caae8a8669..b5aaf43c3743 100644 --- a/src/client/linters/pylint.ts +++ b/src/client/linters/pylint.ts @@ -2,6 +2,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import * as os from 'os'; import * as path from 'path'; import { OutputChannel } from 'vscode'; import { CancellationToken, TextDocument } from 'vscode'; @@ -76,13 +77,12 @@ export class Pylint extends BaseLinter { return true; } - let dir = folder; - if (await fs.fileExistsAsync(path.join(dir, pylintrc)) || await fs.fileExistsAsync(path.join(dir, dotPylintrc))) { + if (await fs.fileExistsAsync(path.join(folder, pylintrc)) || await fs.fileExistsAsync(path.join(folder, dotPylintrc))) { return true; } - let current = dir; - let above = path.dirname(dir); + let current = folder; + let above = path.dirname(folder); do { if (!await fs.fileExistsAsync(path.join(current, '__init__.py'))) { break; @@ -94,11 +94,11 @@ export class Pylint extends BaseLinter { above = path.dirname(above); } while (!fs.arePathsSame(current, above)); - dir = path.resolve('~'); - if (await fs.fileExistsAsync(path.join(dir, dotPylintrc))) { + const home = os.homedir(); + if (await fs.fileExistsAsync(path.join(home, dotPylintrc))) { return true; } - if (await fs.fileExistsAsync(path.join(dir, '.config', pylintrc))) { + if (await fs.fileExistsAsync(path.join(home, '.config', pylintrc))) { return true; } diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index 8a9d09dff517..bb90c3080e5c 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -23,6 +23,7 @@ const yapfFileToAutoFormat = path.join(formatFilesPath, 'yapfFileToAutoFormat.py let formattedYapf = ''; let formattedAutoPep8 = ''; +// tslint:disable-next-line:max-func-body-length suite('Formatting', () => { let ioc: UnitTestIocContainer; @@ -94,7 +95,53 @@ suite('Formatting', () => { }); compareFiles(formattedContents, textEditor.document.getText()); } - test('AutoPep8', async () => await testFormatting(new AutoPep8Formatter(ioc.serviceContainer), formattedAutoPep8, autoPep8FileToFormat, 'autopep8.output')); + test('AutoPep8', async () => await testFormatting(new AutoPep8Formatter(ioc.serviceContainer), formattedAutoPep8, autoPep8FileToFormat, 'autopep8.output')); test('Yapf', async () => await testFormatting(new YapfFormatter(ioc.serviceContainer), formattedYapf, yapfFileToFormat, 'yapf.output')); + + test('Yapf on dirty file', async () => { + const sourceDir = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'formatting'); + const targetDir = path.join(__dirname, '..', 'pythonFiles', 'formatting'); + + const originalName = 'formatWhenDirty.py'; + const resultsName = 'formatWhenDirtyResult.py'; + const fileToFormat = path.join(targetDir, originalName); + const formattedFile = path.join(targetDir, resultsName); + + if (!fs.pathExistsSync(targetDir)) { + fs.mkdirSync(targetDir); + } + fs.copySync(path.join(sourceDir, originalName), fileToFormat, { overwrite: true }); + fs.copySync(path.join(sourceDir, resultsName), formattedFile, { overwrite: true }); + + const textDocument = await vscode.workspace.openTextDocument(fileToFormat); + const textEditor = await vscode.window.showTextDocument(textDocument); + await textEditor.edit(builder => { + // Make file dirty. Trailing blanks will be removed. + builder.insert(new vscode.Position(0, 0), '\n \n'); + }); + + const dir = path.dirname(fileToFormat); + const configFile = path.join(dir, '.style.yapf'); + try { + // Create yapf configuration file + const content = '[style]\nbased_on_style = pep8\nindent_width=5\n'; + fs.writeFileSync(configFile, content); + + const options = { insertSpaces: textEditor.options.insertSpaces! as boolean, tabSize: 1 }; + const formatter = new YapfFormatter(ioc.serviceContainer); + const edits = await formatter.formatDocument(textDocument, options, new CancellationTokenSource().token); + await textEditor.edit(editBuilder => { + edits.forEach(edit => editBuilder.replace(edit.range, edit.newText)); + }); + + const expected = fs.readFileSync(formattedFile).toString(); + const actual = textEditor.document.getText(); + compareFiles(expected, actual); + } finally { + if (fs.existsSync(configFile)) { + fs.unlinkSync(configFile); + } + } + }); }); diff --git a/src/test/linters/pylint.test.ts b/src/test/linters/pylint.test.ts index 75cd78f148f3..ce08b0ac5c95 100644 --- a/src/test/linters/pylint.test.ts +++ b/src/test/linters/pylint.test.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import { Container } from 'inversify'; +import * as os from 'os'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { CancellationTokenSource, OutputChannel, TextDocument, Uri, WorkspaceFolder } from 'vscode'; @@ -100,7 +101,7 @@ suite('Linting - Pylintrc search', () => { expect(result).to.be.equal(true, `'${dotPylintrc}' not detected in the module tree.`); }); test('.pylintrc up the ~ folder', async () => { - const home = path.resolve('~'); + const home = os.homedir(); const rc = path.join(home, dotPylintrc); fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true)); @@ -108,7 +109,7 @@ suite('Linting - Pylintrc search', () => { expect(result).to.be.equal(true, `'${dotPylintrc}' not detected in the ~ folder.`); }); test('pylintrc up the ~/.config folder', async () => { - const home = path.resolve('~'); + const home = os.homedir(); const rc = path.join(home, '.config', pylintrc); fileSystem.setup(x => x.fileExistsAsync(rc)).returns(() => Promise.resolve(true)); diff --git a/src/test/pythonFiles/formatting/formatWhenDirty.py b/src/test/pythonFiles/formatting/formatWhenDirty.py new file mode 100644 index 000000000000..3fe1b80fde86 --- /dev/null +++ b/src/test/pythonFiles/formatting/formatWhenDirty.py @@ -0,0 +1,3 @@ +x = 0 +if x > 0: + x = 1 diff --git a/src/test/pythonFiles/formatting/formatWhenDirtyResult.py b/src/test/pythonFiles/formatting/formatWhenDirtyResult.py new file mode 100644 index 000000000000..d0ae06a2a59b --- /dev/null +++ b/src/test/pythonFiles/formatting/formatWhenDirtyResult.py @@ -0,0 +1,3 @@ +x = 0 +if x > 0: + x = 1