Fix injected services becoming public by mistake (microsoft#209513)

* Fix injected services becoming public by mistake

Fixes cases of `@IFooService readonly foo: IFooService`. This makes the service public, which is likely not expected and also means we can't mangle it

* Fix name

* Remove unused props

Author: mjbvz

.eslintplugin/code-parameter-properties-must-have-explicit-accessibility.ts

@@ -0,0 +1,41 @@
+/*---------------------------------------------------------------------------------------------
+ *  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';
+import { TSESTree } from '@typescript-eslint/experimental-utils';
+
+/**
+ * Enforces that all parameter properties have an explicit access modifier (public, protected, private).
+ *
+ * This catches a common bug where a service is accidentally made public by simply writing: `readonly prop: Foo`
+ */
+export = new class implements eslint.Rule.RuleModule {
+
+	create(context: eslint.Rule.RuleContext): eslint.Rule.RuleListener {
+		function check(inNode: any) {
+			const node: TSESTree.TSParameterProperty = inNode;
+
+			// For now, only apply to injected services
+			const firstDecorator = node.decorators?.at(0);
+			if (
+				firstDecorator?.expression.type !== 'Identifier'
+				|| !firstDecorator.expression.name.endsWith('Service')
+			) {
+				return;
+			}
+
+			if (!node.accessibility) {
+				context.report({
+					node: inNode,
+					message: 'Parameter properties must have an explicit access modifier.'
+				});
+			}
+		}
+
+		return {
+			['TSParameterProperty']: check,
+		};
+	}
+};

.eslintrc.json

@@ -70,6 +70,7 @@
 		],
 		"local/code-translation-remind": "warn",
 		"local/code-no-native-private": "warn",
+		"local/code-parameter-properties-must-have-explicit-accessibility": "warn",
 		"local/code-no-nls-in-standalone-editor": "warn",
 		"local/code-no-standalone-editor": "warn",
 		"local/code-no-unexternalized-strings": "warn",

src/vs/editor/contrib/stickyScroll/browser/stickyScrollModelProvider.ts

