cleaner args validation

fixes #10764

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

@@ -222,6 +222,10 @@ export function parseLineAndColumnAware(rawPath: string): IParsedPath {
 		}
 	});
 
+	if (!path) {
+		throw new Error('Format for `--goto` should be: `FILE:LINE(:COLUMN)`');
+	}
+
 	return {
 		path: path,
 		line: line !== null ? line : void 0,

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

@@ -10,7 +10,7 @@ import * as fs from 'original-fs';
 import { app, ipcMain as ipc } from 'electron';
 import { assign } from 'vs/base/common/objects';
 import * as platform from 'vs/base/common/platform';
-import { parseMainProcessArgv } from 'vs/code/node/argv';
+import { parseMainProcessArgv, ParsedArgs } from 'vs/code/node/argv';
 import { mkdirp } from 'vs/base/node/pfs';
 import { IProcessEnvironment, IEnvService, EnvService } from 'vs/code/electron-main/env';
 import { IWindowsService, WindowsManager } from 'vs/code/electron-main/windows';
@@ -269,22 +269,6 @@ function setupIPC(accessor: ServicesAccessor): TPromise<Server> {
 	return setup(true);
 }
 
-// TODO: isolate
-const services = new ServiceCollection();
-
-services.set(IEnvironmentService, new SyncDescriptor(EnvironmentService, parseMainProcessArgv(process.argv), process.execPath));
-services.set(IEnvService, new SyncDescriptor(EnvService));
-services.set(ILogService, new SyncDescriptor(MainLogService));
-services.set(IWindowsService, new SyncDescriptor(WindowsManager));
-services.set(ILifecycleService, new SyncDescriptor(LifecycleService));
-services.set(IStorageService, new SyncDescriptor(StorageService));
-services.set(IConfigurationService, new SyncDescriptor(NodeConfigurationService));
-services.set(IRequestService, new SyncDescriptor(RequestService));
-services.set(IUpdateService, new SyncDescriptor(UpdateManager));
-services.set(ISettingsService, new SyncDescriptor(SettingsManager));
-
-const instantiationService = new InstantiationService(services);
-
 interface IEnv {
 	[key: string]: string;
 }
@@ -369,20 +353,18 @@ function getShellEnvironment(): TPromise<IEnv> {
  * Such environment needs to be propagated to the renderer/shared
  * processes.
  */
-function getEnvironment(): TPromise<IEnv> {
-	return getShellEnvironment().then(shellEnv => {
-		return instantiationService.invokeFunction(a => {
-			const environmentService = a.get(IEnvironmentService);
-
-			const instanceEnv = {
-				VSCODE_PID: String(process.pid),
-				VSCODE_IPC_HOOK: environmentService.mainIPCHandle,
-				VSCODE_SHARED_IPC_HOOK: environmentService.sharedIPCHandle,
-				VSCODE_NLS_CONFIG: process.env['VSCODE_NLS_CONFIG']
-			};
+function getEnvironment(accessor: ServicesAccessor): TPromise<IEnv> {
+	const environmentService = accessor.get(IEnvironmentService);
 
-			return assign({}, shellEnv, instanceEnv);
-		});
+	return getShellEnvironment().then(shellEnv => {
+		const instanceEnv = {
+			VSCODE_PID: String(process.pid),
+			VSCODE_IPC_HOOK: environmentService.mainIPCHandle,
+			VSCODE_SHARED_IPC_HOOK: environmentService.sharedIPCHandle,
+			VSCODE_NLS_CONFIG: process.env['VSCODE_NLS_CONFIG']
+		};
+
+		return assign({}, shellEnv, instanceEnv);
 	});
 }
 
@@ -391,13 +373,45 @@ function createPaths(environmentService: IEnvironmentService): TPromise<any> {
 	return TPromise.join(paths.map(p => mkdirp(p))) as TPromise<any>;
 }
 
-// On some platforms we need to manually read from the global environment variables
-// and assign them to the process environment (e.g. when doubleclick app on Mac)
-getEnvironment().then(env => {
-	assign(process.env, env);
+function start(): void {
+	let args: ParsedArgs;
+
+	try {
+		args = parseMainProcessArgv(process.argv);
+	} catch (err) {
+		console.error(err.message);
+		process.exit(1);
+		return;
+	}
+
+	// TODO: isolate
+	const services = new ServiceCollection();
+
+	services.set(IEnvironmentService, new SyncDescriptor(EnvironmentService, args, process.execPath));
+	services.set(IEnvService, new SyncDescriptor(EnvService));
+	services.set(ILogService, new SyncDescriptor(MainLogService));
+	services.set(IWindowsService, new SyncDescriptor(WindowsManager));
+	services.set(ILifecycleService, new SyncDescriptor(LifecycleService));
+	services.set(IStorageService, new SyncDescriptor(StorageService));
+	services.set(IConfigurationService, new SyncDescriptor(NodeConfigurationService));
+	services.set(IRequestService, new SyncDescriptor(RequestService));
+	services.set(IUpdateService, new SyncDescriptor(UpdateManager));
+	services.set(ISettingsService, new SyncDescriptor(SettingsManager));
+
+	const instantiationService = new InstantiationService(services);
+
+	// On some platforms we need to manually read from the global environment variables
+	// and assign them to the process environment (e.g. when doubleclick app on Mac)
+	return instantiationService.invokeFunction(accessor => {
+		return getEnvironment(accessor).then(env => {
+			assign(process.env, env);
+
+			return instantiationService.invokeFunction(a => createPaths(a.get(IEnvironmentService)))
+				.then(() => instantiationService.invokeFunction(setupIPC))
+				.then(mainIpcServer => instantiationService.invokeFunction(main, mainIpcServer, env));
+		});
+	})
+	.done(null, err => instantiationService.invokeFunction(quit, err));
+}
 
-	return instantiationService.invokeFunction(a => createPaths(a.get(IEnvironmentService)))
-		.then(() => instantiationService.invokeFunction(setupIPC))
-		.then(mainIpcServer => instantiationService.invokeFunction(main, mainIpcServer, env));
-})
-.done(null, err => instantiationService.invokeFunction(quit, err));
+start();
\ No newline at end of file

src/vs/code/node/argv.ts

@@ -5,6 +5,7 @@
 
 import * as os from 'os';
 import * as minimist from 'minimist';
+import * as assert from 'assert';
 import { firstIndex } from 'vs/base/common/arrays';
 import { localize } from 'vs/nls';
 
@@ -71,22 +72,47 @@ const options: minimist.Opts = {
 	}
 };
 
+function validate(args: ParsedArgs): ParsedArgs {
+	if (args.goto) {
+		args._.forEach(arg => assert(/^[^:]+(:\d+){0,2}$/.test(arg), localize('gotoValidation', "Arguments in `--goto` mode should be in the format of `FILE(:LINE(:COLUMN))`.")));
+	}
+
+	return args;
+}
+
+function stripAppPath(argv: string[]): string[] {
+	const index = firstIndex(argv, a => !/^-/.test(a));
+
+	if (index > -1) {
+		return [...argv.slice(0, index), ...argv.slice(index + 1)];
+	}
+}
+
 /**
  * Use this to parse raw code process.argv such as: `Electron . --verbose --wait`
  */
 export function parseMainProcessArgv(processArgv: string[]): ParsedArgs {
-	const [, ...args] = processArgv;
+	let [, ...args] = processArgv;
 
 	// If dev, remove the first non-option argument: it's the app location
 	if (process.env['VSCODE_DEV']) {
-		const index = firstIndex(args, a => !/^-/.test(a));
+		args = stripAppPath(args);
+	}
 
-		if (index > -1) {
-			args.splice(index, 1);
-		}
+	return validate(parseArgs(args));
+}
+
+/**
+ * Use this to parse raw code CLI process.argv such as: `Electron cli.js . --verbose --wait`
+ */
+export function parseCLIProcessArgv(processArgv: string[]): ParsedArgs {
+	let [,, ...args] = processArgv;
+
+	if (process.env['VSCODE_DEV']) {
+		args = stripAppPath(args);
 	}
 
-	return parseArgs(args);
+	return validate(parseArgs(args));
 }
 
 /**

src/vs/code/node/cli.ts

@@ -6,7 +6,7 @@
 import { spawn } from 'child_process';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { assign } from 'vs/base/common/objects';
-import { parseArgs, buildHelpMessage, ParsedArgs } from 'vs/code/node/argv';
+import { parseCLIProcessArgv, buildHelpMessage, ParsedArgs } from 'vs/code/node/argv';
 import pkg from 'vs/platform/package';
 
 function shouldSpawnCliProcess(argv: ParsedArgs): boolean {
@@ -17,16 +17,23 @@ interface IMainCli {
 	main: (argv: ParsedArgs) => TPromise<void>;
 }
 
-export function main(args: string[]): TPromise<void> {
-	const argv = parseArgs(args);
+export function main(argv: string[]): TPromise<void> {
+	let args: ParsedArgs;
 
-	if (argv.help) {
+	try {
+		args = parseCLIProcessArgv(argv);
+	} catch (err) {
+		console.error(err.message);
+		return TPromise.as(null);
+	}
+
+	if (args.help) {
 		console.log(buildHelpMessage(pkg.version));
-	} else if (argv.version) {
+	} else if (args.version) {
 		console.log(pkg.version);
-	} else if (shouldSpawnCliProcess(argv)) {
+	} else if (shouldSpawnCliProcess(args)) {
 		const mainCli = new TPromise<IMainCli>(c => require(['vs/code/node/cliProcessMain'], c));
-		return mainCli.then(cli => cli.main(argv));
+		return mainCli.then(cli => cli.main(args));
 	} else {
 		const env = assign({}, process.env, {
 			// this will signal Code that it was spawned from this module
@@ -39,18 +46,18 @@ export function main(args: string[]): TPromise<void> {
 			detached: true,
 			env,
 		};
-		if (!argv.verbose) {
+		if (!args.verbose) {
 			options['stdio'] = 'ignore';
 		}
 
-		const child = spawn(process.execPath, args, options);
+		const child = spawn(process.execPath, argv.slice(2), options);
 
-		if (argv.verbose) {
+		if (args.verbose) {
 			child.stdout.on('data', (data) => console.log(data.toString('utf8').trim()));
 			child.stderr.on('data', (data) => console.log(data.toString('utf8').trim()));
 		}
 
-		if (argv.wait || argv.verbose) {
+		if (args.wait || args.verbose) {
 			return new TPromise<void>(c => child.once('exit', () => c(null)));
 		}
 	}
@@ -58,7 +65,7 @@ export function main(args: string[]): TPromise<void> {
 	return TPromise.as(null);
 }
 
-main(process.argv.slice(2))
+main(process.argv)
 	.then(() => process.exit(0))
 	.then(null, err => {
 		console.error(err.stack ? err.stack : err);

src/vs/code/node/cliProcessMain.ts

@@ -7,7 +7,7 @@ import { localize } from 'vs/nls';
 import product from 'vs/platform/product';
 import pkg from 'vs/platform/package';
 import * as path from 'path';
-import { parseArgs, ParsedArgs } from 'vs/code/node/argv';
+import { ParsedArgs } from 'vs/code/node/argv';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { sequence } from 'vs/base/common/async';
 import { IPager } from 'vs/base/common/paging';
@@ -145,7 +145,7 @@ const eventPrefix = 'monacoworkbench';
 
 export function main(argv: ParsedArgs): TPromise<void> {
 	const services = new ServiceCollection();
-	services.set(IEnvironmentService, new SyncDescriptor(EnvironmentService, parseArgs(process.argv), process.execPath));
+	services.set(IEnvironmentService, new SyncDescriptor(EnvironmentService, argv, process.execPath));
 
 	const instantiationService: IInstantiationService = new InstantiationService(services);