Fix #596: yr check should notify the users of all errors, not just the first one

Author: Tom Lancaster

cli/src/commands/check.rs

@@ -212,40 +212,42 @@ pub fn exec_check(args: &ArgMatches, config: &Config) -> anyhow::Result<()> {
 
             compiler.colorize_errors(io::stdout().is_tty());
 
-            match compiler.add_source(src) {
-                Ok(compiler) => {
-                    if compiler.warnings().is_empty() {
-                        state.files_passed.fetch_add(1, Ordering::Relaxed);
-                        lines.push(format!(
-                            "[ {} ] {}",
-                            "PASS".paint(Green).bold(),
-                            file_path.display()
-                        ));
-                    } else {
-                        state.warnings.fetch_add(
-                            compiler.warnings().len(),
-                            Ordering::Relaxed,
-                        );
-                        lines.push(format!(
-                            "[ {} ] {}",
-                            "WARN".paint(Yellow).bold(),
-                            file_path.display()
-                        ));
-                        for warning in compiler.warnings() {
-                            eprintln!("{warning}");
-                        }
-                    }
+            let _ = compiler.add_source(src);
+
+            if !compiler.errors().is_empty() {
+                state.errors.fetch_add(
+                    compiler.errors().len(),
+                    Ordering::Relaxed,
+                );
+                lines.push(format!(
+                    "[ {} ] {}",
+                    "FAIL".paint(Red).bold(),
+                    file_path.display()
+                ));
+                for error in compiler.errors() {
+                    eprintln!("{error}");
                 }
-                Err(err) => {
-                    state.errors.fetch_add(1, Ordering::Relaxed);
-                    lines.push(format!(
-                        "[ {} ] {}\n{}",
-                        "FAIL".paint(Red).bold(),
-                        file_path.display(),
-                        err,
-                    ));
+            } else if !compiler.warnings().is_empty() {
+                state.warnings.fetch_add(
+                    compiler.warnings().len(),
+                    Ordering::Relaxed,
+                );
+                lines.push(format!(
+                    "[ {} ] {}",
+                    "WARN".paint(Yellow).bold(),
+                    file_path.display()
+                ));
+                for warning in compiler.warnings() {
+                    eprintln!("{warning}");
                 }
-            };
+            } else {
+                state.files_passed.fetch_add(1, Ordering::Relaxed);
+                lines.push(format!(
+                    "[ {} ] {}",
+                    "PASS".paint(Green).bold(),
+                    file_path.display()
+                ));
+            }
 
             output.send(Message::Info(lines.join("\n")))?;
 

cli/src/tests/check.rs

@@ -203,3 +203,64 @@ fn config_error() {
             )
         );
 }
+
+#[test]
+fn check_reports_all_errors() {
+    let temp_dir = TempDir::new().unwrap();
+    let config_file = temp_dir.child("config.toml");
+
+    config_file
+        .write_str(
+            r#"
+            [check.rule_name]
+            regexp = "^[A-Z][a-zA-Z0-9]+_[A-Z][a-zA-Z0-9_]+$"
+            error = true
+
+            [check.metadata.author]
+            type = "string"
+            required = true
+            error = true
+
+            [check.metadata.severity]
+            type = "string"
+            regexp = "^(low|medium|high|critical)$"
+            required = true
+            error = true
+            "#,
+        )
+        .unwrap();
+
+    let yar_file = temp_dir.child("test.yar");
+
+    yar_file
+        .write_str(
+            r#"rule bad_rule {
+              meta:
+                description = "test"
+              strings:
+                $a = "test"
+              condition:
+                $a
+            }"#,
+        )
+        .unwrap();
+
+    // All three errors should be reported, not just the first one.
+    Command::new(cargo_bin!("yr"))
+        .arg("--config")
+        .arg(config_file.path())
+        .arg("check")
+        .arg(yar_file.path())
+        .assert()
+        .failure()
+        .code(1)
+        .stderr(predicate::str::contains(
+            "required metadata `author` not found",
+        ))
+        .stderr(predicate::str::contains(
+            "required metadata `severity` not found",
+        ))
+        .stderr(predicate::str::contains(
+            "rule name does not match regex",
+        ));
+}

lib/src/compiler/mod.rs

@@ -1436,6 +1436,7 @@ impl Compiler<'_> {
         }
 
         // Check the rule with all the linters.
+        let mut first_linter_err: Option<CompileError> = None;
         for linter in self.linters.iter() {
             match linter.check(&self.report_builder, rule) {
                 LinterResult::Ok => {}
@@ -1447,9 +1448,18 @@ impl Compiler<'_> {
                         self.warnings.add(|| warning);
                     }
                 }
-                LinterResult::Err(err) => return Err(err),
+                LinterResult::Err(err) => {
+                    if first_linter_err.is_none() {
+                        first_linter_err = Some(err);
+                    } else {
+                        self.errors.push(err);
+                    }
+                }
             }
         }
+        if let Some(err) = first_linter_err {
+            return Err(err);
+        }
 
         // Take snapshot of the current compiler state. In case of error
         // compiling the current rule this snapshot allows restoring the

site/content/docs/cli/commands.md

@@ -1,3 +1,4 @@
+
 ---
 title: "Commands"
 description: "The commands supported by the YARA-X CLI"