From 53f23d9ad21d5d165ed21c60fe18e37ea6e14203 Mon Sep 17 00:00:00 2001 From: Paul Delafosse Date: Sat, 11 Sep 2021 11:10:53 +0200 Subject: [PATCH] feat!: use conventional commit parser instead of custom implementation The old custom implementation of the parser was removed we are now using the conventionl_commit_parser_crate, this breaks the api since some previously valid commit are now errored (mostly du to footer being parsed correctly) --- Cargo.toml | 1 + src/conventional/changelog.rs | 15 +-- src/conventional/commit.rs | 215 ++++------------------------------ src/conventional/version.rs | 51 ++++---- src/lib.rs | 54 ++++++--- src/log/filter.rs | 2 +- src/settings.rs | 5 +- tests/cog_bump_test.rs | 6 +- tests/cog_check_test.rs | 6 +- 9 files changed, 108 insertions(+), 247 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cb00bbe6..a732d727 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ which = "^4" lazy_static = "^1" toml = "^0" clap = { version = "^2", optional = true } +conventional_commit_parser = "0.2.0" [dev-dependencies] assert_cmd = "1.0.3" diff --git a/src/conventional/changelog.rs b/src/conventional/changelog.rs index 791ede88..560a6f59 100644 --- a/src/conventional/changelog.rs +++ b/src/conventional/changelog.rs @@ -95,7 +95,8 @@ impl Changelog { #[cfg(test)] mod test { use crate::conventional::changelog::Changelog; - use crate::conventional::commit::{Commit, CommitMessage, CommitType}; + use crate::conventional::commit::Commit; + use conventional_commit_parser::commit::{CommitType, ConventionalCommit}; use crate::OidOf; use anyhow::Result; use chrono::Utc; @@ -112,26 +113,26 @@ mod test { commits: vec![ Commit { oid: "5375e15770ddf8821d0c1ad393d315e243014c15".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::Feature, scope: None, body: None, - footer: None, - description: "this is a commit message".to_string(), + summary: "this is a commit message".to_string(), is_breaking_change: false, + footers: vec![] }, author: "coco".to_string(), date: Utc::now().naive_local(), }, Commit { oid: "5375e15770ddf8821d0c1ad393d315e243014c15".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::Feature, scope: None, body: None, - footer: None, - description: "this is an other commit message".to_string(), + summary: "this is an other commit message".to_string(), is_breaking_change: false, + footers: vec![] }, author: "cogi".to_string(), date: Utc::now().naive_local(), diff --git a/src/conventional/commit.rs b/src/conventional/commit.rs index 276642fa..18b63dc9 100644 --- a/src/conventional/commit.rs +++ b/src/conventional/commit.rs @@ -1,7 +1,5 @@ -use crate::conventional::commit::CommitType::*; use crate::error::ErrorKind::CommitFormat; use crate::AUTHORS; -use crate::COMMITS_METADATA; use crate::REMOTE_URL; use anyhow::Result; use chrono::{NaiveDateTime, Utc}; @@ -10,41 +8,16 @@ use git2::Commit as Git2Commit; use std::cmp::Ordering; use std::fmt; use std::fmt::Formatter; +use conventional_commit_parser::commit::{ConventionalCommit}; #[derive(Debug, Eq, PartialEq)] pub struct Commit { pub(crate) oid: String, - pub(crate) message: CommitMessage, + pub(crate) message: ConventionalCommit, pub(crate) author: String, pub(crate) date: NaiveDateTime, } -#[derive(Hash, Eq, PartialEq, Debug, Clone)] -pub enum CommitType { - Feature, - BugFix, - Chore, - Revert, - Performances, - Documentation, - Style, - Refactoring, - Test, - Build, - Ci, - Custom(String), -} - -#[derive(Debug, Eq, PartialEq)] -pub struct CommitMessage { - pub(crate) commit_type: CommitType, - pub(crate) scope: Option, - pub(crate) body: Option, - pub(crate) footer: Option, - pub(crate) description: String, - pub(crate) is_breaking_change: bool, -} - #[derive(Debug, Deserialize, Serialize, Clone)] pub struct CommitConfig { pub changelog_title: String, @@ -67,15 +40,20 @@ impl Commit { let message = commit.message(); let git2_message = message.unwrap().to_owned(); let author = commit.author().name().unwrap_or("").to_string(); - let message = Commit::parse_commit_message(&git2_message); - match message { - Ok(message) => Ok(Commit { - oid, - message, - author, - date, - }), + // FIXME : Why suddenly commit message start and finish with '\n' + let message = git2_message.trim_end().trim_start(); + let conventional_commit = conventional_commit_parser::parse(message); + + match conventional_commit { + Ok(message) => { + Ok(Commit { + oid, + message, + author, + date, + }) + }, Err(err) => { let additional_info = if commit.parent_count() == 0 { format!( @@ -115,86 +93,12 @@ impl Commit { } } - // Todo extract to ParseError - pub(crate) fn parse_commit_message(message: &str) -> Result { - let type_separator = message.find(": "); - ensure!( - type_separator.is_some(), - "invalid commit format: missing `{}` separator", - ": ".yellow() - ); - - let idx = type_separator.unwrap(); - - let mut type_and_scope = &message[0..idx]; - let mut is_breaking_change = type_and_scope.ends_with('!'); - - if is_breaking_change { - type_and_scope = &type_and_scope[0..type_and_scope.len() - 1]; - } - - let commit_type_str; - - let scope: Option = if let Some(left_par_idx) = type_and_scope.find('(') { - commit_type_str = &type_and_scope[0..left_par_idx]; - - Some( - type_and_scope - .find(')') - .ok_or_else(|| anyhow!("missing closing parenthesis")) - .map(|right_par_idx| { - type_and_scope[left_par_idx + 1..right_par_idx].to_string() - })?, - ) - } else { - commit_type_str = type_and_scope; - None - }; - - let contents = &message[idx + 2..message.len()]; - let contents: Vec<&str> = contents.split('\n').collect(); - - let description = contents.get(0).map(|desc| desc.to_string()); - - ensure!(description.is_some(), "missing commit description"); - - let description = description.unwrap(); - - let body = contents.get(1).map(|desc| desc.to_string()); - - let footer = contents.get(2).map(|desc| desc.to_string()); - - if let Some(footer) = &footer { - is_breaking_change = is_breaking_change - || footer.contains("BREAKING CHANGE") - || footer.contains("BREAKING-CHANGE") - } - - let commit_type = CommitType::from(commit_type_str); - let allowed_commit = COMMITS_METADATA.get(&commit_type); - - ensure!( - allowed_commit.is_some(), - "unknown commit type `{}`", - commit_type_str.red() - ); - - Ok(CommitMessage { - commit_type, - scope, - body, - footer, - description, - is_breaking_change, - }) - } - pub fn to_markdown(&self, colored: bool) -> String { if colored { format!( "{} - {} - {}\n", self.shorthand().yellow(), - self.message.description, + self.message.summary, self.author.blue() ) } else { @@ -213,14 +117,14 @@ impl Commit { format!( "{} - {} - {}\n\n", oid.unwrap_or_else(|| self.oid[0..6].into()), - self.message.description, + self.message.summary, github_author.unwrap_or_else(|| self.author.to_string()) ) } } pub fn get_log(&self) -> String { - let message_display = self.message.description.replace("\n", " "); + let message_display = self.message.summary.replace("\n", " "); let message_display = if message_display.len() > 80 { format!("{}{}", &message_display[0..80], "...").yellow() } else { @@ -298,77 +202,6 @@ impl fmt::Display for Commit { } } -impl AsRef for CommitType { - fn as_ref(&self) -> &str { - match self { - Feature => "feat", - BugFix => "fix", - Chore => "chore", - Revert => "revert", - Performances => "perf", - Documentation => "docs", - Style => "style", - Refactoring => "refactor", - Test => "test", - Build => "build", - Ci => "ci", - Custom(key) => key, - } - } -} - -impl From<&str> for CommitType { - fn from(commit_type: &str) -> Self { - match commit_type { - "feat" => Feature, - "fix" => BugFix, - "chore" => Chore, - "revert" => Revert, - "perf" => Performances, - "docs" => Documentation, - "style" => Style, - "refactor" => Refactoring, - "test" => Test, - "build" => Build, - "ci" => Ci, - other => Custom(other.to_string()), - } - } -} - -impl ToString for CommitMessage { - fn to_string(&self) -> String { - let mut message = String::new(); - message.push_str(self.commit_type.as_ref()); - - if let Some(scope) = &self.scope { - message.push_str(&format!("({})", scope)); - } - - if self.is_breaking_change { - message.push('!'); - } - - message.push_str(&format!(": {}", &self.description)); - - if let Some(body) = &self.body { - message.push_str(&format!("\n\n{}", body)); - } - - if let Some(footer) = &self.footer { - message.push_str(&format!("\n\n{}", footer)); - } - - message - } -} - -impl fmt::Display for CommitType { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.as_ref()) - } -} - impl PartialOrd for Commit { fn partial_cmp(&self, other: &Self) -> Option { self.message.scope.partial_cmp(&other.message.scope) @@ -382,7 +215,7 @@ impl Ord for Commit { } pub fn verify(author: Option, message: &str) -> Result<()> { - let commit = Commit::parse_commit_message(message); + let commit = conventional_commit_parser::parse(message); match commit { Ok(message) => { @@ -403,8 +236,8 @@ pub fn verify(author: Option, message: &str) -> Result<()> { #[cfg(test)] mod test { - use super::Commit; - use crate::conventional::commit::{verify, CommitType}; + use crate::conventional::commit::verify; + use conventional_commit_parser::commit::CommitType; #[test] fn should_map_conventional_commit_message_to_struct() { @@ -412,16 +245,16 @@ mod test { let message = "feat(database): add postgresql driver"; // Act - let commit = Commit::parse_commit_message(message); + let commit = conventional_commit_parser::parse(message); // Assert let commit = commit.unwrap(); assert_eq!(commit.commit_type, CommitType::Feature); assert_eq!(commit.scope, Some("database".to_owned())); - assert_eq!(commit.description, "add postgresql driver".to_owned()); + assert_eq!(commit.summary, "add postgresql driver".to_owned()); assert!(!commit.is_breaking_change); assert!(commit.body.is_none()); - assert!(commit.footer.is_none()); + assert!(commit.footers.is_empty()); } #[test] diff --git a/src/conventional/version.rs b/src/conventional/version.rs index 993941b4..0b2ab2a1 100644 --- a/src/conventional/version.rs +++ b/src/conventional/version.rs @@ -1,9 +1,10 @@ -use crate::conventional::commit::{Commit, CommitType}; +use crate::conventional::commit::Commit; use crate::git::repository::Repository; use anyhow::Result; use colored::*; use git2::Commit as Git2Commit; use semver::{Identifier, Version}; +use conventional_commit_parser::commit::CommitType; pub enum VersionIncrement { Major, @@ -58,8 +59,7 @@ impl VersionIncrement { let conventional_commits: Vec = commits .iter() .map(|commit| Commit::from_git_commit(commit)) - .filter(|commit| commit.is_ok()) - .map(Result::unwrap) + .filter_map(Result::ok) .collect(); VersionIncrement::get_next_auto_version(current_version, &conventional_commits) @@ -160,11 +160,12 @@ pub fn parse_pre_release(string: &str) -> Result> { #[cfg(test)] mod test { - use crate::conventional::commit::{Commit, CommitMessage, CommitType}; + use crate::conventional::commit::Commit; use crate::conventional::version::{parse_pre_release, VersionIncrement}; use anyhow::Result; use chrono::Utc; use semver::{Identifier, Version}; + use conventional_commit_parser::commit::{CommitType, ConventionalCommit}; // Auto version tests resides in test/ dir since it rely on git log // To generate the version @@ -208,13 +209,13 @@ mod test { fn should_get_next_auto_version_patch() { let patch = Commit { oid: "1234".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::BugFix, scope: None, + summary: "fix".to_string(), body: None, - footer: None, - description: "fix".to_string(), is_breaking_change: false, + footers: vec![] }, author: "".to_string(), date: Utc::now().naive_local(), @@ -230,13 +231,13 @@ mod test { fn should_get_next_auto_version_breaking_changes() { let feature = Commit { oid: "1234".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::Feature, scope: None, body: None, - footer: None, - description: "feature".to_string(), + summary: "feature".to_string(), is_breaking_change: false, + footers: vec![] }, author: "".to_string(), date: Utc::now().naive_local(), @@ -244,13 +245,13 @@ mod test { let breaking_change = Commit { oid: "1234".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::Feature, scope: None, body: None, - footer: None, - description: "feature".to_string(), + summary: "feature".to_string(), is_breaking_change: true, + footers: vec![] }, author: "".to_string(), date: Utc::now().naive_local(), @@ -268,13 +269,13 @@ mod test { fn should_get_next_auto_version_breaking_changes_on_initial_dev_version() { let feature = Commit { oid: "1234".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::Feature, scope: None, body: None, - footer: None, - description: "feature".to_string(), + summary: "feature".to_string(), is_breaking_change: false, + footers: vec![] }, author: "".to_string(), date: Utc::now().naive_local(), @@ -282,13 +283,13 @@ mod test { let breaking_change = Commit { oid: "1234".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::Feature, scope: None, body: None, - footer: None, - description: "feature".to_string(), + summary: "feature".to_string(), is_breaking_change: true, + footers: vec![] }, author: "".to_string(), date: Utc::now().naive_local(), @@ -306,13 +307,13 @@ mod test { fn should_get_next_auto_version_minor() { let patch = Commit { oid: "1234".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::BugFix, scope: None, body: None, - footer: None, - description: "fix".to_string(), + summary: "fix".to_string(), is_breaking_change: false, + footers: vec![] }, author: "".to_string(), date: Utc::now().naive_local(), @@ -320,13 +321,13 @@ mod test { let feature = Commit { oid: "1234".to_string(), - message: CommitMessage { + message: ConventionalCommit { commit_type: CommitType::Feature, scope: None, body: None, - footer: None, - description: "feature".to_string(), + summary: "feature".to_string(), is_breaking_change: false, + footers: vec![] }, author: "".to_string(), date: Utc::now().naive_local(), diff --git a/src/lib.rs b/src/lib.rs index b9d185f3..adb54596 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,12 +17,12 @@ use crate::conventional::commit::verify; use crate::error::ErrorKind::Semver; use crate::error::PreHookError; use crate::settings::{HookType, Settings}; -use anyhow::{Context, Result}; +use anyhow::{Context, Result, Error}; use chrono::Utc; use colored::*; use conventional::changelog::{Changelog, ChangelogWriter}; use conventional::commit::Commit; -use conventional::commit::{CommitConfig, CommitMessage, CommitType}; +use conventional::commit::CommitConfig; use conventional::version::{parse_pre_release, VersionIncrement}; use git::repository::Repository; use git2::{Oid, RebaseOptions}; @@ -40,6 +40,7 @@ use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{exit, Command, Stdio}; use tempfile::TempDir; +use conventional_commit_parser::commit::{CommitType, ConventionalCommit, Footer}; pub type CommitsMetadata = HashMap; @@ -112,7 +113,7 @@ pub fn init + ?Sized>(path: &S) -> Result<()> { toml::to_string(&settings) .map_err(|err| anyhow!("Failed to serialize {} : {}", CONFIG_PATH, err))?, ) - .map_err(|err| anyhow!("Could not write file `{:?}` : {}", &settings_path, err))?; + .map_err(|err| anyhow!("Could not write file `{:?}` : {}", &settings_path, err))?; } // TODO : add coco only" @@ -269,7 +270,6 @@ impl CocoGitto { Ok(()) } - #[allow(unstable_name_collisions)] 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| { @@ -282,23 +282,39 @@ impl CocoGitto { let to = self.repository.get_head_commit_oid()?; let commits = self.repository.get_commit_range(from, to)?; - let errors: Vec = commits + + let conventional_commits: Vec> = commits .iter() .filter(|commit| !commit.message().unwrap_or("").starts_with("Merge ")) - .map(|commit| Commit::from_git_commit(commit)) - .filter(|commit| commit.is_err()) - .map(|err| err.unwrap_err()) + .map(Commit::from_git_commit) + .collect(); + + let (successes, mut errors): (Vec<_>, Vec<_>) = conventional_commits + .into_iter() + .partition_result(); + + let type_errors: Vec = successes.iter() + .map(|commit| { + let commit_type = &commit.message.commit_type; + match &COMMITS_METADATA.get(commit_type) { + Some(_) => Ok(()), + None => Err(anyhow!("Commit type `{}` not allowed", commit_type)) + } + }) + .filter_map(Result::err) .collect(); + errors.extend(type_errors); + if errors.is_empty() { let msg = "No errored commits".green(); println!("{}", msg); Ok(()) } else { - let err: String = errors - .iter() - .map(|err| err.to_string()) - .intersperse("\n".to_string()) + let err: String = itertools::Itertools::intersperse( + errors + .iter() + .map(|err| err.to_string()), "\n".to_string()) .collect(); Err(anyhow!("{}", err)) } @@ -334,22 +350,26 @@ impl CocoGitto { &self, commit_type: &str, scope: Option, - description: String, + summary: String, body: Option, footer: Option, is_breaking_change: bool, ) -> Result<()> { let commit_type = CommitType::from(commit_type); - let message = CommitMessage { + let message = ConventionalCommit { commit_type, scope, body, - footer, - description, + // FIXME + footers: vec![Footer { + token: "".to_string(), + content: "".to_string(), + }], + summary, is_breaking_change, } - .to_string(); + .to_string(); let oid = self.repository.commit(&message)?; let commit = self.repository.0.find_commit(oid)?; diff --git a/src/log/filter.rs b/src/log/filter.rs index 6b7f8e79..01b1e262 100644 --- a/src/log/filter.rs +++ b/src/log/filter.rs @@ -1,6 +1,6 @@ use crate::conventional::commit::Commit; -use crate::conventional::commit::CommitType; use git2::Commit as Git2Commit; +use conventional_commit_parser::commit::CommitType; #[derive(Eq, PartialEq)] pub enum CommitFilter { diff --git a/src/settings.rs b/src/settings.rs index cac8e5aa..c1714aff 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1,10 +1,11 @@ -use crate::conventional::commit::{CommitConfig, CommitType}; +use crate::conventional::commit::CommitConfig; use crate::git::repository::Repository; use crate::{CommitsMetadata, CONFIG_PATH}; use anyhow::Result; use config::{Config, File}; use std::collections::HashMap; use std::path::PathBuf; +use conventional_commit_parser::commit::CommitType; type CommitsMetadataSettings = HashMap; pub(crate) type AuthorSettings = Vec; @@ -97,7 +98,7 @@ impl Settings { CommitConfig::new("Documentation"), ); default_types.insert(CommitType::Style, CommitConfig::new("Style")); - default_types.insert(CommitType::Refactoring, CommitConfig::new("Refactoring")); + default_types.insert(CommitType::Refactor, CommitConfig::new("Refactoring")); default_types.insert(CommitType::Test, CommitConfig::new("Tests")); default_types.insert(CommitType::Build, CommitConfig::new("Build system")); diff --git a/tests/cog_bump_test.rs b/tests/cog_bump_test.rs index f034fa7e..aa386f6a 100644 --- a/tests/cog_bump_test.rs +++ b/tests/cog_bump_test.rs @@ -2,6 +2,7 @@ use anyhow::Result; use assert_cmd::prelude::*; use std::process::Command; use tempfile::TempDir; + mod helper; #[test] @@ -17,7 +18,10 @@ fn auto_bump_from_start_ok() -> Result<()> { helper::git_commit("feat(taef): feature")?; helper::git_commit("fix: bug fix")?; - command.assert().success(); + command + .assert() + .success(); + assert!(temp_dir.path().join("CHANGELOG.md").exists()); helper::assert_tag("0.1.0")?; diff --git a/tests/cog_check_test.rs b/tests/cog_check_test.rs index 50f39268..e57e989e 100644 --- a/tests/cog_check_test.rs +++ b/tests/cog_check_test.rs @@ -30,7 +30,7 @@ fn cog_check_ok() -> Result<()> { #[test] #[cfg(not(tarpaulin))] -fn cog_check_failure() -> Result<()> { +fn cog_check_failure() -> Result<()> { let current_dir = std::env::current_dir()?; let mut command = Command::cargo_bin("cog")?; command.arg("check"); @@ -46,7 +46,7 @@ fn cog_check_failure() -> Result<()> { command .assert() .failure() - .stderr(predicate::str::contains("unknown commit type `toto`")); + .stderr(predicate::str::contains("Commit type `toto` not allowed")); Ok(std::env::set_current_dir(current_dir)?) } @@ -99,7 +99,7 @@ fn cog_check_from_latest_tag_failure() -> Result<()> { command .assert() .failure() - .stderr(predicate::str::contains("unknown commit type `toto`")); + .stderr(predicate::str::contains("Commit type `toto` not allowed")); Ok(std::env::set_current_dir(current_dir)?) }