Fix: Prevents interpreting run options as yarn options (#4152)

* Prevents interpreting run options as yarn options

* Implements a deprecation warning

* Adds tests

* Feedback

* Wording

* Enables a test on Linux

* Disables the tests again on Windows

Author: arcanis

__tests__/integration.js

@@ -73,6 +73,61 @@ test('--mutex network', async () => {
   ]);
 });
 
+// Windows could run these tests, but we currently suffer from an escaping issue that breaks them (#4135)
+if (process.platform !== 'win32') {
+  test('yarn run <script> --opt', async () => {
+    const cwd = await makeTemp();
+
+    await fs.writeFile(
+      path.join(cwd, 'package.json'),
+      JSON.stringify({
+        scripts: {echo: `echo`},
+      }),
+    );
+
+    const command = path.resolve(__dirname, '../bin/yarn');
+    const options = {cwd, env: {YARN_SILENT: 1}};
+
+    const {stderr: stderr, stdout: stdout} = execa(command, ['run', 'echo', '--opt'], options);
+
+    const stdoutPromise = misc.consumeStream(stdout);
+    const stderrPromise = misc.consumeStream(stderr);
+
+    const [stdoutOutput, stderrOutput] = await Promise.all([stdoutPromise, stderrPromise]);
+
+    expect(stdoutOutput.toString().trim()).toEqual('--opt');
+    expect(stderrOutput.toString()).not.toMatch(
+      /From Yarn 1\.0 onwards, scripts don't require "--" for options to be forwarded/,
+    );
+  });
+
+  test('yarn run <script> -- --opt', async () => {
+    const cwd = await makeTemp();
+
+    await fs.writeFile(
+      path.join(cwd, 'package.json'),
+      JSON.stringify({
+        scripts: {echo: `echo`},
+      }),
+    );
+
+    const command = path.resolve(__dirname, '../bin/yarn');
+    const options = {cwd, env: {YARN_SILENT: 1}};
+
+    const {stderr: stderr, stdout: stdout} = execa(command, ['run', 'echo', '--', '--opt'], options);
+
+    const stdoutPromise = misc.consumeStream(stdout);
+    const stderrPromise = misc.consumeStream(stderr);
+
+    const [stdoutOutput, stderrOutput] = await Promise.all([stdoutPromise, stderrPromise]);
+
+    expect(stdoutOutput.toString().trim()).toEqual('--opt');
+    expect(stderrOutput.toString()).toMatch(
+      /From Yarn 1\.0 onwards, scripts don't require "--" for options to be forwarded/,
+    );
+  });
+}
+
 test('cache folder fallback', async () => {
   const cwd = await makeTemp();
   const cacheFolder = path.join(cwd, '.cache');

src/cli/index.js

@@ -110,6 +110,16 @@ export function main({
     command = commands.run;
   }
 
+  let warnAboutRunDashDash = false;
+  // we are using "yarn <script> -abc" or "yarn run <script> -abc", we want -abc to be script options, not yarn options
+  if (command === commands.run) {
+    if (endArgs.length === 0) {
+      endArgs = ['--', ...args.splice(1)];
+    } else {
+      warnAboutRunDashDash = true;
+    }
+  }
+
   command.setFlags(commander);
   commander.parse([
     ...startArgs,
@@ -119,7 +129,7 @@ export function main({
     ...getRcArgs(commandName, args),
     ...args,
   ]);
-  commander.args = commander.args.concat(endArgs);
+  commander.args = commander.args.concat(endArgs.slice(1));
 
   // we strip cmd
   console.assert(commander.args.length >= 1);
@@ -171,6 +181,11 @@ export function main({
   //
   const run = (): Promise<void> => {
     invariant(command, 'missing command');
+
+    if (warnAboutRunDashDash) {
+      reporter.warn(reporter.lang('dashDashDeprecation'));
+    }
+
     return command.run(config, reporter, commander, commander.args).then(exitCode => {
       if (outputWrapper) {
         reporter.footer(false);
@@ -403,7 +418,7 @@ export default function start() {
   const doubleDashIndex = process.argv.findIndex(element => element === '--');
   const startArgs = process.argv.slice(0, 2);
   const args = process.argv.slice(2, doubleDashIndex === -1 ? process.argv.length : doubleDashIndex);
-  const endArgs = doubleDashIndex === -1 ? [] : process.argv.slice(doubleDashIndex + 1, process.argv.length);
+  const endArgs = doubleDashIndex === -1 ? [] : process.argv.slice(doubleDashIndex);
 
   main({startArgs, args, endArgs});
 }

src/reporters/lang/en.js

@@ -188,6 +188,7 @@ const messages = {
 
   execMissingCommand: 'Missing command name.',
 
+  dashDashDeprecation: `From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.`,
   commandNotSpecified: 'No command specified.',
   binCommands: 'Commands available from binary scripts: ',
   possibleCommands: 'Project commands',