Returning anyhow::Result when checking for strictness violations and …

…retrieving packs (#155)

src/packs/checker.rs

@@ -16,6 +16,7 @@ use crate::packs::PackSet;
 
 use anyhow::bail;
 // External imports
+use anyhow::Context;
 use rayon::prelude::IntoParallelIterator;
 use rayon::prelude::IntoParallelRefIterator;
 use rayon::prelude::ParallelIterator;
@@ -41,40 +42,19 @@ pub struct ViolationIdentifier {
 pub fn get_defining_pack<'a>(
     violation: &ViolationIdentifier,
     packset: &'a PackSet,
-) -> &'a Pack {
-    let defining_pack_result = packset.for_pack(&violation.defining_pack_name);
-
-    // For some reason cargo fmt wasn't formatting this closure correctly when passed directly into unwrap_or_else
-    let error_closure = |_| {
-        let error_message = format!(
-            "ViolationIdentifier#defining_pack is {}, but that pack cannot be found in the packset.",
-            &violation.defining_pack_name
-        );
-
-        panic!("{}", &error_message);
-    };
-
-    defining_pack_result.unwrap_or_else(error_closure)
+) -> anyhow::Result<&'a Pack> {
+    packset.for_pack(&violation.defining_pack_name)
+    .context(format!("ViolationIdentifier#defining_pack is {}, but that pack cannot be found in the packset.", 
+    &violation.defining_pack_name))
 }
 
 pub fn get_referencing_pack<'a>(
     violation: &ViolationIdentifier,
     packset: &'a PackSet,
-) -> &'a Pack {
-    let referencing_pack_result =
-        packset.for_pack(&violation.referencing_pack_name);
-
-    // For some reason cargo fmt wasn't formatting this closure correctly when passed directly into unwrap_or_else
-    let error_closure = |_| {
-        let error_message = format!(
-            "ViolationIdentifier#referencing_pack is {}, but that pack cannot be found in the packset.",
-            &violation.referencing_pack_name
-        );
-
-        panic!("{}", &error_message);
-    };
-
-    referencing_pack_result.unwrap_or_else(error_closure)
+) -> anyhow::Result<&'a Pack> {
+    packset.for_pack(&violation.referencing_pack_name)
+    .context(format!("ViolationIdentifier#referencing_pack is {}, but that pack cannot be found in the packset.",
+&violation.referencing_pack_name))
 }
 
 #[derive(PartialEq, Clone, Eq, Hash, Debug)]
@@ -94,7 +74,7 @@ pub(crate) trait CheckerInterface {
         &self,
         offense: &ViolationIdentifier,
         configuration: &Configuration,
-    ) -> bool;
+    ) -> anyhow::Result<bool>;
 
     fn violation_type(&self) -> String;
 }
@@ -182,10 +162,10 @@ impl<'a> CheckAllBuilder<'a> {
         }
     }
 
