diff --git a/.eslintrc.json b/.eslintrc.json index 7b8936e07030b..5bd5bd685a74d 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -151,7 +151,6 @@ "src/vs/base/test/browser/browser.test.ts", "src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts", "src/vs/base/test/browser/ui/scrollbar/scrollbarState.test.ts", - "src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts", "src/vs/editor/test/common/services/languageService.test.ts", "src/vs/editor/test/node/classification/typescript.test.ts", "src/vs/platform/configuration/test/common/configuration.test.ts", diff --git a/src/vs/editor/contrib/codeAction/browser/codeAction.ts b/src/vs/editor/contrib/codeAction/browser/codeAction.ts index c665dd778fe87..84e0fdf84ba98 100644 --- a/src/vs/editor/contrib/codeAction/browser/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/browser/codeAction.ts @@ -155,7 +155,7 @@ export async function getCodeActions( try { const actions = await Promise.all(promises); - const allActions = actions.map(x => x.actions).flat(); + const allActions = actions.flatMap(x => x.actions); const allDocumentation = [ ...coalesce(actions.map(x => x.documentation)), ...getAdditionalDocumentationForShowingActions(registry, model, trigger, allActions) diff --git a/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts b/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts index 9c7e0c73b76bd..1545c8d140707 100644 --- a/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts +++ b/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts @@ -6,7 +6,7 @@ import { CancelablePromise, createCancelablePromise, TimeoutTimer } from 'vs/base/common/async'; import { isCancellationError } from 'vs/base/common/errors'; import { Emitter } from 'vs/base/common/event'; -import { Disposable, MutableDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle'; import { isEqual } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; @@ -163,6 +163,8 @@ export class CodeActionModel extends Disposable { private readonly _onDidChangeState = this._register(new Emitter()); public readonly onDidChangeState = this._onDidChangeState.event; + private readonly disposables = this._register(new DisposableStore()); + private _disposed = false; constructor( @@ -229,7 +231,7 @@ export class CodeActionModel extends Disposable { const actions = createCancelablePromise(async token => { if (this._settingEnabledNearbyQuickfixes() && trigger.trigger.type === CodeActionTriggerType.Invoke && (trigger.trigger.triggerAction === CodeActionTriggerSource.QuickFix || trigger.trigger.filter?.include?.contains(CodeActionKind.QuickFix))) { - const codeActionSet = await getCodeActions(this._registry, model, trigger.selection, trigger.trigger, Progress.None, token); + const codeActionSet = this.disposables.add(await getCodeActions(this._registry, model, trigger.selection, trigger.trigger, Progress.None, token)); const allCodeActions = [...codeActionSet.allActions]; if (token.isCancellationRequested) { return emptyCodeActionSet; @@ -270,7 +272,7 @@ export class CodeActionModel extends Disposable { }; const selectionAsPosition = new Selection(trackedPosition.lineNumber, trackedPosition.column, trackedPosition.lineNumber, trackedPosition.column); - const actionsAtMarker = await getCodeActions(this._registry, model, selectionAsPosition, newCodeActionTrigger, Progress.None, token); + const actionsAtMarker = this.disposables.add(await getCodeActions(this._registry, model, selectionAsPosition, newCodeActionTrigger, Progress.None, token)); if (actionsAtMarker.validActions.length !== 0) { for (const action of actionsAtMarker.validActions) { @@ -315,8 +317,8 @@ export class CodeActionModel extends Disposable { } } } - // temporarilly hiding here as this is enabled/disabled behind a setting. - return getCodeActions(this._registry, model, trigger.selection, trigger.trigger, Progress.None, token); + const codeActionSet = this.disposables.add(await getCodeActions(this._registry, model, trigger.selection, trigger.trigger, Progress.None, token)); + return codeActionSet; }); if (trigger.trigger.type === CodeActionTriggerType.Invoke) { this._progressService?.showWhile(actions, 250); @@ -337,6 +339,10 @@ export class CodeActionModel extends Disposable { } }, undefined); this._codeActionOracle.value.trigger({ type: CodeActionTriggerType.Auto, triggerAction: CodeActionTriggerSource.Default }); + + // once we trigger, remove any disposables added. + this.disposables.dispose(); + this.disposables.clear(); } else { this._supportedCodeActions.reset(); } diff --git a/src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts b/src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts index 71d6df33c0b4f..c06ce4dedbd82 100644 --- a/src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts +++ b/src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts @@ -5,10 +5,10 @@ import * as assert from 'assert'; import { promiseWithResolvers } from 'vs/base/common/async'; -import { DisposableStore } from 'vs/base/common/lifecycle'; import { assertType } from 'vs/base/common/types'; import { URI } from 'vs/base/common/uri'; import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler'; +import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { LanguageFeatureRegistry } from 'vs/editor/common/languageFeatureRegistry'; import * as languages from 'vs/editor/common/languages'; @@ -38,10 +38,8 @@ suite('CodeActionModel', () => { let markerService: MarkerService; let editor: ICodeEditor; let registry: LanguageFeatureRegistry; - const disposables = new DisposableStore(); setup(() => { - disposables.clear(); markerService = new MarkerService(); model = createTextModel('foobar foo bar\nfarboo far boo', languageId, undefined, uri); editor = createTestCodeEditor(model); @@ -49,8 +47,9 @@ suite('CodeActionModel', () => { registry = new LanguageFeatureRegistry(); }); + const store = ensureNoDisposablesAreLeakedInTestSuite(); + teardown(() => { - disposables.clear(); editor.dispose(); model.dispose(); markerService.dispose(); @@ -61,11 +60,11 @@ suite('CodeActionModel', () => { await runWithFakedTimers({ useFakeTimers: true }, () => { const reg = registry.register(languageId, testProvider); - disposables.add(reg); + store.add(reg); const contextKeys = new MockContextKeyService(); - const model = disposables.add(new CodeActionModel(editor, registry, markerService, contextKeys, undefined)); - disposables.add(model.onDidChangeState((e: CodeActionsState.State) => { + const model = store.add(new CodeActionModel(editor, registry, markerService, contextKeys, undefined)); + store.add(model.onDidChangeState((e: CodeActionsState.State) => { assertType(e.type === CodeActionsState.Type.Triggered); assert.strictEqual(e.trigger.type, languages.CodeActionTriggerType.Auto); @@ -93,7 +92,7 @@ suite('CodeActionModel', () => { test('Oracle -> position changed', async () => { await runWithFakedTimers({ useFakeTimers: true }, () => { const reg = registry.register(languageId, testProvider); - disposables.add(reg); + store.add(reg); markerService.changeOne('fake', uri, [{ startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 6, @@ -107,8 +106,8 @@ suite('CodeActionModel', () => { return new Promise((resolve, reject) => { const contextKeys = new MockContextKeyService(); - const model = disposables.add(new CodeActionModel(editor, registry, markerService, contextKeys, undefined)); - disposables.add(model.onDidChangeState((e: CodeActionsState.State) => { + const model = store.add(new CodeActionModel(editor, registry, markerService, contextKeys, undefined)); + store.add(model.onDidChangeState((e: CodeActionsState.State) => { assertType(e.type === CodeActionsState.Type.Triggered); assert.strictEqual(e.trigger.type, languages.CodeActionTriggerType.Auto); @@ -129,12 +128,12 @@ suite('CodeActionModel', () => { const { promise: donePromise, resolve: done } = promiseWithResolvers(); await runWithFakedTimers({ useFakeTimers: true }, () => { const reg = registry.register(languageId, testProvider); - disposables.add(reg); + store.add(reg); let triggerCount = 0; const contextKeys = new MockContextKeyService(); - const model = disposables.add(new CodeActionModel(editor, registry, markerService, contextKeys, undefined)); - disposables.add(model.onDidChangeState((e: CodeActionsState.State) => { + const model = store.add(new CodeActionModel(editor, registry, markerService, contextKeys, undefined)); + store.add(model.onDidChangeState((e: CodeActionsState.State) => { assertType(e.type === CodeActionsState.Type.Triggered); assert.strictEqual(e.trigger.type, languages.CodeActionTriggerType.Auto);