make sure super.dispose is called inside overridden dispose methods (#200342)

This commit is contained in:
Johannes Rieken
2023-12-08 13:45:44 +01:00
committed by GitHub
parent 745cb55422
commit 5bf577b08a
17 changed files with 54 additions and 18 deletions

View File

@@ -0,0 +1,33 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import * as eslint from 'eslint';
export = new class NoAsyncSuite implements eslint.Rule.RuleModule {
create(context: eslint.Rule.RuleContext): eslint.Rule.RuleListener {
function doesCallSuperDispose(node: any) {
if (!node.override) {
return;
}
const body = context.getSourceCode().getText(node)
if (body.includes('super.dispose')) {
return;
}
context.report({
node,
message: 'dispose() should call super.dispose()'
});
}
return {
['MethodDefinition[override][key.name="dispose"]']: doesCallSuperDispose,
};
}
};

View File

@@ -73,6 +73,7 @@
"local/code-no-nls-in-standalone-editor": "warn", "local/code-no-nls-in-standalone-editor": "warn",
"local/code-no-standalone-editor": "warn", "local/code-no-standalone-editor": "warn",
"local/code-no-unexternalized-strings": "warn", "local/code-no-unexternalized-strings": "warn",
"local/code-must-use-super-dispose": "warn",
"local/code-declare-service-brand": "warn", "local/code-declare-service-brand": "warn",
"local/code-layering": [ "local/code-layering": [
"warn", "warn",
@@ -119,6 +120,7 @@
"**/*.test.ts" "**/*.test.ts"
], ],
"rules": { "rules": {
"local/code-must-use-super-dispose": "off",
"local/code-no-test-only": "error", "local/code-no-test-only": "error",
"local/code-no-test-async-suite": "warn", "local/code-no-test-async-suite": "warn",
"local/code-no-unexternalized-strings": "off", "local/code-no-unexternalized-strings": "off",

View File

@@ -14,7 +14,7 @@ import { ScrollbarVisibility } from 'vs/base/common/scrollable';
import 'vs/css!./breadcrumbsWidget'; import 'vs/css!./breadcrumbsWidget';
export abstract class BreadcrumbsItem { export abstract class BreadcrumbsItem {
dispose(): void { } abstract dispose(): void;
abstract equals(other: BreadcrumbsItem): boolean; abstract equals(other: BreadcrumbsItem): boolean;
abstract render(container: HTMLElement): void; abstract render(container: HTMLElement): void;
} }

View File

@@ -34,5 +34,6 @@ export class Client extends IPCClient implements IDisposable {
override dispose(): void { override dispose(): void {
this.protocol.disconnect(); this.protocol.disconnect();
super.dispose();
} }
} }

View File

@@ -687,6 +687,7 @@ class TopBottomDragScrollingOperation extends Disposable {
public override dispose(): void { public override dispose(): void {
this._animationFrameDisposable.dispose(); this._animationFrameDisposable.dispose();
super.dispose();
} }
public setPosition(position: IMouseTargetOutsideEditor, mouseEvent: EditorMouseEvent): void { public setPosition(position: IMouseTargetOutsideEditor, mouseEvent: EditorMouseEvent): void {

View File

@@ -44,6 +44,7 @@ export class BufferLogger extends AbstractMessageLogger {
override dispose(): void { override dispose(): void {
this._logger?.dispose(); this._logger?.dispose();
super.dispose();
} }
override flush(): void { override flush(): void {

View File

@@ -378,10 +378,6 @@ export class ConsoleMainLogger extends AbstractLogger implements ILogger {
} }
} }
override dispose(): void {
// noop
}
flush(): void { flush(): void {
// noop // noop
} }
@@ -445,9 +441,6 @@ export class ConsoleLogger extends AbstractLogger implements ILogger {
} }
} }
override dispose(): void {
// noop
}
flush(): void { flush(): void {
// noop // noop
@@ -499,10 +492,6 @@ export class AdapterLogger extends AbstractLogger implements ILogger {
return toErrorMessage(msg, this.checkLogLevel(LogLevel.Trace)); return toErrorMessage(msg, this.checkLogLevel(LogLevel.Trace));
} }
override dispose(): void {
// noop
}
flush(): void { flush(): void {
// noop // noop
} }
@@ -564,6 +553,7 @@ export class MultiplexLogger extends AbstractLogger implements ILogger {
for (const logger of this.loggers) { for (const logger of this.loggers) {
logger.dispose(); logger.dispose();
} }
super.dispose();
} }
} }