-    pub fn build(mut self) -> CheckAllResult {
+    pub fn build(mut self) -> anyhow::Result<CheckAllResult> {
         let recorded_violations = &self.configuration.pack_set.all_violations;
 
-        CheckAllResult {
+        Ok(CheckAllResult {
             reportable_violations: self
                 .build_reportable_violations(recorded_violations)
                 .into_iter()
@@ -197,11 +177,11 @@ impl<'a> CheckAllBuilder<'a> {
                 .cloned()
                 .collect(),
             strict_mode_violations: self
-                .build_strict_mode_violations(recorded_violations)
+                .build_strict_mode_violations(recorded_violations)?
                 .into_iter()
                 .cloned()
                 .collect(),
-        }
+        })
     }
 
     fn build_reportable_violations(
@@ -258,28 +238,37 @@ impl<'a> CheckAllBuilder<'a> {
     fn build_strict_mode_violations(
         &mut self,
         recorded_violations: &'a HashSet<ViolationIdentifier>,
-    ) -> Vec<&'a ViolationIdentifier> {
-        let mut indexed_checkers: HashMap<
+    ) -> anyhow::Result<Vec<&'a ViolationIdentifier>> {
+        let indexed_checkers: HashMap<
             String,
             &Box<dyn CheckerInterface + Send + Sync>,
-        > = HashMap::new();
-        for checker in &self.found_violations.checkers {
-            indexed_checkers.insert(checker.violation_type(), checker);
-        }
+        > = self
+            .found_violations
+            .checkers
+            .iter()
+            .map(|checker| (checker.violation_type(), checker))
+            .collect();
 
-        let strict_mode_violations: Vec<&'a ViolationIdentifier> =
-            recorded_violations
-                .iter()
-                .filter(|v| {
-                    indexed_checkers
-                        .get(&v.violation_type)
-                        .unwrap()
-                        .is_strict_mode_violation(v, self.configuration)
-                })
-                .collect();
-        strict_mode_violations
+        recorded_violations
+            .iter()
+            .try_fold(vec![], |mut acc, violation| {
+                let checker = indexed_checkers
+                    .get(&violation.violation_type)
+                    .context(format!(
+                    "Checker for violation type {} not found",
+                    violation.violation_type
+                ))?;
+
+                if checker
+                    .is_strict_mode_violation(violation, self.configuration)?
+                {
+                    acc.push(violation);
+                }
+                Ok(acc)
+            })
     }
 }
+
 pub(crate) fn check_all(
     configuration: &Configuration,
     files: Vec<String>,
@@ -297,9 +286,7 @@ pub(crate) fn check_all(
         absolute_paths,
         violations,
     };
-    let check_all_result =
-        CheckAllBuilder::new(configuration, &found_violations).build();
-    Ok(check_all_result)
+    CheckAllBuilder::new(configuration, &found_violations).build()
 }
 
 fn validate(configuration: &Configuration) -> Vec<String> {

src/packs/checker/architecture.rs

@@ -190,11 +190,11 @@ impl CheckerInterface for Checker {
         &self,
         violation: &ViolationIdentifier,
         configuration: &Configuration,
-    ) -> bool {
+    ) -> anyhow::Result<bool> {
         let referencing_pack =
-            get_referencing_pack(violation, &configuration.pack_set);
+            get_referencing_pack(violation, &configuration.pack_set)?;
 
-        referencing_pack.enforce_architecture().is_strict()
+        Ok(referencing_pack.enforce_architecture().is_strict())
     }
 
     fn violation_type(&self) -> String {

src/packs/checker/dependency.rs

@@ -183,11 +183,11 @@ impl CheckerInterface for Checker {
         &self,
         violation: &ViolationIdentifier,
         configuration: &Configuration,
-    ) -> bool {
+    ) -> anyhow::Result<bool> {
         let referencing_pack =
-            get_referencing_pack(violation, &configuration.pack_set);
+            get_referencing_pack(violation, &configuration.pack_set)?;
 
-        referencing_pack.enforce_dependencies().is_strict()
+        Ok(referencing_pack.enforce_dependencies().is_strict())
     }
 
     fn violation_type(&self) -> String {

src/packs/checker/folder_visibility.rs

@@ -53,11 +53,11 @@ impl CheckerInterface for Checker {
         &self,
         violation: &ViolationIdentifier,
         configuration: &Configuration,
-    ) -> bool {
+    ) -> anyhow::Result<bool> {
         let referencing_pack =
-            get_referencing_pack(violation, &configuration.pack_set);
+            get_referencing_pack(violation, &configuration.pack_set)?;
 
-        referencing_pack.enforce_folder_visibility().is_strict()
+        Ok(referencing_pack.enforce_folder_visibility().is_strict())
     }
 
     fn violation_type(&self) -> String {

src/packs/checker/privacy.rs

@@ -118,11 +118,11 @@ impl CheckerInterface for Checker {
         &self,
         violation: &ViolationIdentifier,
         configuration: &Configuration,
-    ) -> bool {
+    ) -> anyhow::Result<bool> {
         let defining_pack =
-            get_defining_pack(violation, &configuration.pack_set);
+            get_defining_pack(violation, &configuration.pack_set)?;
 
-        defining_pack.enforce_privacy().is_strict()
+        Ok(defining_pack.enforce_privacy().is_strict())
     }
 
     fn violation_type(&self) -> String {

src/packs/checker/visibility.rs

@@ -79,11 +79,11 @@ impl CheckerInterface for Checker {
         &self,
         violation: &ViolationIdentifier,
         configuration: &Configuration,
-    ) -> bool {
+    ) -> anyhow::Result<bool> {
         let defining_pack =
-            get_defining_pack(violation, &configuration.pack_set);
+            get_defining_pack(violation, &configuration.pack_set)?;
 
-        defining_pack.enforce_visibility().is_strict()
+        Ok(defining_pack.enforce_visibility().is_strict())
     }
 
     fn violation_type(&self) -> String {