Don't localize markdown icon syntax (#303655)

* Don't localize markdown icon syntax

Co-authored-by: Copilot <copilot@github.com>

* Add eslint rule for localized markdown icons

---------

Co-authored-by: Copilot <copilot@github.com>

.eslint-plugin-local/code-no-icons-in-localized-strings.ts

@@ -0,0 +1,99 @@
+/*---------------------------------------------------------------------------------------------
+ *  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/utils';
+
+/**
+ * Prevents theme icon syntax `$(iconName)` from appearing inside localized
+ * string arguments. Localizers may translate or corrupt the icon syntax,
+ * breaking rendering. Icon references should be kept outside the localized
+ * string - either prepended via concatenation or passed as a placeholder
+ * argument.
+ *
+ * Examples:
+ * ❌ localize('key', "$(gear) Settings")
+ * ✅ '$(gear) ' + localize('key', "Settings")
+ * ✅ localize('key', "Like {0}", '$(gear)')
+ *
+ * ❌ nls.localize('key', "$(loading~spin) Loading...")
+ * ✅ '$(loading~spin) ' + nls.localize('key', "Loading...")
+ */
+export default new class NoIconsInLocalizedStrings implements eslint.Rule.RuleModule {
+
+	readonly meta: eslint.Rule.RuleMetaData = {
+		messages: {
+			noIconInLocalizedString: 'Theme icon syntax $(…) should not appear inside localized strings. Move it outside the localize call or pass it as a placeholder argument.'
+		},
+		docs: {
+			description: 'Prevents $(icon) theme icon syntax inside localize() string arguments',
+		},
+		type: 'problem',
+		schema: false,
+	};
+
+	create(context: eslint.Rule.RuleContext): eslint.Rule.RuleListener {
+
+		// Matches $(iconName) or $(iconName~modifier) but not escaped \$(...)
+		const iconPattern = /(?<!\\)\$\([a-zA-Z][\w~-]*\)/;
+
+		function isLocalizeCall(callee: TSESTree.CallExpression['callee']): { isLocalize: boolean; messageArgIndex: number } {
+			// Direct localize('key', "message", ...) or localize2('key', "message", ...)
+			if (callee.type === 'Identifier' && (callee.name === 'localize' || callee.name === 'localize2')) {
+				return { isLocalize: true, messageArgIndex: 1 };
+			}
+
+			// nls.localize('key', "message", ...) or *.localize(...)
+			if (callee.type === 'MemberExpression' && callee.property.type === 'Identifier' && callee.property.name === 'localize') {
+				return { isLocalize: true, messageArgIndex: 1 };
+			}
+
+			return { isLocalize: false, messageArgIndex: -1 };
+		}
+
+		function getStringValue(node: TSESTree.Node): string | undefined {
+			if (node.type === 'Literal' && typeof node.value === 'string') {
+				return node.value;
+			}
+			if (node.type === 'TemplateLiteral' && node.expressions.length === 0 && node.quasis.length === 1) {
+				return node.quasis[0].value.cooked ?? undefined;
+			}
+			return undefined;
+		}
+
+		function checkCallExpression(node: TSESTree.CallExpression) {
+			const { isLocalize, messageArgIndex } = isLocalizeCall(node.callee);
+			if (!isLocalize) {
+				return;
+			}
+
+			// The first argument may be a string key or an object { key, comment }.
+			// Adjust the message argument index if the first arg is an object.
+			let actualMessageArgIndex = messageArgIndex;
+			const firstArg = node.arguments[0];
+			if (firstArg && firstArg.type === 'ObjectExpression') {
+				// localize({ key: '...', comment: [...] }, "message", ...)
+				actualMessageArgIndex = 1;
+			}
+
+			const messageArg = node.arguments[actualMessageArgIndex];
+			if (!messageArg) {
+				return;
+			}
+
+			const messageValue = getStringValue(messageArg);
+			if (messageValue !== undefined && iconPattern.test(messageValue)) {
+				context.report({
+					node: messageArg,
+					messageId: 'noIconInLocalizedString'
+				});
+			}
+		}
+
+		return {
+			CallExpression: (node: any) => checkCallExpression(node as TSESTree.CallExpression)
+		};
+	}
+};

eslint.config.js

