feat: add output CLI argument with enum for format control

Author: DeveloperC286

src/commits/commit/mod.rs

@@ -1,35 +1,65 @@
-use git2::{Oid, Repository};
+use crate::linting_error::LintingError;
 
-pub(super) struct Commit {
-    commit_hash: git2::Oid,
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct Commit {
+    pub hash: String,
+    pub message: String,
     number_of_parents: usize,
 }
 
 impl Commit {
-    pub(super) fn from_git(repository: &Repository, oid: Oid) -> Result<Self, git2::Error> {
-        let commit = repository.find_commit(oid)?;
+    pub(super) fn from_git(commit: &git2::Commit) -> Commit {
         let number_of_parents = commit.parents().len();
+        let message = match commit.message().map(|m| m.to_string()) {
+            Some(message) => {
+                trace!(
+                    "Found the commit message {message:?} for the commit with the hash '{}'.",
+                    commit.id()
+                );
+                message
+            }
+            None => {
+                warn!(
+                    "Can not find commit message for the commit with the hash '{}'.",
+                    commit.id()
+                );
+                String::new()
+            }
+        };
+
         debug!(
             "The commit with the hash '{}' has {:?} parents.",
             commit.id(),
             number_of_parents,
         );
 
-        Ok(Commit {
-            commit_hash: commit.id(),
+        Commit {
+            hash: commit.id().to_string(),
+            message,
             number_of_parents,
-        })
+        }
     }
 
     pub(super) fn is_merge_commit(&self) -> bool {
         let is_merge_commit = self.number_of_parents > 1;
 
         if is_merge_commit {
-            warn!("Commit {:?} is a merge commit.", self.commit_hash);
+            warn!("Commit {:?} is a merge commit.", self.hash);
         }
 
         is_merge_commit
     }
+
+    /// Lint this commit and return any linting errors found.
+    pub(crate) fn lint(&self) -> Vec<LintingError> {
+        let mut errors = Vec::new();
+
+        if self.is_merge_commit() {
+            errors.push(LintingError::MergeCommit);
+        }
+
+        errors
+    }
 }
 
 #[cfg(test)]

src/commits/mod.rs

@@ -1,11 +1,12 @@
-use std::collections::VecDeque;
+use std::collections::{HashMap, VecDeque};
 
 use anyhow::{bail, Context, Result};
 use git2::{Oid, Repository, Revwalk};
 
-use crate::commits::commit::Commit;
+use crate::linting_results::{CommitLintingErrors, LintingResults, MaxCommitsExceeded};
 
-mod commit;
+pub mod commit;
+pub use commit::Commit;
 
 /// A representation of a range of commits within a Git repository, which can have various lints performed upon it after construction.
 pub struct Commits {
@@ -20,14 +21,54 @@ impl Commits {
         get_commits_till_head_from_oid(repository, oid)
     }
 
-    /// A lint that can be performed on the range of commits, which returns true if any of the
-    /// commits are merge commits, i.e. has multiple parents.
-    pub fn contains_merge_commits(&self) -> bool {
-        self.commits.iter().any(|commit| commit.is_merge_commit())
-    }
+    /// Lint all commits and return the linting results if any issues are found.
+    pub fn lint(&self, max_commits: Option<usize>) -> Option<LintingResults> {
+        let mut commit_errors: HashMap<Commit, Vec<crate::linting_error::LintingError>> =
+            HashMap::new();
+
+        // Check each commit for linting errors
+        for commit in self.commits.iter().cloned() {
+            let errors = commit.lint();
+
+            if !errors.is_empty() {
+                warn!(
+                    "Found {} linting errors for the commit {:?}.",
+                    errors.len(),
+                    commit.hash
+                );
+                commit_errors.insert(commit, errors);
+            }
+        }
+
+        // Check if max commits exceeded
+        let max_commits_exceeded = max_commits.and_then(|max| {
+            if self.commits.len() > max {
+                Some(MaxCommitsExceeded {
+                    max_commits: max,
+                    actual_commits: self.commits.len(),
+                })
+            } else {
+                None
+            }
+        });
 
-    pub fn len(&self) -> usize {
-        self.commits.len()
+        // Return None if no issues found
+        if commit_errors.is_empty() && max_commits_exceeded.is_none() {
+            return None;
+        }
+
+        // Build the linting results
+        Some(LintingResults {
+            commit_errors: if commit_errors.is_empty() {
+                None
+            } else {
+                Some(CommitLintingErrors::new(
+                    self.commits.clone(),
+                    commit_errors,
+                ))
+            },
+            max_commits_exceeded,
+        })
     }
 }
 
@@ -49,14 +90,17 @@ fn get_commits_till_head_from_oid(
     let revwalker = get_revwalker(repository, from_commit_hash)?;
     let mut commits = VecDeque::new();
 
-    for commit in revwalker {
-        let oid = commit?;
-
-        let commit = Commit::from_git(repository, oid)
-            .context(format!("Can not find a commit with the hash '{oid}'."))?;
+    for oid in revwalker {
+        let oid = oid?;
+        let commit = repository.find_commit(oid)?;
+        let commit = Commit::from_git(&commit);
         commits.push_front(commit);
     }
 
+    if commits.is_empty() {
+        bail!("No Git commits within the provided range.");
+    }
+
     info!("Found {} commits within the provided range.", commits.len());
     Ok(Commits { commits })
 }

src/linting_error.rs

@@ -0,0 +1,8 @@
+use serde::Serialize;
+
+/// The representation of a singular linting error a commit can have.
+#[derive(Serialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub enum LintingError {
+    /// Commit is a merge commit (has multiple parents).
+    MergeCommit,
+}

src/linting_results/mod.rs

@@ -0,0 +1,48 @@
+use std::collections::{HashMap, VecDeque};
+
+use crate::commits::commit::Commit;
+use crate::linting_error::LintingError;
+
+mod pretty;
+
+/// Information about exceeding the maximum allowed commits.
+#[derive(Debug, Clone)]
+pub struct MaxCommitsExceeded {
+    pub max_commits: usize,
+    pub actual_commits: usize,
+}
+
+/// Per-commit linting errors.
+pub struct CommitLintingErrors {
+    order: VecDeque<Commit>,
+    errors: HashMap<Commit, Vec<LintingError>>,
+}
+
+impl CommitLintingErrors {
+    pub(crate) fn new(
+        order: VecDeque<Commit>,
+        errors: HashMap<Commit, Vec<LintingError>>,
+    ) -> Self {
+        CommitLintingErrors { order, errors }
+    }
+}
+
+/// A representation of all linting errors found in the range of commits.
+pub struct LintingResults {
+    pub commit_errors: Option<CommitLintingErrors>,
+    pub max_commits_exceeded: Option<MaxCommitsExceeded>,
+}
+
+impl LintingResults {
+    pub fn new() -> Self {
+        LintingResults {
+            commit_errors: None,
+            max_commits_exceeded: None,
+        }
+    }
+
+    /// Get a pretty representation of the linting results as a string, suitable as output for a user.
+    pub fn pretty(&self) -> String {
+        pretty::print_all(self)
+    }
+}

src/linting_results/pretty.rs

@@ -0,0 +1,59 @@
+use std::fmt::Write;
+
+use ansi_term::Colour::Red;
+
+use crate::linting_error::LintingError;
+use crate::linting_results::LintingResults;
+
+pub(crate) fn print_all(results: &LintingResults) -> String {
+    let mut output = String::new();
+    let red = Red.bold();
+
+    // Print per-commit errors
+    if let Some(commit_errors) = &results.commit_errors {
+        for commit in &commit_errors.order {
+            if let Some(errors) = commit_errors.errors.get(commit) {
+                let _ = writeln!(output, "{} - {}", red.paint("Commit Hash"), commit.hash);
+                let _ = writeln!(output, "{} - {:?}", red.paint("Message"), commit.message);
+
+                for error in errors {
+                    match error {
+                        LintingError::MergeCommit => {
+                            let _ = writeln!(
+                                output,
+                                "\t{} - Commit is a merge commit.",
+                                red.paint("X")
+                            );
+                        }
+                    }
+                }
+
+                let _ = writeln!(output);
+            }
+        }
+
+        // Print summary of commit errors
+        let total_linting_errors: usize =
+            commit_errors.errors.values().map(|x| x.len()).sum();
+
+        let _ = writeln!(
+            output,
+            "{} - Found {total_linting_errors} separate linting errors across {} commits.",
+            red.paint("X"),
+            commit_errors.errors.len()
+        );
+    }
+
+    // Print max commits exceeded error
+    if let Some(max_exceeded) = &results.max_commits_exceeded {
+        let _ = writeln!(
+            output,
+            "{} - Maximum commits exceeded: found {} commits, but maximum allowed is {}.",
+            red.paint("X"),
+            max_exceeded.actual_commits,
+            max_exceeded.max_commits
+        );
+    }
+
+    output
+}

src/main.rs

@@ -2,13 +2,17 @@
 extern crate log;
 extern crate pretty_env_logger;
 
-use anyhow::{bail, Context, Result};
+use anyhow::{Context, Result};
 use clap::Parser;
 use git2::Repository;
 
 mod commits;
+mod linting_error;
+mod linting_results;
+mod output;
 
 use crate::commits::Commits;
+use crate::output::Output;
 
 const ERROR_EXIT_CODE: i32 = 1;
 
@@ -27,6 +31,13 @@ pub(crate) struct Arguments {
     )]
     pub(crate) verbose: bool,
 
+    #[arg(
+        long,
+        default_value = "default",
+        help = "Specifies the format for outputting results, acceptable values are quiet, default, and pretty."
+    )]
+    pub(crate) output: Output,
+
     #[arg(
         help = "The Git reference from where to start taking the range of commits from till HEAD to lint. The range is inclusive of HEAD and exclusive of the provided reference.",
         default_value = "origin/HEAD"