@@ -52,8 +52,8 @@ export class StickyModelProvider extends Disposable implements IStickyModelProvi
 	constructor(
 		private readonly _editor: IActiveCodeEditor,
 		onProviderUpdate: () => void,
-		@IInstantiationService readonly _languageConfigurationService: ILanguageConfigurationService,
-		@ILanguageFeaturesService readonly _languageFeaturesService: ILanguageFeaturesService,
+		@IInstantiationService _languageConfigurationService: ILanguageConfigurationService,
+		@ILanguageFeaturesService _languageFeaturesService: ILanguageFeaturesService,
 	) {
 		super();
 

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

@@ -51,17 +51,16 @@ class EntitlementsContribution extends Disposable implements IWorkbenchContribut
 	private accountsMenuBadgeDisposable = this._register(new MutableDisposable());
 
 	constructor(
-		@IContextKeyService readonly contextService: IContextKeyService,
-		@ICommandService readonly commandService: ICommandService,
-		@ITelemetryService readonly telemetryService: ITelemetryService,
-		@IAuthenticationService readonly authenticationService: IAuthenticationService,
-		@IProductService readonly productService: IProductService,
-		@IStorageService readonly storageService: IStorageService,
-		@IExtensionManagementService readonly extensionManagementService: IExtensionManagementService,
-		@IActivityService readonly activityService: IActivityService,
-		@IExtensionService readonly extensionService: IExtensionService,
-		@IConfigurationService readonly configurationService: IConfigurationService,
-		@IRequestService readonly requestService: IRequestService) {
+		@IContextKeyService private readonly contextService: IContextKeyService,
+		@ITelemetryService private readonly telemetryService: ITelemetryService,
+		@IAuthenticationService private readonly authenticationService: IAuthenticationService,
+		@IProductService private readonly productService: IProductService,
+		@IStorageService private readonly storageService: IStorageService,
+		@IExtensionManagementService private readonly extensionManagementService: IExtensionManagementService,
+		@IActivityService private readonly activityService: IActivityService,
+		@IExtensionService private readonly extensionService: IExtensionService,
+		@IConfigurationService private readonly configurationService: IConfigurationService,
+		@IRequestService private readonly requestService: IRequestService) {
 		super();
 
 		if (!this.productService.gitHubEntitlement || isWeb) {

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

@@ -79,7 +79,7 @@ export class ChatEditorOptions extends Disposable {
 		private readonly resultEditorBackgroundColor: string,
 		@IConfigurationService private readonly configurationService: IConfigurationService,
 		@IThemeService private readonly themeService: IThemeService,
-		@IViewDescriptorService readonly viewDescriptorService: IViewDescriptorService
+		@IViewDescriptorService private readonly viewDescriptorService: IViewDescriptorService
 	) {
 		super();
 

src/vs/workbench/contrib/chat/common/chatViewModel.ts

@@ -348,7 +348,7 @@ export class ChatRequestViewModel implements IChatRequestViewModel {
 	currentRenderedHeight: number | undefined;
 
 	constructor(
-		readonly _model: IChatRequestModel,
+		private readonly _model: IChatRequestModel,
 	) { }
 }
 

src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts

@@ -303,8 +303,8 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
 		@INotebookExecutionService private readonly notebookExecutionService: INotebookExecutionService,
 		@INotebookExecutionStateService private readonly notebookExecutionStateService: INotebookExecutionStateService,
 		@IEditorProgressService private editorProgressService: IEditorProgressService,
-		@INotebookLoggingService readonly logService: INotebookLoggingService,
-		@IKeybindingService readonly keybindingService: IKeybindingService,
+		@INotebookLoggingService private readonly logService: INotebookLoggingService,
+		@IKeybindingService private readonly keybindingService: IKeybindingService,
 		@ICodeEditorService codeEditorService: ICodeEditorService
 	) {
 		super();

src/vs/workbench/contrib/notebook/browser/services/notebookEditorServiceImpl.ts

@@ -37,7 +37,7 @@ export class NotebookEditorWidgetService implements INotebookEditorService {
 	private readonly _borrowableEditors = new Map<number, ResourceMap<{ widget: NotebookEditorWidget; token: number | undefined }>>();
 
 	constructor(
-		@IEditorGroupsService readonly editorGroupService: IEditorGroupsService,
+		@IEditorGroupsService private readonly editorGroupService: IEditorGroupsService,
 		@IEditorService editorService: IEditorService,
 		@IContextKeyService contextKeyService: IContextKeyService
 	) {

src/vs/workbench/contrib/remote/browser/remoteExplorer.ts

@@ -185,11 +185,11 @@ export class AutomaticPortForwarding extends Disposable implements IWorkbenchCon
 		@IOpenerService private readonly openerService: IOpenerService,
 		@IExternalUriOpenerService private readonly externalOpenerService: IExternalUriOpenerService,
 		@IRemoteExplorerService private readonly remoteExplorerService: IRemoteExplorerService,
-		@IWorkbenchEnvironmentService readonly environmentService: IWorkbenchEnvironmentService,
+		@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService,
 		@IContextKeyService private readonly contextKeyService: IContextKeyService,
 		@IWorkbenchConfigurationService private readonly configurationService: IWorkbenchConfigurationService,
 		@IDebugService private readonly debugService: IDebugService,
-		@IRemoteAgentService readonly remoteAgentService: IRemoteAgentService,
+		@IRemoteAgentService remoteAgentService: IRemoteAgentService,
 		@ITunnelService private readonly tunnelService: ITunnelService,
 		@IHostService private readonly hostService: IHostService,
 		@ILogService private readonly logService: ILogService,

src/vs/workbench/contrib/scm/browser/scmViewPane.ts

@@ -799,12 +799,12 @@ class HistoryItemGroupRenderer implements ICompressibleTreeRenderer<SCMHistoryIt
 
 	constructor(
 		readonly actionRunner: ActionRunner,
-		@IContextKeyService readonly contextKeyService: IContextKeyService,
-		@IContextMenuService readonly contextMenuService: IContextMenuService,
-		@IKeybindingService	readonly keybindingService: IKeybindingService,
-		@IMenuService readonly menuService: IMenuService,
+		@IContextKeyService private readonly contextKeyService: IContextKeyService,
+		@IContextMenuService private readonly contextMenuService: IContextMenuService,
+		@IKeybindingService private readonly keybindingService: IKeybindingService,
+		@IMenuService private readonly menuService: IMenuService,
 		@ISCMViewService private readonly scmViewService: ISCMViewService,
-		@ITelemetryService readonly telemetryService: ITelemetryService
+		@ITelemetryService private readonly telemetryService: ITelemetryService
 	) { }
 
 	renderTemplate(container: HTMLElement) {
@@ -1115,11 +1115,11 @@ class SeparatorRenderer implements ICompressibleTreeRenderer<SCMViewSeparatorEle
 	get templateId(): string { return SeparatorRenderer.TEMPLATE_ID; }
 
 	constructor(
-		@IContextKeyService readonly contextKeyService: IContextKeyService,
-		@IContextMenuService readonly contextMenuService: IContextMenuService,
-		@IKeybindingService	readonly keybindingService: IKeybindingService,
-		@IMenuService readonly menuService: IMenuService,
-		@ITelemetryService readonly telemetryService: ITelemetryService
+		@IContextKeyService private readonly contextKeyService: IContextKeyService,
+		@IContextMenuService private readonly contextMenuService: IContextMenuService,
+		@IKeybindingService private readonly keybindingService: IKeybindingService,
+		@IMenuService private readonly menuService: IMenuService,
+		@ITelemetryService private readonly telemetryService: ITelemetryService
 	) { }
 
 	renderTemplate(container: HTMLElement): SeparatorTemplate {