@@ -92,6 +92,7 @@ export default tseslint.config(
 			'local/code-no-localized-model-description': 'warn',
 			'local/code-policy-localization-key-match': 'warn',
 			'local/code-no-localization-template-literals': 'error',
+			'local/code-no-icons-in-localized-strings': 'warn',
 			'local/code-no-http-import': ['warn', { target: 'src/vs/**' }],
 			'local/code-no-deep-import-of-internal': ['error', { '.*Internal': true, 'searchExtTypesInternal': false }],
 			'local/code-layering': [

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

@@ -199,7 +199,7 @@ const statusBarItemSchema = {
 		},
 		text: {
 			type: 'string',
-			description: localize('text', 'The text to show for the entry. You can embed icons in the text by leveraging the `$(<name>)`-syntax, like \'Hello $(globe)!\'')
+			description: localize('text', 'The text to show for the entry. You can embed icons in the text by leveraging the `$(<name>)`-syntax, like \'Hello {0}!\'', '$(globe)')
 		},
 		tooltip: {
 			type: 'string',

src/vs/workbench/contrib/chat/browser/chatSessions/chatSessions.contribution.ts

@@ -83,7 +83,7 @@ const extensionPoint = ExtensionsRegistry.registerExtensionPoint<IChatSessionsEx
 					type: 'string'
 				},
 				icon: {
-					description: localize('chatSessionsExtPoint.icon', 'Icon identifier (codicon ID) for the chat session editor tab. For example, "$(github)" or "$(cloud)".'),
+					description: localize('chatSessionsExtPoint.icon', 'Icon identifier (codicon ID) for the chat session editor tab. For example, "{0}" or "{1}".', '$(github)', '$(cloud)'),
 					anyOf: [{
 						type: 'string'
 					},

src/vs/workbench/contrib/chat/browser/chatStatus/chatStatusDashboard.ts

@@ -240,7 +240,7 @@ export class ChatStatusDashboard extends DomWidget {
 					if (count > 0) {
 						const displayName = this.getDisplayNameForChatSessionType(chatSessionType);
 						if (displayName) {
-							const text = localize('inProgressChatSession', "$(loading~spin) {0} in progress", displayName);
+							const text = '$(loading~spin) ' + localize('inProgressChatSession', "{0} in progress", displayName);
 							const chatSessionsElement = this.element.appendChild($('div.description'));
 							const parts = renderLabelWithIcons(text);
 							chatSessionsElement.append(...parts);

src/vs/workbench/contrib/chat/browser/contextContrib/chatContext.contribution.ts

@@ -67,7 +67,7 @@ export class ChatContextContribution extends Disposable implements IWorkbenchCon
 				for (const contribution of ext.value) {
 					const icon = contribution.icon ? ThemeIcon.fromString(contribution.icon) : undefined;
 					if (!icon && contribution.icon) {
-						ext.collector.error(localize('chatContextExtPoint.invalidIcon', "Invalid icon format for chat context contribution '{0}'. Icon must be in the format '$(iconId)' or '$(iconId~spin)', e.g. '$(copilot)'.", contribution.id));
+						ext.collector.error(localize('chatContextExtPoint.invalidIcon', "Invalid icon format for chat context contribution '{0}'. Icon must be in the format '{1}' or '{2}', e.g. '{3}'.", contribution.id, '$(iconId)', '$(iconId~spin)', '$(copilot)'));
 						continue;
 					}
 					if (!icon) {

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts

@@ -1668,8 +1668,8 @@ class ChatTerminalThinkingCollapsibleWrapper extends ChatCollapsibleContentPart
 		labelElement.textContent = '';
 		if (this._isSandboxWrapped) {
 			dom.reset(labelElement, ...renderLabelWithIcons(this._isComplete
-				? localize('chat.terminal.ranInSandbox', "$(lock) Ran `{0}` in sandbox", this._commandText)
-				: localize('chat.terminal.runningInSandbox', "$(lock) Running `{0}` in sandbox", this._commandText)));
+				? '$(lock) ' + localize('chat.terminal.ranInSandbox', "Ran `{0}` in sandbox", this._commandText)
+				: '$(lock) ' + localize('chat.terminal.runningInSandbox', "Running `{0}` in sandbox", this._commandText)));
 			return;
 		}
 

src/vs/workbench/contrib/chat/common/tools/languageModelToolsContribution.ts

@@ -176,7 +176,7 @@ const languageModelToolSetsExtensionPoint = extensionsRegistry.ExtensionsRegistr
 					type: 'string'
 				},
 				icon: {
-					markdownDescription: localize('toolSetIcon', "An icon that represents this tool set, like `$(zap)`"),
+					markdownDescription: localize('toolSetIcon', "An icon that represents this tool set, like {0}", '`$(zap)`'),
 					type: 'string'
 				},
 				tools: {

src/vs/workbench/contrib/files/browser/files.contribution.ts

@@ -459,7 +459,7 @@ configurationRegistry.registerConfiguration({
 								type: 'string', // expression ({ "**/*.js": { "when": "$(basename).js" } })
 								pattern: '\\w*\\$\\(basename\\)\\w*',
 								default: '$(basename).ext',
-								description: nls.localize('explorer.autoRevealExclude.when', 'Additional check on the siblings of a matching file. Use $(basename) as variable for the matching file name.')
+								description: nls.localize('explorer.autoRevealExclude.when', 'Additional check on the siblings of a matching file. Use {0} as variable for the matching file name.', '$(basename)')
 							}
 						}
 					}

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

@@ -1706,7 +1706,7 @@ export class SCMHistoryViewPane extends ViewPane {
 						compact: true,
 						showPointer: true
 					},
-					content: new MarkdownString(localize('scmGraphViewOutdated', "Please refresh the graph using the refresh action ($(refresh))."), { supportThemeIcons: true }),
+					content: new MarkdownString(localize('scmGraphViewOutdated', "Please refresh the graph using the refresh action ({0}).", '$(refresh)'), { supportThemeIcons: true }),
 					position: {
 						hoverPosition: HoverPosition.BELOW
 					}

src/vs/workbench/contrib/tasks/browser/taskQuickPick.ts

@@ -360,7 +360,7 @@ export class TaskQuickPick extends Disposable {
 	public static getSettingEntry(configurationService: IConfigurationService, type: string): (ITaskTwoLevelQuickPickEntry & { settingType: string }) | undefined {
 		if (configurationService.getValue(`${type}.autoDetect`) === 'off') {
 			return {
-				label: nls.localize('TaskQuickPick.changeSettingsOptions', "$(gear) {0} task detection is turned off. Enable {1} task detection...",
+				label: '$(gear) ' + nls.localize('TaskQuickPick.changeSettingsOptions', "{0} task detection is turned off. Enable {1} task detection...",
 					type[0].toUpperCase() + type.slice(1), type),
 				task: null,
 				settingType: type,

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts

@@ -778,8 +778,8 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 		const escapedDisplayCommand = escapeMarkdownSyntaxTokens(displayCommand);
 		const invocationMessage = toolSpecificData.commandLine.isSandboxWrapped
 			? args.isBackground
-				? new MarkdownString(localize('runInTerminal.invocation.sandbox.background', "$(lock) Running `{0}` in sandbox in background", escapedDisplayCommand), { supportThemeIcons: true })
-				: new MarkdownString(localize('runInTerminal.invocation.sandbox', "$(lock) Running `{0}` in sandbox", escapedDisplayCommand), { supportThemeIcons: true })
+				? new MarkdownString('$(lock) ' + localize('runInTerminal.invocation.sandbox.background', "Running `{0}` in sandbox in background", escapedDisplayCommand), { supportThemeIcons: true })
+				: new MarkdownString('$(lock) ' + localize('runInTerminal.invocation.sandbox', "Running `{0}` in sandbox", escapedDisplayCommand), { supportThemeIcons: true })
 			: args.isBackground
 				? new MarkdownString(localize('runInTerminal.invocation.background', "Running `{0}` in background", escapedDisplayCommand))
 				: new MarkdownString(localize('runInTerminal.invocation', "Running `{0}`", escapedDisplayCommand));

src/vs/workbench/contrib/update/browser/updateStatusBarEntry.ts

@@ -71,7 +71,7 @@ export class UpdateStatusBarContribution extends Disposable implements IWorkbenc
 		switch (state.type) {
 			case StateType.CheckingForUpdates:
 				this.updateEntry(
-					localize('updateStatus.checkingForUpdates', "$(loading~spin) Checking for updates..."),
+					'$(loading~spin) ' + localize('updateStatus.checkingForUpdates', "Checking for updates..."),
 					localize('updateStatus.checkingForUpdatesAria', "Checking for updates"),
 					ShowTooltipCommand,
 				);
@@ -79,7 +79,7 @@ export class UpdateStatusBarContribution extends Disposable implements IWorkbenc
 
 			case StateType.AvailableForDownload:
 				this.updateEntry(
-					localize('updateStatus.updateAvailableStatus', "$(circle-filled) Update available, click to download."),
+					'$(circle-filled) ' + localize('updateStatus.updateAvailableStatus', "Update available, click to download."),
 					localize('updateStatus.updateAvailableAria', "Update available, click to download."),
 					'update.downloadNow'
 				);
@@ -95,7 +95,7 @@ export class UpdateStatusBarContribution extends Disposable implements IWorkbenc
 
 			case StateType.Downloaded:
 				this.updateEntry(
-					localize('updateStatus.updateReadyStatus', "$(circle-filled) Update downloaded, click to install."),
+					'$(circle-filled) ' + localize('updateStatus.updateReadyStatus', "Update downloaded, click to install."),
 					localize('updateStatus.updateReadyAria', "Update downloaded, click to install."),
 					'update.install'
 				);
@@ -111,7 +111,7 @@ export class UpdateStatusBarContribution extends Disposable implements IWorkbenc
 
 			case StateType.Ready:
 				this.updateEntry(
-					localize('updateStatus.restartToUpdateStatus', "$(circle-filled) Update is ready, click to restart."),
+					'$(circle-filled) ' + localize('updateStatus.restartToUpdateStatus', "Update is ready, click to restart."),
 					localize('updateStatus.restartToUpdateAria', "Update is ready, click to restart."),
 					'update.restart'
 				);
@@ -119,7 +119,7 @@ export class UpdateStatusBarContribution extends Disposable implements IWorkbenc
 
 			case StateType.Overwriting:
 				this.updateEntry(
-					localize('updateStatus.downloadingNewerUpdateStatus', "$(loading~spin) Downloading update..."),
+					'$(loading~spin) ' + localize('updateStatus.downloadingNewerUpdateStatus', "Downloading update..."),
 					localize('updateStatus.downloadingNewerUpdateAria', "Downloading a newer update"),
 					ShowTooltipCommand
 				);
@@ -155,21 +155,21 @@ export class UpdateStatusBarContribution extends Disposable implements IWorkbenc
 	private getDownloadingText({ downloadedBytes, totalBytes }: Downloading): string {
 		if (downloadedBytes !== undefined && totalBytes !== undefined && totalBytes > 0) {
 			const percent = computeProgressPercent(downloadedBytes, totalBytes) ?? 0;
-			return localize('updateStatus.downloadUpdateProgressStatus', "$(loading~spin) Downloading update: {0} / {1} • {2}%",
+			return '$(loading~spin) ' + localize('updateStatus.downloadUpdateProgressStatus', "Downloading update: {0} / {1} • {2}%",
 				formatBytes(downloadedBytes),
 				formatBytes(totalBytes),
 				percent);
 		} else {
-			return localize('updateStatus.downloadUpdateStatus', "$(loading~spin) Downloading update...");
+			return '$(loading~spin) ' + localize('updateStatus.downloadUpdateStatus', "Downloading update...");
 		}
 	}
 
 	private getUpdatingText({ currentProgress, maxProgress }: Updating): string {
 		const percentage = computeProgressPercent(currentProgress, maxProgress);
 		if (percentage !== undefined) {
-			return localize('updateStatus.installingUpdateProgressStatus', "$(loading~spin) Installing update: {0}%", percentage);
+			return '$(loading~spin) ' + localize('updateStatus.installingUpdateProgressStatus', "Installing update: {0}%", percentage);
 		} else {
-			return localize('updateStatus.installingUpdateStatus', "$(loading~spin) Installing update...");
+			return '$(loading~spin) ' + localize('updateStatus.installingUpdateStatus', "Installing update...");
 		}
 	}
 }