From 74390a4ad711414449a331e0fe88c5eb864470da Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 17 Nov 2021 16:58:39 +0800 Subject: [PATCH] Enhance error message for target auto-discovery 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. --- src/cargo/util/toml/targets.rs | 126 ++++++++++++++++++++++++++++++--- 1 file changed, 118 insertions(+), 8 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 51f367f1ed2..473994b285d 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -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, @@ -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); } @@ -370,7 +378,7 @@ fn clean_examples( warnings: &mut Vec, errors: &mut Vec, ) -> CargoResult> { - 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", @@ -415,7 +423,7 @@ fn clean_tests( warnings: &mut Vec, errors: &mut Vec, ) -> CargoResult> { - 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", @@ -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 } @@ -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)], @@ -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!(),