diff --git a/Cargo.lock b/Cargo.lock index ce409957..9a500f5a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -179,7 +179,6 @@ dependencies = [ "shell-words", "speculoos", "tempfile", - "thiserror", "toml", "which", ] @@ -1007,26 +1006,6 @@ dependencies = [ "unicode-width", ] -[[package]] -name = "thiserror" -version = "1.0.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "602eca064b2d83369e2b2f34b09c70b605402801927c65c11071ac911d299b88" -dependencies = [ - "thiserror-impl", -] - -[[package]] -name = "thiserror-impl" -version = "1.0.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bad553cc2c78e8de258400763a647e80e6d1b31ee237275d756f6836d204494c" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "time" version = "0.1.43" diff --git a/Cargo.toml b/Cargo.toml index ed32b7ce..ea8d762a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,6 @@ and semver specifications. [dependencies] git2 = { version = "^0", default-features = false, features = [] } anyhow = "^1" -thiserror = "^1" colored = "^2" chrono = "^0" config = "^0" diff --git a/src/conventional/commit.rs b/src/conventional/commit.rs index f6984616..11debbaa 100644 --- a/src/conventional/commit.rs +++ b/src/conventional/commit.rs @@ -1,4 +1,4 @@ -use crate::error::ErrorKind::CommitFormat; +use crate::error::CocogittoError::CommitFormat; use crate::AUTHORS; use crate::REMOTE_URL; use anyhow::Result; @@ -54,26 +54,14 @@ impl Commit { date, }), Err(err) => { - let additional_info = if commit.parent_count() == 0 { - format!( - "{} Init commit or commit with no parent cannot be edited", - "warning:".yellow() - ) - } else { - "".to_string() - }; - let message = git2_message.trim_end(); - let commit_message = Commit::format_summary(message).red().to_string(); + let summary = Commit::short_summary_from_str(message); - let cause = format!("{} {}", "cause:".magenta(), err); - let level = "ERROR".red().bold().to_string(); Err(anyhow!(CommitFormat { - level, - shorthand: oid[0..6].into(), - commit_message, - additional_info, - cause + oid, + summary, + author, + cause: err.to_string() })) } } @@ -119,7 +107,7 @@ impl Commit { pub fn get_log(&self) -> String { let summary = &self.message.summary; - let message_display = Commit::format_summary(summary).yellow(); + let message_display = Commit::short_summary_from_str(summary).yellow(); let author_format = "Author:".green().bold(); let type_format = "Type:".green().bold(); let scope_format = "Scope:".green().bold(); @@ -188,7 +176,19 @@ impl Commit { } } - fn format_summary(summary: &str) -> String { + pub(crate) fn format_summary(&self) -> String { + match &self.message.scope { + None => format!("{}: {}", self.message.commit_type, self.message.summary,), + Some(scope) => { + format!( + "{}({}): {}", + self.message.commit_type, scope, self.message.summary, + ) + } + } + } + + fn short_summary_from_str(summary: &str) -> String { if summary.len() > 80 { // display a maximum of 80 char (77 char + ...) let message = summary.chars().take(77).collect::(); @@ -239,8 +239,10 @@ pub fn verify(author: Option, message: &str) -> Result<(), ParseError> { #[cfg(test)] mod test { - use crate::conventional::commit::verify; - use conventional_commit_parser::commit::CommitType; + use crate::conventional::commit::{verify, Commit}; + use chrono::NaiveDateTime; + use conventional_commit_parser::commit::{CommitType, ConventionalCommit}; + use speculoos::prelude::*; #[test] fn should_map_conventional_commit_message_to_struct() { @@ -283,4 +285,54 @@ mod test { // Assert assert!(result.is_err()); } + + #[test] + fn format_summary() { + // Arrange + let commit = Commit { + oid: "1234567".to_string(), + message: ConventionalCommit { + commit_type: CommitType::BugFix, + scope: Some("scope".to_string()), + summary: "this is the message".to_string(), + body: None, + footers: vec![], + is_breaking_change: false, + }, + + author: "".to_string(), + date: NaiveDateTime::from_timestamp(0, 0), + }; + + // Act + let summary = commit.format_summary(); + + // Assert + assert_that(&summary).is_equal_to("fix(scope): this is the message".to_string()); + } + + #[test] + fn format_summary_without_scope() { + // Arrange + let commit = Commit { + oid: "1234567".to_string(), + message: ConventionalCommit { + commit_type: CommitType::BugFix, + scope: None, + summary: "this is the message".to_string(), + body: None, + footers: vec![], + is_breaking_change: false, + }, + + author: "".to_string(), + date: NaiveDateTime::from_timestamp(0, 0), + }; + + // Act + let summary = commit.format_summary(); + + // Assert + assert_that(&summary).is_equal_to("fix: this is the message".to_string()); + } } diff --git a/src/error.rs b/src/error.rs index ea94e4aa..f3fe4cda 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,26 +1,120 @@ +use crate::OidOf; use colored::*; use std::fmt; -use std::fmt::Formatter; -use thiserror::Error; +use std::fmt::{Debug, Display, Formatter}; -#[derive(Error, Debug)] -pub(crate) enum ErrorKind { - #[error("{level} - {commit_message} - ({shorthand})\n\t{cause}\n\t{additional_info}")] +#[derive(Debug)] +pub(crate) enum CocogittoError { CommitFormat { - level: String, - shorthand: String, - commit_message: String, + oid: String, + summary: String, + author: String, cause: String, - additional_info: String, }, - #[error("On branch {branch}\nNothing to commit")] - NothingToCommitWithBranch { branch: String }, - #[error("Nothing to commit")] + CommitTypeNotAllowed { + oid: String, + summary: String, + commit_type: String, + author: String, + }, + NothingToCommitWithBranch { + branch: String, + }, NothingToCommit, - #[error("{level}:\n\t{cause}\n")] - Semver { level: String, cause: String }, - #[error("{level}:\n\t{cause}\n")] - Git { level: String, cause: String }, + Semver { + level: String, + cause: String, + }, + Git { + level: String, + cause: String, + }, +} + +impl Display for CocogittoError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + CocogittoError::CommitFormat { + summary, + oid, + author, + cause, + } => { + let error_header = "Errored commit : ".bold().red(); + let author = format!("<{}>", author).blue(); + writeln!( + f, + "{header}{oid} {author}\n\t{message_title}'{summary}'\n\t{cause_title}{cause}", + header = error_header, + oid = oid, + author = author, + message_title = "Commit message : ".yellow().bold(), + summary = summary.italic(), + cause_title = "Cause : ".yellow().bold(), + cause = cause + ) + } + CocogittoError::CommitTypeNotAllowed { + summary, + commit_type, + oid, + author, + } => { + let error_header = "Errored commit : ".bold().red(); + let author = format!("<{}>", author).blue(); + writeln!( + f, + "{header}{oid} {author}\n\t{message}'{summary}'\n\t{cause}Commit type `{commit_type}` not allowed", + header = error_header, + oid = oid, + author = author, + message = "Commit message : ".yellow().bold(), + cause = "Cause : ".yellow().bold(), + summary = summary.italic(), + commit_type = commit_type.red() + ) + } + CocogittoError::NothingToCommitWithBranch { branch } => { + writeln!(f, "On branch {}\nNothing to commit", branch) + } + CocogittoError::NothingToCommit => { + writeln!(f, "Nothing to commit") + } + CocogittoError::Semver { cause, level } => { + writeln!(f, "{}:\n\t{}\n", level, cause) + } + CocogittoError::Git { level, cause } => { + writeln!(f, "{}:\n\t{}\n", level, cause) + } + } + } +} + +#[derive(Debug)] +pub(crate) struct CogCheckReport { + pub from: OidOf, + pub errors: Vec, +} + +impl Display for CogCheckReport { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let header = format!( + "\nFound {} non compliant commits in {}..HEAD:\n", + self.errors.len(), + self.from + ) + .red() + .bold(); + + writeln!(f, "{}", header)?; + + for err in &self.errors { + let underline = format!("{:>57}", " ").underline(); + writeln!(f, "{:>5}\n", underline)?; + write!(f, "{}", err)?; + } + Ok(()) + } } // This is not meant to be unwrapped like other errors diff --git a/src/git/repository.rs b/src/git/repository.rs index 92bf3ef8..eda85811 100644 --- a/src/git/repository.rs +++ b/src/git/repository.rs @@ -9,8 +9,8 @@ use git2::{ use itertools::Itertools; use semver::Version; -use crate::error::ErrorKind; -use crate::error::ErrorKind::Git; +use crate::error::CocogittoError; +use crate::error::CocogittoError::Git; use crate::OidOf; use super::status::Statuses; @@ -83,8 +83,8 @@ impl Repository { } else { let err = self .get_branch_shorthand() - .map(|branch| ErrorKind::NothingToCommitWithBranch { branch }) - .unwrap_or_else(|| ErrorKind::NothingToCommit); + .map(|branch| CocogittoError::NothingToCommitWithBranch { branch }) + .unwrap_or_else(|| CocogittoError::NothingToCommit); Err(anyhow!(err)) } @@ -177,6 +177,10 @@ impl Repository { .map_err(|err| anyhow!("Could not resolve latest tag : {}", err)) } + pub(crate) fn get_first_commit_oidof(&self) -> Result { + self.get_first_commit().map(OidOf::Other) + } + pub(crate) fn get_first_commit(&self) -> Result { let mut revwalk = self.0.revwalk()?; revwalk.push_head()?; diff --git a/src/lib.rs b/src/lib.rs index 96b7ad34..8bbb46bc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,8 +14,8 @@ pub mod log; pub mod settings; use crate::conventional::commit::verify; -use crate::error::ErrorKind::Semver; use crate::error::PreHookError; +use crate::error::{CocogittoError, CogCheckReport}; use crate::settings::{HookType, Settings}; use anyhow::{Context, Error, Result}; use chrono::Utc; @@ -269,16 +269,18 @@ impl CocoGitto { pub fn check(&self, check_from_latest_tag: bool) -> Result<()> { let from = if check_from_latest_tag { - self.repository.get_latest_tag_oid().unwrap_or_else(|_err| { - println!("No previous tag found, falling back to first commit"); - self.repository.get_first_commit().unwrap() - }) + self.repository + .get_latest_tag_oidof() + .unwrap_or_else(|_err| { + println!("No previous tag found, falling back to first commit"); + self.repository.get_first_commit_oidof().unwrap() + }) } else { - self.repository.get_first_commit()? + self.repository.get_first_commit_oidof()? }; let to = self.repository.get_head_commit_oid()?; - let commits = self.repository.get_commit_range(from, to)?; + let commits = self.repository.get_commit_range(from.get_oid(), to)?; let (successes, mut errors): (Vec<_>, Vec<_>) = commits .iter() @@ -292,7 +294,12 @@ impl CocoGitto { let commit_type = &commit.message.commit_type; match &COMMITS_METADATA.get(commit_type) { Some(_) => Ok(()), - None => Err(anyhow!("Commit type `{}` not allowed", commit_type)), + None => Err(anyhow!(CocogittoError::CommitTypeNotAllowed { + oid: commit.oid.clone(), + summary: commit.format_summary(), + commit_type: commit.message.commit_type.to_string(), + author: commit.author.clone() + })), } }) .filter_map(Result::err) @@ -305,12 +312,8 @@ impl CocoGitto { println!("{}", msg); Ok(()) } else { - let err: String = itertools::Itertools::intersperse( - errors.iter().map(|err| err.to_string()), - "\n".to_string(), - ) - .collect(); - Err(anyhow!("{}", err)) + let report = CogCheckReport { from, errors }; + Err(anyhow!("{}", report)) } } @@ -412,7 +415,7 @@ impl CocoGitto { cause_key, comparison ); - bail!(Semver { + bail!(CocogittoError::Semver { level: "SemVer Error".red().to_string(), cause }); diff --git a/tests/cog_tests/check.rs b/tests/cog_tests/check.rs index 48fd8c72..1cca18b6 100644 --- a/tests/cog_tests/check.rs +++ b/tests/cog_tests/check.rs @@ -41,7 +41,7 @@ fn cog_check_failure() -> Result<()> { // Assert .assert() .failure() - .stderr(predicate::str::contains("Commit type `toto` not allowed")); + .stderr(predicate::str::contains("Found 1 non compliant commits")); Ok(()) }) } @@ -90,7 +90,7 @@ fn cog_check_from_latest_tag_failure() -> Result<()> { // Assert .assert() .failure() - .stderr(predicate::str::contains("Commit type `toto` not allowed")); + .stderr(predicate::str::contains("Found 1 non compliant commits")); Ok(()) }) } diff --git a/tests/lib_tests/cocogitto.rs b/tests/lib_tests/cocogitto.rs index b7fef242..88a829aa 100644 --- a/tests/lib_tests/cocogitto.rs +++ b/tests/lib_tests/cocogitto.rs @@ -152,8 +152,8 @@ fn get_unfiltered_logs() -> Result<()> { let logs = cocogitto.get_log(filters)?; // Assert - assert_that(&logs).contains("ERROR - I am afraid I can't do that Dave"); - assert_that(&logs).contains("cause: Missing commit type separator `:`"); + assert_that(&logs).contains("Commit message : 'I am afraid I can't do that Dave'"); + assert_that(&logs).contains("Cause : Missing commit type separator `:`"); Ok(()) }) @@ -175,8 +175,9 @@ fn get_log_with_no_errors() -> Result<()> { let logs = cocogitto.get_log(filters)?; // Assert - assert_that(&logs).does_not_contain("ERROR - I am afraid I can't do that Dave"); - assert_that(&logs).does_not_contain("cause: Missing commit type separator `:`"); + assert_that(&logs).does_not_contain("Errored commit : "); + assert_that(&logs).does_not_contain("Commit message : 'I am afraid I can't do that Dave'"); + assert_that(&logs).does_not_contain("Missing commit type separator `:`"); Ok(()) })