Move TS/JS to use organize imports code action

Fixes microsoft#47845 Fixes microsoft#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
mjbvz · Apr 17, 2018 · 6fdb5c3 · 6fdb5c3
1 parent cdd9a73
commit 6fdb5c3
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 107 deletions.
diff --git 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
@@ -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."
 }
diff --git 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
@@ -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<TypeScriptServiceClientHost>
+		private readonly client: ITypeScriptServiceClient
 	) { }
 
 	public async execute(): Promise<boolean> {
-		// 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];
 	}
 }
diff --git 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
@@ -22,7 +22,7 @@ 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, HAS_REFACTOR_PROVIDER, HAS_SOURCE_ACTION_PROVIDER, HAS_ORGANIZE_IMPORTS_PROVIDER } from './codeActionModel';
 import { CodeActionAutoApply, CodeActionFilter, CodeActionKind } from './codeActionTrigger';
 import { CodeActionContextMenu } from './codeActionWidget';
 import { LightBulbWidget } from './lightBulbWidget';
@@ -286,4 +286,29 @@ 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, HAS_ORGANIZE_IMPORTS_PROVIDER),
+			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);
+	}
 }
diff --git 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
@@ -19,6 +19,7 @@ import { CodeActionKind, CodeActionTrigger } from './codeActionTrigger';
 
 export const HAS_REFACTOR_PROVIDER = new RawContextKey<boolean>('hasRefactorProvider', false);
 export const HAS_SOURCE_ACTION_PROVIDER = new RawContextKey<boolean>('hasSourceActionProvider', false);
+export const HAS_ORGANIZE_IMPORTS_PROVIDER = new RawContextKey<boolean>('hasOrganizeImportsActionProvider', false);
 
 export class CodeActionOracle {
 
@@ -146,13 +147,15 @@ export class CodeActionModel {
 	private _disposables: IDisposable[] = [];
 	private readonly _hasRefactorProvider: IContextKey<boolean>;
 	private readonly _hasSourceProvider: IContextKey<boolean>;
+	private readonly _hasOrganizeImportsProvider: IContextKey<boolean>;
 
 	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._hasOrganizeImportsProvider = HAS_ORGANIZE_IMPORTS_PROVIDER.bindTo(contextKeyService);
 
 		this._disposables.push(this._editor.onDidChangeModel(() => this._update()));
 		this._disposables.push(this._editor.onDidChangeModelLanguage(() => this._update()));
@@ -184,21 +187,24 @@ export class CodeActionModel {
 
 			let hasRefactorProvider = false;
 			let hasSourceProvider = false;
+			let hasOrgaizeImportsProvider = 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) {
+					hasOrgaizeImportsProvider = hasOrgaizeImportsProvider || CodeActionKind.SourceOrganizeImports.contains(providedKind);
+					if (hasRefactorProvider && hasSourceProvider && hasOrgaizeImportsProvider) {
 						break outer;
 					}
 				}
 			}
 
 			this._hasRefactorProvider.set(hasRefactorProvider);
 			this._hasSourceProvider.set(hasSourceProvider);
+			this._hasOrganizeImportsProvider.set(hasOrgaizeImportsProvider);
 
 			this._codeActionOracle = new CodeActionOracle(this._editor, this._markerService, p => this._onDidChangeFixes.fire(p));
 			this._codeActionOracle.trigger({ type: 'auto' });

diff --git 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
@@ -1942,6 +1942,11 @@ declare module 'vscode' {
 		 */
 		static readonly Source: CodeActionKind;
 
+		/**
+		 * Base kind for an organize imports source action.
+		 */
+		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
@@ -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