Auto merge of #6259 - ordovicia:user-alias-override, r=ehuss

Allow user aliases to override built-in aliases

This PR allows user-defined aliases take precedence over built-in ones, with a warning that tells there exists a built-in alias.
This PR does not allow user aliases override built-in subcommands.

```console
$ cat .cargo/config
[alias]
b = "fetch"
build = "fetch"

$ ./target/debug/cargo b
warning: user-defined alias `b` overrides a built-in alias for `build`

$ ./target/debug/cargo build
warning: user-defined alias `build` is ignored, because it is shadowed by a built-in command
   Compiling proc-macro2 v0.4.19
   Compiling unicode-xid v0.1.0
   Compiling cc v1.0.25
(snip)
```

In the current version of Cargo, user aliases cannot override built-in aliases.
This behavior is keeping us from safely adding new built-in aliases without interfering existing user config.
Merging this PR will allow that.

Fixes #6221
Relating to #6218

src/bin/cargo/cli.rs

@@ -4,8 +4,8 @@ use clap::{AppSettings, Arg, ArgMatches};
 
 use cargo::{self, CliResult, Config};
 
+use super::commands::{self, BuiltinExec};
 use super::list_commands;
-use super::commands;
 use command_prelude::*;
 
 pub fn main(config: &mut Config) -> CliResult {
@@ -107,26 +107,34 @@ fn expand_aliases(
             commands::builtin_exec(cmd),
             super::aliased_command(config, cmd)?,
         ) {
-            (None, Some(mut alias)) => {
-                alias.extend(
+            (
+                Some(BuiltinExec {
+                    alias_for: None, ..
+                }),
+                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(
                     args.values_of("")
                         .unwrap_or_default()
                         .map(|s| s.to_string()),
                 );
                 let args = cli()
                     .setting(AppSettings::NoBinaryName)
-                    .get_matches_from_safe(alias)?;
+                    .get_matches_from_safe(user_alias)?;
                 return expand_aliases(config, args);
             }
-            (Some(_), Some(_)) => {
-                config.shell().warn(format!(
-                    "alias `{}` is ignored, because it is shadowed by a built in command",
-                    cmd
-                ))?;
-            }
             (_, None) => {}
         }
     };
+
     Ok(args)
 }
 
@@ -152,11 +160,12 @@ fn execute_subcommand(config: &mut Config, args: &ArgMatches) -> CliResult {
         args.is_present("frozen"),
         args.is_present("locked"),
         arg_target_dir,
-        &args.values_of_lossy("unstable-features")
+        &args
+            .values_of_lossy("unstable-features")
             .unwrap_or_default(),
     )?;
 
-    if let Some(exec) = commands::builtin_exec(cmd) {
+    if let Some(BuiltinExec { exec, .. }) = commands::builtin_exec(cmd) {
         return exec(config, subcommand_args);
     }
 

src/bin/cargo/commands/build.rs

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

src/bin/cargo/commands/mod.rs

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

src/bin/cargo/commands/run.rs

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

src/bin/cargo/commands/test.rs

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

tests/testsuite/cargo_alias_config.rs

@@ -22,24 +22,6 @@ expected a list, but found a integer for [..]",
         ).run();
 }
 
-#[test]
-fn alias_default_config_overrides_config() {
-    let p = project()
-        .file("Cargo.toml", &basic_bin_manifest("foo"))
-        .file("src/main.rs", "fn main() {}")
-        .file(
-            ".cargo/config",
-            r#"
-            [alias]
-            b = "not_build"
-        "#,
-        ).build();
-
-    p.cargo("b -v")
-        .with_stderr_contains("[COMPILING] foo v0.5.0 [..]")
-        .run();
-}
-
 #[test]
 fn alias_config() {
     let p = project()
@@ -122,7 +104,7 @@ fn alias_with_flags_config() {
 }
 
 #[test]
-fn cant_shadow_builtin() {
+fn alias_cannot_shadow_builtin_command() {
     let p = project()
         .file("Cargo.toml", &basic_bin_manifest("foo"))
         .file("src/main.rs", "fn main() {}")
@@ -137,9 +119,32 @@ fn cant_shadow_builtin() {
     p.cargo("build")
         .with_stderr(
             "\
-[WARNING] alias `build` is ignored, because it is shadowed by a built in command
+[WARNING] user-defined alias `build` is ignored, because it is shadowed by a built-in command
+[COMPILING] foo v0.5.0 ([..])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        ).run();
+}
+
+#[test]
+fn alias_override_builtin_alias() {
+    let p = project()
+        .file("Cargo.toml", &basic_bin_manifest("foo"))
+        .file("src/main.rs", "fn main() {}")
+        .file(
+            ".cargo/config",
+            r#"
+            [alias]
+            b = "run"
+         "#,
+        ).build();
+
+    p.cargo("b")
+        .with_stderr(
+            "\
 [COMPILING] foo v0.5.0 ([..])
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+[RUNNING] `target/debug/foo[EXE]`
 ",
         ).run();
 }