Reworked run Command Parsing

.github/workflows/unit-test.yml

@@ -171,12 +171,12 @@ jobs:
 
             - name: Docker container debug information
               run: |
-                  npm run wp-env run tests-mysql "mysql --version"
-                  npm run wp-env run tests-wordpress "php --version"
-                  npm run wp-env run tests-wordpress "php -m"
-                  npm run wp-env run tests-wordpress "php -i"
-                  npm run wp-env run tests-wordpress "/var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit --version"
-                  npm run wp-env run tests-wordpress "locale -a"
+                  npm run wp-env run tests-mysql mysql -- --version
+                  npm run wp-env run tests-wordpress php -- --version
+                  npm run wp-env run tests-wordpress php -m
+                  npm run wp-env run tests-wordpress php -i
+                  npm run wp-env run tests-wordpress /var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit -- --version
+                  npm run wp-env run tests-wordpress locale -a
 
             - name: Running single site unit tests
               if: ${{ ! matrix.multisite }}

package.json

@@ -272,7 +272,7 @@
 		"fixtures:generate": "cross-env GENERATE_MISSING_FIXTURES=y npm run test:unit test/integration/full-content/ && npm run format test/integration/fixtures/blocks/*.json",
 		"fixtures:regenerate": "npm-run-all fixtures:clean fixtures:generate",
 		"format": "wp-scripts format",
-		"format:php": "wp-env run --env-cwd=\"wp-content/plugins/gutenberg\" cli composer run-script format",
+		"format:php": "wp-env run --env-cwd='wp-content/plugins/gutenberg' cli composer run-script format",
 		"lint": "concurrently \"npm run lint:lockfile\" \"npm run lint:tsconfig\" \"npm run lint:js\" \"npm run lint:pkg-json\" \"npm run lint:css\"",
 		"lint:css": "wp-scripts lint-style \"**/*.scss\"",
 		"lint:css:fix": "npm run lint:css -- --fix",
@@ -282,7 +282,7 @@
 		"lint:tsconfig": "node ./bin/validate-tsconfig.mjs",
 		"lint:md:docs": "wp-scripts lint-md-docs",
 		"prelint:php": "wp-env run --env-cwd='wp-content/plugins/gutenberg' cli composer update --no-interaction",
-		"lint:php": "wp-env run --env-cwd=\"wp-content/plugins/gutenberg\" cli composer run-script lint",
+		"lint:php": "wp-env run --env-cwd='wp-content/plugins/gutenberg' cli composer run-script lint",
 		"lint:pkg-json": "wp-scripts lint-pkg-json . 'packages/*/package.json'",
 		"native": "npm run --prefix packages/react-native-editor",
 		"other:changelog": "node ./bin/plugin/cli.js changelog",
@@ -312,15 +312,15 @@
 		"test:performance": "wp-scripts test-e2e --config packages/e2e-tests/jest.performance.config.js",
 		"test:performance:debug": "wp-scripts --inspect-brk test-e2e --runInBand --no-cache --verbose --config packages/e2e-tests/jest.performance.config.js --puppeteer-devtools",
 		"test:php": "npm-run-all lint:php test:unit:php",
-		"test:php:watch": "wp-env run composer run-script test:watch",
+		"test:php:watch": "wp-env run --env-cwd='wp-content/plugins/gutenberg' tests-cli composer run-script test:watch",
 		"test:unit": "wp-scripts test-unit-js --config test/unit/jest.config.js",
 		"test:unit:date": "bash ./bin/unit-test-date.sh",
 		"test:unit:debug": "wp-scripts --inspect-brk test-unit-js --runInBand --no-cache --verbose --config test/unit/jest.config.js ",
 		"test:unit:profile": "wp-scripts --cpu-prof test-unit-js --runInBand --no-cache --verbose --config test/unit/jest.config.js ",
 		"pretest:unit:php": "wp-env start",
-		"test:unit:php": "wp-env run --env-cwd=\"wp-content/plugins/gutenberg\" tests-wordpress vendor/bin/phpunit -c phpunit.xml.dist --verbose",
+		"test:unit:php": "wp-env run --env-cwd='wp-content/plugins/gutenberg' tests-wordpress vendor/bin/phpunit -c phpunit.xml.dist --verbose",
 		"pretest:unit:php:multisite": "wp-env start",
-		"test:unit:php:multisite": "wp-env run --env-cwd=\"wp-content/plugins/gutenberg\" tests-wordpress vendor/bin/phpunit -c phpunit/multisite.xml --verbose",
+		"test:unit:php:multisite": "wp-env run --env-cwd='wp-content/plugins/gutenberg' tests-wordpress vendor/bin/phpunit -c phpunit/multisite.xml --verbose",
 		"test:unit:update": "npm run test:unit -- --updateSnapshot",
 		"test:unit:watch": "npm run test:unit -- --watch",
 		"wp-env": "wp-env"

