diff --git a/eslint.config.js b/eslint.config.js index f9f120acd41..b3167b40a5c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -219,7 +219,6 @@ export default tseslint.config( { // Files should (only) be removed from the list they adopt the leak detector 'exclude': [ - 'src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts', 'src/vs/platform/configuration/test/common/configuration.test.ts', 'src/vs/platform/opener/test/common/opener.test.ts', 'src/vs/platform/registry/test/common/platform.test.ts', diff --git a/src/vs/editor/contrib/codeAction/browser/codeAction.ts b/src/vs/editor/contrib/codeAction/browser/codeAction.ts index 6120bae3584..c78ecc080d4 100644 --- a/src/vs/editor/contrib/codeAction/browser/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/browser/codeAction.ts @@ -6,8 +6,16 @@ import { coalesce, equals, isNonEmptyArray } from '../../../../base/common/arrays.js'; import { CancellationToken } from '../../../../base/common/cancellation.js'; import { illegalArgument, isCancellationError, onUnexpectedExternalError } from '../../../../base/common/errors.js'; +import { HierarchicalKind } from '../../../../base/common/hierarchicalKind.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; import { URI } from '../../../../base/common/uri.js'; +import * as nls from '../../../../nls.js'; +import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js'; +import { CommandsRegistry, ICommandService } from '../../../../platform/commands/common/commands.js'; +import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; +import { INotificationService } from '../../../../platform/notification/common/notification.js'; +import { IProgress, Progress } from '../../../../platform/progress/common/progress.js'; +import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; import { ICodeEditor } from '../../../browser/editorBrowser.js'; import { IBulkEditService } from '../../../browser/services/bulkEditService.js'; import { Range } from '../../../common/core/range.js'; @@ -18,15 +26,7 @@ import { ITextModel } from '../../../common/model.js'; import { ILanguageFeaturesService } from '../../../common/services/languageFeatures.js'; import { IModelService } from '../../../common/services/model.js'; import { TextModelCancellationTokenSource } from '../../editorState/browser/editorState.js'; -import * as nls from '../../../../nls.js'; -import { CommandsRegistry, ICommandService } from '../../../../platform/commands/common/commands.js'; -import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; -import { INotificationService } from '../../../../platform/notification/common/notification.js'; -import { IProgress, Progress } from '../../../../platform/progress/common/progress.js'; -import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; import { CodeActionFilter, CodeActionItem, CodeActionKind, CodeActionSet, CodeActionTrigger, CodeActionTriggerSource, filtersAction, mayIncludeActionsOfKind } from '../common/types.js'; -import { HierarchicalKind } from '../../../../base/common/hierarchicalKind.js'; -import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js'; export const codeActionCommandId = 'editor.action.codeAction'; export const quickFixCommandId = 'editor.action.quickFix'; @@ -165,6 +165,9 @@ export async function getCodeActions( ...getAdditionalDocumentationForShowingActions(registry, model, trigger, allActions) ]; return new ManagedCodeActionSet(allActions, allDocumentation, disposables); + } catch (err) { + disposables.dispose(); + throw err; } finally { listener.dispose(); cts.dispose(); diff --git a/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts b/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts index 10381ff4ffc..d5289c2cb6b 100644 --- a/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts +++ b/src/vs/editor/contrib/codeAction/browser/codeActionModel.ts @@ -6,24 +6,24 @@ import { CancelablePromise, createCancelablePromise, TimeoutTimer } from '../../../../base/common/async.js'; import { isCancellationError } from '../../../../base/common/errors.js'; import { Emitter } from '../../../../base/common/event.js'; -import { Disposable, MutableDisposable } from '../../../../base/common/lifecycle.js'; +import { HierarchicalKind } from '../../../../base/common/hierarchicalKind.js'; +import { Disposable, IDisposable, MutableDisposable } from '../../../../base/common/lifecycle.js'; import { isEqual } from '../../../../base/common/resources.js'; +import { StopWatch } from '../../../../base/common/stopwatch.js'; import { URI } from '../../../../base/common/uri.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { IContextKey, IContextKeyService, RawContextKey } from '../../../../platform/contextkey/common/contextkey.js'; +import { IMarkerService } from '../../../../platform/markers/common/markers.js'; +import { IEditorProgressService, Progress } from '../../../../platform/progress/common/progress.js'; +import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; import { ICodeEditor } from '../../../browser/editorBrowser.js'; import { EditorOption, ShowLightbulbIconMode } from '../../../common/config/editorOptions.js'; import { Position } from '../../../common/core/position.js'; import { Selection } from '../../../common/core/selection.js'; import { LanguageFeatureRegistry } from '../../../common/languageFeatureRegistry.js'; import { CodeActionProvider, CodeActionTriggerType } from '../../../common/languages.js'; -import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; -import { IContextKey, IContextKeyService, RawContextKey } from '../../../../platform/contextkey/common/contextkey.js'; -import { IMarkerService } from '../../../../platform/markers/common/markers.js'; -import { IEditorProgressService, Progress } from '../../../../platform/progress/common/progress.js'; import { CodeActionKind, CodeActionSet, CodeActionTrigger, CodeActionTriggerSource } from '../common/types.js'; import { getCodeActions } from './codeAction.js'; -import { HierarchicalKind } from '../../../../base/common/hierarchicalKind.js'; -import { StopWatch } from '../../../../base/common/stopwatch.js'; -import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; export const SUPPORTED_CODE_ACTIONS = new RawContextKey('supportedCodeAction', ''); @@ -165,6 +165,8 @@ export class CodeActionModel extends Disposable { private readonly _onDidChangeState = this._register(new Emitter()); public readonly onDidChangeState = this._onDidChangeState.event; + private readonly codeActionsDisposable: MutableDisposable = this._register(new MutableDisposable()); + private _disposed = false; constructor( @@ -233,6 +235,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); + this.codeActionsDisposable.value = codeActionSet; const allCodeActions = [...codeActionSet.allActions]; if (token.isCancellationRequested) { codeActionSet.dispose(); @@ -250,7 +253,7 @@ export class CodeActionModel extends Disposable { } return { validActions: codeActionSet.validActions, allActions: allCodeActions, documentation: codeActionSet.documentation, hasAutoFix: codeActionSet.hasAutoFix, hasAIFix: codeActionSet.hasAIFix, allAIFixes: codeActionSet.allAIFixes, dispose: () => { codeActionSet.dispose(); } }; } else if (!foundQuickfix) { - // If markers exists, and there are no quickfixes found or length is zero, check for quickfixes on that line. + // If markers exist, and there are no quickfixes found or length is zero, check for quickfixes on that line. if (allMarkers.length > 0) { const currPosition = trigger.selection.getPosition(); let trackedPosition = currPosition; @@ -275,6 +278,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); + this.codeActionsDisposable.value = actionsAtMarker; if (actionsAtMarker.validActions.length !== 0) { for (const action of actionsAtMarker.validActions) { @@ -348,8 +352,11 @@ export class CodeActionModel extends Disposable { return codeActions; } - return getCodeActions(this._registry, model, trigger.selection, trigger.trigger, Progress.None, token); + const codeActionSet = await getCodeActions(this._registry, model, trigger.selection, trigger.trigger, Progress.None, token); + this.codeActionsDisposable.value = codeActionSet; + return codeActionSet; }); + if (trigger.trigger.type === CodeActionTriggerType.Invoke) { this._progressService?.showWhile(actions, 250); } @@ -381,6 +388,7 @@ export class CodeActionModel extends Disposable { public trigger(trigger: CodeActionTrigger) { this._codeActionOracle.value?.trigger(trigger); + this.codeActionsDisposable.clear(); } private setState(newState: CodeActionsState.State, skipNotify?: boolean) { 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 666b185bda5..ab6c174582a 100644 --- a/src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts +++ b/src/vs/editor/contrib/codeAction/test/browser/codeActionModel.test.ts @@ -5,19 +5,19 @@ import assert from 'assert'; import { promiseWithResolvers } from '../../../../../base/common/async.js'; -import { DisposableStore } from '../../../../../base/common/lifecycle.js'; import { assertType } from '../../../../../base/common/types.js'; import { URI } from '../../../../../base/common/uri.js'; import { runWithFakedTimers } from '../../../../../base/test/common/timeTravelScheduler.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; +import { MarkerService } from '../../../../../platform/markers/common/markerService.js'; import { ICodeEditor } from '../../../../browser/editorBrowser.js'; import { LanguageFeatureRegistry } from '../../../../common/languageFeatureRegistry.js'; import * as languages from '../../../../common/languages.js'; import { TextModel } from '../../../../common/model/textModel.js'; -import { CodeActionModel, CodeActionsState } from '../../browser/codeActionModel.js'; import { createTestCodeEditor } from '../../../../test/browser/testCodeEditor.js'; import { createTextModel } from '../../../../test/common/testTextModel.js'; -import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; -import { MarkerService } from '../../../../../platform/markers/common/markerService.js'; +import { CodeActionModel, CodeActionsState } from '../../browser/codeActionModel.js'; const testProvider = { provideCodeActions(): languages.CodeActionList { @@ -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);