Skip to content

Commit

Permalink
Enhance error message for target auto-discovery
Browse files Browse the repository at this point in the history
Enhance for following scenarios:

1. Target without `path` specified and cannot be found.
2. Target without `path` specified and cannot be found, but a file
   exists at the commonly wrong path, e.g. `example/a.rs`, `bench/b.rs`.
3. Found multiple candidate files and cannot infer which to use.
  • Loading branch information
weihanglo committed Nov 17, 2021
1 parent 3a3a071 commit 74390a4
Showing 1 changed file with 118 additions and 8 deletions.
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

0 comments on commit 74390a4

Please sign in to comment.