packages/env/CHANGELOG.md

@@ -2,8 +2,13 @@
 
 ## Unreleased
 
+### Breaking Change
+
+-   Rework`run` command to resolve bugs with non-quoted commands. As a consequence it is no longer possible to pass your entire command to `wp-env` wrapped in double-quotes. While `npx wp-env run cli wp help` will still work, `npx wp-env run cli "wp help"` will not. If you are currently escaping any quotes you will need to review those commands and ensure they are compatible with this update.
+
 ### Enhancement
 
+-   Support using double dashes in `wp-env run ...` to pass arguments that would otherwise be consumed by `wp-env`. For example, while normally `--help` would provide the `wp-env` help text, if you use `npx wp-env run cli php -- --help` you will see the PHP help text.
 -   Validate whether or not config options exist to prevent accidentally including ones that don't.
 
 ## 7.0.0 (2023-05-10)

packages/env/README.md

@@ -319,40 +319,38 @@ Options:
   --scripts  Execute any configured lifecycle scripts. [boolean] [default: true]
 ```
 
-### `wp-env run [container] [command]`
+### `wp-env run <container> [command...]`
 
-The run command can be used to open shell sessions or invoke WP-CLI commands.
+The run command can be used to open shell sessions, invoke WP-CLI commands, or run any arbitrary commands inside of a container.
 
-<div class="callout callout-alert">In some cases, <code>wp-env</code> may consume options that you are attempting to pass to
-the container. This happens with options that <code>wp-env</code> has already declared,
-such as <code>--env-cwd</code>, <code>--debug</code>, <code>--help</code>, and <code>--version</code>. When this happens, you should fall
-back to using quotation marks; <code>wp-env</code> considers everything inside the
-quotation marks to be command argument.
-
-For example, to ask <code>WP-CLI</code> for its help text:
-
-<pre>sh
-<code class="language-sh">wp-env run cli "wp --help"</code></pre>
-
-Without the quotation marks, <code>wp-env</code> will print its own help text instead of
-passing it to the container. If you experience any problems where the command
-is not being passed correctly, fall back to using quotation marks.
+<div class="callout callout-alert">
+<p>
+In some cases <code class="language-sh">wp-env run</code> may conflict with options that you are passing to the container.
+When this happens, <code class="language-sh">wp-env</code> will treat the option as its own and take action accordingly.
+For example, if you try <code class="language-sh">wp-env run cli php --help</code>, you will receive the <code class="language-sh">wp-env</code> help text.
+</p>
 
+<p>
+You can get around this by passing any conflicting options after a double dash. <code class="language-sh">wp-env</code> will not process anything after
+the double dash and will simply pass it on to the container. To get the PHP help text you would use <code class="language-sh">wp-env run cli php -- --help</code>.
+</p>
 </div>
 
 ```sh
-wp-env run <container> [command..]
+wp-env run <container> [command...]
 
-Runs an arbitrary command in one of the underlying Docker containers. The
-"container" param should reference one of the underlying Docker services like
-"development", "tests", or "cli". To run a wp-cli command, use the "cli" or
-"tests-cli" service. You can also use this command to open shell sessions like
-bash and the WordPress shell in the WordPress instance. For example, `wp-env run
-cli bash` will open bash in the development WordPress instance.
+Runs an arbitrary command in one of the underlying Docker containers. A double
+dash can be used to pass arguments to the container without parsing them. This
+is necessary if you are using an option that is defined below. You can use
+`bash` to open a shell session and both `composer` and `phpunit` are available
+in all WordPress and CLI containers. WP-CLI is also available in the CLI
+containers.
 
 Positionals:
-  container  The container to run the command on.            [string] [required]
-  command    The command to run.                           [array] [default: []]
+  container  The Docker service to run the command on.
+              [string] [required] [choices: "mysql", "tests-mysql", "wordpress",
+                   "tests-wordpress", "cli", "tests-cli", "composer", "phpunit"]
+  command    The command to run.                                      [required]
 
 Options:
   --debug    Enable debug output.                     [boolean] [default: false]

packages/env/lib/cli.js

@@ -13,6 +13,10 @@ const { execSync } = require( 'child_process' );
  */
 const env = require( './env' );
 const parseXdebugMode = require( './parse-xdebug-mode' );
+const {
+	RUN_CONTAINERS,
+	validateRunContainer,
+} = require( './validate-run-container' );
 
 // Colors.
 const boldWhite = chalk.bold.white;
