Auto merge of #7056 - ehuss:default-run-stabilize, r=alexcrichton

Stabilize default-run

This stabilizes the default-run feature.

I've included some error message changes. If `default-run` specifies an unknown binary, it now tells you that the `default-run` field is incorrect and which manifest it is located in, instead of just saying "could not determine which binary to run".

I also consolidated some of the suggestion code so it is not repeated all over.

Closes #7032

src/bin/cargo/main.rs

@@ -10,7 +10,7 @@ use std::fs;
 use std::path::{Path, PathBuf};
 
 use cargo::core::shell::Shell;
-use cargo::util::{self, command_prelude, lev_distance, CargoResult, CliResult, Config};
+use cargo::util::{self, closest_msg, command_prelude, CargoResult, CliResult, Config};
 use cargo::util::{CliError, ProcessError};
 
 mod cli;
@@ -113,18 +113,6 @@ fn list_commands(config: &Config) -> BTreeSet<CommandInfo> {
     commands
 }
 
-fn find_closest(config: &Config, cmd: &str) -> Option<String> {
-    let cmds = list_commands(config);
-    // Only consider candidates with a lev_distance of 3 or less so we don't
-    // suggest out-of-the-blue options.
-    cmds.into_iter()
-        .map(|c| c.name())
-        .map(|c| (lev_distance(&c, cmd), c))
-        .filter(|&(d, _)| d < 4)
-        .min_by_key(|a| a.0)
-        .map(|slot| slot.1)
-}
-
 fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> CliResult {
     let command_exe = format!("cargo-{}{}", cmd, env::consts::EXE_SUFFIX);
     let path = search_directories(config)
@@ -134,14 +122,9 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> Cli
     let command = match path {
         Some(command) => command,
         None => {
-            let err = match find_closest(config, cmd) {
-                Some(closest) => failure::format_err!(
-                    "no such subcommand: `{}`\n\n\tDid you mean `{}`?\n",
-                    cmd,
-                    closest
-                ),
-                None => failure::format_err!("no such subcommand: `{}`", cmd),
-            };
+            let cmds = list_commands(config);
+            let did_you_mean = closest_msg(cmd, cmds.iter(), |c| c.name());
+            let err = failure::format_err!("no such subcommand: `{}`{}", cmd, did_you_mean);
             return Err(CliError::new(err, 101));
         }
     };

src/cargo/core/features.rs

@@ -196,7 +196,7 @@ features! {
         [unstable] namespaced_features: bool,
 
         // "default-run" manifest option,
-        [unstable] default_run: bool,
+        [stable] default_run: bool,
 
         // Declarative build scripts.
         [unstable] metabuild: bool,

src/cargo/core/manifest.rs

@@ -518,12 +518,6 @@ impl Manifest {
                 })?;
         }
 
-        if self.default_run.is_some() {
-            self.features
-                .require(Feature::default_run())
-                .chain_err(|| failure::format_err!("the `default-run` manifest key is unstable"))?;
-        }
-
         Ok(())
     }
 

src/cargo/core/package.rs

@@ -25,7 +25,7 @@ use crate::ops;
 use crate::util::config::PackageCacheLock;
 use crate::util::errors::{CargoResult, CargoResultExt, HttpNot200};
 use crate::util::network::Retry;
-use crate::util::{self, internal, lev_distance, Config, Progress, ProgressStyle};
+use crate::util::{self, internal, Config, Progress, ProgressStyle};
 
 /// Information about a package that is available somewhere in the file system.
 ///
@@ -193,21 +193,6 @@ impl Package {
         self.targets().iter().any(|t| t.is_custom_build())
     }
 
