Support the Black formatter (#1611)

Co-authored-by: Josh Smeaton <[email protected]>

Author: brettcannon

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)

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.",

requirements.txt

@@ -2,6 +2,7 @@
 # but flake8 has a tighter pinning.
 flake8
 autopep8
+black ; python_version>='3.6'
 yapf
 pylint
 pep8

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);

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>(IApplicationShell);
         this.configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
     }
 
-    public abstract promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse>;
+    public abstract promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse>;
 
-    public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
+    public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         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<boolean | undefined> {
+    public async isInstalled(product: Product, resource?: vscode.Uri): Promise<boolean | undefined> {
         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<InstallerResponse> {
+    public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         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<InstallerResponse> {
+    public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         if (this.serviceContainer.get<IPlatformService>(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<InstallerResponse> {
+    public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
+        // 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>(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<InstallerResponse> {
+    public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         const productName = ProductNames.get(product)!;
         const install = 'Install';
         const disableAllLinting = 'Disable linting';
@@ -173,21 +181,21 @@ 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>(ILinterManager);
         return linterManager.getLinterInfo(product).pathName(resource);
     }
 }
 
 // tslint:disable-next-line:max-classes-per-file
 class TestFrameworkInstaller extends BaseInstaller {
-    public async promptToInstall(product: Product, resource?: Uri): Promise<InstallerResponse> {
+    public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         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>(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<InstallerResponse> {
+    public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         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<InstallerResponse> {
+    public async promptToInstall(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         return this.createInstaller(product).promptToInstall(product, resource);
     }
-    public async install(product: Product, resource?: Uri): Promise<InstallerResponse> {
+    public async install(product: Product, resource?: vscode.Uri): Promise<InstallerResponse> {
         return this.createInstaller(product).install(product, resource);
     }
-    public async isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined> {
+    public async isInstalled(product: Product, resource?: vscode.Uri): Promise<boolean | undefined> {
         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';

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<Product, string>();
 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');

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[];
 }

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<OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
+        this.outputChannel = serviceContainer.get<vscode.OutputChannel>(IOutputChannel, STANDARD_OUTPUT_CHANNEL);
         this.helper = serviceContainer.get<IFormatterHelper>(IFormatterHelper);
         this.workspace = serviceContainer.get<IWorkspaceService>(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<string> {
         return document.isDirty
-            ? await getTempFileWithDocumentContents(document)
+            ? getTempFileWithDocumentContents(document)
             : document.fileName;
     }