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

Enhance error message for target auto-discovery #10090

Merged
merged 2 commits into from
Nov 17, 2021
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
126 changes: 118 additions & 8 deletions src/cargo/util/toml/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ use crate::util::restricted_names;

use anyhow::Context as _;

const DEFAULT_TEST_DIR_NAME: &'static str = "tests";
const DEFAULT_BENCH_DIR_NAME: &'static str = "benches";
const DEFAULT_EXAMPLE_DIR_NAME: &'static str = "examples";
const DEFAULT_BIN_DIR_NAME: &'static str = "bin";

pub fn targets(
features: &Features,
manifest: &TomlManifest,
Expand Down Expand Up @@ -353,7 +358,10 @@ fn clean_bins(
return Some(path);
}

let path = package_root.join("src").join("bin").join("main.rs");
let path = package_root
.join("src")
.join(DEFAULT_BIN_DIR_NAME)
.join("main.rs");
if path.exists() {
return Some(path);
}
Expand All @@ -370,7 +378,7 @@ fn clean_examples(
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
) -> CargoResult<Vec<Target>> {
let inferred = infer_from_directory(&package_root.join("examples"));
let inferred = infer_from_directory(&package_root.join(DEFAULT_EXAMPLE_DIR_NAME));

let targets = clean_targets(
"example",
Expand Down Expand Up @@ -415,7 +423,7 @@ fn clean_tests(
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
) -> CargoResult<Vec<Target>> {
let inferred = infer_from_directory(&package_root.join("tests"));
let inferred = infer_from_directory(&package_root.join(DEFAULT_TEST_DIR_NAME));

let targets = clean_targets(
"test",
Expand Down Expand Up @@ -590,7 +598,9 @@ fn inferred_bins(package_root: &Path, package_name: &str) -> Vec<(String, PathBu
if main.exists() {
result.push((package_name.to_string(), main));
}
result.extend(infer_from_directory(&package_root.join("src").join("bin")));
result.extend(infer_from_directory(
&package_root.join("src").join(DEFAULT_BIN_DIR_NAME),
));

result
}
Expand Down Expand Up @@ -812,6 +822,90 @@ fn configure(features: &Features, toml: &TomlTarget, target: &mut Target) -> Car
Ok(())
}

/// Build an error message for a target path that cannot be determined either
/// by auto-discovery or specifiying.
///
/// This function tries to detect commonly wrong paths for targets:
///
/// test -> tests/*.rs, tests/*/main.rs
/// bench -> benches/*.rs, benches/*/main.rs
/// example -> examples/*.rs, examples/*/main.rs
/// bin -> src/bin/*.rs, src/bin/*/main.rs
///
/// Note that the logic need to sync with [`infer_from_directory`] if changes.
fn target_path_not_found_error_message(
package_root: &Path,
target: &TomlTarget,
target_kind: &str,
) -> String {
fn possible_target_paths(name: &str, kind: &str, commonly_wrong: bool) -> [PathBuf; 2] {
let mut target_path = PathBuf::new();
match (kind, commonly_wrong) {
// commonly wrong paths
("test" | "bench" | "example", true) => target_path.push(kind),
("bin", true) => {
target_path.push("src");
target_path.push("bins");
}
// default inferred paths
("test", false) => target_path.push(DEFAULT_TEST_DIR_NAME),
("bench", false) => target_path.push(DEFAULT_BENCH_DIR_NAME),
("example", false) => target_path.push(DEFAULT_EXAMPLE_DIR_NAME),
("bin", false) => {
target_path.push("src");
target_path.push(DEFAULT_BIN_DIR_NAME);
}
_ => unreachable!("invalid target kind: {}", kind),
}
target_path.push(name);

let target_path_file = {
let mut path = target_path.clone();
path.set_extension("rs");
path
};
let target_path_subdir = {
target_path.push("main.rs");
target_path
};
return [target_path_file, target_path_subdir];
}

let target_name = target.name();
let commonly_wrong_paths = possible_target_paths(&target_name, target_kind, true);
let possible_paths = possible_target_paths(&target_name, target_kind, false);
let existing_wrong_path_index = match (
package_root.join(&commonly_wrong_paths[0]).exists(),
package_root.join(&commonly_wrong_paths[1]).exists(),
) {
(true, _) => Some(0),
(_, true) => Some(1),
_ => None,
};

if let Some(i) = existing_wrong_path_index {
return format!(
"\
can't find `{name}` {kind} at default paths, but found a file at `{wrong_path}`.
Perhaps rename the file to `{possible_path}` for target auto-discovery, \
or specify {kind}.path if you want to use a non-default path.",
name = target_name,
kind = target_kind,
wrong_path = commonly_wrong_paths[i].display(),
possible_path = possible_paths[i].display(),
);
}

format!(
"can't find `{name}` {kind} at `{path_file}` or `{path_dir}`. \
Please specify {kind}.path if you want to use a non-default path.",
name = target_name,
kind = target_kind,
path_file = possible_paths[0].display(),
path_dir = possible_paths[1].display(),
)
}

fn target_path(
target: &TomlTarget,
inferred: &[(String, PathBuf)],
Expand All @@ -835,16 +929,32 @@ fn target_path(
let second = matching.next();
match (first, second) {
(Some(path), None) => Ok(path),
(None, None) | (Some(_), Some(_)) => {
(None, None) => {
if edition == Edition::Edition2015 {
if let Some(path) = legacy_path(target) {
return Ok(path);
}
}
Err(target_path_not_found_error_message(
package_root,
target,
target_kind,
))
}
(Some(p0), Some(p1)) => {
if edition == Edition::Edition2015 {
if let Some(path) = legacy_path(target) {
return Ok(path);
}
}
Err(format!(
"can't find `{name}` {target_kind}, specify {target_kind}.path",
name = name,
target_kind = target_kind
"\
cannot infer path for `{}` {}
Cargo doesn't know which to use because multiple target files found at `{}` and `{}`.",
target.name(),
target_kind,
p0.strip_prefix(package_root).unwrap_or(&p0).display(),
p1.strip_prefix(package_root).unwrap_or(&p1).display(),
))
}
(None, Some(_)) => unreachable!(),
Expand Down
Loading