diff --git a/package.json b/package.json index b9458fd647bb..c2120e547036 100644 --- a/package.json +++ b/package.json @@ -78,8 +78,8 @@ "onCommand:python.startREPL", "onCommand:python.goToPythonObject", "onCommand:python.setLinter", - "onCommand:python.enableLinting", - "onCommand:python.createTerminal" + "onCommand:python.enableLinting", + "onCommand:python.createTerminal" ], "main": "./out/client/extension", "contributes": { diff --git a/src/client/common/application/documentManager.ts b/src/client/common/application/documentManager.ts index 8c3c41df5850..05b064ae1ccd 100644 --- a/src/client/common/application/documentManager.ts +++ b/src/client/common/application/documentManager.ts @@ -4,7 +4,7 @@ // tslint:disable:no-any import { injectable } from 'inversify'; -import { Event, TextDocument, TextDocumentShowOptions, TextEditor, TextEditorOptionsChangeEvent, TextEditorSelectionChangeEvent, TextEditorViewColumnChangeEvent, Uri, ViewColumn, window } from 'vscode'; +import { Event, TextDocument, TextDocumentShowOptions, TextEditor, TextEditorOptionsChangeEvent, TextEditorSelectionChangeEvent, TextEditorViewColumnChangeEvent, Uri, ViewColumn, window, workspace } from 'vscode'; import { IDocumentManager } from './types'; @injectable() @@ -30,10 +30,12 @@ export class DocumentManager implements IDocumentManager { public get onDidChangeTextEditorViewColumn(): Event { return window.onDidChangeTextEditorViewColumn; } + public get onDidSaveTextDocument(): Event { + return workspace.onDidSaveTextDocument; + } public showTextDocument(document: TextDocument, column?: ViewColumn, preserveFocus?: boolean): Thenable; public showTextDocument(document: TextDocument | Uri, options?: TextDocumentShowOptions): Thenable; public showTextDocument(uri: any, options?: any, preserveFocus?: any): Thenable { return window.showTextDocument(uri, options, preserveFocus); } - } diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index e040b35a7e14..4cd05f99592c 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -302,6 +302,11 @@ export interface IDocumentManager { */ readonly onDidChangeTextEditorViewColumn: Event; + /** + * An event that is emitted when a [text document](#TextDocument) is saved to disk. + */ + readonly onDidSaveTextDocument: Event; + /** * Show the given document in a text editor. A [column](#ViewColumn) can be provided * to control where the editor is being shown. Might change the [active editor](#window.activeTextEditor). diff --git a/src/client/common/application/workspace.ts b/src/client/common/application/workspace.ts index c2a3772931cd..71d183ba854b 100644 --- a/src/client/common/application/workspace.ts +++ b/src/client/common/application/workspace.ts @@ -2,36 +2,36 @@ // Licensed under the MIT License. import { injectable } from 'inversify'; -import { CancellationToken, Event, FileSystemWatcher, GlobPattern, Uri, workspace, WorkspaceConfiguration, WorkspaceFolder, WorkspaceFoldersChangeEvent } from 'vscode'; +import * as vscode from 'vscode'; import { IWorkspaceService } from './types'; @injectable() export class WorkspaceService implements IWorkspaceService { - public get onDidChangeConfiguration(): Event { - return workspace.onDidChangeConfiguration; + public get onDidChangeConfiguration(): vscode.Event { + return vscode.workspace.onDidChangeConfiguration; } public get rootPath(): string | undefined { - return workspace.rootPath; + return vscode.workspace.rootPath; } - public get workspaceFolders(): WorkspaceFolder[] | undefined { - return workspace.workspaceFolders; + public get workspaceFolders(): vscode.WorkspaceFolder[] | undefined { + return vscode.workspace.workspaceFolders; } - public get onDidChangeWorkspaceFolders(): Event { - return workspace.onDidChangeWorkspaceFolders; + public get onDidChangeWorkspaceFolders(): vscode.Event { + return vscode.workspace.onDidChangeWorkspaceFolders; } - public getConfiguration(section?: string, resource?: Uri): WorkspaceConfiguration { - return workspace.getConfiguration(section, resource); + public getConfiguration(section?: string, resource?: vscode.Uri): vscode.WorkspaceConfiguration { + return vscode.workspace.getConfiguration(section, resource); } - public getWorkspaceFolder(uri: Uri): WorkspaceFolder | undefined { - return workspace.getWorkspaceFolder(uri); + public getWorkspaceFolder(uri: vscode.Uri): vscode.WorkspaceFolder | undefined { + return vscode.workspace.getWorkspaceFolder(uri); } - public asRelativePath(pathOrUri: string | Uri, includeWorkspaceFolder?: boolean): string { - return workspace.asRelativePath(pathOrUri, includeWorkspaceFolder); + public asRelativePath(pathOrUri: string | vscode.Uri, includeWorkspaceFolder?: boolean): string { + return vscode.workspace.asRelativePath(pathOrUri, includeWorkspaceFolder); } - public createFileSystemWatcher(globPattern: GlobPattern, ignoreCreateEvents?: boolean, ignoreChangeEvents?: boolean, ignoreDeleteEvents?: boolean): FileSystemWatcher { - return workspace.createFileSystemWatcher(globPattern, ignoreChangeEvents, ignoreChangeEvents, ignoreDeleteEvents); + public createFileSystemWatcher(globPattern: vscode.GlobPattern, ignoreCreateEvents?: boolean, ignoreChangeEvents?: boolean, ignoreDeleteEvents?: boolean): vscode.FileSystemWatcher { + return vscode.workspace.createFileSystemWatcher(globPattern, ignoreChangeEvents, ignoreChangeEvents, ignoreDeleteEvents); } - public findFiles(include: GlobPattern, exclude?: GlobPattern, maxResults?: number, token?: CancellationToken): Thenable { - return workspace.findFiles(include, exclude, maxResults, token); + public findFiles(include: vscode.GlobPattern, exclude?: vscode.GlobPattern, maxResults?: number, token?: vscode.CancellationToken): Thenable { + return vscode.workspace.findFiles(include, exclude, maxResults, token); } } diff --git a/src/client/common/editor.ts b/src/client/common/editor.ts index 7da7a2657e8b..24be7cb209fc 100644 --- a/src/client/common/editor.ts +++ b/src/client/common/editor.ts @@ -1,30 +1,32 @@ -import {TextEdit, Position, Range, TextDocument, WorkspaceEdit} from 'vscode'; -import * as vscode from 'vscode'; import * as dmp from 'diff-match-patch'; -import {EOL} from 'os'; import * as fs from 'fs'; +import { EOL } from 'os'; import * as path from 'path'; -const tmp = require('tmp'); +import { Position, Range, TextDocument, TextEdit, WorkspaceEdit } from 'vscode'; +import * as vscode from 'vscode'; // Code borrowed from goFormat.ts (Go Extension for VS Code) -const EDIT_DELETE = 0; -const EDIT_INSERT = 1; -const EDIT_REPLACE = 2; +enum EditAction { + Delete, + Insert, + Replace +} + const NEW_LINE_LENGTH = EOL.length; class Patch { - diffs: dmp.Diff[]; - start1: number; - start2: number; - length1: number; - length2: number; + public diffs: dmp.Diff[]; + public start1: number; + public start2: number; + public length1: number; + public length2: number; } class Edit { - action: number; - start: Position; - end: Position; - text: string; + public action: EditAction; + public start: Position; + public end: Position; + public text: string; constructor(action: number, start: Position) { this.action = action; @@ -32,14 +34,16 @@ class Edit { this.text = ''; } - apply(): TextEdit { + public apply(): TextEdit { switch (this.action) { - case EDIT_INSERT: + case EditAction.Insert: return TextEdit.insert(this.start, this.text); - case EDIT_DELETE: + case EditAction.Delete: return TextEdit.delete(new Range(this.start, this.end)); - case EDIT_REPLACE: + case EditAction.Replace: return TextEdit.replace(new Range(this.start, this.end), this.text); + default: + return new TextEdit(new Range(new Position(0, 0), new Position(0, 0)), ''); } } } @@ -56,26 +60,24 @@ export function getTextEditsFromPatch(before: string, patch: string): TextEdit[] // # Work around missing newline (http://bugs.python.org/issue2142). patch = patch.replace(/\\ No newline at end of file[\r\n]/, ''); - let d = new dmp.diff_match_patch(); - let patches: any[] = patch_fromText.call(d, patch); + const d = new dmp.diff_match_patch(); + const patches = patch_fromText.call(d, patch); if (!Array.isArray(patches) || patches.length === 0) { throw new Error('Unable to parse Patch string'); } - let textEdits: TextEdit[] = []; + const textEdits: TextEdit[] = []; - // Add line feeds - // & build the text edits - patches.forEach(patch => { - patch.diffs.forEach(diff => { + // Add line feeds and build the text edits + patches.forEach(p => { + p.diffs.forEach(diff => { diff[1] += EOL; }); - - getTextEditsInternal(before, patch.diffs, patch.start1).forEach(edit => textEdits.push(edit.apply())); + getTextEditsInternal(before, p.diffs, p.start1).forEach(edit => textEdits.push(edit.apply())); }); return textEdits; } -export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot?:string): WorkspaceEdit { +export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot?: string): WorkspaceEdit { const workspaceEdit = new WorkspaceEdit(); filePatches.forEach(patch => { const indexOfAtAt = patch.indexOf('@@'); @@ -110,8 +112,8 @@ export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot? // # Work around missing newline (http://bugs.python.org/issue2142). patch = patch.replace(/\\ No newline at end of file[\r\n]/, ''); - let d = new dmp.diff_match_patch(); - let patches: any[] = patch_fromText.call(d, patch); + const d = new dmp.diff_match_patch(); + const patches = patch_fromText.call(d, patch); if (!Array.isArray(patches) || patches.length === 0) { throw new Error('Unable to parse Patch string'); } @@ -119,87 +121,88 @@ export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot? const fileSource = fs.readFileSync(fileName).toString('utf8'); const fileUri = vscode.Uri.file(fileName); - // Add line feeds - // & build the text edits - patches.forEach(patch => { - patch.diffs.forEach(diff => { + // Add line feeds and build the text edits + patches.forEach(p => { + p.diffs.forEach(diff => { diff[1] += EOL; }); - getTextEditsInternal(fileSource, patch.diffs, patch.start1).forEach(edit => { + getTextEditsInternal(fileSource, p.diffs, p.start1).forEach(edit => { switch (edit.action) { - case EDIT_DELETE: { + case EditAction.Delete: workspaceEdit.delete(fileUri, new Range(edit.start, edit.end)); - } - case EDIT_INSERT: { + break; + case EditAction.Insert: workspaceEdit.insert(fileUri, edit.start, edit.text); - } - case EDIT_REPLACE: { + break; + case EditAction.Replace: workspaceEdit.replace(fileUri, new Range(edit.start, edit.end), edit.text); - } + break; + default: + break; } }); }); - - }); return workspaceEdit; } export function getTextEdits(before: string, after: string): TextEdit[] { - let d = new dmp.diff_match_patch(); - let diffs = d.diff_main(before, after); + const d = new dmp.diff_match_patch(); + const diffs = d.diff_main(before, after); return getTextEditsInternal(before, diffs).map(edit => edit.apply()); } function getTextEditsInternal(before: string, diffs: [number, string][], startLine: number = 0): Edit[] { let line = startLine; let character = 0; if (line > 0) { - let beforeLines = before.split(/\r?\n/g); + const beforeLines = before.split(/\r?\n/g); beforeLines.filter((l, i) => i < line).forEach(l => character += l.length + NEW_LINE_LENGTH); } const edits: Edit[] = []; - let edit: Edit = null; - - for (let i = 0; i < diffs.length; i++) { - let start = new Position(line, character); + let edit: Edit | null = null; + // tslint:disable-next-line:prefer-for-of + for (let i = 0; i < diffs.length; i += 1) { + const start = new Position(line, character); // Compute the line/character after the diff is applied. - for (let curr = 0; curr < diffs[i][1].length; curr++) { + // tslint:disable-next-line:prefer-for-of + for (let curr = 0; curr < diffs[i][1].length; curr += 1) { if (diffs[i][1][curr] !== '\n') { - character++; + character += 1; } else { character = 0; - line++; + line += 1; } } + // tslint:disable-next-line:switch-default switch (diffs[i][0]) { case dmp.DIFF_DELETE: - if (edit == null) { - edit = new Edit(EDIT_DELETE, start); - } else if (edit.action !== EDIT_DELETE) { + if (edit === null) { + edit = new Edit(EditAction.Delete, start); + } else if (edit.action !== EditAction.Delete) { throw new Error('cannot format due to an internal error.'); } edit.end = new Position(line, character); break; case dmp.DIFF_INSERT: - if (edit == null) { - edit = new Edit(EDIT_INSERT, start); - } else if (edit.action === EDIT_DELETE) { - edit.action = EDIT_REPLACE; + if (edit === null) { + edit = new Edit(EditAction.Insert, start); + } else if (edit.action === EditAction.Delete) { + edit.action = EditAction.Replace; } // insert and replace edits are all relative to the original state // of the document, so inserts should reset the current line/character - // position to the start. + // position to the start. line = start.line; character = start.character; edit.text += diffs[i][1]; break; case dmp.DIFF_EQUAL: - if (edit != null) { + if (edit !== null) { edits.push(edit); edit = null; } @@ -207,7 +210,7 @@ function getTextEditsInternal(before: string, diffs: [number, string][], startLi } } - if (edit != null) { + if (edit !== null) { edits.push(edit); } @@ -216,9 +219,10 @@ function getTextEditsInternal(before: string, diffs: [number, string][], startLi export function getTempFileWithDocumentContents(document: TextDocument): Promise { return new Promise((resolve, reject) => { - let ext = path.extname(document.uri.fsPath); - let tmp = require('tmp'); - tmp.file({ postfix: ext }, function (err, tmpFilePath, fd) { + 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); } @@ -232,85 +236,86 @@ export function getTempFileWithDocumentContents(document: TextDocument): Promise }); } - /** * Parse a textual representation of patches and return a list of Patch objects. * @param {string} textline Text representation of patches. * @return {!Array.} Array of Patch objects. * @throws {!Error} If invalid input. */ -function patch_fromText(textline) { - var patches = []; +function patch_fromText(textline): Patch[] { + const patches: Patch[] = []; if (!textline) { return patches; } // Start Modification by Don Jayamanne 24/06/2016 Support for CRLF - var text = textline.split(/[\r\n]/); + const text = textline.split(/[\r\n]/); // End Modification - var textPointer = 0; - var patchHeader = /^@@ -(\d+),?(\d*) \+(\d+),?(\d*) @@$/; + let textPointer = 0; + const patchHeader = /^@@ -(\d+),?(\d*) \+(\d+),?(\d*) @@$/; while (textPointer < text.length) { - var m = text[textPointer].match(patchHeader); + const m = text[textPointer].match(patchHeader); if (!m) { - throw new Error('Invalid patch string: ' + text[textPointer]); + throw new Error(`Invalid patch string: ${text[textPointer]}`); } - var patch = new (dmp.diff_match_patch).patch_obj(); + // tslint:disable-next-line:no-any + const patch = new (dmp.diff_match_patch).patch_obj(); patches.push(patch); patch.start1 = parseInt(m[1], 10); if (m[2] === '') { - patch.start1--; + patch.start1 -= 1; patch.length1 = 1; - } else if (m[2] == '0') { + } else if (m[2] === '0') { patch.length1 = 0; } else { - patch.start1--; + patch.start1 -= 1; patch.length1 = parseInt(m[2], 10); } patch.start2 = parseInt(m[3], 10); if (m[4] === '') { - patch.start2--; + patch.start2 -= 1; patch.length2 = 1; - } else if (m[4] == '0') { + } else if (m[4] === '0') { patch.length2 = 0; } else { - patch.start2--; + patch.start2 -= 1; patch.length2 = parseInt(m[4], 10); } - textPointer++; + textPointer += 1; while (textPointer < text.length) { - var sign = text[textPointer].charAt(0); + const sign = text[textPointer].charAt(0); + let line: string; try { //var line = decodeURI(text[textPointer].substring(1)); // For some reason the patch generated by python files don't encode any characters // And this patch module (code from Google) is expecting the text to be encoded!! // Temporary solution, disable decoding // Issue #188 - var line = text[textPointer].substring(1); + line = text[textPointer].substring(1); } catch (ex) { // Malformed URI sequence. - throw new Error('Illegal escape in patch_fromText: ' + line); + throw new Error('Illegal escape in patch_fromText'); } - if (sign == '-') { + if (sign === '-') { // Deletion. patch.diffs.push([dmp.DIFF_DELETE, line]); - } else if (sign == '+') { + } else if (sign === '+') { // Insertion. patch.diffs.push([dmp.DIFF_INSERT, line]); - } else if (sign == ' ') { + } else if (sign === ' ') { // Minor equality. patch.diffs.push([dmp.DIFF_EQUAL, line]); - } else if (sign == '@') { + } else if (sign === '@') { // Start of next patch. break; } else if (sign === '') { // Blank line? Whatever. } else { // WTF? - throw new Error('Invalid patch mode "' + sign + '" in: ' + line); + throw new Error(`Invalid patch mode '${sign}' in: ${line}`); } - textPointer++; + textPointer += 1; } } return patches; diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 1a65a0661d05..d2c7ca48a7d5 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -84,87 +84,87 @@ export interface IPathUtils { export const ICurrentProcess = Symbol('ICurrentProcess'); export interface ICurrentProcess { - env: EnvironmentVariables; + readonly env: EnvironmentVariables; } export interface IPythonSettings { - pythonPath: string; - venvPath: string; - jediPath: string; - devOptions: string[]; - linting: ILintingSettings; - formatting: IFormattingSettings; - unitTest: IUnitTestSettings; - autoComplete: IAutoCompeteSettings; - terminal: ITerminalSettings; - sortImports: ISortImportSettings; - workspaceSymbols: IWorkspaceSymbolSettings; - envFile: string; - disablePromptForFeatures: string[]; - disableInstallationChecks: boolean; - globalModuleInstallation: boolean; + readonly pythonPath: string; + readonly venvPath: string; + readonly jediPath: string; + readonly devOptions: string[]; + readonly linting: ILintingSettings; + readonly formatting: IFormattingSettings; + readonly unitTest: IUnitTestSettings; + readonly autoComplete: IAutoCompeteSettings; + readonly terminal: ITerminalSettings; + readonly sortImports: ISortImportSettings; + readonly workspaceSymbols: IWorkspaceSymbolSettings; + readonly envFile: string; + readonly disablePromptForFeatures: string[]; + readonly disableInstallationChecks: boolean; + readonly globalModuleInstallation: boolean; } export interface ISortImportSettings { - path: string; - args: string[]; + readonly path: string; + readonly args: string[]; } export interface IUnitTestSettings { - promptToConfigure: boolean; - debugPort: number; - debugHost?: string; - nosetestsEnabled: boolean; + readonly promptToConfigure: boolean; + readonly debugPort: number; + readonly debugHost?: string; + readonly nosetestsEnabled: boolean; nosetestPath: string; nosetestArgs: string[]; - pyTestEnabled: boolean; + readonly pyTestEnabled: boolean; pyTestPath: string; pyTestArgs: string[]; - unittestEnabled: boolean; + readonly unittestEnabled: boolean; unittestArgs: string[]; cwd?: string; } export interface IPylintCategorySeverity { - convention: DiagnosticSeverity; - refactor: DiagnosticSeverity; - warning: DiagnosticSeverity; - error: DiagnosticSeverity; - fatal: DiagnosticSeverity; + readonly convention: DiagnosticSeverity; + readonly refactor: DiagnosticSeverity; + readonly warning: DiagnosticSeverity; + readonly error: DiagnosticSeverity; + readonly fatal: DiagnosticSeverity; } export interface IPep8CategorySeverity { - W: DiagnosticSeverity; - E: DiagnosticSeverity; + readonly W: DiagnosticSeverity; + readonly E: DiagnosticSeverity; } // tslint:disable-next-line:interface-name export interface Flake8CategorySeverity { - F: DiagnosticSeverity; - E: DiagnosticSeverity; - W: DiagnosticSeverity; + readonly F: DiagnosticSeverity; + readonly E: DiagnosticSeverity; + readonly W: DiagnosticSeverity; } export interface IMypyCategorySeverity { - error: DiagnosticSeverity; - note: DiagnosticSeverity; + readonly error: DiagnosticSeverity; + readonly note: DiagnosticSeverity; } export interface ILintingSettings { - enabled: boolean; - ignorePatterns: string[]; - prospectorEnabled: boolean; - prospectorArgs: string[]; - pylintEnabled: boolean; - pylintArgs: string[]; - pep8Enabled: boolean; - pep8Args: string[]; - pylamaEnabled: boolean; - pylamaArgs: string[]; - flake8Enabled: boolean; - flake8Args: string[]; - pydocstyleEnabled: boolean; - pydocstyleArgs: string[]; - lintOnSave: boolean; - maxNumberOfProblems: number; - pylintCategorySeverity: IPylintCategorySeverity; - pep8CategorySeverity: IPep8CategorySeverity; - flake8CategorySeverity: Flake8CategorySeverity; - mypyCategorySeverity: IMypyCategorySeverity; + readonly enabled: boolean; + readonly ignorePatterns: string[]; + readonly prospectorEnabled: boolean; + readonly prospectorArgs: string[]; + readonly pylintEnabled: boolean; + readonly pylintArgs: string[]; + readonly pep8Enabled: boolean; + readonly pep8Args: string[]; + readonly pylamaEnabled: boolean; + readonly pylamaArgs: string[]; + readonly flake8Enabled: boolean; + readonly flake8Args: string[]; + readonly pydocstyleEnabled: boolean; + readonly pydocstyleArgs: string[]; + readonly lintOnSave: boolean; + readonly maxNumberOfProblems: number; + readonly pylintCategorySeverity: IPylintCategorySeverity; + readonly pep8CategorySeverity: IPep8CategorySeverity; + readonly flake8CategorySeverity: Flake8CategorySeverity; + readonly mypyCategorySeverity: IMypyCategorySeverity; prospectorPath: string; pylintPath: string; pep8Path: string; @@ -174,31 +174,31 @@ export interface ILintingSettings { mypyEnabled: boolean; mypyArgs: string[]; mypyPath: string; - pylintUseMinimalCheckers: boolean; + readonly pylintUseMinimalCheckers: boolean; } export interface IFormattingSettings { - provider: string; + readonly provider: string; autopep8Path: string; - autopep8Args: string[]; + readonly autopep8Args: string[]; yapfPath: string; - yapfArgs: string[]; + readonly yapfArgs: string[]; } export interface IAutoCompeteSettings { - addBrackets: boolean; - extraPaths: string[]; - preloadModules: string[]; + readonly addBrackets: boolean; + readonly extraPaths: string[]; + readonly preloadModules: string[]; } export interface IWorkspaceSymbolSettings { - enabled: boolean; + readonly enabled: boolean; tagFilePath: string; - rebuildOnStart: boolean; - rebuildOnFileSave: boolean; - ctagsPath: string; - exclusionPatterns: string[]; + readonly rebuildOnStart: boolean; + readonly rebuildOnFileSave: boolean; + readonly ctagsPath: string; + readonly exclusionPatterns: string[]; } export interface ITerminalSettings { - executeInFileDir: boolean; - launchArgs: string[]; + readonly executeInFileDir: boolean; + readonly launchArgs: string[]; } export const IConfigurationService = Symbol('IConfigurationService'); diff --git a/src/client/extension.ts b/src/client/extension.ts index 9da8774ad0cc..e76920e2e3f6 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -7,8 +7,8 @@ if ((Reflect as any).metadata === undefined) { } import { Container } from 'inversify'; import * as os from 'os'; -import { Disposable, Memento, OutputChannel, window } from 'vscode'; import * as vscode from 'vscode'; +import { Disposable, Memento, OutputChannel, window } from 'vscode'; import { BannerService } from './banner'; import { PythonSettings } from './common/configSettings'; import * as settings from './common/configSettings'; diff --git a/src/client/formatters/autoPep8Formatter.ts b/src/client/formatters/autoPep8Formatter.ts index 6be2f961acf1..39848c7dc10e 100644 --- a/src/client/formatters/autoPep8Formatter.ts +++ b/src/client/formatters/autoPep8Formatter.ts @@ -1,6 +1,6 @@ import * as vscode from 'vscode'; -import { PythonSettings } from '../common/configSettings'; import { Product } from '../common/installer/productInstaller'; +import { IConfigurationService } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { sendTelemetryWhenDone } from '../telemetry'; import { FORMAT } from '../telemetry/constants'; @@ -14,7 +14,7 @@ export class AutoPep8Formatter extends BaseFormatter { public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { const stopWatch = new StopWatch(); - const settings = PythonSettings.getInstance(document.uri); + const settings = this.serviceContainer.get(IConfigurationService).getSettings(document.uri); const hasCustomArgs = Array.isArray(settings.formatting.autopep8Args) && settings.formatting.autopep8Args.length > 0; const formatSelection = range ? !range.isEmpty : false; diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index 76d789def8cc..f678665fc02d 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -2,6 +2,7 @@ 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 { isNotInstalledError } from '../common/helpers'; import { IPythonToolExecutionService } from '../common/process/types'; @@ -12,10 +13,13 @@ import { IFormatterHelper } from './types'; export abstract class BaseFormatter { protected readonly outputChannel: OutputChannel; + protected readonly workspace: IWorkspaceService; private readonly helper: IFormatterHelper; - constructor(public Id: string, private product: Product, private serviceContainer: IServiceContainer) { - this.outputChannel = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); - this.helper = this.serviceContainer.get(IFormatterHelper); + + constructor(public Id: string, private product: Product, protected serviceContainer: IServiceContainer) { + this.outputChannel = serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); + this.helper = serviceContainer.get(IFormatterHelper); + this.workspace = serviceContainer.get(IWorkspaceService); } public abstract formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable; @@ -26,12 +30,13 @@ export abstract class BaseFormatter { return path.dirname(document.fileName); } protected getWorkspaceUri(document: vscode.TextDocument) { - const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri); + const workspaceFolder = this.workspace.getWorkspaceFolder(document.uri); if (workspaceFolder) { return workspaceFolder.uri; } - if (Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) { - return vscode.workspace.workspaceFolders[0].uri; + const folders = this.workspace.workspaceFolders; + if (Array.isArray(folders) && folders.length > 0) { + return folders[0].uri; } return vscode.Uri.file(__dirname); } diff --git a/src/client/formatters/helper.ts b/src/client/formatters/helper.ts index 4eb20bea34a5..e5930c2f0683 100644 --- a/src/client/formatters/helper.ts +++ b/src/client/formatters/helper.ts @@ -1,16 +1,17 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { injectable } from 'inversify'; +import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Uri } from 'vscode'; -import { PythonSettings } from '../common/configSettings'; -import { IFormattingSettings } from '../common/types'; +import { IConfigurationService, IFormattingSettings } from '../common/types'; import { ExecutionInfo, 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) { } public translateToId(formatter: Product): FormatterId { switch (formatter) { case Product.autopep8: return 'autopep8'; @@ -28,7 +29,7 @@ export class FormatterHelper implements IFormatterHelper { }; } public getExecutionInfo(formatter: Product, customArgs: string[], resource?: Uri): ExecutionInfo { - const settings = PythonSettings.getInstance(resource); + const settings = this.serviceContainer.get(IConfigurationService).getSettings(resource); const names = this.getSettingsPropertyNames(formatter); const execPath = settings.formatting[names.pathName] as string; diff --git a/src/client/formatters/yapfFormatter.ts b/src/client/formatters/yapfFormatter.ts index 4682eb64daec..31a768ca0a57 100644 --- a/src/client/formatters/yapfFormatter.ts +++ b/src/client/formatters/yapfFormatter.ts @@ -1,6 +1,5 @@ import * as vscode from 'vscode'; -import { PythonSettings } from '../common/configSettings'; -import { Product } from '../common/types'; +import { IConfigurationService, Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { sendTelemetryWhenDone } from '../telemetry'; import { FORMAT } from '../telemetry/constants'; @@ -14,7 +13,7 @@ export class YapfFormatter extends BaseFormatter { public formatDocument(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken, range?: vscode.Range): Thenable { const stopWatch = new StopWatch(); - const settings = PythonSettings.getInstance(document.uri); + const settings = this.serviceContainer.get(IConfigurationService).getSettings(document.uri); const hasCustomArgs = Array.isArray(settings.formatting.yapfArgs) && settings.formatting.yapfArgs.length > 0; const formatSelection = range ? !range.isEmpty : false; diff --git a/src/client/providers/formatProvider.ts b/src/client/providers/formatProvider.ts index f905f06f5c2f..cabab87018e8 100644 --- a/src/client/providers/formatProvider.ts +++ b/src/client/providers/formatProvider.ts @@ -1,13 +1,28 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { setTimeout } from 'timers'; import * as vscode from 'vscode'; +import { ICommandManager, IDocumentManager, IWorkspaceService } from '../common/application/types'; +import { IConfigurationService } from '../common/types'; import { IServiceContainer } from '../ioc/types'; -import { PythonSettings } from './../common/configSettings'; import { AutoPep8Formatter } from './../formatters/autoPep8Formatter'; import { BaseFormatter } from './../formatters/baseFormatter'; import { DummyFormatter } from './../formatters/dummyFormatter'; import { YapfFormatter } from './../formatters/yapfFormatter'; -export class PythonFormattingEditProvider implements vscode.DocumentFormattingEditProvider, vscode.DocumentRangeFormattingEditProvider { +export class PythonFormattingEditProvider implements vscode.DocumentFormattingEditProvider, vscode.DocumentRangeFormattingEditProvider, vscode.Disposable { + private readonly config: IConfigurationService; + private readonly workspace: IWorkspaceService; + private readonly documentManager: IDocumentManager; + private readonly commands: ICommandManager; private formatters = new Map(); + private disposables: vscode.Disposable[] = []; + + // Workaround for https://github.com/Microsoft/vscode/issues/41194 + private documentVersionBeforeFormatting = -1; + private formatterMadeChanges = false; + private saving = false; public constructor(context: vscode.ExtensionContext, serviceContainer: IServiceContainer) { const yapfFormatter = new YapfFormatter(serviceContainer); @@ -16,16 +31,70 @@ export class PythonFormattingEditProvider implements vscode.DocumentFormattingEd this.formatters.set(yapfFormatter.Id, yapfFormatter); this.formatters.set(autoPep8.Id, autoPep8); this.formatters.set(dummy.Id, dummy); + + this.commands = serviceContainer.get(ICommandManager); + 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))); + } + + public dispose() { + this.disposables.forEach(d => d.dispose()); } - public provideDocumentFormattingEdits(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken): Thenable { + public provideDocumentFormattingEdits(document: vscode.TextDocument, options: vscode.FormattingOptions, token: vscode.CancellationToken): Promise { return this.provideDocumentRangeFormattingEdits(document, undefined, options, token); } - public provideDocumentRangeFormattingEdits(document: vscode.TextDocument, range: vscode.Range | undefined, options: vscode.FormattingOptions, token: vscode.CancellationToken): Thenable { - const settings = PythonSettings.getInstance(document.uri); + public async provideDocumentRangeFormattingEdits(document: vscode.TextDocument, range: vscode.Range | undefined, options: vscode.FormattingOptions, token: vscode.CancellationToken): Promise { + // Workaround for https://github.com/Microsoft/vscode/issues/41194 + // VSC rejects 'format on save' promise in 750 ms. Python formatting may take quite a bit longer. + // Workaround is to resolve promise to nothing here, then execute format document and force new save. + // However, we need to know if this is 'format document' or formatting on save. + + if (this.saving) { + // We are saving after formatting (see onSaveDocument below) + // so we do not want to format again. + return []; + } + + // Remember content before formatting so we can detect if + // formatting edits have been really applied + const editorConfig = this.workspace.getConfiguration('editor', document.uri); + if (editorConfig.get('formatOnSave') === true) { + this.documentVersionBeforeFormatting = document.version; + } + + const settings = this.config.getSettings(document.uri); const formatter = this.formatters.get(settings.formatting.provider)!; - return formatter.formatDocument(document, options, token, range); + const edits = await formatter.formatDocument(document, options, token, range); + + this.formatterMadeChanges = edits.length > 0; + return edits; } + private async onSaveDocument(document: vscode.TextDocument): Promise { + // Promise was rejected = formatting took too long. + // Don't format inside the event handler, do it on timeout + setTimeout(() => { + try { + if (this.formatterMadeChanges + && !document.isDirty + && document.version === this.documentVersionBeforeFormatting) { + // Formatter changes were not actually applied due to the timeout on save. + // Force formatting now and then save the document. + this.commands.executeCommand('editor.action.formatDocument').then(async () => { + this.saving = true; + await document.save(); + this.saving = false; + }); + } + } finally { + this.documentVersionBeforeFormatting = -1; + this.saving = false; + this.formatterMadeChanges = false; + } + }, 50); + } } diff --git a/src/test/format/extension.format.test.ts b/src/test/format/extension.format.test.ts index a078a4c92d98..03a44962bb06 100644 --- a/src/test/format/extension.format.test.ts +++ b/src/test/format/extension.format.test.ts @@ -56,7 +56,6 @@ suite('Formatting', () => { }); teardown(async () => { ioc.dispose(); - await closeActiveWindows(); }); function initializeDI() { diff --git a/src/test/format/extension.formatOnSave.test.ts b/src/test/format/extension.formatOnSave.test.ts new file mode 100644 index 000000000000..d6bbd442bc1b --- /dev/null +++ b/src/test/format/extension.formatOnSave.test.ts @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { expect } from 'chai'; +import * as fs from 'fs-extra'; +import * as path from 'path'; +import * as TypeMoq from 'typemoq'; +import * as vscode from 'vscode'; +import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; +import { PythonSettings } from '../../client/common/configSettings'; +import { IConfigurationService } from '../../client/common/types'; +import { PythonFormattingEditProvider } from '../../client/providers/formatProvider'; +import { closeActiveWindows } from '../initialize'; +import { UnitTestIocContainer } from '../unittests/serviceRegistry'; + +const formatFilesPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'formatting'); +const unformattedFile = path.join(formatFilesPath, 'fileToFormat.py'); + +suite('Formating On Save', () => { + let ioc: UnitTestIocContainer; + let config: TypeMoq.IMock; + let editorConfig: TypeMoq.IMock; + let workspace: TypeMoq.IMock; + let documentManager: TypeMoq.IMock; + let commands: TypeMoq.IMock; + let options: TypeMoq.IMock; + let listener: (d: vscode.TextDocument) => Promise; + + setup(initializeDI); + suiteTeardown(async () => { + await closeActiveWindows(); + }); + teardown(async () => { + ioc.dispose(); + await closeActiveWindows(); + }); + + function initializeDI() { + ioc = new UnitTestIocContainer(); + ioc.registerFormatterTypes(); + ioc.registerProcessTypes(); + ioc.registerVariableTypes(); + ioc.registerMockProcess(); + + config = TypeMoq.Mock.ofType(); + config.setup(x => x.getSettings(TypeMoq.It.isAny())).returns(() => PythonSettings.getInstance()); + + editorConfig = TypeMoq.Mock.ofType(); + + workspace = TypeMoq.Mock.ofType(); + workspace.setup(x => x.getConfiguration('editor', TypeMoq.It.isAny())).returns(() => editorConfig.object); + + const event = TypeMoq.Mock.ofType>(); + event.setup(x => x(TypeMoq.It.isAny())).callback((s) => { + listener = s as ((d: vscode.TextDocument) => Promise); + // tslint:disable-next-line:no-empty + }).returns(() => new vscode.Disposable(() => { })); + + documentManager = TypeMoq.Mock.ofType(); + documentManager.setup(x => x.onDidSaveTextDocument).returns(() => event.object); + + options = TypeMoq.Mock.ofType(); + options.setup(x => x.insertSpaces).returns(() => true); + options.setup(x => x.tabSize).returns(() => 4); + + commands = TypeMoq.Mock.ofType(); + commands.setup(x => x.executeCommand('editor.action.formatDocument')).returns(() => + new Promise((resolve, reject) => resolve()) + ); + ioc.serviceManager.addSingletonInstance(IConfigurationService, config.object); + ioc.serviceManager.addSingletonInstance(ICommandManager, commands.object); + ioc.serviceManager.addSingletonInstance(IWorkspaceService, workspace.object); + ioc.serviceManager.addSingletonInstance(IDocumentManager, documentManager.object); + } + + test('Workaround VS Code 41194', async () => { + editorConfig.setup(x => x.get('formatOnSave')).returns(() => true); + + const content = await fs.readFile(unformattedFile, 'utf8'); + let version = 1; + + const document = TypeMoq.Mock.ofType(); + document.setup(x => x.getText()).returns(() => content); + document.setup(x => x.uri).returns(() => vscode.Uri.file(unformattedFile)); + document.setup(x => x.isDirty).returns(() => false); + document.setup(x => x.fileName).returns(() => unformattedFile); + document.setup(x => x.save()).callback(() => version += 1); + document.setup(x => x.version).returns(() => version); + + const context = TypeMoq.Mock.ofType(); + const provider = new PythonFormattingEditProvider(context.object, ioc.serviceContainer); + const edits = await provider.provideDocumentFormattingEdits(document.object, options.object, new vscode.CancellationTokenSource().token); + expect(edits.length).be.greaterThan(0, 'Formatter produced no edits'); + + await listener(document.object); + await new Promise((resolve, reject) => setTimeout(resolve, 500)); + + commands.verify(x => x.executeCommand('editor.action.formatDocument'), TypeMoq.Times.once()); + document.verify(x => x.save(), TypeMoq.Times.once()); + }); +}); diff --git a/src/test/format/format.helper.test.ts b/src/test/format/format.helper.test.ts index a77c4eb3b9b7..51afda1a64db 100644 --- a/src/test/format/format.helper.test.ts +++ b/src/test/format/format.helper.test.ts @@ -1,15 +1,28 @@ import * as assert from 'assert'; +import * as TypeMoq from 'typemoq'; import { PythonSettings } from '../../client/common/configSettings'; import { EnumEx } from '../../client/common/enumUtils'; -import { IFormattingSettings, Product } from '../../client/common/types'; +import { IConfigurationService, IFormattingSettings, Product } from '../../client/common/types'; import { FormatterHelper } from '../../client/formatters/helper'; import { FormatterId } from '../../client/formatters/types'; import { initialize } from '../initialize'; +import { UnitTestIocContainer } from '../unittests/serviceRegistry'; // tslint:disable-next-line:max-func-body-length suite('Formatting - Helper', () => { - const formatHelper = new FormatterHelper(); + let ioc: UnitTestIocContainer; + let formatHelper: FormatterHelper; + suiteSetup(initialize); + setup(() => { + ioc = new UnitTestIocContainer(); + + const config = TypeMoq.Mock.ofType(); + config.setup(x => x.getSettings(TypeMoq.It.isAny())).returns(() => PythonSettings.getInstance()); + + ioc.serviceManager.addSingletonInstance(IConfigurationService, config.object); + formatHelper = new FormatterHelper(ioc.serviceManager); + }); test('Ensure product is set in Execution Info', async () => { [Product.autopep8, Product.yapf].forEach(formatter => {