Skip to content

Commit

Permalink
Wrap initial ManifestError in toml module
Browse files Browse the repository at this point in the history
Filter out ManifestErrors when generating error output
  • Loading branch information
alexheretic committed Oct 12, 2018
1 parent 10b3ebf commit 25b7ad2
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 34 deletions.
6 changes: 1 addition & 5 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,11 +495,7 @@ impl<'cfg> Workspace<'cfg> {
self.members.push(manifest_path.clone());

let candidates = {
let pkg = match *self
.packages
.load(&manifest_path)
.map_err(|err| ManifestError::new(err, manifest_path.clone()))?
{
let pkg = match *self.packages.load(&manifest_path)? {
MaybePackage::Package(ref p) => p,
MaybePackage::Virtual(_) => return Ok(()),
};
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_read_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ fn read_nested_packages(
"skipping malformed package found at `{}`",
path.to_string_lossy()
);
errors.push(err);
errors.push(err.into());
return Ok(());
}
Ok(tuple) => tuple,
Expand Down
36 changes: 32 additions & 4 deletions src/cargo/util/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,25 +73,37 @@ impl fmt::Display for Internal {
}
}

/// Error related to a particular manifest and providing it's path.
/// Error wrapper related to a particular manifest and providing it's path.
///
/// This error adds no displayable info of it's own.
pub struct ManifestError {
cause: Error,
manifest: PathBuf,
}

impl ManifestError {
pub fn new(cause: Error, manifest: PathBuf) -> Self {
Self { cause, manifest }
pub fn new<E: Into<Error>>(cause: E, manifest: PathBuf) -> Self {
Self {
cause: cause.into(),
manifest,
}
}

pub fn manifest_path(&self) -> &PathBuf {
&self.manifest
}

/// Returns an iterator over the `ManifestError` chain of causes.
///
/// So if this error was not caused by another `ManifestError` this will be empty.
pub fn manifest_causes(&self) -> ManifestCauses {
ManifestCauses { current: self }
}
}

impl Fail for ManifestError {
fn cause(&self) -> Option<&Fail> {
self.cause.as_fail().into()
self.cause.as_fail().cause()
}
}

Expand All @@ -107,6 +119,22 @@ impl fmt::Display for ManifestError {
}
}

/// An iterator over the `ManifestError` chain of causes.
pub struct ManifestCauses<'a> {
current: &'a ManifestError,
}

impl<'a> Iterator for ManifestCauses<'a> {
type Item = &'a ManifestError;

fn next(&mut self) -> Option<Self::Item> {
self.current = self.current.cause.downcast_ref()?;
Some(self.current)
}
}

impl<'a> ::std::iter::FusedIterator for ManifestCauses<'a> {}

// =============================================================================
// Process errors
#[derive(Debug, Fail)]
Expand Down
12 changes: 6 additions & 6 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use core::{Dependency, Manifest, PackageId, Summary, Target};
use core::{Edition, EitherManifest, Feature, Features, VirtualManifest};
use core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig};
use sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
use util::errors::{CargoError, CargoResult, CargoResultExt};
use util::errors::{CargoError, CargoResult, CargoResultExt, ManifestError};
use util::paths;
use util::{self, Config, ToUrl};

Expand All @@ -30,17 +30,17 @@ pub fn read_manifest(
path: &Path,
source_id: &SourceId,
config: &Config,
) -> CargoResult<(EitherManifest, Vec<PathBuf>)> {
) -> Result<(EitherManifest, Vec<PathBuf>), ManifestError> {
trace!(
"read_manifest; path={}; source-id={}",
path.display(),
source_id
);
let contents = paths::read(path)?;
let contents = paths::read(path).map_err(|err| ManifestError::new(err, path.into()))?;

let ret = do_read_manifest(&contents, path, source_id, config)
.chain_err(|| format!("failed to parse manifest at `{}`", path.display()))?;
Ok(ret)
do_read_manifest(&contents, path, source_id, config)
.chain_err(|| format!("failed to parse manifest at `{}`", path.display()))
.map_err(|err| ManifestError::new(err, path.into()))
}

fn do_read_manifest(
Expand Down
30 changes: 12 additions & 18 deletions tests/testsuite/member_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,12 @@ fn toml_deserialize_manifest_error() {
let error = Workspace::new(&root_manifest_path, &Config::default().unwrap()).unwrap_err();
eprintln!("{:?}", error);

let manifest_errs: Vec<_> = error
.iter_chain()
.filter_map(|err| err.downcast_ref::<ManifestError>())
.map(|err| err.manifest_path())
.collect();

assert_eq!(manifest_errs.len(), 2, "{:?}", manifest_errs);
assert_eq!(manifest_errs[0], &root_manifest_path);
assert_eq!(manifest_errs[1], &member_manifest_path);
let manifest_err: &ManifestError = error.downcast_ref().expect("Not a ManifestError");
assert_eq!(manifest_err.manifest_path(), &root_manifest_path);

let causes: Vec<_> = manifest_err.manifest_causes().collect();
assert_eq!(causes.len(), 1, "{:?}", causes);
assert_eq!(causes[0].manifest_path(), &member_manifest_path);
}

/// Tests inclusion of a `ManifestError` pointing to a member manifest
Expand Down Expand Up @@ -97,14 +94,11 @@ fn member_manifest_path_io_error() {
let error = Workspace::new(&root_manifest_path, &Config::default().unwrap()).unwrap_err();
eprintln!("{:?}", error);

let manifest_errs: Vec<_> = error
.iter_chain()
.filter_map(|err| err.downcast_ref::<ManifestError>())
.map(|err| err.manifest_path())
.collect();
let manifest_err: &ManifestError = error.downcast_ref().expect("Not a ManifestError");
assert_eq!(manifest_err.manifest_path(), &root_manifest_path);

assert_eq!(manifest_errs.len(), 3, "{:?}", manifest_errs);
assert_eq!(manifest_errs[0], &root_manifest_path);
assert_eq!(manifest_errs[1], &member_manifest_path);
assert_eq!(manifest_errs[2], &missing_manifest_path);
let causes: Vec<_> = manifest_err.manifest_causes().collect();
assert_eq!(causes.len(), 2, "{:?}", causes);
assert_eq!(causes[0].manifest_path(), &member_manifest_path);
assert_eq!(causes[1].manifest_path(), &missing_manifest_path);
}

0 comments on commit 25b7ad2

Please sign in to comment.