Silent warning when argv.json has syntax errors (fix #212671) (#215551)

src/main.js

@@ -21,7 +21,7 @@ const os = require('os');
 const bootstrap = require('./bootstrap');
 const bootstrapNode = require('./bootstrap-node');
 const { getUserDataPath } = require('./vs/platform/environment/node/userDataPath');
-const { stripComments } = require('./vs/base/common/stripComments');
+const { parse } = require('./vs/base/common/jsonc');
 const { getUNCHost, addUNCHostToAllowlist } = require('./vs/base/node/unc');
 /** @type {Partial<IProductConfiguration>} */
 // @ts-ignore
@@ -316,7 +316,7 @@ function readArgvConfigSync() {
 	const argvConfigPath = getArgvConfigPath();
 	let argvConfig;
 	try {
-		argvConfig = JSON.parse(stripComments(fs.readFileSync(argvConfigPath).toString()));
+		argvConfig = parse(fs.readFileSync(argvConfigPath).toString());
 	} catch (error) {
 		if (error && error.code === 'ENOENT') {
 			createDefaultArgvConfigSync(argvConfigPath);

src/vs/base/common/json.ts

@@ -1308,40 +1308,6 @@ export function visit(text: string, visitor: JSONVisitor, options: ParseOptions
 	return true;
 }
 
-/**
- * Takes JSON with JavaScript-style comments and remove
- * them. Optionally replaces every none-newline character
- * of comments with a replaceCharacter
- */
-export function stripComments(text: string, replaceCh?: string): string {
-
-	const _scanner = createScanner(text);
-	const parts: string[] = [];
-	let kind: SyntaxKind;
-	let offset = 0;
-	let pos: number;
-
-	do {
-		pos = _scanner.getPosition();
-		kind = _scanner.scan();
-		switch (kind) {
-			case SyntaxKind.LineCommentTrivia:
-			case SyntaxKind.BlockCommentTrivia:
-			case SyntaxKind.EOF:
-				if (offset !== pos) {
-					parts.push(text.substring(offset, pos));
-				}
-				if (replaceCh !== undefined) {
-					parts.push(_scanner.getTokenValue().replace(/[^\r\n]/g, replaceCh));
-				}
-				offset = _scanner.getPosition();
-				break;
-		}
-	} while (kind !== SyntaxKind.EOF);
-
-	return parts.join('');
-}
-
 export function getNodeType(value: any): NodeType {
 	switch (typeof value) {
 		case 'boolean': return 'boolean';

src/vs/base/common/stripComments.d.ts → src/vs/base/common/jsonc.d.ts

@@ -3,11 +3,21 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+/**
+ * A drop-in replacement for JSON.parse that can parse
+ * JSON with comments and trailing commas.
+ *
+ * @param content the content to strip comments from
+ * @returns the parsed content as JSON
+*/
+export function parse(content: string): any;
+
 /**
  * Strips single and multi line JavaScript comments from JSON
  * content. Ignores characters in strings BUT doesn't support
  * string continuation across multiple lines since it is not
  * supported in JSON.
+ *
  * @param content the content to strip comments from
  * @returns the content without comments
 */

src/vs/base/common/stripComments.js → src/vs/base/common/jsonc.js

@@ -3,9 +3,10 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-'use strict';
+/// <reference path="../../../typings/require.d.ts" />
 
 //@ts-check
+'use strict';
 
 (function () {
 	function factory(path, os, productName, cwd) {
@@ -17,7 +18,6 @@
 		const regexp = /("[^"\\]*(?:\\.[^"\\]*)*")|('[^'\\]*(?:\\.[^'\\]*)*')|(\/\*[^\/\*]*(?:(?:\*|\/)[^\/\*]*)*?\*\/)|(\/{2,}.*?(?:(?:\r?\n)|$))|(,\s*[}\]])/g;
 
 		/**
-		 *
 		 * @param {string} content
 		 * @returns {string}
 		 */
@@ -46,19 +46,34 @@
 				}
 			});
 		}
+
+		/**
+		 * @param {string} content
+		 * @returns {any}
+		 */
+		function parse(content) {
+			const commentsStripped = stripComments(content);
+
+			try {
+				return JSON.parse(commentsStripped);
+			} catch (error) {
+				const trailingCommasStriped = commentsStripped.replace(/,\s*([}\]])/g, '$1');
+				return JSON.parse(trailingCommasStriped);
+			}
+		}
 		return {
-			stripComments
+			stripComments,
+			parse
 		};
 	}
 
-
 	if (typeof define === 'function') {
 		// amd
 		define([], function () { return factory(); });
 	} else if (typeof module === 'object' && typeof module.exports === 'object') {
 		// commonjs
 		module.exports = factory();
 	} else {
-		console.trace('strip comments defined in UNKNOWN context (neither requirejs or commonjs)');
+		console.trace('jsonc defined in UNKNOWN context (neither requirejs or commonjs)');
 	}
 })();