View File

@@ -123,6 +123,7 @@ export class SpdLogLogger extends AbstractMessageLogger implements ILogger {
} else { } else {
this._loggerCreationPromise.then(() => this.disposeLogger()); this._loggerCreationPromise.then(() => this.disposeLogger());
} }
super.dispose();
} }
private disposeLogger(): void { private disposeLogger(): void {

View File

@@ -291,6 +291,7 @@ class ManualSyncTaskChannelClient extends Disposable implements IUserDataManualS
override dispose(): void { override dispose(): void {
this.channel.call('dispose'); this.channel.call('dispose');
super.dispose();
} }
} }

View File

@@ -320,10 +320,6 @@ class ServerLogger extends AbstractLogger {
} }
} }
override dispose(): void {
// noop
}
flush(): void { flush(): void {
// noop // noop
} }

View File

@@ -42,6 +42,7 @@ export class ExtHostDocumentData extends MirrorTextModel {
super(uri, lines, eol, versionId); super(uri, lines, eol, versionId);
} }
// eslint-disable-next-line local/code-must-use-super-dispose
override dispose(): void { override dispose(): void {
// we don't really dispose documents but let // we don't really dispose documents but let
// extensions still read from them. some // extensions still read from them. some

View File

@@ -53,7 +53,7 @@ class OutlineItem extends BreadcrumbsItem {
super(); super();
} }
override dispose(): void { dispose(): void {
this._disposables.dispose(); this._disposables.dispose();
} }
@@ -113,7 +113,7 @@ class FileItem extends BreadcrumbsItem {
super(); super();
} }
override dispose(): void { dispose(): void {
this._disposables.dispose(); this._disposables.dispose();
} }
@@ -331,6 +331,9 @@ export class BreadcrumbsControl {
equals(other: BreadcrumbsItem): boolean { equals(other: BreadcrumbsItem): boolean {
return other === this; return other === this;
} }
dispose(): void {
}
}]); }]);
} else { } else {
this._widget.setEnabled(true); this._widget.setEnabled(true);

View File

@@ -102,6 +102,7 @@ export class BulkEditPane extends ViewPane {
override dispose(): void { override dispose(): void {
this._tree.dispose(); this._tree.dispose();
this._disposables.dispose(); this._disposables.dispose();
super.dispose();
} }
protected override renderBody(parent: HTMLElement): void { protected override renderBody(parent: HTMLElement): void {

View File

@@ -174,6 +174,7 @@ export class StartDebugActionViewItem extends BaseActionViewItem {
override dispose(): void { override dispose(): void {
this.toDispose = dispose(this.toDispose); this.toDispose = dispose(this.toDispose);
super.dispose();
} }
private updateOptions(): void { private updateOptions(): void {

View File

@@ -222,6 +222,7 @@ function _createTestNotebookEditor(instantiationService: TestInstantiationServic
let visibleRanges: ICellRange[] = [{ start: 0, end: 100 }]; let visibleRanges: ICellRange[] = [{ start: 0, end: 100 }];
const notebookEditor: IActiveNotebookEditorDelegate = new class extends mock<IActiveNotebookEditorDelegate>() { const notebookEditor: IActiveNotebookEditorDelegate = new class extends mock<IActiveNotebookEditorDelegate>() {
// eslint-disable-next-line local/code-must-use-super-dispose
override dispose() { override dispose() {
viewModel.dispose(); viewModel.dispose();
} }

View File

@@ -47,6 +47,7 @@ class ViewContainerActivityByView extends Disposable {
override dispose() { override dispose() {
this.activityDisposable.dispose(); this.activityDisposable.dispose();
super.dispose();
} }
} }

View File

@@ -170,6 +170,8 @@ export class RPCProtocol extends Disposable implements IRPCProtocol {
delete this._pendingRPCReplies[msgId]; delete this._pendingRPCReplies[msgId];
pending.resolveErr(errors.canceled()); pending.resolveErr(errors.canceled());
}); });
super.dispose();
} }
public drain(): Promise<void> { public drain(): Promise<void> {