Add eslint rule for potentially unsafe disposable patterns

.eslintplugin/code-no-potentially-unsafe-disposables.ts

@@ -0,0 +1,37 @@
+/*---------------------------------------------------------------------------------------------
+ *  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';
+
+/**
+ * Checks for potentially unsafe usage of `DisposableStore` / `MutableDisposable`.
+ *
+ * These have been the source of leaks in the past.
+ */
+export = new class implements eslint.Rule.RuleModule {
+
+	create(context: eslint.Rule.RuleContext): eslint.Rule.RuleListener {
+		function checkVariableDeclaration(inNode: any) {
+			context.report({
+				node: inNode,
+				message: `Use const for 'DisposableStore' to avoid leaks by accidental reassignment.`
+			});
+		}
+
+		function checkProperty(inNode: any) {
+			context.report({
+				node: inNode,
+				message: `Use readonly for DisposableStore/MutableDisposable to avoid leaks through accidental reassignment.`
+			});
+		}
+
+		return {
+			'VariableDeclaration[kind!="const"] NewExpression[callee.name="DisposableStore"]': checkVariableDeclaration,
+
+			'PropertyDefinition[readonly!=true][typeAnnotation.typeAnnotation.typeName.name=/DisposableStore|MutableDisposable/]': checkProperty,
+			'PropertyDefinition[readonly!=true] NewExpression[callee.name=/DisposableStore|MutableDisposable/]': checkProperty,
+		};
+	}
+};

.eslintrc.json

@@ -71,6 +71,7 @@
 		"local/code-translation-remind": "warn",
 		"local/code-no-native-private": "warn",
 		"local/code-no-nls-in-standalone-editor": "warn",
+		"local/code-no-potentially-unsafe-disposables": "warn",
 		"local/code-no-standalone-editor": "warn",
 		"local/code-no-unexternalized-strings": "warn",
 		"local/code-must-use-super-dispose": "warn",

src/vs/base/browser/ui/findinput/findInput.ts

