From b4a00ca33fc2252b88040092467b89dd238300a3 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 6 Jun 2019 22:11:11 -0700 Subject: [PATCH] Replacing heapservice with lifecycle for code actions Part of #74846 Code Actions can use commands internally, which must be disposed of. We were previously using the heap service for this but this will not work for the web. Add a custom lifecycle instead --- src/vs/editor/common/modes.ts | 6 +- .../editor/contrib/codeAction/codeAction.ts | 23 +++-- .../contrib/codeAction/codeActionCommands.ts | 16 ++- .../codeAction/test/codeAction.test.ts | 97 ++++++++++--------- .../codeAction/test/codeActionModel.test.ts | 25 +++-- .../editor/contrib/hover/modesContentHover.ts | 40 +++++--- .../standalone/browser/standaloneLanguages.ts | 4 +- src/vs/monaco.d.ts | 6 +- .../common/configurationRegistry.ts | 2 +- .../api/browser/mainThreadLanguageFeatures.ts | 26 ++--- .../api/browser/mainThreadSaveParticipant.ts | 2 + .../workbench/api/common/extHost.protocol.ts | 8 +- .../api/common/extHostLanguageFeatures.ts | 40 +++++--- .../markers/browser/markersTreeViewer.ts | 4 +- 14 files changed, 189 insertions(+), 110 deletions(-) diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index ed6e0782e9f..0584508a387 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -550,6 +550,10 @@ export interface CodeActionContext { trigger: CodeActionTrigger; } +export interface CodeActionList extends IDisposable { + readonly actions: ReadonlyArray; +} + /** * The code action interface defines the contract between extensions and * the [light bulb](https://code.visualstudio.com/docs/editor/editingevolved#_code-action) feature. @@ -559,7 +563,7 @@ export interface CodeActionProvider { /** * Provide commands for the given document and range. */ - provideCodeActions(model: model.ITextModel, range: Range | Selection, context: CodeActionContext, token: CancellationToken): ProviderResult; + provideCodeActions(model: model.ITextModel, range: Range | Selection, context: CodeActionContext, token: CancellationToken): ProviderResult; /** * Optional list of CodeActionKinds that this provider returns. diff --git a/src/vs/editor/contrib/codeAction/codeAction.ts b/src/vs/editor/contrib/codeAction/codeAction.ts index 854af117f7d..8a38c956230 100644 --- a/src/vs/editor/contrib/codeAction/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/codeAction.ts @@ -15,8 +15,9 @@ import { CodeAction, CodeActionContext, CodeActionProviderRegistry, CodeActionTr import { IModelService } from 'vs/editor/common/services/modelService'; import { CodeActionFilter, CodeActionKind, CodeActionTrigger, filtersAction, mayIncludeActionsOfKind } from './codeActionTrigger'; import { TextModelCancellationTokenSource } from 'vs/editor/browser/core/editorState'; +import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; -export class CodeActionSet { +export class CodeActionSet extends Disposable { private static codeActionsComparator(a: CodeAction, b: CodeAction): number { if (isNonEmptyArray(a.diagnostics)) { @@ -34,7 +35,9 @@ export class CodeActionSet { public readonly actions: readonly CodeAction[]; - public constructor(actions: readonly CodeAction[]) { + public constructor(actions: readonly CodeAction[], disposables: DisposableStore) { + super(); + this._register(disposables); this.actions = mergeSort([...actions], CodeActionSet.codeActionsComparator); } @@ -59,12 +62,14 @@ export function getCodeActions( const cts = new TextModelCancellationTokenSource(model, token); const providers = getCodeActionProviders(model, filter); + const disposables = new DisposableStore(); const promises = providers.map(provider => { return Promise.resolve(provider.provideCodeActions(model, rangeOrSelection, codeActionContext, cts.token)).then(providedCodeActions => { - if (cts.token.isCancellationRequested || !Array.isArray(providedCodeActions)) { + if (cts.token.isCancellationRequested || !providedCodeActions) { return []; } - return providedCodeActions.filter(action => action && filtersAction(filter, action)); + disposables.add(providedCodeActions); + return providedCodeActions.actions.filter(action => action && filtersAction(filter, action)); }, (err): CodeAction[] => { if (isPromiseCanceledError(err)) { throw err; @@ -84,7 +89,7 @@ export function getCodeActions( return Promise.all(promises) .then(flatten) - .then(actions => new CodeActionSet(actions)) + .then(actions => new CodeActionSet(actions, disposables)) .finally(() => { listener.dispose(); cts.dispose(); @@ -106,7 +111,7 @@ function getCodeActionProviders( }); } -registerLanguageCommand('_executeCodeActionProvider', function (accessor, args): Promise> { +registerLanguageCommand('_executeCodeActionProvider', async function (accessor, args): Promise> { const { resource, range, kind } = args; if (!(resource instanceof URI) || !Range.isIRange(range)) { throw illegalArgument(); @@ -117,9 +122,11 @@ registerLanguageCommand('_executeCodeActionProvider', function (accessor, args): throw illegalArgument(); } - return getCodeActions( + const codeActionSet = await getCodeActions( model, model.validateRange(range), { type: 'manual', filter: { includeSourceActions: true, kind: kind && kind.value ? new CodeActionKind(kind.value) : undefined } }, - CancellationToken.None).then(actions => actions.actions); + CancellationToken.None); + codeActionSet.dispose(); + return codeActionSet.actions; }); diff --git a/src/vs/editor/contrib/codeAction/codeActionCommands.ts b/src/vs/editor/contrib/codeAction/codeActionCommands.ts index 150f483052e..6263a6308a3 100644 --- a/src/vs/editor/contrib/codeAction/codeActionCommands.ts +++ b/src/vs/editor/contrib/codeAction/codeActionCommands.ts @@ -5,7 +5,7 @@ import { CancelablePromise } from 'vs/base/common/async'; import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, dispose } from 'vs/base/common/lifecycle'; import { escapeRegExpCharacters } from 'vs/base/common/strings'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { EditorAction, EditorCommand, ServicesAccessor } from 'vs/editor/browser/editorExtensions'; @@ -47,6 +47,7 @@ export class QuickFixController extends Disposable implements IEditorContributio private readonly _model: CodeActionModel; private readonly _codeActionContextMenu: CodeActionContextMenu; private readonly _lightBulbWidget: LightBulbWidget; + private _currentCodeActions: CodeActionSet | undefined; private _activeRequest: CancelablePromise | undefined; @@ -63,7 +64,7 @@ export class QuickFixController extends Disposable implements IEditorContributio super(); this._editor = editor; - this._model = new CodeActionModel(this._editor, markerService, contextKeyService, progressService); + this._model = this._register(new CodeActionModel(this._editor, markerService, contextKeyService, progressService)); this._codeActionContextMenu = this._register(new CodeActionContextMenu(editor, contextMenuService, action => this._onApplyCodeAction(action))); this._lightBulbWidget = this._register(new LightBulbWidget(editor)); @@ -75,9 +76,9 @@ export class QuickFixController extends Disposable implements IEditorContributio this._register(this._keybindingService.onDidUpdateKeybindings(this._updateLightBulbTitle, this)); } - public dispose(): void { + dipose() { super.dispose(); - this._model.dispose(); + dispose(this._currentCodeActions); } private _onDidChangeCodeActionsState(newState: CodeActionsState.State): void { @@ -89,6 +90,11 @@ export class QuickFixController extends Disposable implements IEditorContributio if (newState.type === CodeActionsState.Type.Triggered) { this._activeRequest = newState.actions; + newState.actions.then(actions => { + dispose(this._currentCodeActions); + this._currentCodeActions = actions; + }); + if (newState.trigger.filter && newState.trigger.filter.kind) { // Triggered for specific scope newState.actions.then(fixes => { @@ -115,6 +121,8 @@ export class QuickFixController extends Disposable implements IEditorContributio } } } else { + dispose(this._currentCodeActions); + this._currentCodeActions = undefined; this._lightBulbWidget.hide(); } } diff --git a/src/vs/editor/contrib/codeAction/test/codeAction.test.ts b/src/vs/editor/contrib/codeAction/test/codeAction.test.ts index 5fd325be68a..07c61c66067 100644 --- a/src/vs/editor/contrib/codeAction/test/codeAction.test.ts +++ b/src/vs/editor/contrib/codeAction/test/codeAction.test.ts @@ -7,15 +7,27 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { Range } from 'vs/editor/common/core/range'; import { TextModel } from 'vs/editor/common/model/textModel'; -import { CodeAction, CodeActionContext, CodeActionProvider, CodeActionProviderRegistry, Command, LanguageIdentifier, ResourceTextEdit, WorkspaceEdit } from 'vs/editor/common/modes'; +import * as modes from 'vs/editor/common/modes'; import { getCodeActions } from 'vs/editor/contrib/codeAction/codeAction'; import { CodeActionKind } from 'vs/editor/contrib/codeAction/codeActionTrigger'; import { IMarkerData, MarkerSeverity } from 'vs/platform/markers/common/markers'; import { CancellationToken } from 'vs/base/common/cancellation'; +function staticCodeActionProvider(...actions: modes.CodeAction[]): modes.CodeActionProvider { + return new class implements modes.CodeActionProvider { + provideCodeActions(): modes.CodeActionList { + return { + actions: actions, + dispose: () => { } + }; + } + }; +} + + suite('CodeAction', () => { - let langId = new LanguageIdentifier('fooLang', 17); + let langId = new modes.LanguageIdentifier('fooLang', 17); let uri = URI.parse('untitled:path'); let model: TextModel; const disposables = new DisposableStore(); @@ -46,7 +58,7 @@ suite('CodeAction', () => { }, command: { abc: { - command: new class implements Command { + command: new class implements modes.Command { id: '1'; title: 'abc'; }, @@ -56,8 +68,8 @@ suite('CodeAction', () => { spelling: { bcd: { diagnostics: [], - edit: new class implements WorkspaceEdit { - edits: ResourceTextEdit[]; + edit: new class implements modes.WorkspaceEdit { + edits: modes.ResourceTextEdit[]; }, title: 'abc' } @@ -90,20 +102,16 @@ suite('CodeAction', () => { test('CodeActions are sorted by type, #38623', async function () { - const provider = new class implements CodeActionProvider { - provideCodeActions() { - return [ - testData.command.abc, - testData.diagnostics.bcd, - testData.spelling.bcd, - testData.tsLint.bcd, - testData.tsLint.abc, - testData.diagnostics.abc - ]; - } - }; + const provider = staticCodeActionProvider( + testData.command.abc, + testData.diagnostics.bcd, + testData.spelling.bcd, + testData.tsLint.bcd, + testData.tsLint.abc, + testData.diagnostics.abc + ); - disposables.add(CodeActionProviderRegistry.register('fooLang', provider)); + disposables.add(modes.CodeActionProviderRegistry.register('fooLang', provider)); const expected = [ // CodeActions with a diagnostics array are shown first ordered by diagnostics.message @@ -123,17 +131,13 @@ suite('CodeAction', () => { }); test('getCodeActions should filter by scope', async function () { - const provider = new class implements CodeActionProvider { - provideCodeActions(): CodeAction[] { - return [ - { title: 'a', kind: 'a' }, - { title: 'b', kind: 'b' }, - { title: 'a.b', kind: 'a.b' } - ]; - } - }; + const provider = staticCodeActionProvider( + { title: 'a', kind: 'a' }, + { title: 'b', kind: 'b' }, + { title: 'a.b', kind: 'a.b' } + ); - disposables.add(CodeActionProviderRegistry.register('fooLang', provider)); + disposables.add(modes.CodeActionProviderRegistry.register('fooLang', provider)); { const { actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a') } }, CancellationToken.None); @@ -155,15 +159,18 @@ suite('CodeAction', () => { }); test('getCodeActions should forward requested scope to providers', async function () { - const provider = new class implements CodeActionProvider { - provideCodeActions(_model: any, _range: Range, context: CodeActionContext, _token: any): CodeAction[] { - return [ - { title: context.only || '', kind: context.only } - ]; + const provider = new class implements modes.CodeActionProvider { + provideCodeActions(_model: any, _range: Range, context: modes.CodeActionContext, _token: any): modes.CodeActionList { + return { + actions: [ + { title: context.only || '', kind: context.only } + ], + dispose: () => { } + }; } }; - disposables.add(CodeActionProviderRegistry.register('fooLang', provider)); + disposables.add(modes.CodeActionProviderRegistry.register('fooLang', provider)); const { actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', filter: { kind: new CodeActionKind('a') } }, CancellationToken.None); assert.equal(actions.length, 1); @@ -171,16 +178,12 @@ suite('CodeAction', () => { }); test('getCodeActions should not return source code action by default', async function () { - const provider = new class implements CodeActionProvider { - provideCodeActions(): CodeAction[] { - return [ - { title: 'a', kind: CodeActionKind.Source.value }, - { title: 'b', kind: 'b' } - ]; - } - }; + const provider = staticCodeActionProvider( + { title: 'a', kind: CodeActionKind.Source.value }, + { title: 'b', kind: 'b' } + ); - disposables.add(CodeActionProviderRegistry.register('fooLang', provider)); + disposables.add(modes.CodeActionProviderRegistry.register('fooLang', provider)); { const { actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto' }, CancellationToken.None); @@ -197,16 +200,16 @@ suite('CodeAction', () => { test('getCodeActions should not invoke code action providers filtered out by providedCodeActionKinds', async function () { let wasInvoked = false; - const provider = new class implements CodeActionProvider { - provideCodeActions() { + const provider = new class implements modes.CodeActionProvider { + provideCodeActions(): modes.CodeActionList { wasInvoked = true; - return []; + return { actions: [], dispose: () => { } }; } providedCodeActionKinds = [CodeActionKind.Refactor.value]; }; - disposables.add(CodeActionProviderRegistry.register('fooLang', provider)); + disposables.add(modes.CodeActionProviderRegistry.register('fooLang', provider)); const { actions } = await getCodeActions(model, new Range(1, 1, 2, 1), { type: 'auto', diff --git a/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts b/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts index e0680174e80..f79fb9c5258 100644 --- a/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts +++ b/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts @@ -9,19 +9,24 @@ import { URI } from 'vs/base/common/uri'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { Selection } from 'vs/editor/common/core/selection'; import { TextModel } from 'vs/editor/common/model/textModel'; -import { CodeActionProviderRegistry, LanguageIdentifier } from 'vs/editor/common/modes'; +import * as modes from 'vs/editor/common/modes'; import { CodeActionOracle, CodeActionsState } from 'vs/editor/contrib/codeAction/codeActionModel'; import { createTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor'; import { MarkerService } from 'vs/platform/markers/common/markerService'; const testProvider = { - provideCodeActions() { - return [{ id: 'test-command', title: 'test', arguments: [] }]; + provideCodeActions(): modes.CodeActionList { + return { + actions: [ + { title: 'test', command: { id: 'test-command', title: 'test', arguments: [] } } + ], + dispose() { /* noop*/ } + }; } }; suite('CodeAction', () => { - const languageIdentifier = new LanguageIdentifier('foo-lang', 3); + const languageIdentifier = new modes.LanguageIdentifier('foo-lang', 3); let uri = URI.parse('untitled:path'); let model: TextModel; let markerService: MarkerService; @@ -44,7 +49,7 @@ suite('CodeAction', () => { }); test('Orcale -> marker added', done => { - const reg = CodeActionProviderRegistry.register(languageIdentifier.language, testProvider); + const reg = modes.CodeActionProviderRegistry.register(languageIdentifier.language, testProvider); disposables.add(reg); const oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsState.Triggered) => { @@ -70,7 +75,7 @@ suite('CodeAction', () => { }); test('Orcale -> position changed', () => { - const reg = CodeActionProviderRegistry.register(languageIdentifier.language, testProvider); + const reg = modes.CodeActionProviderRegistry.register(languageIdentifier.language, testProvider); disposables.add(reg); markerService.changeOne('fake', uri, [{ @@ -100,9 +105,9 @@ suite('CodeAction', () => { }); test('Lightbulb is in the wrong place, #29933', async function () { - const reg = CodeActionProviderRegistry.register(languageIdentifier.language, { - provideCodeActions(_doc, _range) { - return []; + const reg = modes.CodeActionProviderRegistry.register(languageIdentifier.language, { + provideCodeActions(_doc, _range): modes.CodeActionList { + return { actions: [], dispose() { /* noop*/ } }; } }); disposables.add(reg); @@ -138,7 +143,7 @@ suite('CodeAction', () => { }); test('Orcale -> should only auto trigger once for cursor and marker update right after each other', done => { - const reg = CodeActionProviderRegistry.register(languageIdentifier.language, testProvider); + const reg = modes.CodeActionProviderRegistry.register(languageIdentifier.language, testProvider); disposables.add(reg); let triggerCount = 0; diff --git a/src/vs/editor/contrib/hover/modesContentHover.ts b/src/vs/editor/contrib/hover/modesContentHover.ts index 6e067b7e1bf..b195cec1938 100644 --- a/src/vs/editor/contrib/hover/modesContentHover.ts +++ b/src/vs/editor/contrib/hover/modesContentHover.ts @@ -187,6 +187,10 @@ class ModesContentComputer implements IHoverComputer { } } +interface ActionSet extends IDisposable { + readonly actions: Action[]; +} + export class ModesContentHoverWidget extends ContentHoverWidget { static readonly ID = 'editor.contrib.modesContentHoverWidget'; @@ -535,10 +539,11 @@ export class ModesContentHoverWidget extends ContentHoverWidget { const codeActionsPromise = this.getCodeActions(markerHover.marker); disposables.add(toDisposable(() => codeActionsPromise.cancel())); const actions = await codeActionsPromise; + disposables.add(actions); const elementPosition = dom.getDomNodePagePosition(target); this._contextMenuService.showContextMenu({ getAnchor: () => ({ x: elementPosition.left + 6, y: elementPosition.top + elementPosition.height + 6 }), - getActions: () => actions + getActions: () => actions.actions }); } })); @@ -557,20 +562,33 @@ export class ModesContentHoverWidget extends ContentHoverWidget { return hoverElement; } - private getCodeActions(marker: IMarker): CancelablePromise { + private getCodeActions(marker: IMarker): CancelablePromise { return createCancelablePromise(async cancellationToken => { const codeActions = await getCodeActions(this._editor.getModel()!, new Range(marker.startLineNumber, marker.startColumn, marker.endLineNumber, marker.endColumn), { type: 'manual', filter: { kind: CodeActionKind.QuickFix } }, cancellationToken); if (codeActions.actions.length) { - return codeActions.actions.map(codeAction => new Action( - codeAction.command ? codeAction.command.id : codeAction.title, - codeAction.title, - undefined, - true, - () => applyCodeAction(codeAction, this._bulkEditService, this._commandService))); + const disposables = new DisposableStore(); + const actions: Action[] = []; + for (const codeAction of codeActions.actions) { + disposables.add(disposables); + actions.push(new Action( + codeAction.command ? codeAction.command.id : codeAction.title, + codeAction.title, + undefined, + true, + () => applyCodeAction(codeAction, this._bulkEditService, this._commandService))); + } + return { + actions: actions, + dispose: () => disposables.dispose() + }; } - return [ - new Action('', nls.localize('editor.action.quickFix.noneMessage', "No code actions available")) - ]; + + return { + actions: [ + new Action('', nls.localize('editor.action.quickFix.noneMessage', "No code actions available")) + ], + dispose() { } + }; }); } diff --git a/src/vs/editor/standalone/browser/standaloneLanguages.ts b/src/vs/editor/standalone/browser/standaloneLanguages.ts index f3ea17ca812..7c0d721dd7c 100644 --- a/src/vs/editor/standalone/browser/standaloneLanguages.ts +++ b/src/vs/editor/standalone/browser/standaloneLanguages.ts @@ -427,7 +427,7 @@ export function registerCodeLensProvider(languageId: string, provider: modes.Cod */ export function registerCodeActionProvider(languageId: string, provider: CodeActionProvider): IDisposable { return modes.CodeActionProviderRegistry.register(languageId, { - provideCodeActions: (model: model.ITextModel, range: Range, context: modes.CodeActionContext, token: CancellationToken): (modes.Command | modes.CodeAction)[] | Promise<(modes.Command | modes.CodeAction)[]> => { + provideCodeActions: (model: model.ITextModel, range: Range, context: modes.CodeActionContext, token: CancellationToken): modes.CodeActionList | Promise => { let markers = StaticServices.markerService.get().read({ resource: model.uri }).filter(m => { return Range.areIntersectingOrTouching(m, range); }); @@ -510,7 +510,7 @@ export interface CodeActionProvider { /** * Provide commands for the given document and range. */ - provideCodeActions(model: model.ITextModel, range: Range, context: CodeActionContext, token: CancellationToken): (modes.Command | modes.CodeAction)[] | Promise<(modes.Command | modes.CodeAction)[]>; + provideCodeActions(model: model.ITextModel, range: Range, context: CodeActionContext, token: CancellationToken): modes.CodeActionList | Promise; } /** diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 9a44a9563e4..c292ab5e9d0 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -4433,7 +4433,7 @@ declare namespace monaco.languages { /** * Provide commands for the given document and range. */ - provideCodeActions(model: editor.ITextModel, range: Range, context: CodeActionContext, token: CancellationToken): (Command | CodeAction)[] | Promise<(Command | CodeAction)[]>; + provideCodeActions(model: editor.ITextModel, range: Range, context: CodeActionContext, token: CancellationToken): CodeActionList | Promise; } /** @@ -4894,6 +4894,10 @@ declare namespace monaco.languages { isPreferred?: boolean; } + export interface CodeActionList extends IDisposable { + readonly actions: ReadonlyArray; + } + /** * Represents a parameter of a callable-signature. A parameter can * have a label and a doc-comment. diff --git a/src/vs/platform/configuration/common/configurationRegistry.ts b/src/vs/platform/configuration/common/configurationRegistry.ts index 1dce4d68093..1752c23aa2e 100644 --- a/src/vs/platform/configuration/common/configurationRegistry.ts +++ b/src/vs/platform/configuration/common/configurationRegistry.ts @@ -471,7 +471,7 @@ export function validateProperty(property: string): string | null { } export function getScopes(): { [key: string]: ConfigurationScope } { - const scopes: { [key: string]: ConfigurationScope } = {}; + const scopes = {}; const configurationProperties = configurationRegistry.getConfigurationProperties(); for (const key of Object.keys(configurationProperties)) { scopes[key] = configurationProperties[key].scope; diff --git a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts index 8e5dc539dc4..5b25bf02257 100644 --- a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts +++ b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts @@ -11,7 +11,7 @@ import * as search from 'vs/workbench/contrib/search/common/search'; import { CancellationToken } from 'vs/base/common/cancellation'; import { Position as EditorPosition } from 'vs/editor/common/core/position'; import { Range as EditorRange, IRange } from 'vs/editor/common/core/range'; -import { ExtHostContext, MainThreadLanguageFeaturesShape, ExtHostLanguageFeaturesShape, MainContext, IExtHostContext, ISerializedLanguageConfiguration, ISerializedRegExp, ISerializedIndentationRule, ISerializedOnEnterRule, LocationDto, WorkspaceSymbolDto, CodeActionDto, reviveWorkspaceEditDto, ISerializedDocumentFilter, DefinitionLinkDto, ISerializedSignatureHelpProviderMetadata, LinkDto, CallHierarchyDto, SuggestDataDto } from '../common/extHost.protocol'; +import { ExtHostContext, MainThreadLanguageFeaturesShape, ExtHostLanguageFeaturesShape, MainContext, IExtHostContext, ISerializedLanguageConfiguration, ISerializedRegExp, ISerializedIndentationRule, ISerializedOnEnterRule, LocationDto, WorkspaceSymbolDto, reviveWorkspaceEditDto, ISerializedDocumentFilter, DefinitionLinkDto, ISerializedSignatureHelpProviderMetadata, LinkDto, CallHierarchyDto, SuggestDataDto, CodeActionDto } from '../common/extHost.protocol'; import { LanguageConfigurationRegistry } from 'vs/editor/common/modes/languageConfigurationRegistry'; import { LanguageConfiguration, IndentationRule, OnEnterRule } from 'vs/editor/common/modes/languageConfiguration'; import { IModeService } from 'vs/editor/common/services/modeService'; @@ -20,24 +20,20 @@ import { URI } from 'vs/base/common/uri'; import { Selection } from 'vs/editor/common/core/selection'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import * as callh from 'vs/workbench/contrib/callHierarchy/common/callHierarchy'; -import { IHeapService } from 'vs/workbench/services/heap/common/heap'; import { mixin } from 'vs/base/common/objects'; @extHostNamedCustomer(MainContext.MainThreadLanguageFeatures) export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesShape { private readonly _proxy: ExtHostLanguageFeaturesShape; - private readonly _heapService: IHeapService; private readonly _modeService: IModeService; private readonly _registrations: { [handle: number]: IDisposable; } = Object.create(null); constructor( extHostContext: IExtHostContext, - @IHeapService heapService: IHeapService, @IModeService modeService: IModeService, ) { this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostLanguageFeatures); - this._heapService = heapService; this._modeService = modeService; } @@ -100,7 +96,7 @@ export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesSha } } - private static _reviveCodeActionDto(data: CodeActionDto[] | undefined): modes.CodeAction[] { + private static _reviveCodeActionDto(data: ReadonlyArray): modes.CodeAction[] { if (data) { data.forEach(code => reviveWorkspaceEditDto(code.edit)); } @@ -239,13 +235,19 @@ export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesSha $registerQuickFixSupport(handle: number, selector: ISerializedDocumentFilter[], providedCodeActionKinds?: string[]): void { this._registrations[handle] = modes.CodeActionProviderRegistry.register(selector, { - provideCodeActions: (model: ITextModel, rangeOrSelection: EditorRange | Selection, context: modes.CodeActionContext, token: CancellationToken): Promise => { - return this._proxy.$provideCodeActions(handle, model.uri, rangeOrSelection, context, token).then(dto => { - if (dto) { - dto.forEach(obj => { this._heapService.trackObject(obj.command); }); + provideCodeActions: async (model: ITextModel, rangeOrSelection: EditorRange | Selection, context: modes.CodeActionContext, token: CancellationToken): Promise => { + const listDto = await this._proxy.$provideCodeActions(handle, model.uri, rangeOrSelection, context, token); + if (!listDto) { + return undefined; + } + return { + actions: MainThreadLanguageFeatures._reviveCodeActionDto(listDto.actions), + dispose: () => { + if (typeof listDto.cacheId === 'number') { + this._proxy.$releaseCodeActions(handle, listDto.cacheId); + } } - return MainThreadLanguageFeatures._reviveCodeActionDto(dto); - }); + }; }, providedCodeActionKinds }); diff --git a/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts b/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts index acdf95bfb5b..9376f6d1bbb 100644 --- a/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts +++ b/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts @@ -305,6 +305,8 @@ class CodeActionOnSaveParticipant implements ISaveParticipant { await this.applyCodeActions(actionsToRun.actions); } catch { // Failure to apply a code action should not block other on save actions + } finally { + actionsToRun.dispose(); } } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 713f0d64bb9..2403367f755 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -993,6 +993,11 @@ export interface CodeActionDto { isPreferred?: boolean; } +export interface CodeActionListDto { + cacheId: number; + actions: ReadonlyArray; +} + export type CacheId = number; export type ChainedCacheId = [CacheId, CacheId]; @@ -1041,7 +1046,8 @@ export interface ExtHostLanguageFeaturesShape { $provideHover(handle: number, resource: UriComponents, position: IPosition, token: CancellationToken): Promise; $provideDocumentHighlights(handle: number, resource: UriComponents, position: IPosition, token: CancellationToken): Promise; $provideReferences(handle: number, resource: UriComponents, position: IPosition, context: modes.ReferenceContext, token: CancellationToken): Promise; - $provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise; + $provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise; + $releaseCodeActions(handle: number, cacheId: number): void; $provideDocumentFormattingEdits(handle: number, resource: UriComponents, options: modes.FormattingOptions, token: CancellationToken): Promise; $provideDocumentRangeFormattingEdits(handle: number, resource: UriComponents, range: IRange, options: modes.FormattingOptions, token: CancellationToken): Promise; $provideOnTypeFormattingEdits(handle: number, resource: UriComponents, position: IPosition, ch: string, options: modes.FormattingOptions, token: CancellationToken): Promise; diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index 5ee506bf01a..76ec31ba4c3 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -15,7 +15,7 @@ import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments'; import { ExtHostCommands, CommandsConverter } from 'vs/workbench/api/common/extHostCommands'; import { ExtHostDiagnostics } from 'vs/workbench/api/common/extHostDiagnostics'; import { asPromise } from 'vs/base/common/async'; -import { MainContext, MainThreadLanguageFeaturesShape, ExtHostLanguageFeaturesShape, ObjectIdentifier, IRawColorInfo, IMainContext, IdObject, ISerializedRegExp, ISerializedIndentationRule, ISerializedOnEnterRule, ISerializedLanguageConfiguration, WorkspaceSymbolDto, SuggestResultDto, WorkspaceSymbolsDto, CodeActionDto, ISerializedDocumentFilter, WorkspaceEditDto, ISerializedSignatureHelpProviderMetadata, LinkDto, CodeLensDto, SuggestDataDto, LinksListDto, ChainedCacheId, CodeLensListDto } from './extHost.protocol'; +import { MainContext, MainThreadLanguageFeaturesShape, ExtHostLanguageFeaturesShape, ObjectIdentifier, IRawColorInfo, IMainContext, IdObject, ISerializedRegExp, ISerializedIndentationRule, ISerializedOnEnterRule, ISerializedLanguageConfiguration, WorkspaceSymbolDto, SuggestResultDto, WorkspaceSymbolsDto, CodeActionDto, ISerializedDocumentFilter, WorkspaceEditDto, ISerializedSignatureHelpProviderMetadata, LinkDto, CodeLensDto, SuggestDataDto, LinksListDto, ChainedCacheId, CodeLensListDto, CodeActionListDto } from './extHost.protocol'; import { regExpLeadsToEndlessLoop, regExpFlags } from 'vs/base/common/strings'; import { IPosition } from 'vs/editor/common/core/position'; import { IRange, Range as EditorRange } from 'vs/editor/common/core/range'; @@ -307,6 +307,9 @@ export interface CustomCodeAction extends CodeActionDto { class CodeActionAdapter { private static readonly _maxCodeActionsPerFile: number = 1000; + private readonly _cache = new Cache(); + private readonly _disposables = new Map(); + constructor( private readonly _documents: ExtHostDocuments, private readonly _commands: CommandsConverter, @@ -316,7 +319,7 @@ class CodeActionAdapter { private readonly _extensionId: ExtensionIdentifier ) { } - provideCodeActions(resource: URI, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise { + provideCodeActions(resource: URI, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise { const doc = this._documents.getDocument(resource); const ran = Selection.isISelection(rangeOrSelection) @@ -339,34 +342,39 @@ class CodeActionAdapter { }; return asPromise(() => this._provider.provideCodeActions(doc, ran, codeActionContext, token)).then(commandsOrActions => { - if (!isNonEmptyArray(commandsOrActions)) { + if (!isNonEmptyArray(commandsOrActions) || token.isCancellationRequested) { return undefined; } - const result: CustomCodeAction[] = []; + + const cacheId = this._cache.add(commandsOrActions); + const disposables = new DisposableStore(); + this._disposables.set(cacheId, disposables); + + const actions: CustomCodeAction[] = []; for (const candidate of commandsOrActions) { if (!candidate) { continue; } if (CodeActionAdapter._isCommand(candidate)) { // old school: synthetic code action - result.push({ + actions.push({ _isSynthetic: true, title: candidate.title, - command: this._commands.toInternal(candidate), + command: this._commands.toInternal2(candidate, disposables), }); } else { if (codeActionContext.only) { if (!candidate.kind) { this._logService.warn(`${this._extensionId.value} - Code actions of kind '${codeActionContext.only.value} 'requested but returned code action does not have a 'kind'. Code action will be dropped. Please set 'CodeAction.kind'.`); } else if (!codeActionContext.only.contains(candidate.kind)) { - this._logService.warn(`${this._extensionId.value} -Code actions of kind '${codeActionContext.only.value} 'requested but returned code action is of kind '${candidate.kind.value}'. Code action will be dropped. Please check 'CodeActionContext.only' to only return requested code actions.`); + this._logService.warn(`${this._extensionId.value} - Code actions of kind '${codeActionContext.only.value} 'requested but returned code action is of kind '${candidate.kind.value}'. Code action will be dropped. Please check 'CodeActionContext.only' to only return requested code actions.`); } } // new school: convert code action - result.push({ + actions.push({ title: candidate.title, - command: candidate.command && this._commands.toInternal(candidate.command), + command: candidate.command && this._commands.toInternal2(candidate.command, disposables), diagnostics: candidate.diagnostics && candidate.diagnostics.map(typeConvert.Diagnostic.from), edit: candidate.edit && typeConvert.WorkspaceEdit.from(candidate.edit), kind: candidate.kind && candidate.kind.value, @@ -375,10 +383,16 @@ class CodeActionAdapter { } } - return result; + return { cacheId, actions }; }); } + public releaseCodeActions(cachedId: number): void { + dispose(this._disposables.get(cachedId)); + this._disposables.delete(cachedId); + this._cache.delete(cachedId); + } + private static _isCommand(thing: any): thing is vscode.Command { return typeof (thing).command === 'string' && typeof (thing).title === 'string'; } @@ -1276,10 +1290,14 @@ export class ExtHostLanguageFeatures implements ExtHostLanguageFeaturesShape { } - $provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise { + $provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext, token: CancellationToken): Promise { return this._withAdapter(handle, CodeActionAdapter, adapter => adapter.provideCodeActions(URI.revive(resource), rangeOrSelection, context, token), undefined); } + $releaseCodeActions(handle: number, cacheId: number): void { + this._withAdapter(handle, CodeActionAdapter, adapter => Promise.resolve(adapter.releaseCodeActions(cacheId)), undefined); + } + // --- formatting registerDocumentFormattingEditProvider(extension: IExtensionDescription, selector: vscode.DocumentSelector, provider: vscode.DocumentFormattingEditProvider): vscode.Disposable { diff --git a/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts b/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts index cb51cf61d64..c7dafaacfd3 100644 --- a/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts +++ b/src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts @@ -548,7 +548,9 @@ export class MarkerViewModel extends Disposable { if (model) { if (!this.codeActionsPromise) { this.codeActionsPromise = createCancelablePromise(cancellationToken => { - return getCodeActions(model, new Range(this.marker.range.startLineNumber, this.marker.range.startColumn, this.marker.range.endLineNumber, this.marker.range.endColumn), { type: 'manual', filter: { kind: CodeActionKind.QuickFix } }, cancellationToken); + return getCodeActions(model, new Range(this.marker.range.startLineNumber, this.marker.range.startColumn, this.marker.range.endLineNumber, this.marker.range.endColumn), { type: 'manual', filter: { kind: CodeActionKind.QuickFix } }, cancellationToken).then(actions => { + return this._register(actions); + }); }); } return this.codeActionsPromise; -- GitLab