API allowing CodeActionProviders to reliably get the current editor selection (#49081)

Fixes #49024
Fixes #49060

See #49024 for details on the problem. Makes code action providers take a range | selection. Always pass the selection if one is available.

extensions/typescript-language-features/src/features/refactorProvider.ts

@@ -102,7 +102,7 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv
 
 	public async provideCodeActions(
 		document: vscode.TextDocument,
-		_range: vscode.Range,
+		rangeOrSelection: vscode.Range | vscode.Selection,
 		context: vscode.CodeActionContext,
 		token: vscode.CancellationToken
 	): Promise<vscode.CodeAction[]> {
@@ -114,22 +114,16 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv
 			return [];
 		}
 
-		if (!vscode.window.activeTextEditor) {
+		if (!(rangeOrSelection instanceof vscode.Selection) || rangeOrSelection.isEmpty) {
 			return [];
 		}
 
-		const editor = vscode.window.activeTextEditor;
 		const file = this.client.normalizePath(document.uri);
-		if (!file || editor.document.uri.fsPath !== document.uri.fsPath) {
+		if (!file) {
 			return [];
 		}
 
-		if (editor.selection.isEmpty) {
-			return [];
-		}
-
-		const range = editor.selection;
-		const args: Proto.GetApplicableRefactorsRequestArgs = typeConverters.Range.toFileRangeRequestArgs(file, range);
+		const args: Proto.GetApplicableRefactorsRequestArgs = typeConverters.Range.toFileRangeRequestArgs(file, rangeOrSelection);
 		try {
 			const response = await this.client.execute('getApplicableRefactors', args, token);
 			if (!response || !response.body) {
@@ -143,7 +137,7 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv
 					codeAction.command = {
 						title: info.description,
 						command: SelectRefactorCommand.ID,
-						arguments: [document, file, info, range]
+						arguments: [document, file, info, rangeOrSelection]
 					};
 					actions.push(codeAction);
 				} else {
@@ -152,7 +146,7 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv
 						codeAction.command = {
 							title: action.description,
 							command: ApplyRefactoringCommand.ID,
-							arguments: [document, file, info.name, action.name, range]
+							arguments: [document, file, info.name, action.name, rangeOrSelection]
 						};
 						actions.push(codeAction);
 					}

src/vs/editor/common/modes.ts

@@ -18,6 +18,7 @@ import { Color } from 'vs/base/common/color';
 import { IMarkerData } from 'vs/platform/markers/common/markers';
 import * as model from 'vs/editor/common/model';
 import { isObject } from 'vs/base/common/types';
+import { Selection } from 'vs/editor/common/core/selection';
 
 /**
  * Open ended enum at runtime
@@ -363,7 +364,7 @@ export interface CodeActionProvider {
 	/**
 	 * Provide commands for the given document and range.
 	 */
-	provideCodeActions(model: model.ITextModel, range: Range, context: CodeActionContext, token: CancellationToken): CodeAction[] | Thenable<CodeAction[]>;
+	provideCodeActions(model: model.ITextModel, range: Range | Selection, context: CodeActionContext, token: CancellationToken): CodeAction[] | Thenable<CodeAction[]>;
 
 	/**
 	 * Optional list of of CodeActionKinds that this provider returns.

src/vs/editor/contrib/codeAction/codeAction.ts

@@ -11,15 +11,18 @@ import { TPromise } from 'vs/base/common/winjs.base';
 import { registerLanguageCommand } from 'vs/editor/browser/editorExtensions';
 import { Range } from 'vs/editor/common/core/range';
 import { ITextModel } from 'vs/editor/common/model';
-import { CodeAction, CodeActionProviderRegistry } from 'vs/editor/common/modes';
+import { CodeAction, CodeActionProviderRegistry, CodeActionContext } from 'vs/editor/common/modes';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { CodeActionFilter, CodeActionKind } from './codeActionTrigger';
+import { Selection } from 'vs/editor/common/core/selection';
 
-export function getCodeActions(model: ITextModel, range: Range, filter?: CodeActionFilter): TPromise<CodeAction[]> {
-	const codeActionContext = { only: filter && filter.kind ? filter.kind.value : undefined };
+export function getCodeActions(model: ITextModel, rangeOrSelection: Range | Selection, filter?: CodeActionFilter): TPromise<CodeAction[]> {
+	const codeActionContext: CodeActionContext = {
+		only: filter && filter.kind ? filter.kind.value : undefined,
+	};
 
 	const promises = CodeActionProviderRegistry.all(model).map(support => {
-		return asWinJsPromise(token => support.provideCodeActions(model, range, codeActionContext, token)).then(providedCodeActions => {
+		return asWinJsPromise(token => support.provideCodeActions(model, rangeOrSelection, codeActionContext, token)).then(providedCodeActions => {
 			if (!Array.isArray(providedCodeActions)) {
 				return [];
 			}
@@ -80,5 +83,5 @@ registerLanguageCommand('_executeCodeActionProvider', function (accessor, args)
 		throw illegalArgument();
 	}
 
-	return getCodeActions(model, model.validateRange(range));
+	return getCodeActions(model, model.validateRange(range), undefined);
 });

src/vs/editor/contrib/codeAction/codeActionModel.ts

@@ -40,11 +40,8 @@ export class CodeActionOracle {
 	}
 
 	trigger(trigger: CodeActionTrigger) {
-		let rangeOrSelection = this._getRangeOfMarker() || this._getRangeOfSelectionUnlessWhitespaceEnclosed();
-		if (!rangeOrSelection && trigger.type === 'manual') {
-			rangeOrSelection = this._editor.getSelection();
-		}
-		return this._createEventAndSignalChange(trigger, rangeOrSelection);
+		const selection = this._getRangeOfSelectionUnlessWhitespaceEnclosed();
+		return this._createEventAndSignalChange(trigger, selection);
 	}
 
 	private _onMarkerChanges(resources: URI[]): void {
@@ -61,8 +58,7 @@ export class CodeActionOracle {
 		this.trigger({ type: 'auto' });
 	}
 
-	private _getRangeOfMarker(): Range {
-		const selection = this._editor.getSelection();
+	private _getRangeOfMarker(selection: Selection): Range {
 		const model = this._editor.getModel();
 		for (const marker of this._markerService.read({ resource: model.uri })) {
 			if (Range.intersectRanges(marker, selection)) {
@@ -72,7 +68,7 @@ export class CodeActionOracle {
 		return undefined;
 	}
 
-	private _getRangeOfSelectionUnlessWhitespaceEnclosed(): Selection {
+	private _getRangeOfSelectionUnlessWhitespaceEnclosed(): Selection | undefined {
 		const model = this._editor.getModel();
 		const selection = this._editor.getSelection();
 		if (selection.isEmpty()) {
@@ -101,26 +97,25 @@ export class CodeActionOracle {
 		return selection;
 	}
 
-	private _createEventAndSignalChange(trigger: CodeActionTrigger, rangeOrSelection: Range | Selection): TPromise<CodeAction[] | undefined> {
-		if (!rangeOrSelection) {
+	private _createEventAndSignalChange(trigger: CodeActionTrigger, selection: Selection | undefined): TPromise<CodeAction[] | undefined> {
+		if (!selection) {
 			// cancel
 			this._signalChange({
 				trigger,
-				range: undefined,
+				rangeOrSelection: undefined,
 				position: undefined,
 				actions: undefined,
 			});
 			return TPromise.as(undefined);
 		} else {
-			// actual
 			const model = this._editor.getModel();
-			const range = model.validateRange(rangeOrSelection);
-			const position = rangeOrSelection instanceof Selection ? rangeOrSelection.getPosition() : rangeOrSelection.getStartPosition();
-			const actions = getCodeActions(model, range, trigger && trigger.filter);
+			const markerRange = this._getRangeOfMarker(selection);
+			const position = markerRange ? markerRange.getStartPosition() : selection.getStartPosition();
+			const actions = getCodeActions(model, selection, trigger && trigger.filter);
 
 			this._signalChange({
 				trigger,
-				range,
+				rangeOrSelection: selection,
 				position,
 				actions
 			});
@@ -131,7 +126,7 @@ export class CodeActionOracle {
 
 export interface CodeActionsComputeEvent {
 	trigger: CodeActionTrigger;
-	range: Range;
+	rangeOrSelection: Range | Selection;
 	position: Position;
 	actions: TPromise<CodeAction[]>;
 }

src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts

@@ -6,15 +6,14 @@
 
 import * as assert from 'assert';
 import URI from 'vs/base/common/uri';
-import { TPromise } from 'vs/base/common/winjs.base';
 import { TextModel } from 'vs/editor/common/model/textModel';
 import { createTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor';
 import { MarkerService } from 'vs/platform/markers/common/markerService';
 import { CodeActionOracle } from 'vs/editor/contrib/codeAction/codeActionModel';
 import { CodeActionProviderRegistry, LanguageIdentifier } from 'vs/editor/common/modes';
 import { IDisposable } from 'vs/base/common/lifecycle';
-import { Range } from 'vs/editor/common/core/range';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
+import { Selection } from 'vs/editor/common/core/selection';
 
 suite('CodeAction', () => {
 
@@ -96,49 +95,6 @@ suite('CodeAction', () => {
 		});
 	});
 
-	test('Oracle -> marker wins over selection', () => {
-
-		let range: Range;
-		let reg = CodeActionProviderRegistry.register(languageIdentifier.language, {
-			provideCodeActions(doc, _range) {
-				range = _range;
-				return [];
-			}
-		});
-
-		markerService.changeOne('fake', uri, [{
-			startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 6,
-			message: 'error',
-			severity: 1,
-			code: '',
-			source: ''
-		}]);
-
-		let fixes: TPromise<any>[] = [];
-		let oracle = new CodeActionOracle(editor, markerService, e => {
-			fixes.push(e.actions);
-		}, 10);
-
-		editor.setSelection({ startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 13 });
-
-		return TPromise.join<any>([TPromise.timeout(20)].concat(fixes)).then(_ => {
-
-			// -> marker wins
-			assert.deepEqual(range, { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 6 });
-
-			// 'auto' triggered, non-empty selection BUT within a marker
-			editor.setSelection({ startLineNumber: 1, startColumn: 2, endLineNumber: 1, endColumn: 4 });
-
-			return TPromise.join([TPromise.timeout(20)].concat(fixes)).then(_ => {
-				reg.dispose();
-				oracle.dispose();
-
-				// assert marker
-				assert.deepEqual(range, { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 6 });
-			});
-		});
-	});
-
 	test('Lightbulb is in the wrong place, #29933', async function () {
 		let reg = CodeActionProviderRegistry.register(languageIdentifier.language, {
 			provideCodeActions(doc, _range) {
@@ -161,7 +117,11 @@ suite('CodeAction', () => {
 
 			let oracle = new CodeActionOracle(editor, markerService, e => {
 				assert.equal(e.trigger.type, 'auto');
-				assert.deepEqual(e.range, { startLineNumber: 3, startColumn: 1, endLineNumber: 3, endColumn: 4 });
+				const selection = <Selection>e.rangeOrSelection;
+				assert.deepEqual(selection.selectionStartLineNumber, 1);
+				assert.deepEqual(selection.selectionStartColumn, 1);
+				assert.deepEqual(selection.endLineNumber, 4);
+				assert.deepEqual(selection.endColumn, 1);
 				assert.deepEqual(e.position, { lineNumber: 3, column: 1 });
 
 				oracle.dispose();

src/vs/vscode.d.ts

@@ -2053,13 +2053,14 @@ declare module 'vscode' {
 		 * Provide commands for the given document and range.
 		 *
 		 * @param document The document in which the command was invoked.
-		 * @param range The range for which the command was invoked.
+		 * @param range The selector or range for which the command was invoked. This will always be a selection if
+		 * there is a currently active editor.
 		 * @param context Context carrying additional information.
 		 * @param token A cancellation token.
 		 * @return An array of commands, quick fixes, or refactorings or a thenable of such. The lack of a result can be
 		 * signaled by returning `undefined`, `null`, or an empty array.
 		 */
-		provideCodeActions(document: TextDocument, range: Range, context: CodeActionContext, token: CancellationToken): ProviderResult<(Command | CodeAction)[]>;
+		provideCodeActions(document: TextDocument, range: Range | Selection, context: CodeActionContext, token: CancellationToken): ProviderResult<(Command | CodeAction)[]>;
 	}
 
 	/**

src/vs/workbench/api/electron-browser/mainThreadLanguageFeatures.ts

@@ -22,6 +22,7 @@ import { IModeService } from 'vs/editor/common/services/modeService';
 import { extHostNamedCustomer } from 'vs/workbench/api/electron-browser/extHostCustomers';
 import * as typeConverters from 'vs/workbench/api/node/extHostTypeConverters';
 import URI from 'vs/base/common/uri';
+import { Selection } from 'vs/editor/common/core/selection';
 
 @extHostNamedCustomer(MainContext.MainThreadLanguageFeatures)
 export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesShape {
@@ -193,8 +194,8 @@ export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesSha
 
 	$registerQuickFixSupport(handle: number, selector: ISerializedDocumentFilter[], providedCodeActionKinds?: string[]): void {
 		this._registrations[handle] = modes.CodeActionProviderRegistry.register(typeConverters.LanguageSelector.from(selector), <modes.CodeActionProvider>{
-			provideCodeActions: (model: ITextModel, range: EditorRange, context: modes.CodeActionContext, token: CancellationToken): Thenable<modes.CodeAction[]> => {
-				return this._heapService.trackRecursive(wireCancellationToken(token, this._proxy.$provideCodeActions(handle, model.uri, range, context))).then(MainThreadLanguageFeatures._reviveCodeActionDto);
+			provideCodeActions: (model: ITextModel, rangeOrSelection: EditorRange | Selection, context: modes.CodeActionContext, token: CancellationToken): Thenable<modes.CodeAction[]> => {
+				return this._heapService.trackRecursive(wireCancellationToken(token, this._proxy.$provideCodeActions(handle, model.uri, rangeOrSelection, context))).then(MainThreadLanguageFeatures._reviveCodeActionDto);
 			},
 			providedCodeActionKinds
 		});

src/vs/workbench/api/node/extHost.protocol.ts

@@ -720,7 +720,7 @@ export interface ExtHostLanguageFeaturesShape {
 	$provideHover(handle: number, resource: UriComponents, position: IPosition): TPromise<modes.Hover>;
 	$provideDocumentHighlights(handle: number, resource: UriComponents, position: IPosition): TPromise<modes.DocumentHighlight[]>;
 	$provideReferences(handle: number, resource: UriComponents, position: IPosition, context: modes.ReferenceContext): TPromise<LocationDto[]>;
-	$provideCodeActions(handle: number, resource: UriComponents, range: IRange, context: modes.CodeActionContext): TPromise<CodeActionDto[]>;
+	$provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext): TPromise<CodeActionDto[]>;
 	$provideDocumentFormattingEdits(handle: number, resource: UriComponents, options: modes.FormattingOptions): TPromise<ISingleEditOperation[]>;
 	$provideDocumentRangeFormattingEdits(handle: number, resource: UriComponents, range: IRange, options: modes.FormattingOptions): TPromise<ISingleEditOperation[]>;
 	$provideOnTypeFormattingEdits(handle: number, resource: UriComponents, position: IPosition, ch: string, options: modes.FormattingOptions): TPromise<ISingleEditOperation[]>;

src/vs/workbench/api/node/extHostLanguageFeatures.ts

@@ -23,6 +23,7 @@ import { IPosition } from 'vs/editor/common/core/position';
 import { IRange } from 'vs/editor/common/core/range';
 import { isFalsyOrEmpty } from 'vs/base/common/arrays';
 import { isObject } from 'vs/base/common/types';
+import { ISelection, Selection } from 'vs/editor/common/core/selection';
 
 // --- adapter
 
@@ -267,14 +268,16 @@ class CodeActionAdapter {
 		private readonly _provider: vscode.CodeActionProvider
 	) { }
 
-	provideCodeActions(resource: URI, range: IRange, context: modes.CodeActionContext): TPromise<CodeActionDto[]> {
+	provideCodeActions(resource: URI, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext): TPromise<CodeActionDto[]> {
 
 		const doc = this._documents.getDocumentData(resource).document;
-		const ran = <vscode.Range>typeConvert.Range.to(range);
+		const ran = Selection.isISelection(rangeOrSelection)
+			? <vscode.Selection>typeConvert.Selection.to(rangeOrSelection)
+			: <vscode.Range>typeConvert.Range.to(rangeOrSelection);
 		const allDiagnostics: vscode.Diagnostic[] = [];
 
 		for (const diagnostic of this._diagnostics.getDiagnostics(resource)) {
-			if (ran.contains(diagnostic.range)) {
+			if (ran.intersection(diagnostic.range)) {
 				allDiagnostics.push(diagnostic);
 			}
 		}
@@ -283,6 +286,7 @@ class CodeActionAdapter {
 			diagnostics: allDiagnostics,
 			only: context.only ? new CodeActionKind(context.only) : undefined
 		};
+
 		return asWinJsPromise(token =>
 			this._provider.provideCodeActions(doc, ran, codeActionContext, token)
 		).then(commandsOrActions => {
@@ -1037,8 +1041,8 @@ export class ExtHostLanguageFeatures implements ExtHostLanguageFeaturesShape {
 	}
 
 
-	$provideCodeActions(handle: number, resource: UriComponents, range: IRange, context: modes.CodeActionContext): TPromise<CodeActionDto[]> {
-		return this._withAdapter(handle, CodeActionAdapter, adapter => adapter.provideCodeActions(URI.revive(resource), range, context));
+	$provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext): TPromise<CodeActionDto[]> {
+		return this._withAdapter(handle, CodeActionAdapter, adapter => adapter.provideCodeActions(URI.revive(resource), rangeOrSelection, context));
 	}
 
 	// --- formatting

src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts

@@ -654,7 +654,7 @@ suite('ExtHostLanguageFeatures', function () {
 		}));
 
 		return rpcProtocol.sync().then(() => {
-			return getCodeActions(model, model.getFullModelRange()).then(value => {
+			return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
 				assert.equal(value.length, 2);
 
 				const [first, second] = value;
@@ -681,7 +681,7 @@ suite('ExtHostLanguageFeatures', function () {
 		}));
 
 		return rpcProtocol.sync().then(() => {
-			return getCodeActions(model, model.getFullModelRange()).then(value => {
+			return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
 				assert.equal(value.length, 1);
 
 				const [first] = value;
@@ -707,7 +707,7 @@ suite('ExtHostLanguageFeatures', function () {
 		}));
 
 		return rpcProtocol.sync().then(() => {
-			return getCodeActions(model, model.getFullModelRange()).then(value => {
+			return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
 				assert.equal(value.length, 1);
 			});
 		});
@@ -727,7 +727,7 @@ suite('ExtHostLanguageFeatures', function () {
 		}));
 
 		return rpcProtocol.sync().then(() => {
-			return getCodeActions(model, model.getFullModelRange()).then(value => {
+			return getCodeActions(model, model.getFullModelRange(), undefined).then(value => {
 				assert.equal(value.length, 1);
 			});
 		});