@@ -51,7 +51,7 @@ export class FindInput extends Widget {
 	private readonly showCommonFindToggles: boolean;
 	private fixFocusOnOptionClickEnabled = true;
 	private imeSessionInProgress = false;
-	private additionalTogglesDisposables: MutableDisposable<DisposableStore> = this._register(new MutableDisposable());
+	private readonly additionalTogglesDisposables: MutableDisposable<DisposableStore> = this._register(new MutableDisposable());
 
 	protected readonly controls: HTMLDivElement;
 	protected readonly regex?: RegexToggle;

src/vs/base/browser/ui/toolbar/toolbar.ts

@@ -55,7 +55,7 @@ export class ToolBar extends Disposable {
 
 	private _onDidChangeDropdownVisibility = this._register(new EventMultiplexer<boolean>());
 	readonly onDidChangeDropdownVisibility = this._onDidChangeDropdownVisibility.event;
-	private disposables = this._register(new DisposableStore());
+	private readonly disposables = this._register(new DisposableStore());
 
 	constructor(container: HTMLElement, contextMenuProvider: IContextMenuProvider, options: IToolBarOptions = { orientation: ActionsOrientation.HORIZONTAL }) {
 		super();

src/vs/base/browser/ui/tree/abstractTree.ts

@@ -64,7 +64,7 @@ class TreeNodeListDragAndDrop<T, TFilterData, TRef> implements IListDragAndDrop<
 
 	private autoExpandNode: ITreeNode<T, TFilterData> | undefined;
 	private autoExpandDisposable: IDisposable = Disposable.None;
-	private disposables = new DisposableStore();
+	private readonly disposables = new DisposableStore();
 
 	constructor(private modelProvider: () => ITreeModel<T, TFilterData, TRef>, private dnd: ITreeDragAndDrop<T>) { }
 
@@ -1561,7 +1561,7 @@ class StickyScrollWidget<T, TFilterData, TRef> implements IDisposable {
 	private readonly _rootDomNode: HTMLElement;
 	private _previousState: StickyScrollState<T, TFilterData, TRef> | undefined;
 	private _previousElements: HTMLElement[] = [];
-	private _previousStateDisposables: DisposableStore = new DisposableStore();
+	private readonly _previousStateDisposables: DisposableStore = new DisposableStore();
 
 	private stickyScrollFocus: StickyScrollFocus<T, TFilterData, TRef>;
 	readonly onDidChangeHasFocus: Event<boolean>;

src/vs/base/common/lifecycle.ts

@@ -561,7 +561,7 @@ export class MutableDisposable<T extends IDisposable> implements IDisposable {
  * exist and cannot be undefined.
  */
 export class MandatoryMutableDisposable<T extends IDisposable> implements IDisposable {
-	private _disposable = new MutableDisposable<T>();
+	private readonly _disposable = new MutableDisposable<T>();
 	private _isDisposed = false;
 
 	constructor(initialValue: T) {

src/vs/base/parts/ipc/common/ipc.net.ts

@@ -827,6 +827,7 @@ export class PersistentProtocol implements IMessagePassingProtocol {
 	private _socket: ISocket;
 	private _socketWriter: ProtocolWriter;
 	private _socketReader: ProtocolReader;
+	// eslint-disable-next-line local/code-no-potentially-unsafe-disposables
 	private _socketDisposables: DisposableStore;
 
 	private readonly _loadEstimator: ILoadEstimator;

src/vs/editor/browser/controller/textAreaInput.ts

@@ -196,7 +196,7 @@ export class TextAreaInput extends Disposable {
 
 	private readonly _asyncTriggerCut: RunOnceScheduler;
 
-	private _asyncFocusGainWriteScreenReaderContent: MutableDisposable<RunOnceScheduler> = this._register(new MutableDisposable());
+	private readonly _asyncFocusGainWriteScreenReaderContent: MutableDisposable<RunOnceScheduler> = this._register(new MutableDisposable());
 
 	private _textAreaState: TextAreaState;
 

src/vs/editor/browser/editorDom.ts

@@ -359,7 +359,7 @@ export interface CssProperties {
 class RefCountedCssRule {
 	private _referenceCount: number = 0;
 	private _styleElement: HTMLStyleElement | undefined;
-	private _styleElementDisposables: DisposableStore;
+	private readonly _styleElementDisposables: DisposableStore;
 
 	constructor(
 		public readonly key: string,

src/vs/editor/common/model/textModel.ts

@@ -245,7 +245,7 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
 	private _buffer: model.ITextBuffer;
 	private _bufferDisposable: IDisposable;
 	private _options: model.TextModelResolvedOptions;
-	private _languageSelectionListener = this._register(new MutableDisposable<IDisposable>());
+	private readonly _languageSelectionListener = this._register(new MutableDisposable<IDisposable>());
 
 	private _isDisposed: boolean;
 	private __isDisposing: boolean;

src/vs/editor/contrib/colorPicker/browser/colorDetector.ts

@@ -195,7 +195,7 @@ export class ColorDetector extends Disposable implements IEditorContribution {
 		});
 	}
 
-	private _colorDecorationClassRefs = this._register(new DisposableStore());
+	private readonly _colorDecorationClassRefs = this._register(new DisposableStore());
 
 	private updateColorDecorators(colorData: IColorData[]): void {
 		this._colorDecorationClassRefs.clear();

src/vs/editor/contrib/rename/browser/renameWidget.ts

@@ -569,7 +569,7 @@ class RenameCandidateListView {
 	private _minimumWidth: number;
 	private _typicalHalfwidthCharacterWidth: number;
 
-	private _disposables: DisposableStore;
+	private readonly _disposables: DisposableStore;
 
 	// FIXME@ulugbekna: rewrite using event emitters
 	constructor(parent: HTMLElement, opts: { fontInfo: FontInfo; onFocusChange: (newSymbolName: string) => void; onSelectionChange: () => void }) {
@@ -788,7 +788,7 @@ class RenameInput implements IDisposable {
 	private readonly _onDidChange = new Emitter<void>();
 	public readonly onDidChange = this._onDidChange.event;
 
-	private _disposables = new DisposableStore();
+	private readonly _disposables = new DisposableStore();
 
 	get domNode() {
 		if (!this._domNode) {

src/vs/platform/contextview/browser/contextViewService.ts

@@ -12,7 +12,7 @@ import { getWindow } from 'vs/base/browser/dom';
 
 export class ContextViewHandler extends Disposable implements IContextViewProvider {
 
-	private currentViewDisposable = this._register(new MutableDisposable<IDisposable>());
+	private readonly currentViewDisposable = this._register(new MutableDisposable<IDisposable>());
 	protected readonly contextView = this._register(new ContextView(this.layoutService.mainContainer, ContextViewDOMPosition.ABSOLUTE));
 
 	constructor(

src/vs/platform/hover/browser/hover.ts

@@ -30,7 +30,7 @@ export class WorkbenchHoverDelegate extends Disposable implements IHoverDelegate
 		return this._delay;
 	}
 
-	private hoverDisposables = this._register(new DisposableStore());
+	private readonly hoverDisposables = this._register(new DisposableStore());
 
 	constructor(
 		public readonly placement: 'mouse' | 'element',

src/vs/platform/policy/node/nativePolicyService.ts

@@ -13,7 +13,7 @@ import { ILogService } from 'vs/platform/log/common/log';
 export class NativePolicyService extends AbstractPolicyService implements IPolicyService {
 
 	private throttler = new Throttler();
-	private watcher = this._register(new MutableDisposable<Watcher>());
+	private readonly watcher = this._register(new MutableDisposable<Watcher>());
 
 	constructor(
 		@ILogService private readonly logService: ILogService,

src/vs/platform/quickinput/browser/quickInputTree.ts

@@ -707,7 +707,7 @@ export class QuickInputTree extends Disposable {
 	private _elementTree = new Array<IQuickPickElement>();
 	private _itemElements = new Array<QuickPickItemElement>();
 	// Elements that apply to the current set of elements
-	private _elementDisposable = this._register(new DisposableStore());
+	private readonly _elementDisposable = this._register(new DisposableStore());
 	private _lastHover: IHoverWidget | undefined;
 	// This is used to prevent setting the checked state of a single element from firing the checked events
 	// so that we can batch them together. This can probably be improved by handling events differently,

src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts

@@ -35,7 +35,7 @@ export class CommandDetectionCapability extends Disposable implements ICommandDe
 	private _commitCommandFinished?: RunOnceScheduler;
 
 	private _ptyHeuristicsHooks: ICommandDetectionHeuristicsHooks;
-	private _ptyHeuristics: MandatoryMutableDisposable<IPtyHeuristics>;
+	private readonly _ptyHeuristics: MandatoryMutableDisposable<IPtyHeuristics>;
 
 	get commands(): readonly TerminalCommand[] { return this._commands; }
 	get executingCommand(): string | undefined { return this._currentCommand.command; }
@@ -514,7 +514,7 @@ const enum AdjustCommandStartMarkerConstants {
  */
 class WindowsPtyHeuristics extends Disposable {
 
-	private _onCursorMoveListener = this._register(new MutableDisposable());
+	private readonly _onCursorMoveListener = this._register(new MutableDisposable());
 
 	private _tryAdjustCommandStartMarkerScheduler?: RunOnceScheduler;
 	private _tryAdjustCommandStartMarkerScannedLineCount: number = 0;

src/vs/workbench/api/browser/mainThreadTerminalService.ts

@@ -49,7 +49,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
 	 * provided through this, even from multiple ext link providers. Xterm should remove lower
 	 * priority intersecting links itself.
 	 */
-	private _linkProvider = this._store.add(new MutableDisposable());
+	private readonly _linkProvider = this._store.add(new MutableDisposable());
 
 	private _os: OperatingSystem = OS;
 

src/vs/workbench/api/common/extHostComments.ts

@@ -397,7 +397,7 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo
 
 		private _commentsMap: Map<vscode.Comment, number> = new Map<vscode.Comment, number>();
 
-		private _acceptInputDisposables = new MutableDisposable<DisposableStore>();
+		private readonly _acceptInputDisposables = new MutableDisposable<DisposableStore>();
 
 		readonly value: vscode.CommentThread2;
 

src/vs/workbench/api/common/extHostSCM.ts

@@ -559,7 +559,7 @@ class ExtHostSourceControl implements vscode.SourceControl {
 	}
 
 	private _historyProvider: vscode.SourceControlHistoryProvider | undefined;
-	private _historyProviderDisposable = new MutableDisposable<DisposableStore>();
+	private readonly _historyProviderDisposable = new MutableDisposable<DisposableStore>();
 	private _historyProviderCurrentHistoryItemGroup: vscode.SourceControlHistoryItemGroup | undefined;
 
 	get historyProvider(): vscode.SourceControlHistoryProvider | undefined {
@@ -598,7 +598,7 @@ class ExtHostSourceControl implements vscode.SourceControl {
 		this.#proxy.$updateSourceControl(this.handle, { commitTemplate });
 	}
 
-	private _acceptInputDisposables = new MutableDisposable<DisposableStore>();
+	private readonly _acceptInputDisposables = new MutableDisposable<DisposableStore>();
 	private _acceptInputCommand: vscode.Command | undefined = undefined;
 
 	get acceptInputCommand(): vscode.Command | undefined {
@@ -614,7 +614,7 @@ class ExtHostSourceControl implements vscode.SourceControl {
 		this.#proxy.$updateSourceControl(this.handle, { acceptInputCommand: internal });
 	}
 
-	private _actionButtonDisposables = new MutableDisposable<DisposableStore>();
+	private readonly _actionButtonDisposables = new MutableDisposable<DisposableStore>();
 	private _actionButton: vscode.SourceControlActionButton | undefined;
 	get actionButton(): vscode.SourceControlActionButton | undefined {
 		checkProposedApiEnabled(this._extension, 'scmActionButton');
@@ -639,7 +639,7 @@ class ExtHostSourceControl implements vscode.SourceControl {
 	}
 
 
-	private _statusBarDisposables = new MutableDisposable<DisposableStore>();
+	private readonly _statusBarDisposables = new MutableDisposable<DisposableStore>();
 	private _statusBarCommands: vscode.Command[] | undefined = undefined;
 
 	get statusBarCommands(): vscode.Command[] | undefined {

src/vs/workbench/api/common/extHostStatusBar.ts

@@ -46,6 +46,7 @@ export class ExtHostStatusBarEntry implements vscode.StatusBarItem {
 	private _name?: string;
 	private _color?: string | ThemeColor;
 	private _backgroundColor?: ThemeColor;
+	// eslint-disable-next-line local/code-no-potentially-unsafe-disposables
 	private _latestCommandRegistration?: DisposableStore;
 	private readonly _staleCommandRegistrations = new DisposableStore();
 	private _command?: {

src/vs/workbench/api/common/extHostTerminalService.ts

@@ -393,7 +393,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I
 	protected _environmentVariableCollections: Map<string, UnifiedEnvironmentVariableCollection> = new Map();
 	private _defaultProfile: ITerminalProfile | undefined;
 	private _defaultAutomationProfile: ITerminalProfile | undefined;
-	private _lastQuickFixCommands: MutableDisposable<IDisposable> = this._register(new MutableDisposable());
+	private readonly _lastQuickFixCommands: MutableDisposable<IDisposable> = this._register(new MutableDisposable());
 
 	private readonly _bufferer: TerminalDataBufferer;
 	private readonly _linkProviders: Set<vscode.TerminalLinkProvider> = new Set();

src/vs/workbench/browser/actions.ts

@@ -23,7 +23,7 @@ class MenuActions extends Disposable {
 	private readonly _onDidChange = this._register(new Emitter<void>());
 	readonly onDidChange = this._onDidChange.event;
 
-	private disposables = this._register(new DisposableStore());
+	private readonly disposables = this._register(new DisposableStore());
 
 	constructor(
 		menuId: MenuId,

src/vs/workbench/browser/labels.ts

@@ -291,7 +291,7 @@ class ResourceLabelWidget extends IconLabel {
 	readonly onDidRender = this._onDidRender.event;
 
 	private label: IResourceLabelProps | undefined = undefined;
-	private decoration = this._register(new MutableDisposable<IDecoration>());
+	private readonly decoration = this._register(new MutableDisposable<IDecoration>());
 	private options: IResourceLabelOptions | undefined = undefined;
 
 	private computedIconClasses: string[] | undefined = undefined;

src/vs/workbench/browser/parts/editor/editorAutoSave.ts

@@ -29,7 +29,7 @@ export class EditorAutoSave extends Disposable implements IWorkbenchContribution
 	// Auto save: focus change & window change
 	private lastActiveEditor: EditorInput | undefined = undefined;
 	private lastActiveGroupId: GroupIdentifier | undefined = undefined;
-	private lastActiveEditorControlDisposable = this._register(new DisposableStore());
+	private readonly lastActiveEditorControlDisposable = this._register(new DisposableStore());
 
 	// Auto save: waiting on specific condition
 	private readonly waitingOnConditionAutoSaveWorkingCopies = new ResourceMap<{ readonly workingCopy: IWorkingCopy; readonly reason: SaveReason; condition: AutoSaveDisabledReason }>(resource => this.uriIdentityService.extUri.getComparisonKey(resource));

src/vs/workbench/browser/parts/editor/editorPlaceholder.ts

@@ -52,7 +52,7 @@ export abstract class EditorPlaceholder extends EditorPane {
 
 	private container: HTMLElement | undefined;
 	private scrollbar: DomScrollableElement | undefined;
-	private inputDisposable = this._register(new MutableDisposable());
+	private readonly inputDisposable = this._register(new MutableDisposable());
 
 	constructor(
 		id: string,

src/vs/workbench/browser/parts/editor/editorTitleControl.ts

@@ -36,10 +36,10 @@ export interface IEditorTitleControlDimensions {
 export class EditorTitleControl extends Themable {
 
 	private editorTabsControl: IEditorTabsControl;
-	private editorTabsControlDisposable = this._register(new DisposableStore());
+	private readonly editorTabsControlDisposable = this._register(new DisposableStore());
 
 	private breadcrumbsControlFactory: BreadcrumbsControlFactory | undefined;
-	private breadcrumbsControlDisposables = this._register(new DisposableStore());
+	private readonly breadcrumbsControlDisposables = this._register(new DisposableStore());
 	private get breadcrumbsControl() { return this.breadcrumbsControlFactory?.control; }
 
 	constructor(

src/vs/workbench/browser/parts/paneCompositePart.ts

@@ -116,9 +116,9 @@ export abstract class AbstractPaneCompositePart extends CompositePart<PaneCompos
 	private readonly location: ViewContainerLocation;
 	private titleContainer: HTMLElement | undefined;
 	private headerFooterCompositeBarContainer: HTMLElement | undefined;
-	protected headerFooterCompositeBarDispoables = this._register(new DisposableStore());
+	protected readonly headerFooterCompositeBarDispoables = this._register(new DisposableStore());
 	private paneCompositeBarContainer: HTMLElement | undefined;
-	private paneCompositeBar = this._register(new MutableDisposable<PaneCompositeBar>());
+	private readonly paneCompositeBar = this._register(new MutableDisposable<PaneCompositeBar>());
 	private compositeBarPosition: CompositeBarPosition | undefined = undefined;
 	private emptyPaneMessageElement: HTMLElement | undefined;
 

src/vs/workbench/browser/parts/titlebar/menubarControl.ts

@@ -143,7 +143,7 @@ export abstract class MenubarControl extends Disposable {
 
 	protected topLevelTitles: { [menu: string]: string } = {};
 
-	protected mainMenuDisposables: DisposableStore;
+	protected readonly mainMenuDisposables: DisposableStore;
 
 	protected recentlyOpened: IRecentlyOpened = { files: [], workspaces: [] };
 
@@ -564,7 +564,7 @@ export class CustomMenubarControl extends MenubarControl {
 		return result;
 	}
 
-	private reinstallDisposables = this._register(new DisposableStore());
+	private readonly reinstallDisposables = this._register(new DisposableStore());
 	private setupCustomMenubar(firstTime: boolean): void {
 		// If there is no container, we cannot setup the menubar
 		if (!this.container) {

src/vs/workbench/browser/parts/titlebar/titlebarPart.ts

@@ -235,8 +235,8 @@ export class BrowserTitlebarPart extends Part implements ITitlebarPart {
 	private lastLayoutDimensions: Dimension | undefined;
 
 	private actionToolBar!: WorkbenchToolBar;
-	private actionToolBarDisposable = this._register(new DisposableStore());
-	private editorActionsChangeDisposable = this._register(new DisposableStore());
+	private readonly actionToolBarDisposable = this._register(new DisposableStore());
+	private readonly editorActionsChangeDisposable = this._register(new DisposableStore());
 	private actionToolBarElement!: HTMLElement;
 
 	private layoutToolbarMenu: IMenu | undefined;

src/vs/workbench/contrib/accountEntitlements/browser/accountsEntitlements.contribution.ts

@@ -48,7 +48,7 @@ class EntitlementsContribution extends Disposable implements IWorkbenchContribut
 	private isInitialized = false;
 	private showAccountsBadgeContextKey = new RawContextKey<boolean>(accountsBadgeConfigKey, false).bindTo(this.contextService);
 	private showChatWelcomeViewContextKey = new RawContextKey<boolean>(chatWelcomeViewConfigKey, false).bindTo(this.contextService);
-	private accountsMenuBadgeDisposable = this._register(new MutableDisposable());
+	private readonly accountsMenuBadgeDisposable = this._register(new MutableDisposable());
 
 	constructor(
 		@IContextKeyService readonly contextService: IContextKeyService,

src/vs/workbench/contrib/chat/browser/chatInputPart.ts

@@ -93,7 +93,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
 	private inputSideToolbarContainer?: HTMLElement;
 
 	private followupsContainer!: HTMLElement;
-	private followupsDisposables = this._register(new DisposableStore());
+	private readonly followupsDisposables = this._register(new DisposableStore());
 
 	private implicitContextContainer!: HTMLElement;
 	private implicitContextLabel!: HTMLElement;

src/vs/workbench/contrib/chat/browser/chatQuick.ts

@@ -156,7 +156,7 @@ class QuickChat extends Disposable {
 	private sash!: Sash;
 	private model: ChatModel | undefined;
 	private _currentQuery: string | undefined;
-	private maintainScrollTimer: MutableDisposable<IDisposable> = this._register(new MutableDisposable<IDisposable>());
+	private readonly maintainScrollTimer: MutableDisposable<IDisposable> = this._register(new MutableDisposable<IDisposable>());
 	private _deferUpdatingDynamicLayout: boolean = false;
 
 	constructor(