Revert "Filter as skips (#38)" (#39)

This commit introduced an unwanted behavior with the some keyword while
fixing for [*] clauses to fail when empty.

This reverts commit 611de1f.

cfn-guard/src/commands/test.rs

@@ -94,7 +94,14 @@ or failure testing.
                     match crate::rules::parser::rules_file(span) {
                         Err(e) => println!("Parse Error on ruleset file {}", e),
                         Ok(rules) => {
-                            exit_code = test_with_data(&data_test_files, &rules, verbose)?
+                            match test_with_data(&data_test_files, &rules, verbose) {
+                                Ok(code) => {
+                                    exit_code = code;
+                                },
+                                Err(_) => {
+                                    exit_code = 5;
+                                }
+                            }
                         }
                     }
                 }

cfn-guard/src/commands/validate.rs

@@ -230,12 +230,8 @@ fn find_all_failing_clauses(context: &StatusContext) -> Vec<&StatusContext> {
     for each in &context.children {
         if each.status.map_or(false, |s| s == Status::FAIL) {
             match each.eval_type {
-                EvaluationType::Clause |
-                EvaluationType::BlockClause => {
+                EvaluationType::Clause => {
                     failed.push(each);
-                    if each.eval_type == EvaluationType::BlockClause {
-                        failed.extend(find_all_failing_clauses(each));
-                    }
                 },
 
                 EvaluationType::Filter |

cfn-guard/src/rules/evaluate.rs

@@ -138,7 +138,7 @@ fn compare<F>(lhs: &Vec<&PathAwareValue>,
     where F: Fn(&PathAwareValue, &PathAwareValue) -> Result<bool>
 {
     if lhs.is_empty() || rhs.is_empty() {
-        return Ok((Status::SKIP, None, None))
+        return Ok((Status::FAIL, None, None))
     }
 
     let lhs_elem = lhs[0];
@@ -243,7 +243,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> {
 
 
         let (lhs, retrieve_error) =
-            match resolve_query(true,
+            match resolve_query(clause.access_clause.query.match_all,
                                 &clause.access_clause.query.query,
                                 context,
                                 var_resolver)
@@ -364,10 +364,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> {
                         Some(l[0].clone())
                     } else { None }
                 }
-            );
-            if r == Status::FAIL {
-                auto_reporter.message(message.to_string());
-            }
+            ).message(message.to_string());
             return Ok(r)
         }
 
@@ -378,11 +375,9 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> {
                                   .message(retrieve_error.map_or("".to_string(), |e| e)).get_status())
                 }
                 else {
-                    let status = if retrieve_error.is_none() { Status::SKIP } else { Status::FAIL };
-                    return Ok(auto_reporter.status(status)
+                    return Ok(auto_reporter.status(Status::FAIL)
                         .message(retrieve_error.map_or("".to_string(), |e| e)).get_status())
-                },
-
+                }
             Some(l) => l,
         };
 
@@ -411,7 +406,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> {
         let (rhs_resolved, rhs_query) = if let Some(expr) = &clause.access_clause.compare_with {
             match expr {
                 LetValue::AccessClause(query) =>
-                    (Some(resolve_query(true, &query.query, context, var_resolver)?), Some(query.query.as_slice())),
+                    (Some(resolve_query(query.match_all, &query.query, context, var_resolver)?), Some(query.query.as_slice())),
                 _ => (None, None)
             }
         } else {
@@ -526,10 +521,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> {
             Some(msg) => msg,
             None => "(DEFAULT: NO_MESSAGE)"
         };
-        auto_reporter.comparison(result, from, to);
-        if result == Status::FAIL {
-            auto_reporter.message(message.to_string());
-        }
+        auto_reporter.comparison(result, from, to).message(message.to_string());
         Ok(result)
     }
 }
@@ -648,28 +640,27 @@ impl<'loc, T: Evaluate + 'loc> Evaluate for Conjunctions<T> {
 
 impl<'loc> Evaluate for BlockGuardClause<'loc> {
     fn evaluate<'s>(&self, context: &'s PathAwareValue, var_resolver: &'s dyn EvaluationContext) -> Result<Status> {
-        let blk_context = format!("Block[{}]", self.location);
-        let mut report = AutoReport::new(
-            EvaluationType::BlockClause,
-            var_resolver,
-            &blk_context
-        );
         let all = self.query.match_all;
-        let block_values = match resolve_query(true, &self.query.query, context, var_resolver) {
+        let block_values = match resolve_query(all, &self.query.query, context, var_resolver) {
             Err(Error(ErrorKind::RetrievalError(e))) |
             Err(Error(ErrorKind::IncompatibleRetrievalError(e))) => {
+                let context = format!("Block[{}]", self.location);
+                let mut report = AutoReport::new(
+                    EvaluationType::Clause,
+                    var_resolver,
+                    &context
+                );
                 return Ok(report.message(e).status(Status::FAIL).get_status())
             },
 
-            Ok(v) => if v.is_empty() {
-                return Ok(report.message(format!("Query {} returned no results", SliceDisplay(&self.query.query))).status(
-                    if self.not_empty { Status::FAIL } else { Status::SKIP }).get_status())
+            Ok(v) => if v.is_empty() { // one or more
+                return Ok(Status::FAIL)
             } else { v },
 
             Err(e) => return Err(e)
         };
 
-        Ok(report.status(loop {
+        Ok(loop {
             let mut num_fail = 0;
             let mut num_pass = 0;
             for each in block_values {
@@ -690,7 +681,7 @@ impl<'loc> Evaluate for BlockGuardClause<'loc> {
                 if num_fail > 0 { break Status::FAIL }
                 break Status::SKIP
             }
-        }).get_status())
+        })
     }
 }