diff --git a/news/1 Enhancements/1153.md b/news/1 Enhancements/1153.md new file mode 100644 index 000000000000..9f19892e6d51 --- /dev/null +++ b/news/1 Enhancements/1153.md @@ -0,0 +1,2 @@ +Add support for the [Black formatter](https://pypi.org/project/black/) +(thanks to [Josh Smeaton](https://github.com/jarshwah) for the initial patch) diff --git a/package.json b/package.json index 3568488dee4f..96dc0a2eb896 100644 --- a/package.json +++ b/package.json @@ -1200,14 +1200,30 @@ "python.formatting.provider": { "type": "string", "default": "autopep8", - "description": "Provider for formatting. Possible options include 'autopep8' and 'yapf'.", + "description": "Provider for formatting. Possible options include 'autopep8', 'black', and 'yapf'.", "enum": [ "autopep8", + "black", "yapf", "none" ], "scope": "resource" }, + "python.formatting.blackArgs": { + "type": "array", + "description": "Arguments passed in. Each argument is a separate item in the array.", + "default": [], + "items": { + "type": "string" + }, + "scope": "resource" + }, + "python.formatting.blackPath": { + "type": "string", + "default": "black", + "description": "Path to Black, you can use a custom version of Black by modifying this setting to include the full path.", + "scope": "resource" + }, "python.formatting.yapfArgs": { "type": "array", "description": "Arguments passed in. Each argument is a separate item in the array.", diff --git a/requirements.txt b/requirements.txt index 71762bf0eae3..01c6059a10cb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ # but flake8 has a tighter pinning. flake8 autopep8 +black ; python_version>='3.6' yapf pylint pep8 diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 99c111d64f70..86d8ad57a218 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -34,12 +34,12 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { public venvFolders: string[] = []; public devOptions: string[] = []; public linting?: ILintingSettings; - public formatting?: IFormattingSettings; + public formatting!: IFormattingSettings; public autoComplete?: IAutoCompleteSettings; - public unitTest?: IUnitTestSettings; + public unitTest!: IUnitTestSettings; public terminal!: ITerminalSettings; public sortImports?: ISortImportSettings; - public workspaceSymbols?: IWorkspaceSymbolSettings; + public workspaceSymbols!: IWorkspaceSymbolSettings; public disableInstallationChecks = false; public globalModuleInstallation = false; @@ -213,6 +213,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings { this.formatting = this.formatting ? this.formatting : { autopep8Args: [], autopep8Path: 'autopep8', provider: 'autopep8', + blackArgs: [], blackPath: 'black', yapfArgs: [], yapfPath: 'yapf' }; this.formatting.autopep8Path = getAbsolutePath(systemVariables.resolveAny(this.formatting.autopep8Path), workspaceRoot); diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 15610472ad80..a0929e86de66 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -1,7 +1,6 @@ import { inject, injectable, named } from 'inversify'; import * as os from 'os'; import * as path from 'path'; -import { OutputChannel, Uri } from 'vscode'; import * as vscode from 'vscode'; import { IFormatterHelper } from '../../formatters/types'; import { IServiceContainer } from '../../ioc/types'; @@ -33,14 +32,14 @@ abstract class BaseInstaller { protected appShell: IApplicationShell; protected configService: IConfigurationService; - constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) { + constructor(protected serviceContainer: IServiceContainer, protected outputChannel: vscode.OutputChannel) { this.appShell = serviceContainer.get(IApplicationShell); this.configService = serviceContainer.get(IConfigurationService); } - public abstract promptToInstall(product: Product, resource?: Uri): Promise; + public abstract promptToInstall(product: Product, resource?: vscode.Uri): Promise; - public async install(product: Product, resource?: Uri): Promise { + public async install(product: Product, resource?: vscode.Uri): Promise { if (product === Product.unittest) { return InstallerResponse.Installed; } @@ -60,7 +59,7 @@ abstract class BaseInstaller { .then(isInstalled => isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore); } - public async isInstalled(product: Product, resource?: Uri): Promise { + public async isInstalled(product: Product, resource?: vscode.Uri): Promise { if (product === Product.unittest) { return true; } @@ -85,22 +84,22 @@ abstract class BaseInstaller { } } - protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { + protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string { throw new Error('getExecutableNameFromSettings is not supported on this object'); } } class CTagsInstaller extends BaseInstaller { - constructor(serviceContainer: IServiceContainer, outputChannel: OutputChannel) { + constructor(serviceContainer: IServiceContainer, outputChannel: vscode.OutputChannel) { super(serviceContainer, outputChannel); } - public async promptToInstall(product: Product, resource?: Uri): Promise { + public async promptToInstall(product: Product, resource?: vscode.Uri): Promise { const item = await this.appShell.showErrorMessage('Install CTags to enable Python workspace symbols?', 'Yes', 'No'); return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore; } - public async install(product: Product, resource?: Uri): Promise { + public async install(product: Product, resource?: vscode.Uri): Promise { if (this.serviceContainer.get(IPlatformService).isWindows) { this.outputChannel.appendLine('Install Universal Ctags Win32 to enable support for Workspace Symbols'); this.outputChannel.appendLine('Download the CTags binary from the Universal CTags site.'); @@ -117,32 +116,41 @@ class CTagsInstaller extends BaseInstaller { return InstallerResponse.Ignore; } - protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { + protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string { const settings = this.configService.getSettings(resource); return settings.workspaceSymbols.ctagsPath; } } class FormatterInstaller extends BaseInstaller { - public async promptToInstall(product: Product, resource?: Uri): Promise { + public async promptToInstall(product: Product, resource?: vscode.Uri): Promise { + // Hard-coded on purpose because the UI won't necessarily work having + // another formatter. + const formatters = [Product.autopep8, Product.black, Product.yapf]; + const formatterNames = formatters.map((formatter) => ProductNames.get(formatter)!); const productName = ProductNames.get(product)!; + formatterNames.splice(formatterNames.indexOf(productName), 1); + const useOptions = formatterNames.map((name) => `Use ${name}`); + const yesChoice = 'Yes'; - const installThis = `Install ${productName}`; - const alternateFormatter = product === Product.autopep8 ? 'yapf' : 'autopep8'; - const useOtherFormatter = `Use '${alternateFormatter}' formatter`; - const item = await this.appShell.showErrorMessage(`Formatter ${productName} is not installed.`, installThis, useOtherFormatter); - - if (item === installThis) { + const item = await this.appShell.showErrorMessage(`Formatter ${productName} is not installed. Install?`, yesChoice, ...useOptions); + if (item === yesChoice) { return this.install(product, resource); + } else if (typeof item === 'string') { + for (const formatter of formatters) { + const formatterName = ProductNames.get(formatter)!; + + if (item.endsWith(formatterName)) { + await this.configService.updateSettingAsync('formatting.provider', formatterName, resource); + return this.install(formatter, resource); + } + } } - if (item === useOtherFormatter) { - await this.configService.updateSettingAsync('formatting.provider', alternateFormatter, resource); - return InstallerResponse.Installed; - } + return InstallerResponse.Ignore; } - protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { + protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string { const settings = this.configService.getSettings(resource); const formatHelper = this.serviceContainer.get(IFormatterHelper); const settingsPropNames = formatHelper.getSettingsPropertyNames(product); @@ -152,7 +160,7 @@ class FormatterInstaller extends BaseInstaller { // tslint:disable-next-line:max-classes-per-file class LinterInstaller extends BaseInstaller { - public async promptToInstall(product: Product, resource?: Uri): Promise { + public async promptToInstall(product: Product, resource?: vscode.Uri): Promise { const productName = ProductNames.get(product)!; const install = 'Install'; const disableAllLinting = 'Disable linting'; @@ -173,7 +181,7 @@ class LinterInstaller extends BaseInstaller { } return InstallerResponse.Ignore; } - protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { + protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string { const linterManager = this.serviceContainer.get(ILinterManager); return linterManager.getLinterInfo(product).pathName(resource); } @@ -181,13 +189,13 @@ class LinterInstaller extends BaseInstaller { // tslint:disable-next-line:max-classes-per-file class TestFrameworkInstaller extends BaseInstaller { - public async promptToInstall(product: Product, resource?: Uri): Promise { + public async promptToInstall(product: Product, resource?: vscode.Uri): Promise { const productName = ProductNames.get(product)!; const item = await this.appShell.showErrorMessage(`Test framework ${productName} is not installed. Install?`, 'Yes', 'No'); return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore; } - protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { + protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string { const testHelper = this.serviceContainer.get(ITestsHelper); const settingsPropNames = testHelper.getSettingsPropertyNames(product); if (!settingsPropNames.pathName) { @@ -201,12 +209,12 @@ class TestFrameworkInstaller extends BaseInstaller { // tslint:disable-next-line:max-classes-per-file class RefactoringLibraryInstaller extends BaseInstaller { - public async promptToInstall(product: Product, resource?: Uri): Promise { + public async promptToInstall(product: Product, resource?: vscode.Uri): Promise { const productName = ProductNames.get(product)!; const item = await this.appShell.showErrorMessage(`Refactoring library ${productName} is not installed. Install?`, 'Yes', 'No'); return item === 'Yes' ? this.install(product, resource) : InstallerResponse.Ignore; } - protected getExecutableNameFromSettings(product: Product, resource?: Uri): string { + protected getExecutableNameFromSettings(product: Product, resource?: vscode.Uri): string { return translateProductToModule(product, ModuleNamePurpose.run); } } @@ -230,19 +238,20 @@ export class ProductInstaller implements IInstaller { this.ProductTypes.set(Product.pytest, ProductType.TestFramework); this.ProductTypes.set(Product.unittest, ProductType.TestFramework); this.ProductTypes.set(Product.autopep8, ProductType.Formatter); + this.ProductTypes.set(Product.black, ProductType.Formatter); this.ProductTypes.set(Product.yapf, ProductType.Formatter); this.ProductTypes.set(Product.rope, ProductType.RefactoringLibrary); } // tslint:disable-next-line:no-empty public dispose() { } - public async promptToInstall(product: Product, resource?: Uri): Promise { + public async promptToInstall(product: Product, resource?: vscode.Uri): Promise { return this.createInstaller(product).promptToInstall(product, resource); } - public async install(product: Product, resource?: Uri): Promise { + public async install(product: Product, resource?: vscode.Uri): Promise { return this.createInstaller(product).install(product, resource); } - public async isInstalled(product: Product, resource?: Uri): Promise { + public async isInstalled(product: Product, resource?: vscode.Uri): Promise { return this.createInstaller(product).isInstalled(product, resource); } public translateProductToModuleName(product: Product, purpose: ModuleNamePurpose): string { @@ -280,6 +289,7 @@ function translateProductToModule(product: Product, purpose: ModuleNamePurpose): case Product.pylint: return 'pylint'; case Product.pytest: return 'pytest'; case Product.autopep8: return 'autopep8'; + case Product.black: return 'black'; case Product.pep8: return 'pep8'; case Product.pydocstyle: return 'pydocstyle'; case Product.yapf: return 'yapf'; diff --git a/src/client/common/installer/productNames.ts b/src/client/common/installer/productNames.ts index 9371f540c778..dc58605cc57d 100644 --- a/src/client/common/installer/productNames.ts +++ b/src/client/common/installer/productNames.ts @@ -6,6 +6,7 @@ import { Product } from '../types'; // tslint:disable-next-line:variable-name export const ProductNames = new Map(); ProductNames.set(Product.autopep8, 'autopep8'); +ProductNames.set(Product.black, 'black'); ProductNames.set(Product.flake8, 'flake8'); ProductNames.set(Product.mypy, 'mypy'); ProductNames.set(Product.nosetest, 'nosetest'); diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 240b6f5809e3..35d77cae9846 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -62,7 +62,8 @@ export enum Product { unittest = 12, ctags = 13, rope = 14, - isort = 15 + isort = 15, + black = 16 } export enum ModuleNamePurpose { @@ -103,12 +104,12 @@ export interface IPythonSettings { readonly jediMemoryLimit: number; readonly devOptions: string[]; readonly linting?: ILintingSettings; - readonly formatting?: IFormattingSettings; - readonly unitTest?: IUnitTestSettings; + readonly formatting: IFormattingSettings; + readonly unitTest: IUnitTestSettings; readonly autoComplete?: IAutoCompleteSettings; readonly terminal: ITerminalSettings; readonly sortImports?: ISortImportSettings; - readonly workspaceSymbols?: IWorkspaceSymbolSettings; + readonly workspaceSymbols: IWorkspaceSymbolSettings; readonly envFile: string; readonly disablePromptForFeatures: string[]; readonly disableInstallationChecks: boolean; @@ -191,6 +192,8 @@ export interface IFormattingSettings { readonly provider: string; autopep8Path: string; readonly autopep8Args: string[]; + blackPath: string; + readonly blackArgs: string[]; yapfPath: string; readonly yapfArgs: string[]; } diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index 9e91b00a5a19..d72edac84532 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -1,7 +1,6 @@ import * as fs from 'fs-extra'; import * as path from 'path'; 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'; @@ -13,12 +12,12 @@ import { getTempFileWithDocumentContents, getTextEditsFromPatch } from './../com import { IFormatterHelper } from './types'; export abstract class BaseFormatter { - protected readonly outputChannel: OutputChannel; + protected readonly outputChannel: vscode.OutputChannel; protected readonly workspace: IWorkspaceService; private readonly helper: IFormatterHelper; constructor(public Id: string, private product: Product, protected serviceContainer: IServiceContainer) { - this.outputChannel = serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); + this.outputChannel = serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); this.helper = serviceContainer.get(IFormatterHelper); this.workspace = serviceContainer.get(IWorkspaceService); } @@ -49,8 +48,8 @@ export abstract class BaseFormatter { // autopep8 and yapf have the ability to read from the process input stream and return the formatted code out of the output stream. // 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. + // Yet 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. less responsive solution. const tempFile = await this.createTempFile(document); if (this.checkCancellation(document.fileName, tempFile, token)) { return []; @@ -63,17 +62,17 @@ export abstract class BaseFormatter { .then(output => output.stdout) .then(data => { if (this.checkCancellation(document.fileName, tempFile, token)) { - return [] as TextEdit[]; + return [] as vscode.TextEdit[]; } return getTextEditsFromPatch(document.getText(), data); }) .catch(error => { if (this.checkCancellation(document.fileName, tempFile, token)) { - return [] as TextEdit[]; + return [] as vscode.TextEdit[]; } // tslint:disable-next-line:no-empty this.handleError(this.Id, error, document.uri).catch(() => { }); - return [] as TextEdit[]; + return [] as vscode.TextEdit[]; }) .then(edits => { this.deleteTempFile(document.fileName, tempFile).ignoreErrors(); @@ -83,7 +82,7 @@ export abstract class BaseFormatter { return promise; } - protected async handleError(expectedFileName: string, error: Error, resource?: Uri) { + protected async handleError(expectedFileName: string, error: Error, resource?: vscode.Uri) { let customError = `Formatting with ${this.Id} failed.`; if (isNotInstalledError(error)) { @@ -100,7 +99,7 @@ export abstract class BaseFormatter { private async createTempFile(document: vscode.TextDocument): Promise { return document.isDirty - ? await getTempFileWithDocumentContents(document) + ? getTempFileWithDocumentContents(document) : document.fileName; } diff --git a/src/client/formatters/blackFormatter.ts b/src/client/formatters/blackFormatter.ts new file mode 100644 index 000000000000..d1bb356dc584 --- /dev/null +++ b/src/client/formatters/blackFormatter.ts @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as vscode from 'vscode'; +import { Product } from '../common/installer/productInstaller'; +import { StopWatch } from '../common/stopWatch'; +import { IConfigurationService } from '../common/types'; +import { IServiceContainer } from '../ioc/types'; +import { sendTelemetryWhenDone } from '../telemetry'; +import { FORMAT } from '../telemetry/constants'; +import { BaseFormatter } from './baseFormatter'; + +export class BlackFormatter extends BaseFormatter { + constructor(serviceContainer: IServiceContainer) { + super('black', Product.black, serviceContainer); + } + + public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { + const stopWatch = new StopWatch(); + const settings = this.serviceContainer.get(IConfigurationService).getSettings(document.uri); + const hasCustomArgs = Array.isArray(settings.formatting.blackArgs) && settings.formatting.blackArgs.length > 0; + const formatSelection = range ? !range.isEmpty : false; + + if (formatSelection) { + const errorMessage = async () => { + // Black does not support partial formatting on purpose. + await vscode.window.showErrorMessage('Black does not support the "Format Selection" command'); + return [] as vscode.TextEdit[]; + }; + + return errorMessage(); + } + + const blackArgs = ['--diff', '--quiet']; + const promise = super.provideDocumentFormattingEdits(document, options, token, blackArgs); + sendTelemetryWhenDone(FORMAT, promise, stopWatch, { tool: 'black', hasCustomArgs, formatSelection }); + return promise; + } +} diff --git a/src/client/formatters/helper.ts b/src/client/formatters/helper.ts index b491c40baaa2..95383e69b538 100644 --- a/src/client/formatters/helper.ts +++ b/src/client/formatters/helper.ts @@ -4,17 +4,17 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Uri } from 'vscode'; -import { IConfigurationService, IFormattingSettings } from '../common/types'; -import { ExecutionInfo, Product } from '../common/types'; +import { ExecutionInfo, IConfigurationService, IFormattingSettings, Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { FormatterId, FormatterSettingsPropertyNames, IFormatterHelper } from './types'; @injectable() export class FormatterHelper implements IFormatterHelper { - constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer) { } + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { } public translateToId(formatter: Product): FormatterId { switch (formatter) { case Product.autopep8: return 'autopep8'; + case Product.black: return 'black'; case Product.yapf: return 'yapf'; default: { throw new Error(`Unrecognized Formatter '${formatter}'`); diff --git a/src/client/formatters/types.ts b/src/client/formatters/types.ts index d21ace78bb4f..7f4bcf5b7524 100644 --- a/src/client/formatters/types.ts +++ b/src/client/formatters/types.ts @@ -6,7 +6,7 @@ import { ExecutionInfo, IFormattingSettings, Product } from '../common/types'; export const IFormatterHelper = Symbol('IFormatterHelper'); -export type FormatterId = 'autopep8' | 'yapf'; +export type FormatterId = 'autopep8' | 'black' | 'yapf'; export type FormatterSettingsPropertyNames = { argsName: keyof IFormattingSettings; diff --git a/src/client/providers/formatProvider.ts b/src/client/providers/formatProvider.ts index cabab87018e8..9398fd7b0e58 100644 --- a/src/client/providers/formatProvider.ts +++ b/src/client/providers/formatProvider.ts @@ -8,6 +8,7 @@ import { IConfigurationService } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { AutoPep8Formatter } from './../formatters/autoPep8Formatter'; import { BaseFormatter } from './../formatters/baseFormatter'; +import { BlackFormatter } from './../formatters/blackFormatter'; import { DummyFormatter } from './../formatters/dummyFormatter'; import { YapfFormatter } from './../formatters/yapfFormatter'; @@ -27,8 +28,10 @@ export class PythonFormattingEditProvider implements vscode.DocumentFormattingEd public constructor(context: vscode.ExtensionContext, serviceContainer: IServiceContainer) { const yapfFormatter = new YapfFormatter(serviceContainer); const autoPep8 = new AutoPep8Formatter(serviceContainer); + const black = new BlackFormatter(serviceContainer); const dummy = new DummyFormatter(serviceContainer); this.formatters.set(yapfFormatter.Id, yapfFormatter); + this.formatters.set(black.Id, black); this.formatters.set(autoPep8.Id, autoPep8); this.formatters.set(dummy.Id, dummy); @@ -36,7 +39,7 @@ export class PythonFormattingEditProvider implements vscode.DocumentFormattingEd this.workspace = serviceContainer.get(IWorkspaceService); this.documentManager = serviceContainer.get(IDocumentManager); this.config = serviceContainer.get(IConfigurationService); - this.disposables.push(this.documentManager.onDidSaveTextDocument(async document => await this.onSaveDocument(document))); + this.disposables.push(this.documentManager.onDidSaveTextDocument(async document => this.onSaveDocument(document))); } public dispose() { diff --git a/src/client/telemetry/types.ts b/src/client/telemetry/types.ts index dcb355155f45..818f892b00c4 100644 --- a/src/client/telemetry/types.ts +++ b/src/client/telemetry/types.ts @@ -7,7 +7,7 @@ export type EditorLoadTelemetry = { condaVersion: string; }; export type FormatTelemetry = { - tool: 'autopep8' | 'yapf'; + tool: 'autopep8' | 'black' | 'yapf'; hasCustomArgs: boolean; formatSelection: boolean; }; diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index fb74ecb0e522..f50e678b3886 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -1,9 +1,9 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import * as vscode from 'vscode'; -import { CancellationTokenSource, Uri } from 'vscode'; import { IProcessService, IPythonExecutionFactory } from '../../client/common/process/types'; import { AutoPep8Formatter } from '../../client/formatters/autoPep8Formatter'; +import { BlackFormatter } from '../../client/formatters/blackFormatter'; import { YapfFormatter } from '../../client/formatters/yapfFormatter'; import { closeActiveWindows, initialize, initializeTest } from '../initialize'; import { MockProcessService } from '../mocks/proc'; @@ -16,11 +16,12 @@ const workspaceRootPath = path.join(__dirname, '..', '..', '..', 'src', 'test'); const originalUnformattedFile = path.join(formatFilesPath, 'fileToFormat.py'); const autoPep8FileToFormat = path.join(formatFilesPath, 'autoPep8FileToFormat.py'); -const autoPep8FileToAutoFormat = path.join(formatFilesPath, 'autoPep8FileToAutoFormat.py'); +const blackFileToFormat = path.join(formatFilesPath, 'blackFileToFormat.py'); +const blackReferenceFile = path.join(formatFilesPath, 'blackFileReference.py'); const yapfFileToFormat = path.join(formatFilesPath, 'yapfFileToFormat.py'); -const yapfFileToAutoFormat = path.join(formatFilesPath, 'yapfFileToAutoFormat.py'); let formattedYapf = ''; +let formattedBlack = ''; let formattedAutoPep8 = ''; // tslint:disable-next-line:max-func-body-length @@ -30,16 +31,30 @@ suite('Formatting', () => { suiteSetup(async () => { await initialize(); initializeDI(); - [autoPep8FileToFormat, autoPep8FileToAutoFormat, yapfFileToFormat, yapfFileToAutoFormat].forEach(file => { + [autoPep8FileToFormat, blackFileToFormat, blackReferenceFile, yapfFileToFormat].forEach(file => { fs.copySync(originalUnformattedFile, file, { overwrite: true }); }); fs.ensureDirSync(path.dirname(autoPep8FileToFormat)); - const pythonProcess = await ioc.serviceContainer.get(IPythonExecutionFactory).create(Uri.file(workspaceRootPath)); + const pythonProcess = await ioc.serviceContainer.get(IPythonExecutionFactory).create(vscode.Uri.file(workspaceRootPath)); + const py2 = await ioc.getPythonMajorVersion(vscode.Uri.parse(originalUnformattedFile)) === 2; const yapf = pythonProcess.execModule('yapf', [originalUnformattedFile], { cwd: workspaceRootPath }); const autoPep8 = pythonProcess.execModule('autopep8', [originalUnformattedFile], { cwd: workspaceRootPath }); - await Promise.all([yapf, autoPep8]).then(formattedResults => { + const formatters = [yapf, autoPep8]; + // When testing against 3.5 and older, this will break. + if (!py2) { + // Black doesn't support emitting only to stdout; it either works + // through a pipe, emits a diff, or rewrites the file in-place. + // Thus it's easier to let it do its in-place rewrite and then + // read the reference file from there. + const black = pythonProcess.execModule('black', [blackReferenceFile], { cwd: workspaceRootPath }); + formatters.push(black); + } + await Promise.all(formatters).then(formattedResults => { formattedYapf = formattedResults[0].stdout; formattedAutoPep8 = formattedResults[1].stdout; + if (!py2) { + formattedBlack = fs.readFileSync(blackReferenceFile).toString(); + } }); }); setup(async () => { @@ -47,7 +62,7 @@ suite('Formatting', () => { initializeDI(); }); suiteTeardown(async () => { - [autoPep8FileToFormat, autoPep8FileToAutoFormat, yapfFileToFormat, yapfFileToAutoFormat].forEach(file => { + [autoPep8FileToFormat, blackFileToFormat, blackReferenceFile, yapfFileToFormat].forEach(file => { if (fs.existsSync(file)) { fs.unlinkSync(file); } @@ -82,22 +97,30 @@ suite('Formatting', () => { }); } - async function testFormatting(formatter: AutoPep8Formatter | YapfFormatter, formattedContents: string, fileToFormat: string, outputFileName: string) { + async function testFormatting(formatter: AutoPep8Formatter | BlackFormatter | YapfFormatter, formattedContents: string, fileToFormat: string, outputFileName: string) { const textDocument = await vscode.workspace.openTextDocument(fileToFormat); const textEditor = await vscode.window.showTextDocument(textDocument); const options = { insertSpaces: textEditor.options.insertSpaces! as boolean, tabSize: textEditor.options.tabSize! as number }; injectFormatOutput(outputFileName); - const edits = await formatter.formatDocument(textDocument, options, new CancellationTokenSource().token); + const edits = await formatter.formatDocument(textDocument, options, new vscode.CancellationTokenSource().token); await textEditor.edit(editBuilder => { edits.forEach(edit => editBuilder.replace(edit.range, edit.newText)); }); compareFiles(formattedContents, textEditor.document.getText()); } - 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('AutoPep8', async () => testFormatting(new AutoPep8Formatter(ioc.serviceContainer), formattedAutoPep8, autoPep8FileToFormat, 'autopep8.output')); + test('Black', async function() { + if (await ioc.getPythonMajorVersion(vscode.Uri.parse(blackFileToFormat)) === 2) { + // tslint:disable-next-line:no-invalid-this + return this.skip(); + } + + await testFormatting(new BlackFormatter(ioc.serviceContainer), formattedBlack, blackFileToFormat, 'black.output'); + }); + test('Yapf', async () => testFormatting(new YapfFormatter(ioc.serviceContainer), formattedYapf, yapfFileToFormat, 'yapf.output')); test('Yapf on dirty file', async () => { const sourceDir = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'formatting'); @@ -130,7 +153,7 @@ suite('Formatting', () => { 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); + const edits = await formatter.formatDocument(textDocument, options, new vscode.CancellationTokenSource().token); await textEditor.edit(editBuilder => { edits.forEach(edit => editBuilder.replace(edit.range, edit.newText)); }); diff --git a/src/test/format/format.helper.test.ts b/src/test/format/format.helper.test.ts index 51afda1a64db..d6c732e3e131 100644 --- a/src/test/format/format.helper.test.ts +++ b/src/test/format/format.helper.test.ts @@ -25,7 +25,7 @@ suite('Formatting - Helper', () => { }); test('Ensure product is set in Execution Info', async () => { - [Product.autopep8, Product.yapf].forEach(formatter => { + [Product.autopep8, Product.black, Product.yapf].forEach(formatter => { const info = formatHelper.getExecutionInfo(formatter, []); assert.equal(info.product, formatter, `Incorrect products for ${formatHelper.translateToId(formatter)}`); }); @@ -34,7 +34,7 @@ suite('Formatting - Helper', () => { test('Ensure executable is set in Execution Info', async () => { const settings = PythonSettings.getInstance(); - [Product.autopep8, Product.yapf].forEach(formatter => { + [Product.autopep8, Product.black, Product.yapf].forEach(formatter => { const info = formatHelper.getExecutionInfo(formatter, []); const names = formatHelper.getSettingsPropertyNames(formatter); const execPath = settings.formatting[names.pathName] as string; @@ -47,7 +47,7 @@ suite('Formatting - Helper', () => { const settings = PythonSettings.getInstance(); const customArgs = ['1', '2', '3']; - [Product.autopep8, Product.yapf].forEach(formatter => { + [Product.autopep8, Product.black, Product.yapf].forEach(formatter => { const names = formatHelper.getSettingsPropertyNames(formatter); const args: string[] = Array.isArray(settings.formatting[names.argsName]) ? settings.formatting[names.argsName] as string[] : []; const expectedArgs = args.concat(customArgs).join(','); @@ -58,7 +58,7 @@ suite('Formatting - Helper', () => { }); test('Ensure correct setting names are returned', async () => { - [Product.autopep8, Product.yapf].forEach(formatter => { + [Product.autopep8, Product.black, Product.yapf].forEach(formatter => { const translatedId = formatHelper.translateToId(formatter)!; const settings = { argsName: `${translatedId}Args` as keyof IFormattingSettings, @@ -72,9 +72,10 @@ suite('Formatting - Helper', () => { test('Ensure translation of ids works', async () => { const formatterMapping = new Map(); formatterMapping.set(Product.autopep8, 'autopep8'); + formatterMapping.set(Product.black, 'black'); formatterMapping.set(Product.yapf, 'yapf'); - [Product.autopep8, Product.yapf].forEach(formatter => { + [Product.autopep8, Product.black, Product.yapf].forEach(formatter => { const translatedId = formatHelper.translateToId(formatter); assert.equal(translatedId, formatterMapping.get(formatter)!, `Incorrect translation for product ${formatHelper.translateToId(formatter)}`); }); @@ -83,6 +84,7 @@ suite('Formatting - Helper', () => { EnumEx.getValues(Product).forEach(product => { const formatterMapping = new Map(); formatterMapping.set(Product.autopep8, 'autopep8'); + formatterMapping.set(Product.black, 'black'); formatterMapping.set(Product.yapf, 'yapf'); if (formatterMapping.has(product)) { return; diff --git a/src/test/pythonFiles/formatting/black.output b/src/test/pythonFiles/formatting/black.output new file mode 100644 index 000000000000..be709f2d720a --- /dev/null +++ b/src/test/pythonFiles/formatting/black.output @@ -0,0 +1,54 @@ +--- src/test/pythonFiles/formatting/fileToFormat.py (original) ++++ src/test/pythonFiles/formatting/fileToFormat.py (formatted) +@@ -1,22 +1,38 @@ +-import math, sys; ++import math, sys ++ + + def example1(): + ####This is a long comment. This should be wrapped to fit within 72 characters. +- some_tuple=( 1,2, 3,'a' ); +- some_variable={'long':'Long code lines should be wrapped within 79 characters.', +- 'other':[math.pi, 100,200,300,9876543210,'This is a long string that goes on'], +- 'more':{'inner':'This whole logical line should be wrapped.',some_tuple:[1, +- 20,300,40000,500000000,60000000000000000]}} ++ some_tuple = (1, 2, 3, "a") ++ some_variable = { ++ "long": "Long code lines should be wrapped within 79 characters.", ++ "other": [ ++ math.pi, 100, 200, 300, 9876543210, "This is a long string that goes on" ++ ], ++ "more": { ++ "inner": "This whole logical line should be wrapped.", ++ some_tuple: [1, 20, 300, 40000, 500000000, 60000000000000000], ++ }, ++ } + return (some_tuple, some_variable) +-def example2(): return {'has_key() is deprecated':True}.has_key({'f':2}.has_key('')); +-class Example3( object ): +- def __init__ ( self, bar ): +- #Comments should have a space after the hash. +- if bar : bar+=1; bar=bar* bar ; return bar +- else: +- some_string = """ ++ ++ ++def example2(): ++ return {"has_key() is deprecated": True}.has_key({"f": 2}.has_key("")) ++ ++ ++class Example3(object): ++ ++ def __init__(self, bar): ++ # Comments should have a space after the hash. ++ if bar: ++ bar += 1 ++ bar = bar * bar ++ return bar ++ else: ++ some_string = """ + Indentation in multiline strings should not be touched. + Only actual code should be reindented. + """ +- return (sys.path, some_string) ++ return (sys.path, some_string)