Improved Executable Argument Parsing (#99)

.github/workflows/ci.yml

@@ -44,10 +44,10 @@ jobs:
     name: Windows
     runs-on: windows-latest
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
 
     - name: Install Node
-      uses: actions/setup-node@v3
+      uses: actions/setup-node@v4
       with:
         node-version-file: '.nvmrc'
 

.nvmrc

@@ -1 +1 @@
-lts/gallium
+lts/iron

CHANGELOG.md

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Fixed
+- Improved the argument parsing for the PHPCS executable options.
 
 ## [3.0.1] - 2024-01-29
 ### Fixed

package.json

@@ -164,7 +164,8 @@
     ]
   },
   "dependencies": {
-    "minimatch": "^5.1.2"
+    "minimatch": "^5.1.2",
+    "split-string": "^6.1.0"
   },
   "devDependencies": {
     "@types/jest": "^29.5.0",
@@ -178,6 +179,7 @@
     "eslint-plugin-prettier": "^4.2.1",
     "jest": "^29.5.0",
     "prettier": "^2.8.7",
+    "rimraf": "^6.0.1",
     "ts-jest": "^29.1.0",
     "ts-loader": "^9.4.2",
     "typescript": "^5.0.3",

src/phpcs-report/__tests__/worker-integration.test.ts

@@ -31,7 +31,9 @@ describe('Worker/WorkerPool Integration', () => {
 		);
 
 		try {
-			child_process.execFileSync(phpcsPath, ['--version']);
+			child_process.execFileSync(phpcsPath, ['--version'], {
+				shell: true,
+			});
 		} catch (e) {
 			throw new Error(
 				'PHPCS could not be found at "' +

src/phpcs-report/__tests__/worker.spec.ts

@@ -31,8 +31,11 @@ describe('Worker', () => {
 		);
 
 		try {
-			child_process.execFileSync(phpcsPath, ['--version']);
+			child_process.execFileSync(phpcsPath, ['--version'], {
+				shell: true,
+			});
 		} catch (e) {
+			console.log(e);
 			throw new Error(
 				'PHPCS could not be found at "' +
 					phpcsPath +
@@ -202,7 +205,7 @@ describe('Worker', () => {
 		expect(onCompletion).toHaveBeenLastCalledWith(worker);
 	});
 
-	it('should support executables with spaces', async () => {
+	it('should support executables with spaces and quotes', async () => {
 		const worker = new Worker();
 
 		const request: Request<ReportType.Diagnostic> = {
@@ -212,7 +215,7 @@ describe('Worker', () => {
 			documentContent: '<?php class Test {}',
 			options: {
 				// Since we use custom reports, adding `-s` for sources won't break anything.
-				executable: phpcsPath + ' -s',
+				executable: '"' + phpcsPath + '" -s',
 				standard: 'PSR12',
 				autoloadPHPCSIntegration: false,
 			},

src/phpcs-report/worker.ts

@@ -1,6 +1,7 @@
 import { ChildProcess, spawn, SpawnOptionsWithoutStdio } from 'child_process';
 import { resolve as resolvePath } from 'path';
 import { CancellationError, CancellationToken, Disposable } from 'vscode';
+import * as splitString from 'split-string';
 import { Request } from './request';
 import { ReportType, Response } from './response';
 import { PHPCS_INTEGRATION_VERSION } from '../services/configuration';
@@ -209,25 +210,51 @@ export class Worker {
 			'1',
 		];
 
-		// Since the executable may also include arguments, we need to break the given option
-		// apart and track the specific process and add the args to the array we will use
-		// to spawn the process. We will break on spaces but also support quoted strings.
-		const executableMatches = request.options.executable.match(
-			/`((?:[^`\\]|\\`)*)`|'((?:[^'\\]|\\')*)'|"((?:[^"\\]|\\")*)"|([^\s"]+)/g
-		);
-		if (!executableMatches) {
+		// We support the use of arguments in the executable option. This allows for
+		// users to build more complex commands such as those that should be ran
+		// in a container or with specific arguments.
+		const supportedQuotes = ["'", '"'];
+		const parsedExecutable = splitString(request.options.executable, {
+			quotes: supportedQuotes,
+			brackets: false,
+			separator: ' ',
+			keep: () => true,
+		});
+		if (!parsedExecutable.length) {
 			throw new Error('No executable was given.');
 		}
 
+		// Trim quotes from the start/end of each argument since Node will
+		// handle quoting any arguments for us when we spawn the process.
+		for (const key in parsedExecutable) {
+			const segment = parsedExecutable[key];
+
+			const first = segment.at(0);
+			if (!first) {
+				continue;
+			}
+
+			if (!supportedQuotes.includes(first)) {
+				continue;
+			}
+
+			// Only remove matching quotes.
+			if (segment.at(-1) !== first) {
+				continue;
+			}
+
+			parsedExecutable[key] = segment.slice(1, -1);
+		}
+
 		// The first segment will always be the executable.
-		const executable = executableMatches.shift();
+		const executable = parsedExecutable.shift();
 		if (!executable) {
 			throw new Error('No executable was given.');
 		}
 
 		// Any remaining matches will be arguments that we pass to the executable.
 		// Make sure to add them to the front so it runs PHPCS correctly.
-		processArguments.unshift(...executableMatches);
+		processArguments.unshift(...parsedExecutable);
 
 		// Only set the standard when the user has selected one.
 		if (request.options.standard) {
@@ -247,6 +274,8 @@ export class Worker {
 				}),
 			},
 			windowsHide: true,
+			// Node requires that the shell option be used on Windows to execute batch scripts.
+			shell: process.platform === 'win32' ? true : false,
 		};
 
 		// Make sure PHPCS knows to read from STDIN.

tsconfig.json

@@ -2,9 +2,9 @@
 	"compilerOptions": {
         "rootDir": "src",
 		"outDir": "dist",
-		"target": "es2017",
+		"target": "es2022",
 		"module": "commonjs",
-		"lib": [ "ES2017" ],
+		"lib": [ "ES2022" ],
 		"sourceMap": true,
 		"noImplicitAny": true,
 		"noImplicitReturns": true,

types/split-string.d.ts

@@ -0,0 +1,48 @@
+// Taken from: https://github.com/jonschlinkert/split-string/pull/14
+
+declare module 'split-string' {
+	function split(
+		input: string,
+		options?: split.Options | split.SplitFunc
+	): string[];
+	function split(
+		input: string,
+		options: split.Options,
+		fn: split.SplitFunc
+	): string[];
+
+	namespace split {
+		interface ASTNode {
+			type: 'root' | 'bracket';
+			nodes: ASTNode[];
+			stash: string[];
+		}
+
+		interface State {
+			input: string;
+			separator: string;
+			stack: ASTNode[];
+
+			bos(): boolean;
+
+			eos(): boolean;
+
+			prev(): string;
+
+			next(): string;
+		}
+
+		interface Options {
+			brackets?: { [key: string]: string } | boolean;
+			quotes?: string[] | boolean;
+			separator?: string;
+			strict?: boolean;
+
+			keep?(value: string, state: State): boolean;
+		}
+
+		type SplitFunc = (state: State) => boolean;
+	}
+
+	export = split;
+}