Auto merge of #10090 - weihanglo:issue-9014, r=alexcrichton

Enhance error message for target auto-discovery

resolves #9014
resolves #9117

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.

For the suggestion in [the thread in #9116], I can't see any feasible way to list potential candidates without addditional I/O checking file existences. This PR is the best effort I can think of at this time. Feel free to comment. Thanks!

[the thread in #9116]: #9116 (comment)

Author: bors

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<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",
@@ -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",
@@ -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!(),