-    pub fn find_closest_target(
-        &self,
-        target: &str,
-        is_expected_kind: fn(&Target) -> bool,
-    ) -> Option<&Target> {
-        let targets = self.targets();
-
-        let matches = targets
-            .iter()
-            .filter(|t| is_expected_kind(t))
-            .map(|t| (lev_distance(target, t.name()), t))
-            .filter(|&(d, _)| d < 4);
-        matches.min_by_key(|t| t.0).map(|t| t.1)
-    }
-
     pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Package {
         Package {
             manifest: self.manifest.map_source(to_replace, replace_with),

src/cargo/core/profiles.rs

@@ -7,9 +7,8 @@ use crate::core::compiler::CompileMode;
 use crate::core::interning::InternedString;
 use crate::core::{Features, PackageId, PackageIdSpec, PackageSet, Shell};
 use crate::util::errors::CargoResultExt;
-use crate::util::lev_distance::lev_distance;
 use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool};
-use crate::util::{CargoResult, Config};
+use crate::util::{closest_msg, CargoResult, Config};
 
 /// Collection of all user profiles.
 #[derive(Clone, Debug)]
@@ -290,23 +289,12 @@ impl ProfileMaker {
                 })
                 .collect();
             if name_matches.is_empty() {
-                let suggestion = packages
-                    .package_ids()
-                    .map(|p| (lev_distance(&*spec.name(), &p.name()), p.name()))
-                    .filter(|&(d, _)| d < 4)
-                    .min_by_key(|p| p.0)
-                    .map(|p| p.1);
-                match suggestion {
-                    Some(p) => shell.warn(format!(
-                        "profile override spec `{}` did not match any packages\n\n\
-                         Did you mean `{}`?",
-                        spec, p
-                    ))?,
-                    None => shell.warn(format!(
-                        "profile override spec `{}` did not match any packages",
-                        spec
-                    ))?,
-                }
+                let suggestion =
+                    closest_msg(&spec.name(), packages.package_ids(), |p| p.name().as_str());
+                shell.warn(format!(
+                    "profile override spec `{}` did not match any packages{}",
+                    spec, suggestion
+                ))?;
             } else {
                 shell.warn(format!(
                     "version or URL in profile override spec `{}` does not \

src/cargo/ops/cargo_compile.rs

@@ -35,7 +35,7 @@ use crate::core::{Package, Target};
 use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
 use crate::ops;
 use crate::util::config::Config;
-use crate::util::{lev_distance, profile, CargoResult};
+use crate::util::{closest_msg, profile, CargoResult};
 
 /// Contains information about how a package should be compiled.
 #[derive(Debug)]
@@ -906,26 +906,18 @@ fn find_named_targets<'a>(
     let filter = |t: &Target| t.name() == target_name && is_expected_kind(t);
     let proposals = filter_targets(packages, filter, true, mode);
     if proposals.is_empty() {
-        let suggestion = packages
-            .iter()
-            .flat_map(|pkg| {
-                pkg.targets()
-                    .iter()
-                    .filter(|target| is_expected_kind(target))
-            })
-            .map(|target| (lev_distance(target_name, target.name()), target))
-            .filter(|&(d, _)| d < 4)
-            .min_by_key(|t| t.0)
-            .map(|t| t.1);
-        match suggestion {
-            Some(s) => failure::bail!(
-                "no {} target named `{}`\n\nDid you mean `{}`?",
-                target_desc,
-                target_name,
-                s.name()
-            ),
-            None => failure::bail!("no {} target named `{}`", target_desc, target_name),
-        }
+        let targets = packages.iter().flat_map(|pkg| {
+            pkg.targets()
+                .iter()
+                .filter(|target| is_expected_kind(target))
+        });
+        let suggestion = closest_msg(target_name, targets, |t| t.name());
+        failure::bail!(
+            "no {} target named `{}`{}",
+            target_desc,
+            target_name,
+            suggestion
+        );
     }
     Ok(proposals)
 }

src/cargo/ops/cargo_run.rs

@@ -2,7 +2,7 @@ use std::ffi::OsString;
 use std::iter;
 use std::path::Path;
 
-use crate::core::{nightly_features_allowed, TargetKind, Workspace};
+use crate::core::{TargetKind, Workspace};
 use crate::ops;
 use crate::util::{CargoResult, ProcessError};
 
@@ -55,22 +55,13 @@ pub fn run(
                 .into_iter()
                 .map(|(_pkg, target)| target.name())
                 .collect();
-            if nightly_features_allowed() {
-                failure::bail!(
-                    "`cargo run` could not determine which binary to run. \
-                     Use the `--bin` option to specify a binary, \
-                     or (on nightly) the `default-run` manifest key.\n\
-                     available binaries: {}",
-                    names.join(", ")
-                )
-            } else {
-                failure::bail!(
-                    "`cargo run` requires that a package only have one \
-                     executable; use the `--bin` option to specify which one \
-                     to run\navailable binaries: {}",
-                    names.join(", ")
-                )
-            }
+            failure::bail!(
+                "`cargo run` could not determine which binary to run. \
+                 Use the `--bin` option to specify a binary, \
+                 or the `default-run` manifest key.\n\
+                 available binaries: {}",
+                names.join(", ")
+            )
         } else {
             failure::bail!(
                 "`cargo run` can run at most one executable, but \

src/cargo/util/command_prelude.rs

@@ -514,10 +514,10 @@ pub enum CommandInfo {
 }
 
 impl CommandInfo {
-    pub fn name(&self) -> String {
+    pub fn name(&self) -> &str {
         match self {
-            CommandInfo::BuiltIn { name, .. } => name.to_string(),
-            CommandInfo::External { name, .. } => name.to_string(),
+            CommandInfo::BuiltIn { name, .. } => &name,
+            CommandInfo::External { name, .. } => &name,
         }
     }
 }

src/cargo/util/lev_distance.rs

@@ -33,6 +33,34 @@ pub fn lev_distance(me: &str, t: &str) -> usize {
     dcol[t_last + 1]
 }
 
+/// Find the closest element from `iter` matching `choice`. The `key` callback
+/// is used to select a `&str` from the iterator to compare against `choice`.
+pub fn closest<'a, T>(
+    choice: &str,
+    iter: impl Iterator<Item = T>,
+    key: impl Fn(&T) -> &'a str,
+) -> Option<T> {
+    // Only consider candidates with a lev_distance of 3 or less so we don't
+    // suggest out-of-the-blue options.
+    iter.map(|e| (lev_distance(choice, key(&e)), e))
+        .filter(|&(d, _)| d < 4)
+        .min_by_key(|t| t.0)
+        .map(|t| t.1)
+}
+
+/// Version of `closest` that returns a common "suggestion" that can be tacked
+/// onto the end of an error message.
+pub fn closest_msg<'a, T>(
+    choice: &str,
+    iter: impl Iterator<Item = T>,
+    key: impl Fn(&T) -> &'a str,
+) -> String {
+    match closest(choice, iter, &key) {
+        Some(e) => format!("\n\n\tDid you mean `{}`?", key(&e)),
+        None => String::new(),
+    }
+}
+
 #[test]
 fn test_lev_distance() {
     use std::char::{from_u32, MAX};

src/cargo/util/mod.rs

@@ -12,7 +12,7 @@ pub use self::graph::Graph;
 pub use self::hex::{hash_u64, short_hash, to_hex};
 pub use self::into_url::IntoUrl;
 pub use self::into_url_with_base::IntoUrlWithBase;
-pub use self::lev_distance::lev_distance;
+pub use self::lev_distance::{closest, closest_msg, lev_distance};
 pub use self::lockserver::{LockServer, LockServerClient, LockServerStarted};
 pub use self::paths::{bytes2path, dylib_path, join_paths, path2bytes};
 pub use self::paths::{dylib_path_envvar, normalize_path};

src/cargo/util/toml/mod.rs

@@ -21,8 +21,7 @@ use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest};
 use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig};
 use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
 use crate::util::errors::{CargoResult, CargoResultExt, ManifestError};
-use crate::util::paths;
-use crate::util::{self, validate_package_name, Config, IntoUrl};
+use crate::util::{self, paths, validate_package_name, Config, IntoUrl};
 
 mod targets;
 use self::targets::targets;
@@ -1052,6 +1051,18 @@ impl TomlManifest {
             )
         }
 
+        if let Some(run) = &project.default_run {
+            if !targets
+                .iter()
+                .filter(|t| t.is_bin())
+                .any(|t| t.name() == run)
+            {
+                let suggestion =
+                    util::closest_msg(&run, targets.iter().filter(|t| t.is_bin()), |t| t.name());
+                bail!("default-run target `{}` not found{}", run, suggestion);
+            }
+        }
+
         let custom_metadata = project.metadata.clone();
         let mut manifest = Manifest::new(
             summary,

src/doc/man/cargo-run.adoc

@@ -29,7 +29,8 @@ include::options-package.adoc[]
 
 When no target selection options are given, `cargo run` will run the binary
 target. If there are multiple binary targets, you must pass a target flag to
-choose one.
+choose one. Or, the `default-run` field may be specified in the `[package]`
+section of `Cargo.toml` to choose the name of the binary to run by default.
 
 *--bin* _NAME_::
     Run the specified binary.

src/doc/man/generated/cargo-run.html

@@ -48,7 +48,8 @@ <h3 id="cargo_run_target_selection">Target Selection</h3>
 <div class="paragraph">
 <p>When no target selection options are given, <code>cargo run</code> will run the binary
 target. If there are multiple binary targets, you must pass a target flag to
-choose one.</p>
+choose one. Or, the <code>default-run</code> field may be specified in the <code>[package]</code>
+section of <code>Cargo.toml</code> to choose the name of the binary to run by default.</p>
 </div>
 <div class="dlist">
 <dl>