feat: add output CLI argument with enum for format control

Author: DeveloperC286

Cargo.lock

@@ -19,6 +19,9 @@ clap = { version = "4.4.6", features = ["derive"] }
 log = "0.4.20"
 pretty_env_logger = "0.5.0"
 
+# For fancy output.
+ansi_term = "0.12.1"
+
 # For error handling.
 anyhow = "1.0.89"
 

Cargo.toml

@@ -22,7 +22,10 @@ def assert_git_history_is_not_clean(context):
     result = execute_clean_git_history(context)
 
     # Then
-    assert_no_output(result)
+    # Output now goes to stdout instead of stderr
+    assert result.stdout != "", "Expected standard output to contain error information.\n" + \
+        f"Standard output = {result.stdout.encode()}.\n"
+    assert_no_errors(result)
     assert_command_unsuccessful(result)
     return result
 

end-to-end-tests/features/steps/then.py

@@ -1,36 +1,63 @@
-use git2::{Oid, Repository};
+use crate::linting_results::CommitError;
 
-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
     }
-}
 
-#[cfg(test)]
-mod tests;
+    /// Lint this commit and return any linting errors found.
+    pub(crate) fn lint(&self) -> Vec<CommitError> {
+        let mut errors = Vec::new();
+
+        if self.is_merge_commit() {
+            errors.push(CommitError::MergeCommit);
+        }
+
+        errors
+    }
+}

src/commits/commit/mod.rs

@@ -1,11 +1,14 @@
-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::{
+    CommitError, CommitErrors, CommitsError, CommitsErrors, LintingResults,
+};
 
-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 +23,52 @@ 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<CommitError>> = 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);
+            }
+        }
 
-    pub fn len(&self) -> usize {
-        self.commits.len()
+        // Check for aggregate errors
+        let commits_errors = max_commits.and_then(|max| {
+            if self.commits.len() > max {
+                Some(vec![CommitsError::MaxCommitsExceeded {
+                    max_commits: max,
+                    actual_commits: self.commits.len(),
+                }])
+            } else {
+                None
+            }
+        });
+
+        // Return None if no issues found, otherwise build LintingResults
+        match (&commit_errors.is_empty(), &commits_errors) {
+            (true, None) => None,
+            (true, Some(ce)) => Some(LintingResults {
+                commit_errors: None,
+                commits_errors: Some(CommitsErrors::new(ce.clone())),
+            }),
+            (false, None) => Some(LintingResults {
+                commit_errors: Some(CommitErrors::new(self.commits.clone(), commit_errors)),
+                commits_errors: None,
+            }),
+            (false, Some(ce)) => Some(LintingResults {
+                commit_errors: Some(CommitErrors::new(self.commits.clone(), commit_errors)),
+                commits_errors: Some(CommitsErrors::new(ce.clone())),
+            }),
+        }
     }
 }
 
@@ -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/commits/commit/tests/mod.rs

@@ -0,0 +1,58 @@
+use std::collections::{HashMap, VecDeque};
+
+use crate::commits::commit::Commit;
+
+mod pretty;
+
+/// The representation of an error that an individual commit can have.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub enum CommitError {
+    /// Commit is a merge commit (has multiple parents).
+    MergeCommit,
+}
+
+/// The representation of an error for the collection of commits as a whole.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum CommitsError {
+    /// The number of commits exceeds the maximum allowed.
+    MaxCommitsExceeded {
+        max_commits: usize,
+        actual_commits: usize,
+    },
+}
+
+/// Per-commit linting errors.
+pub struct CommitErrors {
+    pub(crate) order: VecDeque<Commit>,
+    pub(crate) errors: HashMap<Commit, Vec<CommitError>>,
+}
+
+impl CommitErrors {
+    pub(crate) fn new(order: VecDeque<Commit>, errors: HashMap<Commit, Vec<CommitError>>) -> Self {
+        CommitErrors { order, errors }
+    }
+}
+
+/// Aggregate linting errors for the commits collection.
+pub struct CommitsErrors {
+    pub(crate) errors: Vec<CommitsError>,
+}
+
+impl CommitsErrors {
+    pub(crate) fn new(errors: Vec<CommitsError>) -> Self {
+        CommitsErrors { errors }
+    }
+}
+
+/// A representation of all linting errors found in the range of commits.
+pub struct LintingResults {
+    pub commit_errors: Option<CommitErrors>,
+    pub commits_errors: Option<CommitsErrors>,
+}
+
+impl LintingResults {
+    /// 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)
+    }
+}