Merge pull request #841 from Michael-F-Bryan/host-errors

Make model load errors more specific

crates/fj-host/src/lib.rs

@@ -50,13 +50,12 @@ impl Model {
     pub fn from_path(
         path: PathBuf,
         target_dir: Option<PathBuf>,
-    ) -> io::Result<Self> {
+    ) -> Result<Self, Error> {
         let crate_dir = path.canonicalize()?;
 
         let metadata = cargo_metadata::MetadataCommand::new()
             .current_dir(&crate_dir)
-            .exec()
-            .map_err(metadata_error_to_io)?;
+            .exec()?;
 
         let pkg = package_associated_with_directory(&metadata, &crate_dir)?;
         let src_path = crate_dir.join("src");
@@ -207,17 +206,10 @@ impl Model {
     }
 }
 
-fn metadata_error_to_io(e: cargo_metadata::Error) -> std::io::Error {
-    match e {
-        cargo_metadata::Error::Io(io) => io,
-        _ => std::io::Error::new(io::ErrorKind::Other, e),
-    }
-}
-
 fn package_associated_with_directory<'m>(
     metadata: &'m cargo_metadata::Metadata,
     dir: &Path,
-) -> Result<&'m cargo_metadata::Package, io::Error> {
+) -> Result<&'m cargo_metadata::Package, Error> {
     for pkg in metadata.workspace_packages() {
         let crate_dir = pkg
             .manifest_path
@@ -229,9 +221,33 @@ fn package_associated_with_directory<'m>(
         }
     }
 
-    let msg = format!("\"{}\" doesn't point to a crate", dir.display());
+    Err(ambiguous_path_error(metadata, dir))
+}
 
-    Err(io::Error::new(io::ErrorKind::Other, msg))
+fn ambiguous_path_error(
+    metadata: &cargo_metadata::Metadata,
+    dir: &Path,
+) -> Error {
+    let mut possible_paths = Vec::new();
+
+    for id in &metadata.workspace_members {
+        let cargo_toml = &metadata[id].manifest_path;
+        let crate_dir = cargo_toml
+            .parent()
+            .expect("A Cargo.toml always has a parent");
+        // Try to make the path relative to the workspace root so error messages
+        // aren't super long.
+        let simplified_path = crate_dir
+            .strip_prefix(&metadata.workspace_root)
+            .unwrap_or(crate_dir);
+
+        possible_paths.push(simplified_path.into());
+    }
+
+    Error::AmbiguousPath {
+        dir: dir.to_path_buf(),
+        possible_paths,
+    }
 }
 
 /// Watches a model for changes, reloading it continually
@@ -334,6 +350,28 @@ pub enum Error {
     /// Error while watching the model code for changes
     #[error("Error watching model for changes")]
     Notify(#[from] notify::Error),
+
+    /// An error occurred while trying to use evaluate
+    /// [`cargo_metadata::MetadataCommand`].
+    #[error("Unable to determine the crate's metadata")]
+    CargoMetadata(#[from] cargo_metadata::Error),
+
+    /// The user pointed us to a directory, but it doesn't look like that was
+    /// a crate root (i.e. the folder containing `Cargo.toml`).
+    #[error(
+        "It doesn't look like \"{}\" is a crate directory. Did you mean one of {}?",
+        dir.display(),
+        possible_paths.iter().map(|p| p.display().to_string())
+        .collect::<Vec<_>>()
+        .join(", ")
+    )]
+    AmbiguousPath {
+        /// The model directory supplied by the user.
+        dir: PathBuf,
+        /// The directories for each crate in the workspace, relative to the
+        /// workspace root.
+        possible_paths: Vec<PathBuf>,
+    },
 }
 
 type ModelFn = unsafe extern "C" fn(args: &Parameters) -> fj::Shape;