manifest: Be more specific about not supporting multiple packages yet (#21)

When running `cargo-subcommand` without `-p` package selection against a
workspace currently only the first found package is returned.  This
should be implemented over time to return all packages (and artifacts
adhering to target selection), but return a more specific error for the
time being instead of making it seem like there's only one package.

Author: MarijnS95

src/config.rs

@@ -17,6 +17,8 @@ pub enum EnvError {
     Var(VarError),
 }
 
+pub type Result<T, E = EnvError> = std::result::Result<T, E>;
+
 impl From<VarError> for EnvError {
     fn from(var: VarError) -> Self {
         Self::Var(var)
@@ -34,8 +36,6 @@ impl Display for EnvError {
 
 impl std::error::Error for EnvError {}
 
-type Result<T, E = EnvError> = std::result::Result<T, E>;
-
 #[derive(Clone, Debug, Deserialize, PartialEq)]
 #[serde(rename_all = "kebab-case")]
 pub struct Config {

src/error.rs

@@ -1,5 +1,5 @@
 use glob::{GlobError, PatternError};
-use std::fmt::{Display, Formatter, Result};
+use std::fmt::{Display, Formatter, Result as FmtResult};
 use std::io::Error as IoError;
 use std::path::PathBuf;
 use toml::de::Error as TomlError;
@@ -10,18 +10,24 @@ pub enum Error {
     ManifestNotFound,
     RustcNotFound,
     ManifestPathNotFound,
+    MultiplePackagesNotSupported,
     GlobPatternError(&'static str),
     Glob(GlobError),
     Io(PathBuf, IoError),
     Toml(PathBuf, TomlError),
 }
 
+pub type Result<T, E = Error> = std::result::Result<T, E>;
+
 impl Display for Error {
-    fn fmt(&self, f: &mut Formatter) -> Result {
+    fn fmt(&self, f: &mut Formatter) -> FmtResult {
         f.write_str(match self {
             Self::InvalidArgs => "Invalid args.",
             Self::ManifestNotFound => "Didn't find Cargo.toml.",
             Self::ManifestPathNotFound => "The manifest-path must be a path to a Cargo.toml file",
+            Self::MultiplePackagesNotSupported => {
+                "Multiple packages are not yet supported (ie. in a `[workspace]`). Use `-p` to select a specific package."
+            }
             Self::RustcNotFound => "Didn't find rustc.",
             Self::GlobPatternError(error) => error,
             Self::Glob(error) => return error.fmt(f),

src/manifest.rs

@@ -1,4 +1,4 @@
-use crate::error::Error;
+use crate::error::{Error, Result};
 use serde::Deserialize;
 use std::path::Path;
 
@@ -9,7 +9,7 @@ pub struct Manifest {
 }
 
 impl Manifest {
-    pub fn parse_from_toml(path: &Path) -> Result<Self, Error> {
+    pub fn parse_from_toml(path: &Path) -> Result<Self> {
         let contents = std::fs::read_to_string(path).map_err(|e| Error::Io(path.to_owned(), e))?;
         toml::from_str(&contents).map_err(|e| Error::Toml(path.to_owned(), e))
     }

src/subcommand.rs

@@ -1,6 +1,6 @@
 use crate::args::Args;
 use crate::artifact::{Artifact, CrateType};
-use crate::error::Error;
+use crate::error::{Error, Result};
 use crate::profile::Profile;
 use crate::{utils, LocalizedConfig};
 use std::ffi::OsStr;
@@ -21,7 +21,7 @@ pub struct Subcommand {
 }
 
 impl Subcommand {
-    pub fn new(args: Args) -> Result<Self, Error> {
+    pub fn new(args: Args) -> Result<Self> {
         // TODO: support multiple packages properly
         assert!(
             args.package.len() < 2,

src/utils.rs

@@ -1,10 +1,10 @@
 use crate::config::Config;
-use crate::error::Error;
+use crate::error::{Error, Result};
 use crate::manifest::Manifest;
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 
-pub fn list_rust_files(dir: &Path) -> Result<Vec<String>, Error> {
+pub fn list_rust_files(dir: &Path) -> Result<Vec<String>> {
     let mut files = Vec::new();
     if dir.exists() && dir.is_dir() {
         let entries = std::fs::read_dir(dir).map_err(|e| Error::Io(dir.to_owned(), e))?;
@@ -19,7 +19,7 @@ pub fn list_rust_files(dir: &Path) -> Result<Vec<String>, Error> {
     Ok(files)
 }
 
-fn member(manifest: &Path, members: &[String], package: &str) -> Result<Option<PathBuf>, Error> {
+fn member(manifest: &Path, members: &[String], package: &str) -> Result<Option<PathBuf>> {
     let workspace_dir = manifest.parent().unwrap();
     for member in members {
         for manifest_dir in glob::glob(workspace_dir.join(member).to_str().unwrap())? {
@@ -35,7 +35,7 @@ fn member(manifest: &Path, members: &[String], package: &str) -> Result<Option<P
     Ok(None)
 }
 
-pub fn find_package(path: &Path, name: Option<&str>) -> Result<(PathBuf, String), Error> {
+pub fn find_package(path: &Path, name: Option<&str>) -> Result<(PathBuf, String)> {
     let path = dunce::canonicalize(path).map_err(|e| Error::Io(path.to_owned(), e))?;
     for manifest_path in path
         .ancestors()
@@ -44,15 +44,17 @@ pub fn find_package(path: &Path, name: Option<&str>) -> Result<(PathBuf, String)
     {
         let manifest = Manifest::parse_from_toml(&manifest_path)?;
         if let Some(p) = manifest.package.as_ref() {
-            if let (Some(n1), n2) = (name, &p.name) {
-                if n1 == n2 {
+            if let Some(name) = name {
+                if name == p.name {
                     return Ok((manifest_path, p.name.clone()));
                 }
             } else {
                 return Ok((manifest_path, p.name.clone()));
             }
         }
-        if let (Some(w), Some(name)) = (manifest.workspace.as_ref(), name) {
+        if let Some(w) = manifest.workspace.as_ref() {
+            // TODO: This should also work if name is None - then all packages should simply be returned
+            let name = name.ok_or(Error::MultiplePackagesNotSupported)?;
             if let Some(manifest_path) = member(&manifest_path, &w.members, name)? {
                 return Ok((manifest_path, name.to_string()));
             }
@@ -61,7 +63,7 @@ pub fn find_package(path: &Path, name: Option<&str>) -> Result<(PathBuf, String)
     Err(Error::ManifestNotFound)
 }
 
-pub fn find_workspace(manifest: &Path, name: &str) -> Result<Option<PathBuf>, Error> {
+pub fn find_workspace(manifest: &Path, name: &str) -> Result<Option<PathBuf>> {
     let dir = manifest.parent().unwrap();
     for manifest_path in dir
         .ancestors()
@@ -81,7 +83,7 @@ pub fn find_workspace(manifest: &Path, name: &str) -> Result<Option<PathBuf>, Er
 /// Returns the [`target-dir`] configured in `.cargo/config.toml` or `"target"` if not set.
 ///
 /// [`target-dir`]: https://doc.rust-lang.org/cargo/reference/config.html#buildtarget-dir
-pub fn get_target_dir_name(config: Option<&Config>) -> Result<String, Error> {
+pub fn get_target_dir_name(config: Option<&Config>) -> Result<String> {
     if let Some(config) = config {
         if let Some(build) = config.build.as_ref() {
             if let Some(target_dir) = &build.target_dir {