Auto merge of #6385 - ehuss:beta-fix-builtin-alias, r=alexcrichton

[BETA] Fix built-in aliases taking arguments.

The built-in aliases weren't parsing their arguments. Change implementation to treat built-in aliases the same as user aliases.

Fixes #6381

src/bin/cargo/cli.rs

@@ -4,7 +4,7 @@ use clap::{AppSettings, Arg, ArgMatches};
 
 use cargo::{self, CliResult, Config};
 
-use super::commands::{self, BuiltinExec};
+use super::commands;
 use super::list_commands;
 use command_prelude::*;
 
@@ -107,28 +107,22 @@ fn expand_aliases(
             commands::builtin_exec(cmd),
             super::aliased_command(config, cmd)?,
         ) {
-            (
-                Some(BuiltinExec {
-                    alias_for: None, ..
-                }),
-                Some(_),
-            ) => {
+            (Some(_), Some(_)) => {
                 // User alias conflicts with a built-in subcommand
                 config.shell().warn(format!(
                     "user-defined alias `{}` is ignored, because it is shadowed by a built-in command",
                     cmd,
                 ))?;
             }
-            (_, Some(mut user_alias)) => {
-                // User alias takes precedence over built-in aliases
-                user_alias.extend(
+            (_, Some(mut alias)) => {
+                alias.extend(
                     args.values_of("")
                         .unwrap_or_default()
                         .map(|s| s.to_string()),
                 );
                 let args = cli()
                     .setting(AppSettings::NoBinaryName)
-                    .get_matches_from_safe(user_alias)?;
+                    .get_matches_from_safe(alias)?;
                 return expand_aliases(config, args);
             }
             (_, None) => {}
@@ -165,7 +159,7 @@ fn execute_subcommand(config: &mut Config, args: &ArgMatches) -> CliResult {
             .unwrap_or_default(),
     )?;
 
-    if let Some(BuiltinExec { exec, .. }) = commands::builtin_exec(cmd) {
+    if let Some(exec) = commands::builtin_exec(cmd) {
         return exec(config, subcommand_args);
     }
 

src/bin/cargo/commands/build.rs

@@ -4,7 +4,7 @@ use cargo::ops;
 
 pub fn cli() -> App {
     subcommand("build")
-        // subcommand aliases are handled in commands::builtin_exec() and cli::expand_aliases()
+        // subcommand aliases are handled in aliased_command()
         // .alias("b")
         .about("Compile a local package and all of its dependencies")
         .arg_package_spec(

src/bin/cargo/commands/check.rs

@@ -4,7 +4,7 @@ use cargo::ops;
 
 pub fn cli() -> App {
     subcommand("check")
-        // subcommand aliases are handled in commands::builtin_exec() and cli::expand_aliases()
+        // subcommand aliases are handled in aliased_command()
         // .alias("c")
         .about("Check a local package and all of its dependencies for errors")
         .arg_package_spec(

src/bin/cargo/commands/mod.rs

@@ -35,16 +35,11 @@ pub fn builtin() -> Vec<App> {
     ]
 }
 
-pub struct BuiltinExec<'a> {
-    pub exec: fn(&'a mut Config, &'a ArgMatches) -> CliResult,
-    pub alias_for: Option<&'static str>,
-}
-
-pub fn builtin_exec(cmd: &str) -> Option<BuiltinExec> {
-    let exec = match cmd {
+ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResult> {
+     let f = match cmd {
         "bench" => bench::exec,
-        "build" | "b" => build::exec,
-        "check" | "c" => check::exec,
+        "build" => build::exec,
+        "check" => check::exec,
         "clean" => clean::exec,
         "doc" => doc::exec,
         "fetch" => fetch::exec,
@@ -62,28 +57,19 @@ pub fn builtin_exec(cmd: &str) -> Option<BuiltinExec> {
         "pkgid" => pkgid::exec,
         "publish" => publish::exec,
         "read-manifest" => read_manifest::exec,
-        "run" | "r" => run::exec,
+        "run" => run::exec,
         "rustc" => rustc::exec,
         "rustdoc" => rustdoc::exec,
         "search" => search::exec,
-        "test" | "t" => test::exec,
+        "test" => test::exec,
         "uninstall" => uninstall::exec,
         "update" => update::exec,
         "verify-project" => verify_project::exec,
         "version" => version::exec,
         "yank" => yank::exec,
         _ => return None,
     };
-
-    let alias_for = match cmd {
-        "b" => Some("build"),
-        "c" => Some("check"),
-        "r" => Some("run"),
-        "t" => Some("test"),
-        _ => None,
-    };
-
-    Some(BuiltinExec { exec, alias_for })
+    Some(f)
 }
 
 pub mod bench;

src/bin/cargo/commands/run.rs

@@ -5,7 +5,7 @@ use cargo::ops::{self, CompileFilter};
 
 pub fn cli() -> App {
     subcommand("run")
-        // subcommand aliases are handled in commands::builtin_exec() and cli::expand_aliases()
+        // subcommand aliases are handled in aliased_command()
         // .alias("r")
         .setting(AppSettings::TrailingVarArg)
         .about("Run the main binary of the local package (src/main.rs)")

src/bin/cargo/commands/test.rs

@@ -4,7 +4,7 @@ use cargo::ops::{self, CompileFilter};
 
 pub fn cli() -> App {
     subcommand("test")
-        // subcommand aliases are handled in commands::builtin_exec() and cli::expand_aliases()
+        // subcommand aliases are handled in aliased_command()
         // .alias("t")
         .setting(AppSettings::TrailingVarArg)
         .about("Execute all unit and integration tests of a local package")

src/bin/cargo/main.rs

@@ -63,28 +63,27 @@ fn main() {
 
 fn aliased_command(config: &Config, command: &str) -> CargoResult<Option<Vec<String>>> {
     let alias_name = format!("alias.{}", command);
-    let mut result = Ok(None);
-    match config.get_string(&alias_name) {
-        Ok(value) => {
-            if let Some(record) = value {
-                let alias_commands = record
-                    .val
-                    .split_whitespace()
-                    .map(|s| s.to_string())
-                    .collect();
-                result = Ok(Some(alias_commands));
-            }
-        }
-        Err(_) => {
-            let value = config.get_list(&alias_name)?;
-            if let Some(record) = value {
-                let alias_commands: Vec<String> =
-                    record.val.iter().map(|s| s.0.to_string()).collect();
-                result = Ok(Some(alias_commands));
-            }
-        }
-    }
-    result
+    let user_alias = match config.get_string(&alias_name) {
+        Ok(Some(record)) => Some(
+            record
+                .val
+                .split_whitespace()
+                .map(|s| s.to_string())
+                .collect(),
+        ),
+        Ok(None) => None,
+        Err(_) => config
+            .get_list(&alias_name)?
+            .map(|record| record.val.iter().map(|s| s.0.to_string()).collect()),
+    };
+    let result = user_alias.or_else(|| match command {
+        "b" => Some(vec!["build".to_string()]),
+        "c" => Some(vec!["check".to_string()]),
+        "r" => Some(vec!["run".to_string()]),
+        "t" => Some(vec!["test".to_string()]),
+        _ => None,
+    });
+    Ok(result)
 }
 
 /// List all runnable commands

tests/testsuite/cargo_alias_config.rs

@@ -148,3 +148,17 @@ fn alias_override_builtin_alias() {
 ",
         ).run();
 }
+
+#[test]
+fn builtin_alias_takes_options() {
+    // #6381
+    let p = project()
+        .file("src/lib.rs", "")
+        .file(
+            "examples/ex1.rs",
+            r#"fn main() { println!("{}", std::env::args().skip(1).next().unwrap()) }"#,
+        )
+        .build();
+
+    p.cargo("r --example ex1 -- asdf").with_stdout("asdf").run();
+}