src/vs/base/test/common/stripComments.test.ts → src/vs/base/test/common/jsonParse.test.ts

@@ -4,12 +4,10 @@
  *--------------------------------------------------------------------------------------------*/
 import assert from 'assert';
 
-import { stripComments } from 'vs/base/common/stripComments';
+import { parse, stripComments } from 'vs/base/common/jsonc';
 import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
 
-// We use this regular expression quite often to strip comments in JSON files.
-
-suite('Strip Comments', () => {
+suite('JSON Parse', () => {
 	ensureNoDisposablesAreLeakedInTestSuite();
 
 	test('Line comment', () => {
@@ -23,7 +21,7 @@ suite('Strip Comments', () => {
 			"  \"prop\": 10 ",
 			"}",
 		].join('\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('Line comment - EOF', () => {
 		const content: string = [
@@ -36,7 +34,7 @@ suite('Strip Comments', () => {
 			"}",
 			""
 		].join('\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('Line comment - \\r\\n', () => {
 		const content: string = [
@@ -49,7 +47,7 @@ suite('Strip Comments', () => {
 			"  \"prop\": 10 ",
 			"}",
 		].join('\r\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('Line comment - EOF - \\r\\n', () => {
 		const content: string = [
@@ -62,7 +60,7 @@ suite('Strip Comments', () => {
 			"}",
 			""
 		].join('\r\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('Block comment - single line', () => {
 		const content: string = [
@@ -75,7 +73,7 @@ suite('Strip Comments', () => {
 			"  \"prop\": 10",
 			"}",
 		].join('\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('Block comment - multi line', () => {
 		const content: string = [
@@ -92,7 +90,7 @@ suite('Strip Comments', () => {
 			"  \"prop\": 10",
 			"}",
 		].join('\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('Block comment - shortest match', () => {
 		const content = "/* abc */ */";
@@ -110,7 +108,7 @@ suite('Strip Comments', () => {
 			"  \"/* */\": 10",
 			"}"
 		].join('\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('No strings - single quote', () => {
 		const content: string = [
@@ -136,7 +134,7 @@ suite('Strip Comments', () => {
 			`  "a": 10`,
 			"}"
 		].join('\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
 	});
 	test('Trailing comma in array', () => {
 		const content: string = [
@@ -145,6 +143,52 @@ suite('Strip Comments', () => {
 		const expected: string = [
 			`[ "a", "b", "c" ]`
 		].join('\n');
-		assert.strictEqual(stripComments(content), expected);
+		assert.deepEqual(parse(content), JSON.parse(expected));
+	});
+
+	test('Trailing comma', () => {
+		const content: string = [
+			"{",
+			"  \"propA\": 10, // a comment",
+			"  \"propB\": false, // a trailing comma",
+			"}",
+		].join('\n');
+		const expected = [
+			"{",
+			"  \"propA\": 10,",
+			"  \"propB\": false",
+			"}",
+		].join('\n');
+		assert.deepEqual(parse(content), JSON.parse(expected));
+	});
+
+	test('Trailing comma - EOF', () => {
+		const content = `
+// This configuration file allows you to pass permanent command line arguments to VS Code.
+// Only a subset of arguments is currently supported to reduce the likelihood of breaking
+// the installation.
+//
+// PLEASE DO NOT CHANGE WITHOUT UNDERSTANDING THE IMPACT
+//
+// NOTE: Changing this file requires a restart of VS Code.
+{
+	// Use software rendering instead of hardware accelerated rendering.
+	// This can help in cases where you see rendering issues in VS Code.
+	// "disable-hardware-acceleration": true,
+	// Allows to disable crash reporting.
+	// Should restart the app if the value is changed.
+	"enable-crash-reporter": true,
+	// Unique id used for correlating crash reports sent from this instance.
+	// Do not edit this value.
+	"crash-reporter-id": "aaaaab31-7453-4506-97d0-93411b2c21c7",
+	"locale": "en",
+	// "log-level": "trace"
+}
+`;
+		assert.deepEqual(parse(content), {
+			"enable-crash-reporter": true,
+			"crash-reporter-id": "aaaaab31-7453-4506-97d0-93411b2c21c7",
+			"locale": "en"
+		});
 	});
 });

src/vs/code/electron-main/app.ts

@@ -12,7 +12,7 @@ import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { isSigPipeError, onUnexpectedError, setUnexpectedErrorHandler } from 'vs/base/common/errors';
 import { isEqualOrParent } from 'vs/base/common/extpath';
 import { Event } from 'vs/base/common/event';
-import { stripComments } from 'vs/base/common/json';
+import { parse } from 'vs/base/common/jsonc';
 import { getPathLabel } from 'vs/base/common/labels';
 import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
 import { Schemas, VSCODE_AUTHORITY } from 'vs/base/common/network';
@@ -1394,10 +1394,10 @@ export class CodeApplication extends Disposable {
 		// Crash reporter
 		this.updateCrashReporterEnablement();
 
+		// macOS: rosetta translation warning
 		if (isMacintosh && app.runningUnderARM64Translation) {
 			this.windowsMainService?.sendToFocused('vscode:showTranslatedBuildWarning');
 		}
-
 	}
 
 	private async installMutex(): Promise<void> {
@@ -1437,7 +1437,7 @@ export class CodeApplication extends Disposable {
 		try {
 			const argvContent = await this.fileService.readFile(this.environmentMainService.argvResource);
 			const argvString = argvContent.value.toString();
-			const argvJSON = JSON.parse(stripComments(argvString));
+			const argvJSON = parse(argvString);
 			const telemetryLevel = getTelemetryLevel(this.configurationService);
 			const enableCrashReporter = telemetryLevel >= TelemetryLevel.CRASH;
 
@@ -1468,6 +1468,9 @@ export class CodeApplication extends Disposable {
 			}
 		} catch (error) {
 			this.logService.error(error);
+
+			// Inform the user via notification
+			this.windowsMainService?.sendToFocused('vscode:showArgvParseWarning');
 		}
 	}
 }

src/vs/workbench/contrib/encryption/electron-sandbox/encryption.contribution.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { isLinux } from 'vs/base/common/platform';
-import { stripComments } from 'vs/base/common/stripComments';
+import { parse } from 'vs/base/common/jsonc';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { IFileService } from 'vs/platform/files/common/files';
 import { Registry } from 'vs/platform/registry/common/platform';
@@ -34,7 +34,7 @@ class EncryptionContribution implements IWorkbenchContribution {
 		}
 		try {
 			const content = await this.fileService.readFile(this.environmentService.argvResource);
-			const argv = JSON.parse(stripComments(content.value.toString()));
+			const argv = parse(content.value.toString());
 			if (argv['password-store'] === 'gnome' || argv['password-store'] === 'gnome-keyring') {
 				this.jsonEditingService.write(this.environmentService.argvResource, [{ path: ['password-store'], value: 'gnome-libsecret' }], true);
 			}

src/vs/workbench/electron-sandbox/window.ts

@@ -204,7 +204,10 @@ export class NativeWindow extends BaseWindow {
 				[{
 					label: localize('restart', "Restart"),
 					run: () => this.nativeHostService.relaunch()
-				}]
+				}],
+				{
+					priority: NotificationPriority.URGENT
+				}
 			);
 		});
 
@@ -248,7 +251,7 @@ export class NativeWindow extends BaseWindow {
 			);
 		});
 
-		ipcRenderer.on('vscode:showTranslatedBuildWarning', (event: unknown, message: string) => {
+		ipcRenderer.on('vscode:showTranslatedBuildWarning', () => {
 			this.notificationService.prompt(
 				Severity.Warning,
 				localize("runningTranslated", "You are running an emulated version of {0}. For better performance download the native arm64 version of {0} build for your machine.", this.productService.nameLong),
@@ -260,7 +263,24 @@ export class NativeWindow extends BaseWindow {
 						const insidersURL = 'https://code.visualstudio.com/docs/?dv=osx&build=insiders';
 						this.openerService.open(quality === 'stable' ? stableURL : insidersURL);
 					}
-				}]
+				}],
+				{
+					priority: NotificationPriority.URGENT
+				}
+			);
+		});
+
+		ipcRenderer.on('vscode:showArgvParseWarning', (event: unknown, message: string) => {
+			this.notificationService.prompt(
+				Severity.Warning,
+				localize("showArgvParseWarning", "The runtime arguments file 'argv.json' contains errors. Please correct them and restart."),
+				[{
+					label: localize('showArgvParseWarningAction', "Open File"),
+					run: () => this.editorService.openEditor({ resource: this.nativeEnvironmentService.argvResource })
+				}],
+				{
+					priority: NotificationPriority.URGENT
+				}
 			);
 		});