@@ -47,29 +58,34 @@ fn main() {
     info!("Version {}.", env!("CARGO_PKG_VERSION"));
     debug!("The command line arguments provided are {arguments:?}.");
 
-    if let Err(err) = run(arguments) {
-        error!("{err:?}");
-        std::process::exit(ERROR_EXIT_CODE);
+    match run(arguments) {
+        Ok(exit_code) => std::process::exit(exit_code),
+        Err(err) => {
+            error!("{err:?}");
+            std::process::exit(ERROR_EXIT_CODE);
+        }
     }
 }
 
-fn run(arguments: Arguments) -> Result<()> {
+fn run(arguments: Arguments) -> Result<i32> {
     let repository = Repository::open_from_env().context("Unable to open the Git repository.")?;
     let commits = Commits::from_git(&repository, arguments.from)?;
 
-    if commits.contains_merge_commits() {
-        bail!("Contains merge commits.");
-    }
-
-    if let Some(max_commits) = arguments.max_commits {
-        if commits.len() > max_commits {
-            bail!(format!(
-                "Exceeded the maximum number of commits {:?} with {:?} commits.",
-                arguments.max_commits,
-                commits.len()
-            ));
+    if let Some(linting_results) = commits.lint(arguments.max_commits) {
+        match arguments.output {
+            Output::Quiet => {}
+            Output::Default => {
+                println!("{}", linting_results.pretty());
+            }
+            Output::Pretty => {
+                println!("{}", linting_results.pretty());
+            }
         }
+
+        // As we don't want an error printed but linting failed so want to exit unsuccessfully.
+        return Ok(1);
     }
 
-    Ok(())
+    info!("Successfully linted all commits.");
+    Ok(0)
 }

src/output.rs

@@ -0,0 +1,8 @@
+use clap::ValueEnum;
+
+#[derive(Clone, Debug, PartialEq, ValueEnum)]
+pub enum Output {
+    Quiet,
+    Default,
+    Pretty,
+}