adopt ensureNoDisposablesAreLeakedInTestSuite (#239536)

* adopt

* fix import change

* remove disposablestore and organize imports

* fix disposable leak error

* more disposables fix

* rename to code actions disposable

---------

Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com>
This commit is contained in:
Justin Chen
2025-02-04 14:37:31 -08:00
committed by GitHub
parent c18871a3c0
commit 363d424f91
4 changed files with 44 additions and 35 deletions

View File

@@ -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',

View File

@@ -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();

View File

@@ -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<string>('supportedCodeAction', '');
@@ -165,6 +165,8 @@ export class CodeActionModel extends Disposable {
private readonly _onDidChangeState = this._register(new Emitter<CodeActionsState.State>());
public readonly onDidChangeState = this._onDidChangeState.event;
private readonly codeActionsDisposable: MutableDisposable<IDisposable> = 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) {

View File

@@ -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<languages.CodeActionProvider>;
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<void>();
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);