Provide more information when validating configs and manifests (#1182)

* Provide more information when validating configs and manifests

* Fix tests

* Fix other snapshot

* Write to printer to allow hide output with `-q`

---------

Co-authored-by: Jo <[email protected]>

Author: MatthewMckee4

src/cli/mod.rs

@@ -42,7 +42,7 @@ pub(crate) use self_update::self_update;
 pub(crate) use try_repo::try_repo;
 pub(crate) use validate::{validate_configs, validate_manifest};
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, Eq)]
 pub(crate) enum ExitStatus {
     /// The command succeeded.
     Success,

src/cli/validate.rs

@@ -1,41 +1,81 @@
 use std::error::Error;
+use std::fmt::Write;
 use std::iter;
 use std::path::PathBuf;
 
 use anstream::eprintln;
+use anyhow::Result;
 use owo_colors::OwoColorize;
 
 use crate::cli::ExitStatus;
 use crate::config::{read_config, read_manifest};
+use crate::printer::Printer;
+use crate::warn_user;
 
-pub(crate) fn validate_configs(configs: Vec<PathBuf>) -> ExitStatus {
+pub(crate) fn validate_configs(configs: Vec<PathBuf>, printer: Printer) -> Result<ExitStatus> {
     let mut status = ExitStatus::Success;
 
+    if configs.is_empty() {
+        warn_user!("No configs to check");
+        return Ok(ExitStatus::Success);
+    }
+
     for config in configs {
         if let Err(err) = read_config(&config) {
-            eprintln!("{}: {}", "error".red().bold(), err);
+            writeln!(printer.stderr(), "{}: {}", "error".red().bold(), err)?;
             for source in iter::successors(err.source(), |&err| err.source()) {
-                eprintln!("  {}: {}", "caused by".red().bold(), source);
+                writeln!(
+                    printer.stderr(),
+                    "  {}: {}",
+                    "caused by".red().bold(),
+                    source
+                )?;
             }
             status = ExitStatus::Failure;
         }
     }
 
-    status
+    if status == ExitStatus::Success {
+        writeln!(
+            printer.stderr(),
+            "{}: All configs are valid",
+            "success".green().bold()
+        )?;
+    }
+
+    Ok(status)
 }
 
-pub(crate) fn validate_manifest(configs: Vec<PathBuf>) -> ExitStatus {
+pub(crate) fn validate_manifest(manifests: Vec<PathBuf>, printer: Printer) -> Result<ExitStatus> {
     let mut status = ExitStatus::Success;
 
-    for config in configs {
-        if let Err(err) = read_manifest(&config) {
-            eprintln!("{}: {}", "error".red().bold(), err);
+    if manifests.is_empty() {
+        warn_user!("No manifests to check");
+        return Ok(ExitStatus::Success);
+    }
+
+    for manifest in manifests {
+        if let Err(err) = read_manifest(&manifest) {
+            writeln!(printer.stderr(), "{}: {}", "error".red().bold(), err)?;
             for source in iter::successors(err.source(), |&err| err.source()) {
-                eprintln!("  {}: {}", "caused by".red().bold(), source);
+                writeln!(
+                    printer.stderr(),
+                    "  {}: {}",
+                    "caused by".red().bold(),
+                    source
+                )?;
             }
             status = ExitStatus::Failure;
         }
     }
 
-    status
+    if status == ExitStatus::Success {
+        writeln!(
+            printer.stderr(),
+            "{}: All manifests are valid",
+            "success".green().bold()
+        )?;
+    }
+
+    Ok(status)
 }

src/main.rs

@@ -315,12 +315,12 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
         Command::ValidateConfig(args) => {
             show_settings!(args);
 
-            Ok(cli::validate_configs(args.configs))
+            cli::validate_configs(args.configs, printer)
         }
         Command::ValidateManifest(args) => {
             show_settings!(args);
 
-            Ok(cli::validate_manifest(args.manifests))
+            cli::validate_manifest(args.manifests, printer)
         }
         Command::SampleConfig(args) => cli::sample_config(args.file, printer),
         Command::AutoUpdate(args) => {

tests/validate.rs

@@ -10,13 +10,14 @@ fn validate_config() -> anyhow::Result<()> {
     let context = TestContext::new();
 
     // No files to validate.
-    cmd_snapshot!(context.filters(), context.validate_config(), @r#"
+    cmd_snapshot!(context.filters(), context.validate_config(), @r"
     success: true
     exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-    "#);
+    warning: No configs to check
+    ");
 
     context.write_pre_commit_config(indoc::indoc! {r"
             repos:
@@ -28,13 +29,14 @@ fn validate_config() -> anyhow::Result<()> {
                   - id: check-json
         "});
     // Validate one file.
-    cmd_snapshot!(context.filters(), context.validate_config().arg(CONFIG_FILE), @r#"
+    cmd_snapshot!(context.filters(), context.validate_config().arg(CONFIG_FILE), @r"
     success: true
     exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-    "#);
+    success: All configs are valid
+    ");
 
     context
         .work_dir()
@@ -63,13 +65,14 @@ fn validate_manifest() -> anyhow::Result<()> {
     let context = TestContext::new();
 
     // No files to validate.
-    cmd_snapshot!(context.filters(), context.validate_manifest(), @r#"
+    cmd_snapshot!(context.filters(), context.validate_manifest(), @r"
     success: true
     exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-    "#);
+    warning: No manifests to check
+    ");
 
     context
         .work_dir()
@@ -84,13 +87,14 @@ fn validate_manifest() -> anyhow::Result<()> {
                 minimum_pre_commit_version: 3.2.0
         "})?;
     // Validate one file.
-    cmd_snapshot!(context.filters(), context.validate_manifest().arg(".pre-commit-hooks.yaml"), @r#"
+    cmd_snapshot!(context.filters(), context.validate_manifest().arg(".pre-commit-hooks.yaml"), @r"
     success: true
     exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-    "#);
+    success: All manifests are valid
+    ");
 
     context
         .work_dir()
@@ -143,6 +147,7 @@ fn unexpected_keys_warning() {
 
     ----- stderr -----
     warning: Ignored unexpected keys in `.pre-commit-config.yaml`: `another_unknown`, `unexpected_top_level_key`, `repos[0].unexpected_repo_key`
+    success: All configs are valid
     ");
 
     context.write_pre_commit_config(indoc::indoc! {r"
@@ -177,5 +182,6 @@ fn unexpected_keys_warning() {
       - `repos[0].hooks[0].unexpected_hook_key_2`
       - `repos[0].hooks[0].unexpected_hook_key_3`
       - `repos[0].hooks[0].unexpected_hook_key_4`
+    success: All configs are valid
     ");
 }