Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): Improve errors related to cargo script #13346

Merged
merged 5 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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