@@ -99,9 +103,12 @@ module.exports = function cli() {
 		default: false,
 	} );
 
-	// Make sure any unknown arguments are passed to the command as arguments.
-	// This allows options to be passed to "run" without being quoted.
-	yargs.parserConfiguration( { 'unknown-options-as-args': true } );
+	yargs.parserConfiguration( {
+		// Treats unknown options as arguments for commands to deal with instead of discarding them.
+		'unknown-options-as-args': true,
+		// Populates '--' in the command options with arguments after the double dash.
+		'populate--': true,
+	} );
 
 	yargs.command(
 		'start',
@@ -184,8 +191,8 @@ module.exports = function cli() {
 		'Displays the latest logs for the e2e test environment without watching.'
 	);
 	yargs.command(
-		'run <container> [command..]',
-		'Runs an arbitrary command in one of the underlying Docker containers. The "container" param should reference one of the underlying Docker services like "development", "tests", or "cli". To run a wp-cli command, use the "cli" or "tests-cli" service. You can also use this command to open shell sessions like bash and the WordPress shell in the WordPress instance. For example, `wp-env run cli bash` will open bash in the development WordPress instance. When using long commands with arguments and quotation marks, you need to wrap the "command" param in quotation marks. For example: `wp-env run tests-cli "wp post create --post_type=page --post_title=\'Test\'"` will create a post on the tests WordPress instance.',
+		'run <container> [command...]',
+		'Runs an arbitrary command in one of the underlying Docker containers. A double dash can be used to pass arguments to the container without parsing them. This is necessary if you are using an option that is defined below. You can use `bash` to open a shell session and both `composer` and `phpunit` are available in all WordPress and CLI containers. WP-CLI is also available in the CLI containers.',
 		( args ) => {
 			args.option( 'env-cwd', {
 				type: 'string',
@@ -196,7 +203,10 @@ module.exports = function cli() {
 			} );
 			args.positional( 'container', {
 				type: 'string',
-				describe: 'The container to run the command on.',
+				describe:
+					'The underlying Docker service to run the command on.',
+				choices: RUN_CONTAINERS,
+				coerce: validateRunContainer,
 			} );
 			args.positional( 'command', {
 				type: 'array',

packages/env/lib/commands/run.js

@@ -10,7 +10,6 @@ const path = require( 'path' );
  */
 const initConfig = require( '../init-config' );
 const getHostUser = require( '../get-host-user' );
-const { ValidationError } = require( '../config' );
 
 /**
  * @typedef {import('../config').WPConfig} WPConfig
@@ -22,73 +21,42 @@ const { ValidationError } = require( '../config' );
  * @param {Object}   options
  * @param {string}   options.container The Docker container to run the command on.
  * @param {string[]} options.command   The command to run.
+ * @param {string[]} options.'--'      Any arguments that were passed after a double dash.
  * @param {string}   options.envCwd    The working directory for the command to be executed from.
  * @param {Object}   options.spinner   A CLI spinner which indicates progress.
  * @param {boolean}  options.debug     True if debug mode is enabled.
  */
 module.exports = async function run( {
 	container,
 	command,
+	'--': doubleDashedArgs,
 	envCwd,
 	spinner,
 	debug,
 } ) {
-	validateContainerExistence( container );
-
 	const config = await initConfig( { spinner, debug } );
 
-	command = command.join( ' ' );
+	// Include any double dashed arguments in the command so that we can pass them to Docker.
+	// This lets users pass options that the command defines without them being parsed.
+	if ( Array.isArray( doubleDashedArgs ) ) {
+		command.push( ...doubleDashedArgs );
+	}
 
 	// Shows a contextual tip for the given command.
-	showCommandTips( command, container, spinner );
+	const joinedCommand = command.join( ' ' );
+	showCommandTips( joinedCommand, container, spinner );
 
 	await spawnCommandDirectly( config, container, command, envCwd, spinner );
 
-	spinner.text = `Ran \`${ command }\` in '${ container }'.`;
+	spinner.text = `Ran \`${ joinedCommand }\` in '${ container }'.`;
 };
 
-/**
- * Validates the container option and throws if it is invalid.
- *
- * @param {string} container The Docker container to run the command on.
- */
-function validateContainerExistence( container ) {
-	// Give better errors for containers that we have removed.
-	if ( container === 'phpunit' ) {
-		throw new ValidationError(
-			"The 'phpunit' container has been removed. Please use 'wp-env run tests-cli --env-cwd=wp-content/path/to/plugin phpunit' instead."
-		);
-	}
-	if ( container === 'composer' ) {
-		throw new ValidationError(
-			"The 'composer' container has been removed. Please use 'wp-env run cli --env-cwd=wp-content/path/to/plugin composer' instead."
-		);
-	}
-
-	// Provide better error output than Docker's "service does not exist" messaging.
-	const validContainers = [
-		'mysql',
-		'tests-mysql',
-		'wordpress',
-		'tests-wordpress',
-		'cli',
-		'tests-cli',
-	];
-	if ( ! validContainers.includes( container ) ) {
-		throw new ValidationError(
-			`The '${ container }' container does not exist. Valid selections are: ${ validContainers.join(
-				', '
-			) }`
-		);
-	}
-}
-
 /**
  * Runs an arbitrary command on the given Docker container.
  *
  * @param {WPConfig} config    The wp-env configuration.
  * @param {string}   container The Docker container to run the command on.
- * @param {string}   command   The command to run.
+ * @param {string[]} command   The command to run.
  * @param {string}   envCwd    The working directory for the command to be executed from.
  * @param {Object}   spinner   A CLI spinner which indicates progress.
  */
@@ -108,31 +76,30 @@ function spawnCommandDirectly( config, container, command, envCwd, spinner ) {
 		envCwd
 	);
 
-	const isTTY = process.stdout.isTTY;
 	const composeCommand = [
 		'-f',
 		config.dockerComposeConfigPath,
 		'exec',
-		! isTTY ? '-T' : '',
 		'-w',
 		envCwd,
 		'--user',
 		hostUser.fullUser,
-		container,
-		...command.split( ' ' ), // The command will fail if passed as a complete string.
 	];
 
+	if ( ! process.stdout.isTTY ) {
+		composeCommand.push( '-T' );
+	}
+
+	composeCommand.push( container, ...command );
+
 	return new Promise( ( resolve, reject ) => {
 		// Note: since the npm docker-compose package uses the -T option, we
 		// cannot use it to spawn an interactive command. Thus, we run docker-
 		// compose on the CLI directly.
 		const childProc = spawn(
 			'docker-compose',
 			composeCommand,
-			{
-				stdio: 'inherit',
-				shell: true,
-			},
+			{ stdio: 'inherit' },
 			spinner
 		);
 		childProc.on( 'error', reject );
@@ -153,17 +120,17 @@ function spawnCommandDirectly( config, container, command, envCwd, spinner ) {
  * bash) may have weird behavior (exit with ctrl-d instead of ctrl-c or ctrl-z),
  * so we want the user to have that information without having to ask someone.
  *
- * @param {string} command   The command for which to show a tip.
- * @param {string} container The container the command will be run on.
- * @param {Object} spinner   A spinner object to show progress.
+ * @param {string} joinedCommand The command for which to show a tip joined by spaces.
+ * @param {string} container     The container the command will be run on.
+ * @param {Object} spinner       A spinner object to show progress.
  */
-function showCommandTips( command, container, spinner ) {
-	if ( ! command.length ) {
+function showCommandTips( joinedCommand, container, spinner ) {
+	if ( ! joinedCommand.length ) {
 		return;
 	}
 
-	const tip = `Starting '${ command }' on the ${ container } container. ${ ( () => {
-		switch ( command ) {
+	const tip = `Starting '${ joinedCommand }' on the ${ container } container. ${ ( () => {
+		switch ( joinedCommand ) {
 			case 'bash':
 				return 'Exit bash with ctrl-d.';
 			case 'wp shell':

packages/env/lib/validate-run-container.js

@@ -0,0 +1,41 @@
+'use strict';
+
+/**
+ * A list of the containers that we can use `run` on.
+ */
+const RUN_CONTAINERS = [
+	'mysql',
+	'tests-mysql',
+	'wordpress',
+	'tests-wordpress',
+	'cli',
+	'tests-cli',
+];
+
+/**
+ * Custom parsing and validation for the "run" command's container argument.
+ *
+ * @param {string} value The user-set container.
+ *
+ * @return {string} The container name to use.
+ */
+function validateRunContainer( value ) {
+	// Give special errors for deprecated containers.
+	if ( value === 'phpunit' ) {
+		throw new Error(
+			"The 'phpunit' container has been removed. Please use 'wp-env run tests-cli --env-cwd=wp-content/path/to/plugin phpunit' instead."
+		);
+	}
+	if ( value === 'composer' ) {
+		throw new Error(
+			"The 'composer' container has been removed. Please use 'wp-env run cli --env-cwd=wp-content/path/to/plugin composer' instead."
+		);
+	}
+
+	return value;
+}
+
+module.exports = {
+	RUN_CONTAINERS,
+	validateRunContainer,
+};