Skip to content

Commit

Permalink
Auto merge of #13346 - epage:error, r=weihanglo
Browse files Browse the repository at this point in the history
fix(cli): Improve errors related to cargo script

### What does this PR try to resolve?

Fixes #13332

### How should we test and review this PR?

See tests in last commit to see how this changes error messages.

This is a lot of duplication with minor tweaking that will go away on stabilization

### Additional information
  • Loading branch information
bors committed Jan 25, 2024
2 parents 29386b9 + 51b1200 commit f4bafa7
Show file tree
Hide file tree
Showing 3 changed files with 217 additions and 15 deletions.
111 changes: 108 additions & 3 deletions src/bin/cargo/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::util::restricted_names::is_glob_pattern;
use cargo::core::Verbosity;
use cargo::core::Workspace;
use cargo::ops::{self, CompileFilter, Packages};
use cargo::util::closest;
use cargo_util::ProcessError;
use itertools::Itertools as _;

pub fn cli() -> Command {
subcommand("run")
Expand Down Expand Up @@ -97,11 +99,81 @@ pub fn is_manifest_command(arg: &str) -> bool {
}

pub fn exec_manifest_command(config: &mut Config, cmd: &str, args: &[OsString]) -> CliResult {
if !config.cli_unstable().script {
return Err(anyhow::anyhow!("running `{cmd}` requires `-Zscript`").into());
let manifest_path = Path::new(cmd);
match (manifest_path.is_file(), config.cli_unstable().script) {
(true, true) => {}
(true, false) => {
return Err(anyhow::anyhow!("running the file `{cmd}` requires `-Zscript`").into());
}
(false, true) => {
let possible_commands = crate::list_commands(config);
let is_dir = if manifest_path.is_dir() {
format!("\n\t`{cmd}` is a directory")
} else {
"".to_owned()
};
let suggested_command = if let Some(suggested_command) = possible_commands
.keys()
.filter(|c| cmd.starts_with(c.as_str()))
.max_by_key(|c| c.len())
{
let actual_args = cmd.strip_prefix(suggested_command).unwrap();
let args = if args.is_empty() {
"".to_owned()
} else {
format!(
" {}",
args.into_iter().map(|os| os.to_string_lossy()).join(" ")
)
};
format!("\n\tDid you mean the command `{suggested_command} {actual_args}{args}`")
} else {
"".to_owned()
};
let suggested_script = if let Some(suggested_script) = suggested_script(cmd) {
format!("\n\tDid you mean the file `{suggested_script}`")
} else {
"".to_owned()
};
return Err(anyhow::anyhow!(
"no such file or subcommand `{cmd}`{is_dir}{suggested_command}{suggested_script}"
)
.into());
}
(false, false) => {
// HACK: duplicating the above for minor tweaks but this will all go away on
// stabilization
let possible_commands = crate::list_commands(config);
let suggested_command = if let Some(suggested_command) = possible_commands
.keys()
.filter(|c| cmd.starts_with(c.as_str()))
.max_by_key(|c| c.len())
{
let actual_args = cmd.strip_prefix(suggested_command).unwrap();
let args = if args.is_empty() {
"".to_owned()
} else {
format!(
" {}",
args.into_iter().map(|os| os.to_string_lossy()).join(" ")
)
};
format!("\n\tDid you mean the command `{suggested_command} {actual_args}{args}`")
} else {
"".to_owned()
};
let suggested_script = if let Some(suggested_script) = suggested_script(cmd) {
format!("\n\tDid you mean the file `{suggested_script}` with `-Zscript`")
} else {
"".to_owned()
};
return Err(anyhow::anyhow!(
"no such subcommand `{cmd}`{suggested_command}{suggested_script}"
)
.into());
}
}

let manifest_path = Path::new(cmd);
let manifest_path = root_manifest(Some(manifest_path), config)?;

// Treat `cargo foo.rs` like `cargo install --path foo` and re-evaluate the config based on the
Expand All @@ -123,6 +195,39 @@ pub fn exec_manifest_command(config: &mut Config, cmd: &str, args: &[OsString])
cargo::ops::run(&ws, &compile_opts, args).map_err(|err| to_run_error(config, err))
}

fn suggested_script(cmd: &str) -> Option<String> {
let cmd_path = Path::new(cmd);
let mut suggestion = Path::new(".").to_owned();
for cmd_part in cmd_path.components() {
let exact_match = suggestion.join(cmd_part);
suggestion = if exact_match.exists() {
exact_match
} else {
let possible: Vec<_> = std::fs::read_dir(suggestion)
.into_iter()
.flatten()
.filter_map(|e| e.ok())
.map(|e| e.path())
.filter(|p| p.to_str().is_some())
.collect();
if let Some(possible) = closest(
cmd_part.as_os_str().to_str().unwrap(),
possible.iter(),
|p| p.file_name().unwrap().to_str().unwrap(),
) {
possible.to_owned()
} else {
return None;
}
};
}
if suggestion.is_dir() {
None
} else {
suggestion.into_os_string().into_string().ok()
}
}

fn to_run_error(config: &cargo::util::Config, err: anyhow::Error) -> CliError {
let proc_err = match err.downcast_ref::<ProcessError>() {
Some(e) => e,
Expand Down
15 changes: 11 additions & 4 deletions src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,19 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&OsStr]) -> C
let command = match path {
Some(command) => command,
None => {
let script_suggestion = if config.cli_unstable().script
&& std::path::Path::new(cmd).is_file()
{
let sep = std::path::MAIN_SEPARATOR;
format!("\n\tTo run the file `{cmd}`, provide a relative path like `.{sep}{cmd}`")
} else {
"".to_owned()
};
let err = if cmd.starts_with('+') {
anyhow::format_err!(
"no such command: `{}`\n\n\t\
"no such command: `{cmd}`\n\n\t\
Cargo does not handle `+toolchain` directives.\n\t\
Did you mean to invoke `cargo` through `rustup` instead?",
cmd
Did you mean to invoke `cargo` through `rustup` instead?{script_suggestion}",
)
} else {
let suggestions = list_commands(config);
Expand All @@ -192,7 +199,7 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&OsStr]) -> C
anyhow::format_err!(
"no such command: `{cmd}`{did_you_mean}\n\n\t\
View all installed commands with `cargo --list`\n\t\
Find a package to install `{cmd}` with `cargo search cargo-{cmd}`",
Find a package to install `{cmd}` with `cargo search cargo-{cmd}`{script_suggestion}",
)
};

Expand Down
106 changes: 98 additions & 8 deletions tests/testsuite/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ error: no such command: `echo`
<tab>View all installed commands with `cargo --list`
<tab>Find a package to install `echo` with `cargo search cargo-echo`
<tab>To run the file `echo`, provide a relative path like `./echo`
",
)
.run();
Expand Down Expand Up @@ -182,7 +183,7 @@ fn requires_nightly() {
.with_stdout("")
.with_stderr(
"\
error: running `echo.rs` requires `-Zscript`
[ERROR] running the file `echo.rs` requires `-Zscript`
",
)
.run();
Expand All @@ -200,7 +201,7 @@ fn requires_z_flag() {
.with_stdout("")
.with_stderr(
"\
error: running `echo.rs` requires `-Zscript`
[ERROR] running the file `echo.rs` requires `-Zscript`
",
)
.run();
Expand Down Expand Up @@ -591,30 +592,119 @@ args: []
#[cargo_test]
fn script_like_dir() {
let p = cargo_test_support::project()
.file("script.rs/foo", "something")
.file("foo.rs/foo", "something")
.build();

p.cargo("-Zscript -v script.rs")
p.cargo("-Zscript -v foo.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stderr(
"\
error: manifest path `script.rs` is a directory but expected a file
[ERROR] no such file or subcommand `foo.rs`
<tab>`foo.rs` is a directory
",
)
.run();
}

#[cargo_test]
fn missing_script_rs() {
fn non_existent_rs() {
let p = cargo_test_support::project().build();

p.cargo("-Zscript -v script.rs")
p.cargo("-Zscript -v foo.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stderr(
"\
[ERROR] manifest path `script.rs` does not exist
[ERROR] no such file or subcommand `foo.rs`
",
)
.run();
}

#[cargo_test]
fn non_existent_rs_stable() {
let p = cargo_test_support::project().build();

p.cargo("-v foo.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stdout("")
.with_stderr(
"\
[ERROR] no such subcommand `foo.rs`
",
)
.run();
}

#[cargo_test]
fn did_you_mean_file() {
let p = cargo_test_support::project()
.file("food.rs", ECHO_SCRIPT)
.build();

p.cargo("-Zscript -v foo.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stdout("")
.with_stderr(
"\
[ERROR] no such file or subcommand `foo.rs`
<tab>Did you mean the file `./food.rs`
",
)
.run();
}

#[cargo_test]
fn did_you_mean_file_stable() {
let p = cargo_test_support::project()
.file("food.rs", ECHO_SCRIPT)
.build();

p.cargo("-v foo.rs")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stdout("")
.with_stderr(
"\
[ERROR] no such subcommand `foo.rs`
<tab>Did you mean the file `./food.rs` with `-Zscript`
",
)
.run();
}

#[cargo_test]
fn did_you_mean_command() {
let p = cargo_test_support::project().build();

p.cargo("-Zscript -v build--manifest-path=./Cargo.toml")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stdout("")
.with_stderr(
"\
[ERROR] no such file or subcommand `build--manifest-path=./Cargo.toml`
<tab>Did you mean the command `build --manifest-path=./Cargo.toml`
",
)
.run();
}

#[cargo_test]
fn did_you_mean_command_stable() {
let p = cargo_test_support::project().build();

p.cargo("-v build--manifest-path=./Cargo.toml")
.masquerade_as_nightly_cargo(&["script"])
.with_status(101)
.with_stdout("")
.with_stderr(
"\
[ERROR] no such subcommand `build--manifest-path=./Cargo.toml`
<tab>Did you mean the command `build --manifest-path=./Cargo.toml`
",
)
.run();
Expand Down

0 comments on commit f4bafa7

Please sign in to comment.