From 2fadb901982adf9f52e743a95a4f290d3752c920 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 18 Apr 2018 10:51:33 -0700 Subject: [PATCH] Move TS/JS to use organize imports codeAction instead of command (#47850) * Move TS/JS to use organize imports code action Fixes #47845 Fixes #46647 - Defines a new standard `SourceOrganizeImports` `CodeActionKind` to be used to implement organize imports in a consistent way. - Add a new `Organize imports` command and keybinding that executes these actions. - Move over the existing js/ts organize imports command to use the new code action kind * Use supportedCodeActions context key * Document code action kind values * Fix regular expression Make sure we only match whole scopes and not `unicorn.source.organizeImports` --- .../typescript-language-features/package.json | 34 -------- .../package.nls.json | 1 - .../src/extension.ts | 7 -- .../src/features/organizeImports.ts | 81 ++++++------------- .../src/languageProvider.ts | 10 ++- .../contrib/codeAction/codeActionCommands.ts | 45 ++++++++++- .../codeAction/codeActionContributions.ts | 3 +- .../contrib/codeAction/codeActionModel.ts | 32 +++----- .../contrib/codeAction/codeActionTrigger.ts | 1 + src/vs/vscode.d.ts | 17 ++-- src/vs/workbench/api/node/extHostTypes.ts | 1 + 11 files changed, 95 insertions(+), 137 deletions(-) diff --git a/extensions/typescript-language-features/package.json b/extensions/typescript-language-features/package.json index 649154ef5fee3..836a370e78545 100644 --- a/extensions/typescript-language-features/package.json +++ b/extensions/typescript-language-features/package.json @@ -39,7 +39,6 @@ "onCommand:javascript.goToProjectConfig", "onCommand:typescript.goToProjectConfig", "onCommand:typescript.openTsServerLog", - "onCommand:typescript.organizeImports", "onCommand:workbench.action.tasks.runTask" ], "main": "./out/extension", @@ -464,23 +463,6 @@ "command": "typescript.restartTsServer", "title": "%typescript.restartTsServer%", "category": "TypeScript" - }, - { - "command": "typescript.organizeImports", - "title": "%typescript.organizeImports%", - "category": "TypeScript" - }, - { - "command": "javascript.organizeImports", - "title": "%typescript.organizeImports%", - "category": "JavaScript" - } - ], - "keybindings": [ - { - "command": "typescript.organizeImports", - "key": "shift+alt+o", - "when": "typescript.isManagedFile && typescript.canOrganizeImports" } ], "menus": { @@ -528,22 +510,6 @@ { "command": "typescript.restartTsServer", "when": "typescript.isManagedFile" - }, - { - "command": "typescript.organizeImports", - "when": "editorLangId == typescriptreact && typescript.isManagedFile && typescript.canOrganizeImports" - }, - { - "command": "typescript.organizeImports", - "when": "editorLangId == typescript && typescript.isManagedFile && typescript.canOrganizeImports" - }, - { - "command": "javascript.organizeImports", - "when": "editorLangId == javascriptreact && typescript.isManagedFile && typescript.canOrganizeImports" - }, - { - "command": "javascript.organizeImports", - "when": "editorLangId == javascript && typescript.isManagedFile && typescript.canOrganizeImports" } ] }, diff --git a/extensions/typescript-language-features/package.nls.json b/extensions/typescript-language-features/package.nls.json index d3d8458eef014..18ab9fadd9a12 100644 --- a/extensions/typescript-language-features/package.nls.json +++ b/extensions/typescript-language-features/package.nls.json @@ -53,7 +53,6 @@ "typescript.autoImportSuggestions.enabled": "Enable/disable auto import suggestions. Requires TypeScript >=2.6.1", "typescript.experimental.syntaxFolding": "Enables/disables syntax aware folding markers.", "taskDefinition.tsconfig.description": "The tsconfig file that defines the TS build.", - "typescript.organizeImports": "Organize Imports", "javascript.suggestionActions.enabled": "Enable/disable suggestion diagnostics for JavaScript files in the editor. Requires TypeScript >= 2.8", "typescript.suggestionActions.enabled": "Enable/disable suggestion diagnostics for TypeScript files in the editor. Requires TypeScript >= 2.8." } \ No newline at end of file diff --git a/extensions/typescript-language-features/src/extension.ts b/extensions/typescript-language-features/src/extension.ts index ac33918d1c264..960296b049187 100644 --- a/extensions/typescript-language-features/src/extension.ts +++ b/extensions/typescript-language-features/src/extension.ts @@ -19,7 +19,6 @@ import ManagedFileContextManager from './utils/managedFileContext'; import { lazy, Lazy } from './utils/lazy'; import * as fileSchemes from './utils/fileSchemes'; import LogDirectoryProvider from './utils/logDirectoryProvider'; -import { OrganizeImportsCommand, OrganizeImportsContextManager } from './features/organizeImports'; export function activate( context: vscode.ExtensionContext @@ -74,11 +73,6 @@ function createLazyClientHost( context.subscriptions.push(clientHost); - const organizeImportsContext = new OrganizeImportsContextManager(); - clientHost.serviceClient.onTsServerStarted(api => { - organizeImportsContext.onDidChangeApiVersion(api); - }, null, context.subscriptions); - clientHost.serviceClient.onReady(() => { context.subscriptions.push( ProjectStatus.create( @@ -103,7 +97,6 @@ function registerCommands( commandManager.register(new commands.RestartTsServerCommand(lazyClientHost)); commandManager.register(new commands.TypeScriptGoToProjectConfigCommand(lazyClientHost)); commandManager.register(new commands.JavaScriptGoToProjectConfigCommand(lazyClientHost)); - commandManager.register(new OrganizeImportsCommand(lazyClientHost)); } function isSupportedDocument( diff --git a/extensions/typescript-language-features/src/features/organizeImports.ts b/extensions/typescript-language-features/src/features/organizeImports.ts index 96bf276d3cb1a..2fe5f9e71d2ef 100644 --- a/extensions/typescript-language-features/src/features/organizeImports.ts +++ b/extensions/typescript-language-features/src/features/organizeImports.ts @@ -4,37 +4,27 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; - +import * as nls from 'vscode-nls'; import * as Proto from '../protocol'; -import { Command } from '../utils/commandManager'; +import { ITypeScriptServiceClient } from '../typescriptService'; +import { Command, CommandManager } from '../utils/commandManager'; +import { isSupportedLanguageMode } from '../utils/languageModeIds'; import * as typeconverts from '../utils/typeConverters'; -import { isSupportedLanguageMode } from '../utils/languageModeIds'; -import API from '../utils/api'; -import { Lazy } from '../utils/lazy'; -import TypeScriptServiceClientHost from '../typeScriptServiceClientHost'; -import { ITypeScriptServiceClient } from '../typescriptService'; -import * as nls from 'vscode-nls'; const localize = nls.loadMessageBundle(); -export class OrganizeImportsCommand implements Command { - public static readonly Ids = ['javascript.organizeImports', 'typescript.organizeImports']; +class OrganizeImportsCommand implements Command { + public static readonly Id = '_typescript.organizeImports'; - public readonly id = OrganizeImportsCommand.Ids; + public readonly id = OrganizeImportsCommand.Id; constructor( - private readonly lazyClientHost: Lazy + private readonly client: ITypeScriptServiceClient ) { } public async execute(): Promise { - // Don't force activation - if (!this.lazyClientHost.hasValue) { - return false; - } - - const client = this.lazyClientHost.value.serviceClient; - if (!client.apiVersion.has280Features()) { + if (!this.client.apiVersion.has280Features()) { return false; } @@ -43,7 +33,7 @@ export class OrganizeImportsCommand implements Command { return false; } - const file = client.normalizePath(editor.document.uri); + const file = this.client.normalizePath(editor.document.uri); if (!file) { return false; } @@ -56,67 +46,42 @@ export class OrganizeImportsCommand implements Command { } } }; - const response = await client.execute('organizeImports', args); + const response = await this.client.execute('organizeImports', args); if (!response || !response.success) { return false; } - const edits = typeconverts.WorkspaceEdit.fromFromFileCodeEdits(client, response.body); + const edits = typeconverts.WorkspaceEdit.fromFromFileCodeEdits(this.client, response.body); return await vscode.workspace.applyEdit(edits); } } -/** - * When clause context set when the ts version supports organize imports. - */ -const contextName = 'typescript.canOrganizeImports'; - -export class OrganizeImportsContextManager { - - private currentValue: boolean = false; - - public onDidChangeApiVersion(apiVersion: API): any { - this.updateContext(apiVersion.has280Features()); - } - - private updateContext(newValue: boolean) { - if (newValue === this.currentValue) { - return; - } - - vscode.commands.executeCommand('setContext', contextName, newValue); - this.currentValue = newValue; - } -} - - export class OrganizeImportsCodeActionProvider implements vscode.CodeActionProvider { - private static readonly organizeImportsKind = vscode.CodeActionKind.Source.append('organizeImports'); - public constructor( - private readonly client: ITypeScriptServiceClient - ) { } + private readonly client: ITypeScriptServiceClient, + commandManager: CommandManager + ) { + commandManager.register(new OrganizeImportsCommand(client)); + } public readonly metadata: vscode.CodeActionProviderMetadata = { - providedCodeActionKinds: [OrganizeImportsCodeActionProvider.organizeImportsKind] + providedCodeActionKinds: [vscode.CodeActionKind.SourceOrganizeImports] }; public provideCodeActions( - document: vscode.TextDocument, + _document: vscode.TextDocument, _range: vscode.Range, _context: vscode.CodeActionContext, _token: vscode.CancellationToken ): vscode.CodeAction[] { - if (!isSupportedLanguageMode(document)) { - return []; - } - if (!this.client.apiVersion.has280Features()) { return []; } - const action = new vscode.CodeAction(localize('oraganizeImportsAction.title', "Organize Imports"), OrganizeImportsCodeActionProvider.organizeImportsKind); - action.command = { title: '', command: OrganizeImportsCommand.Ids[0] }; + const action = new vscode.CodeAction( + localize('oraganizeImportsAction.title', "Organize Imports"), + vscode.CodeActionKind.SourceOrganizeImports); + action.command = { title: '', command: OrganizeImportsCommand.Id }; return [action]; } } \ No newline at end of file diff --git a/extensions/typescript-language-features/src/languageProvider.ts b/extensions/typescript-language-features/src/languageProvider.ts index f23c7fab62052..b2cdbf56456aa 100644 --- a/extensions/typescript-language-features/src/languageProvider.ts +++ b/extensions/typescript-language-features/src/languageProvider.ts @@ -43,7 +43,7 @@ export default class LanguageProvider { constructor( private readonly client: TypeScriptServiceClient, private readonly description: LanguageDescription, - commandManager: CommandManager, + private readonly commandManager: CommandManager, typingsStatus: TypingsStatus ) { this.formattingOptionsManager = new FormattingConfigurationManager(client); @@ -123,9 +123,6 @@ export default class LanguageProvider { const refactorProvider = new (await import('./features/refactorProvider')).default(client, this.formattingOptionsManager, commandManager); this.disposables.push(languages.registerCodeActionsProvider(selector, refactorProvider, refactorProvider.metadata)); - const organizeImportsProvider = new (await import('./features/organizeImports')).OrganizeImportsCodeActionProvider(client); - this.disposables.push(languages.registerCodeActionsProvider(selector, organizeImportsProvider, organizeImportsProvider.metadata)); - await this.initFoldingProvider(); this.disposables.push(workspace.onDidChangeConfiguration(c => { if (c.affectsConfiguration(foldingSetting)) { @@ -247,6 +244,11 @@ export default class LanguageProvider { if (this.client.apiVersion.has213Features()) { this.versionDependentDisposables.push(languages.registerTypeDefinitionProvider(selector, new (await import('./features/typeDefinitionProvider')).default(this.client))); } + + if (this.client.apiVersion.has280Features()) { + const organizeImportsProvider = new (await import('./features/organizeImports')).OrganizeImportsCodeActionProvider(this.client, this.commandManager); + this.versionDependentDisposables.push(languages.registerCodeActionsProvider(selector, organizeImportsProvider, organizeImportsProvider.metadata)); + } } public triggerAllDiagnostics(): void { diff --git a/src/vs/editor/contrib/codeAction/codeActionCommands.ts b/src/vs/editor/contrib/codeAction/codeActionCommands.ts index 9f9464324a37f..3083b047c8d3b 100644 --- a/src/vs/editor/contrib/codeAction/codeActionCommands.ts +++ b/src/vs/editor/contrib/codeAction/codeActionCommands.ts @@ -22,10 +22,17 @@ import { IFileService } from 'vs/platform/files/common/files'; import { optional } from 'vs/platform/instantiation/common/instantiation'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { IMarkerService } from 'vs/platform/markers/common/markers'; -import { CodeActionModel, CodeActionsComputeEvent, HAS_REFACTOR_PROVIDER, HAS_SOURCE_ACTION_PROVIDER } from './codeActionModel'; +import { CodeActionModel, CodeActionsComputeEvent, SUPPORTED_CODE_ACTIONS } from './codeActionModel'; import { CodeActionAutoApply, CodeActionFilter, CodeActionKind } from './codeActionTrigger'; import { CodeActionContextMenu } from './codeActionWidget'; import { LightBulbWidget } from './lightBulbWidget'; +import { escapeRegExpCharacters } from 'vs/base/common/strings'; + +function contextKeyForSupportedActions(kind: CodeActionKind) { + return ContextKeyExpr.regex( + SUPPORTED_CODE_ACTIONS.keys()[0], + new RegExp('(\\s|^)' + escapeRegExpCharacters(kind.value) + '\\b')); +} export class QuickFixController implements IEditorContribution { @@ -247,7 +254,9 @@ export class RefactorAction extends EditorAction { menuOpts: { group: '1_modification', order: 2, - when: ContextKeyExpr.and(EditorContextKeys.writable, HAS_REFACTOR_PROVIDER), + when: ContextKeyExpr.and( + EditorContextKeys.writable, + contextKeyForSupportedActions(CodeActionKind.Refactor)), } }); } @@ -274,8 +283,9 @@ export class SourceAction extends EditorAction { menuOpts: { group: '1_modification', order: 2.1, - when: ContextKeyExpr.and(EditorContextKeys.writable, HAS_SOURCE_ACTION_PROVIDER), - + when: ContextKeyExpr.and( + EditorContextKeys.writable, + contextKeyForSupportedActions(CodeActionKind.Source)), } }); } @@ -286,4 +296,31 @@ export class SourceAction extends EditorAction { { kind: CodeActionKind.Source, includeSourceActions: true }, CodeActionAutoApply.Never); } +} + +export class OrganizeImportsAction extends EditorAction { + + static readonly Id = 'editor.action.organizeImports'; + + constructor() { + super({ + id: OrganizeImportsAction.Id, + label: nls.localize('organizeImports.label', "Organize Imports"), + alias: 'Organize Imports', + precondition: ContextKeyExpr.and( + EditorContextKeys.writable, + contextKeyForSupportedActions(CodeActionKind.SourceOrganizeImports)), + kbOpts: { + kbExpr: EditorContextKeys.editorTextFocus, + primary: KeyMod.Shift | KeyMod.Alt | KeyCode.KEY_O + } + }); + } + + public run(accessor: ServicesAccessor, editor: ICodeEditor): void { + return showCodeActionsForEditorSelection(editor, + nls.localize('editor.action.organize.noneMessage', "No organize imports action available"), + { kind: CodeActionKind.SourceOrganizeImports, includeSourceActions: true }, + CodeActionAutoApply.IfSingle); + } } \ No newline at end of file diff --git a/src/vs/editor/contrib/codeAction/codeActionContributions.ts b/src/vs/editor/contrib/codeAction/codeActionContributions.ts index 52df471057fcf..8653b9354709e 100644 --- a/src/vs/editor/contrib/codeAction/codeActionContributions.ts +++ b/src/vs/editor/contrib/codeAction/codeActionContributions.ts @@ -4,11 +4,12 @@ *--------------------------------------------------------------------------------------------*/ import { registerEditorAction, registerEditorCommand, registerEditorContribution } from 'vs/editor/browser/editorExtensions'; -import { SourceAction, QuickFixController, QuickFixAction, CodeActionCommand, RefactorAction } from 'vs/editor/contrib/codeAction/codeActionCommands'; +import { SourceAction, QuickFixController, QuickFixAction, CodeActionCommand, RefactorAction, OrganizeImportsAction } from 'vs/editor/contrib/codeAction/codeActionCommands'; registerEditorContribution(QuickFixController); registerEditorAction(QuickFixAction); registerEditorAction(RefactorAction); registerEditorAction(SourceAction); +registerEditorAction(OrganizeImportsAction); registerEditorCommand(new CodeActionCommand()); diff --git a/src/vs/editor/contrib/codeAction/codeActionModel.ts b/src/vs/editor/contrib/codeAction/codeActionModel.ts index 7e7fe02e1f236..a369f31e28e74 100644 --- a/src/vs/editor/contrib/codeAction/codeActionModel.ts +++ b/src/vs/editor/contrib/codeAction/codeActionModel.ts @@ -15,10 +15,9 @@ import { CodeAction, CodeActionProviderRegistry } from 'vs/editor/common/modes'; import { IContextKey, IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey'; import { IMarkerService } from 'vs/platform/markers/common/markers'; import { getCodeActions } from './codeAction'; -import { CodeActionKind, CodeActionTrigger } from './codeActionTrigger'; +import { CodeActionTrigger } from './codeActionTrigger'; -export const HAS_REFACTOR_PROVIDER = new RawContextKey('hasRefactorProvider', false); -export const HAS_SOURCE_ACTION_PROVIDER = new RawContextKey('hasSourceActionProvider', false); +export const SUPPORTED_CODE_ACTIONS = new RawContextKey('supportedCodeAction', ''); export class CodeActionOracle { @@ -144,15 +143,13 @@ export class CodeActionModel { private _codeActionOracle: CodeActionOracle; private _onDidChangeFixes = new Emitter(); private _disposables: IDisposable[] = []; - private readonly _hasRefactorProvider: IContextKey; - private readonly _hasSourceProvider: IContextKey; + private readonly _supportedCodeActions: IContextKey; constructor(editor: ICodeEditor, markerService: IMarkerService, contextKeyService: IContextKeyService) { this._editor = editor; this._markerService = markerService; - this._hasRefactorProvider = HAS_REFACTOR_PROVIDER.bindTo(contextKeyService); - this._hasSourceProvider = HAS_SOURCE_ACTION_PROVIDER.bindTo(contextKeyService); + this._supportedCodeActions = SUPPORTED_CODE_ACTIONS.bindTo(contextKeyService); this._disposables.push(this._editor.onDidChangeModel(() => this._update())); this._disposables.push(this._editor.onDidChangeModelLanguage(() => this._update())); @@ -182,28 +179,19 @@ export class CodeActionModel { && CodeActionProviderRegistry.has(this._editor.getModel()) && !this._editor.getConfiguration().readOnly) { - let hasRefactorProvider = false; - let hasSourceProvider = false; - outer: for (const provider of CodeActionProviderRegistry.all(this._editor.getModel())) { - if (!provider.providedCodeActionKinds) { - continue; - } - for (const providedKind of provider.providedCodeActionKinds) { - hasRefactorProvider = hasRefactorProvider || CodeActionKind.Refactor.contains(providedKind); - hasSourceProvider = hasSourceProvider || CodeActionKind.Source.contains(providedKind); - if (hasRefactorProvider && hasSourceProvider) { - break outer; - } + const supportedActions: string[] = []; + for (const provider of CodeActionProviderRegistry.all(this._editor.getModel())) { + if (Array.isArray(provider.providedCodeActionKinds)) { + supportedActions.push(...provider.providedCodeActionKinds); } } - this._hasRefactorProvider.set(hasRefactorProvider); - this._hasSourceProvider.set(hasSourceProvider); + this._supportedCodeActions.set(supportedActions.join(' ')); this._codeActionOracle = new CodeActionOracle(this._editor, this._markerService, p => this._onDidChangeFixes.fire(p)); this._codeActionOracle.trigger({ type: 'auto' }); } else { - this._hasRefactorProvider.reset(); + this._supportedCodeActions.reset(); } } diff --git a/src/vs/editor/contrib/codeAction/codeActionTrigger.ts b/src/vs/editor/contrib/codeAction/codeActionTrigger.ts index 382c8ebda682a..4f3c53b5dc9c4 100644 --- a/src/vs/editor/contrib/codeAction/codeActionTrigger.ts +++ b/src/vs/editor/contrib/codeAction/codeActionTrigger.ts @@ -11,6 +11,7 @@ export class CodeActionKind { public static readonly Empty = new CodeActionKind(''); public static readonly Refactor = new CodeActionKind('refactor'); public static readonly Source = new CodeActionKind('source'); + public static readonly SourceOrganizeImports = new CodeActionKind('source.organizeImports'); constructor( public readonly value: string diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 73c2a2ee36416..85bb26a4ed085 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -1887,17 +1887,17 @@ declare module 'vscode' { static readonly Empty: CodeActionKind; /** - * Base kind for quickfix actions. + * Base kind for quickfix actions: `quickfix` */ static readonly QuickFix: CodeActionKind; /** - * Base kind for refactoring actions. + * Base kind for refactoring actions: `refactor` */ static readonly Refactor: CodeActionKind; /** - * Base kind for refactoring extraction actions. + * Base kind for refactoring extraction actions: `refactor.extract` * * Example extract actions: * @@ -1910,7 +1910,7 @@ declare module 'vscode' { static readonly RefactorExtract: CodeActionKind; /** - * Base kind for refactoring inline actions. + * Base kind for refactoring inline actions: `refactor.inline` * * Example inline actions: * @@ -1922,7 +1922,7 @@ declare module 'vscode' { static readonly RefactorInline: CodeActionKind; /** - * Base kind for refactoring rewrite actions. + * Base kind for refactoring rewrite actions: `refactor.rewrite` * * Example rewrite actions: * @@ -1936,12 +1936,17 @@ declare module 'vscode' { static readonly RefactorRewrite: CodeActionKind; /** - * Base kind for source actions. + * Base kind for source actions: `source` * * Source code actions apply to the entire file. */ static readonly Source: CodeActionKind; + /** + * Base kind for an organize imports source action: `source.organizeImports` + */ + static readonly SourceOrganizeImports: CodeActionKind; + private constructor(value: string); /** diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index d4da587fb9e75..f877921b5d5c3 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -904,6 +904,7 @@ export class CodeActionKind { public static readonly RefactorInline = CodeActionKind.Refactor.append('inline'); public static readonly RefactorRewrite = CodeActionKind.Refactor.append('rewrite'); public static readonly Source = CodeActionKind.Empty.append('source'); + public static readonly SourceOrganizeImports = CodeActionKind.Source.append('organizeImports'); constructor( public readonly value: string