fix(commit): limit complete line limit to 72 chars

This commit is contained in:
2026-02-06 00:54:56 +01:00
parent 50f500e65c
commit d94ec2c5ec
4 changed files with 315 additions and 108 deletions

View File

@@ -3,14 +3,14 @@
pub struct Description(String); pub struct Description(String);
impl Description { impl Description {
pub const MAX_LENGTH: usize = 72; pub const MAX_LENGTH: usize = 50;
/// Parse and validate a description string /// Parse and validate a description string
/// ///
/// # Validation /// # Validation
/// - Trims leading/trailing whitespace /// - Trims leading/trailing whitespace
/// - Rejects empty or whitespace-only input /// - Rejects empty or whitespace-only input
/// - Validates maximum length (72 chars after trim) /// - Validates maximum length (50 chars after trim - soft limit)
pub fn parse(value: impl Into<String>) -> Result<Self, DescriptionError> { pub fn parse(value: impl Into<String>) -> Result<Self, DescriptionError> {
let value = value.into().trim().to_owned(); let value = value.into().trim().to_owned();
if value.is_empty() { if value.is_empty() {
@@ -55,7 +55,6 @@ impl std::fmt::Display for Description {
} }
} }
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
pub enum DescriptionError { pub enum DescriptionError {
#[error("Description cannot be empty")] #[error("Description cannot be empty")]
@@ -181,26 +180,32 @@ mod tests {
assert_eq!(result.unwrap().as_str(), "add multiple spaces"); assert_eq!(result.unwrap().as_str(), "add multiple spaces");
} }
/// Test that exactly 72 characters is accepted (boundary) /// Test that 72 characters (old limit) is now rejected
#[test] #[test]
fn seventy_two_characters_accepted() { fn seventy_two_characters_now_rejected() {
let desc_72 = "a".repeat(72); let desc_72 = "a".repeat(72);
let result = Description::parse(&desc_72); let result = Description::parse(&desc_72);
assert!(result.is_ok());
assert_eq!(result.unwrap().as_str().len(), 72);
}
/// Test that 73 characters is rejected
#[test]
fn seventy_three_characters_rejected() {
let desc_73 = "a".repeat(73);
let result = Description::parse(&desc_73);
assert!(result.is_err()); assert!(result.is_err());
assert_eq!( assert_eq!(
result.unwrap_err(), result.unwrap_err(),
DescriptionError::TooLong { DescriptionError::TooLong {
actual: 73, actual: 72,
max: 72 max: 50
}
);
}
/// Test that 51 characters is rejected (boundary)
#[test]
fn fifty_one_characters_rejected() {
let desc_51 = "a".repeat(51);
let result = Description::parse(&desc_51);
assert!(result.is_err());
assert_eq!(
result.unwrap_err(),
DescriptionError::TooLong {
actual: 51,
max: 50
} }
); );
} }
@@ -215,7 +220,7 @@ mod tests {
result.unwrap_err(), result.unwrap_err(),
DescriptionError::TooLong { DescriptionError::TooLong {
actual: 100, actual: 100,
max: 72 max: 50
} }
); );
} }
@@ -223,11 +228,11 @@ mod tests {
/// Test that length is checked after trimming /// Test that length is checked after trimming
#[test] #[test]
fn length_checked_after_trimming() { fn length_checked_after_trimming() {
// 72 chars + leading/trailing spaces = should be valid after trim // 50 chars + leading/trailing spaces = should be valid after trim
let desc_with_spaces = format!(" {} ", "a".repeat(72)); let desc_with_spaces = format!(" {} ", "a".repeat(50));
let result = Description::parse(&desc_with_spaces); let result = Description::parse(&desc_with_spaces);
assert!(result.is_ok()); assert!(result.is_ok());
assert_eq!(result.unwrap().as_str().len(), 72); assert_eq!(result.unwrap().as_str().len(), 50);
} }
/// Test that 50 characters is accepted without issue /// Test that 50 characters is accepted without issue
@@ -239,10 +244,10 @@ mod tests {
assert_eq!(result.unwrap().as_str().len(), 50); assert_eq!(result.unwrap().as_str().len(), 50);
} }
/// Test MAX_LENGTH constant is 72 /// Test MAX_LENGTH constant is 50 (soft limit)
#[test] #[test]
fn max_length_constant_is_72() { fn max_length_constant_is_50() {
assert_eq!(Description::MAX_LENGTH, 72); assert_eq!(Description::MAX_LENGTH, 50);
} }
/// Test as_str() returns inner string /// Test as_str() returns inner string
@@ -320,13 +325,13 @@ mod tests {
#[test] #[test]
fn too_long_error_display() { fn too_long_error_display() {
let err = DescriptionError::TooLong { let err = DescriptionError::TooLong {
actual: 73, actual: 51,
max: 72, max: 50,
}; };
let msg = format!("{}", err); let msg = format!("{}", err);
assert!(msg.contains("too long")); assert!(msg.contains("too long"));
assert!(msg.contains("73")); assert!(msg.contains("51"));
assert!(msg.contains("72")); assert!(msg.contains("50"));
} }
/// Test description with only whitespace after trim becomes empty /// Test description with only whitespace after trim becomes empty
@@ -349,25 +354,25 @@ mod tests {
/// Test description at exact boundary after trimming /// Test description at exact boundary after trimming
#[test] #[test]
fn boundary_length_after_trim() { fn boundary_length_after_trim() {
// 72 chars + 2 spaces on each side = 76 chars total, but 72 after trim // 50 chars + 2 spaces on each side = 54 chars total, but 50 after trim
let desc = format!(" {} ", "x".repeat(72)); let desc = format!(" {} ", "x".repeat(50));
let result = Description::parse(&desc); let result = Description::parse(&desc);
assert!(result.is_ok()); assert!(result.is_ok());
assert_eq!(result.unwrap().len(), 72); assert_eq!(result.unwrap().len(), 50);
} }
/// Test description just over boundary after trimming /// Test description just over boundary after trimming
#[test] #[test]
fn over_boundary_after_trim() { fn over_boundary_after_trim() {
// 73 chars + spaces = should fail even after trim // 51 chars + spaces = should fail even after trim
let desc = format!(" {} ", "x".repeat(73)); let desc = format!(" {} ", "x".repeat(51));
let result = Description::parse(&desc); let result = Description::parse(&desc);
assert!(result.is_err()); assert!(result.is_err());
assert_eq!( assert_eq!(
result.unwrap_err(), result.unwrap_err(),
DescriptionError::TooLong { DescriptionError::TooLong {
actual: 73, actual: 51,
max: 72 max: 50
} }
); );
} }

View File

@@ -1,4 +1,13 @@
use super::{CommitType, Description, Scope}; use super::{CommitType, Description, Scope};
use thiserror::Error;
/// Errors that can occur when creating a ConventionalCommit
#[derive(Debug, Clone, PartialEq, Eq, Error)]
pub enum CommitMessageError {
/// The complete first line exceeds the maximum allowed length
#[error("first line too long: {actual} characters (max {max})")]
FirstLineTooLong { actual: usize, max: usize },
}
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct ConventionalCommit { pub struct ConventionalCommit {
@@ -8,15 +17,52 @@ pub struct ConventionalCommit {
} }
impl ConventionalCommit { impl ConventionalCommit {
/// Maximum allowed length for the complete first line (type + scope + description)
pub const FIRST_LINE_MAX_LENGTH: usize = 72;
/// Create a new conventional commit message /// Create a new conventional commit message
/// ///
/// # Arguments /// # Arguments
/// All arguments are pre-validated types, so this cannot fail /// All arguments are pre-validated types, but the combined first line
pub fn new(commit_type: CommitType, scope: Scope, description: Description) -> Self { /// length is validated here (max 72 characters).
Self { ///
/// # Errors
/// Returns `CommitMessageError::FirstLineTooLong` if the formatted first
/// line exceeds 72 characters.
pub fn new(
commit_type: CommitType,
scope: Scope,
description: Description,
) -> Result<Self, CommitMessageError> {
let commit = Self {
commit_type, commit_type,
scope, scope,
description, description,
};
let len = commit.first_line_len();
if len > Self::FIRST_LINE_MAX_LENGTH {
return Err(CommitMessageError::FirstLineTooLong {
actual: len,
max: Self::FIRST_LINE_MAX_LENGTH,
});
}
Ok(commit)
}
/// Calculate the length of the formatted first line
///
/// Formula:
/// - With scope: `len(type) + len(scope) + 4 + len(description)`
/// (the 4 accounts for parentheses, colon, and space: "() ")
/// - Without scope: `len(type) + 2 + len(description)`
/// (the 2 accounts for colon and space: ": ")
pub fn first_line_len(&self) -> usize {
if self.scope.is_empty() {
// type: description
self.commit_type.as_str().len() + 2 + self.description.len()
} else {
// type(scope): description
self.commit_type.as_str().len() + self.scope.as_str().len() + 4 + self.description.len()
} }
} }
@@ -54,7 +100,6 @@ impl std::fmt::Display for ConventionalCommit {
} }
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
@@ -69,10 +114,20 @@ mod tests {
Description::parse(value).expect("test description should be valid") Description::parse(value).expect("test description should be valid")
} }
/// Helper to create a valid ConventionalCommit for testing
fn test_commit(
commit_type: CommitType,
scope: Scope,
description: Description,
) -> ConventionalCommit {
ConventionalCommit::new(commit_type, scope, description)
.expect("test commit should have valid line length")
}
/// Test that ConventionalCommit::new() creates a valid commit with all fields /// Test that ConventionalCommit::new() creates a valid commit with all fields
#[test] #[test]
fn new_creates_commit_with_all_fields() { fn new_creates_commit_with_all_fields() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("add new feature"), test_description("add new feature"),
@@ -85,7 +140,7 @@ mod tests {
/// Test that ConventionalCommit::new() works with empty scope /// Test that ConventionalCommit::new() works with empty scope
#[test] #[test]
fn new_creates_commit_with_empty_scope() { fn new_creates_commit_with_empty_scope() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Fix, CommitType::Fix,
Scope::empty(), Scope::empty(),
test_description("fix critical bug"), test_description("fix critical bug"),
@@ -98,7 +153,7 @@ mod tests {
/// Test that format() produces "type(scope): description" when scope is non-empty /// Test that format() produces "type(scope): description" when scope is non-empty
#[test] #[test]
fn format_with_scope_produces_correct_output() { fn format_with_scope_produces_correct_output() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("auth"), test_scope("auth"),
test_description("add login"), test_description("add login"),
@@ -110,7 +165,7 @@ mod tests {
#[test] #[test]
fn format_with_various_scopes() { fn format_with_various_scopes() {
// Hyphenated scope // Hyphenated scope
let commit1 = ConventionalCommit::new( let commit1 = test_commit(
CommitType::Fix, CommitType::Fix,
test_scope("user-auth"), test_scope("user-auth"),
test_description("fix token refresh"), test_description("fix token refresh"),
@@ -118,7 +173,7 @@ mod tests {
assert_eq!(commit1.format(), "fix(user-auth): fix token refresh"); assert_eq!(commit1.format(), "fix(user-auth): fix token refresh");
// Underscored scope // Underscored scope
let commit2 = ConventionalCommit::new( let commit2 = test_commit(
CommitType::Docs, CommitType::Docs,
test_scope("api_docs"), test_scope("api_docs"),
test_description("update README"), test_description("update README"),
@@ -126,7 +181,7 @@ mod tests {
assert_eq!(commit2.format(), "docs(api_docs): update README"); assert_eq!(commit2.format(), "docs(api_docs): update README");
// Scope with slash (Jira-style) // Scope with slash (Jira-style)
let commit3 = ConventionalCommit::new( let commit3 = test_commit(
CommitType::Chore, CommitType::Chore,
test_scope("PROJ-123/cleanup"), test_scope("PROJ-123/cleanup"),
test_description("remove unused code"), test_description("remove unused code"),
@@ -140,7 +195,7 @@ mod tests {
/// Test that format() produces "type: description" when scope is empty /// Test that format() produces "type: description" when scope is empty
#[test] #[test]
fn format_without_scope_produces_correct_output() { fn format_without_scope_produces_correct_output() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
Scope::empty(), Scope::empty(),
test_description("add login"), test_description("add login"),
@@ -151,14 +206,14 @@ mod tests {
/// Test format without scope for various descriptions /// Test format without scope for various descriptions
#[test] #[test]
fn format_without_scope_various_descriptions() { fn format_without_scope_various_descriptions() {
let commit1 = ConventionalCommit::new( let commit1 = test_commit(
CommitType::Fix, CommitType::Fix,
Scope::empty(), Scope::empty(),
test_description("fix critical bug"), test_description("fix critical bug"),
); );
assert_eq!(commit1.format(), "fix: fix critical bug"); assert_eq!(commit1.format(), "fix: fix critical bug");
let commit2 = ConventionalCommit::new( let commit2 = test_commit(
CommitType::Docs, CommitType::Docs,
Scope::empty(), Scope::empty(),
test_description("update installation guide"), test_description("update installation guide"),
@@ -187,7 +242,7 @@ mod tests {
]; ];
for (commit_type, expected) in expected_formats { for (commit_type, expected) in expected_formats {
let commit = ConventionalCommit::new(commit_type, scope.clone(), desc.clone()); let commit = test_commit(commit_type, scope.clone(), desc.clone());
assert_eq!( assert_eq!(
commit.format(), commit.format(),
expected, expected,
@@ -217,7 +272,7 @@ mod tests {
]; ];
for (commit_type, expected) in expected_formats { for (commit_type, expected) in expected_formats {
let commit = ConventionalCommit::new(commit_type, Scope::empty(), desc.clone()); let commit = test_commit(commit_type, Scope::empty(), desc.clone());
assert_eq!( assert_eq!(
commit.format(), commit.format(),
expected, expected,
@@ -230,7 +285,7 @@ mod tests {
/// Test that Display implementation delegates to format() /// Test that Display implementation delegates to format()
#[test] #[test]
fn display_delegates_to_format() { fn display_delegates_to_format() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("auth"), test_scope("auth"),
test_description("add login"), test_description("add login"),
@@ -243,7 +298,7 @@ mod tests {
/// Test Display with scope /// Test Display with scope
#[test] #[test]
fn display_with_scope() { fn display_with_scope() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Fix, CommitType::Fix,
test_scope("api"), test_scope("api"),
test_description("handle null response"), test_description("handle null response"),
@@ -254,7 +309,7 @@ mod tests {
/// Test Display without scope /// Test Display without scope
#[test] #[test]
fn display_without_scope() { fn display_without_scope() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Docs, CommitType::Docs,
Scope::empty(), Scope::empty(),
test_description("improve README"), test_description("improve README"),
@@ -267,11 +322,8 @@ mod tests {
fn display_equals_format_for_all_types() { fn display_equals_format_for_all_types() {
for commit_type in CommitType::all() { for commit_type in CommitType::all() {
// With scope // With scope
let commit_with_scope = ConventionalCommit::new( let commit_with_scope =
*commit_type, test_commit(*commit_type, test_scope("test"), test_description("change"));
test_scope("test"),
test_description("change"),
);
assert_eq!( assert_eq!(
format!("{}", commit_with_scope), format!("{}", commit_with_scope),
commit_with_scope.format(), commit_with_scope.format(),
@@ -281,7 +333,7 @@ mod tests {
// Without scope // Without scope
let commit_without_scope = let commit_without_scope =
ConventionalCommit::new(*commit_type, Scope::empty(), test_description("change")); test_commit(*commit_type, Scope::empty(), test_description("change"));
assert_eq!( assert_eq!(
format!("{}", commit_without_scope), format!("{}", commit_without_scope),
commit_without_scope.format(), commit_without_scope.format(),
@@ -295,8 +347,7 @@ mod tests {
#[test] #[test]
fn commit_type_accessor_returns_correct_type() { fn commit_type_accessor_returns_correct_type() {
for commit_type in CommitType::all() { for commit_type in CommitType::all() {
let commit = let commit = test_commit(*commit_type, Scope::empty(), test_description("test"));
ConventionalCommit::new(*commit_type, Scope::empty(), test_description("test"));
assert_eq!(commit.commit_type(), *commit_type); assert_eq!(commit.commit_type(), *commit_type);
} }
} }
@@ -304,7 +355,7 @@ mod tests {
/// Test scope() returns reference to scope /// Test scope() returns reference to scope
#[test] #[test]
fn scope_accessor_returns_reference() { fn scope_accessor_returns_reference() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("auth"), test_scope("auth"),
test_description("add feature"), test_description("add feature"),
@@ -315,7 +366,7 @@ mod tests {
/// Test scope() returns reference to empty scope /// Test scope() returns reference to empty scope
#[test] #[test]
fn scope_accessor_returns_empty_scope() { fn scope_accessor_returns_empty_scope() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
Scope::empty(), Scope::empty(),
test_description("add feature"), test_description("add feature"),
@@ -326,7 +377,7 @@ mod tests {
/// Test description() returns reference to description /// Test description() returns reference to description
#[test] #[test]
fn description_accessor_returns_reference() { fn description_accessor_returns_reference() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
Scope::empty(), Scope::empty(),
test_description("add new authentication flow"), test_description("add new authentication flow"),
@@ -337,7 +388,7 @@ mod tests {
/// Test Clone trait /// Test Clone trait
#[test] #[test]
fn conventional_commit_is_cloneable() { fn conventional_commit_is_cloneable() {
let original = ConventionalCommit::new( let original = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("add feature"), test_description("add feature"),
@@ -349,12 +400,12 @@ mod tests {
/// Test PartialEq trait - equal commits /// Test PartialEq trait - equal commits
#[test] #[test]
fn conventional_commit_equality() { fn conventional_commit_equality() {
let commit1 = ConventionalCommit::new( let commit1 = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("add feature"), test_description("add feature"),
); );
let commit2 = ConventionalCommit::new( let commit2 = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("add feature"), test_description("add feature"),
@@ -365,12 +416,12 @@ mod tests {
/// Test PartialEq trait - different commit types /// Test PartialEq trait - different commit types
#[test] #[test]
fn conventional_commit_inequality_different_type() { fn conventional_commit_inequality_different_type() {
let commit1 = ConventionalCommit::new( let commit1 = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("change"), test_description("change"),
); );
let commit2 = ConventionalCommit::new( let commit2 = test_commit(
CommitType::Fix, CommitType::Fix,
test_scope("cli"), test_scope("cli"),
test_description("change"), test_description("change"),
@@ -381,12 +432,12 @@ mod tests {
/// Test PartialEq trait - different scopes /// Test PartialEq trait - different scopes
#[test] #[test]
fn conventional_commit_inequality_different_scope() { fn conventional_commit_inequality_different_scope() {
let commit1 = ConventionalCommit::new( let commit1 = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("change"), test_description("change"),
); );
let commit2 = ConventionalCommit::new( let commit2 = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("api"), test_scope("api"),
test_description("change"), test_description("change"),
@@ -397,12 +448,12 @@ mod tests {
/// Test PartialEq trait - different descriptions /// Test PartialEq trait - different descriptions
#[test] #[test]
fn conventional_commit_inequality_different_description() { fn conventional_commit_inequality_different_description() {
let commit1 = ConventionalCommit::new( let commit1 = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("add feature"), test_description("add feature"),
); );
let commit2 = ConventionalCommit::new( let commit2 = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("fix bug"), test_description("fix bug"),
@@ -413,7 +464,7 @@ mod tests {
/// Test Debug trait /// Test Debug trait
#[test] #[test]
fn conventional_commit_has_debug() { fn conventional_commit_has_debug() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("cli"), test_scope("cli"),
test_description("add feature"), test_description("add feature"),
@@ -426,7 +477,7 @@ mod tests {
/// Test real-world commit message example: feature with scope /// Test real-world commit message example: feature with scope
#[test] #[test]
fn real_world_feature_with_scope() { fn real_world_feature_with_scope() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("auth"), test_scope("auth"),
test_description("implement OAuth2 login flow"), test_description("implement OAuth2 login flow"),
@@ -437,7 +488,7 @@ mod tests {
/// Test real-world commit message example: bug fix without scope /// Test real-world commit message example: bug fix without scope
#[test] #[test]
fn real_world_bugfix_without_scope() { fn real_world_bugfix_without_scope() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Fix, CommitType::Fix,
Scope::empty(), Scope::empty(),
test_description("prevent crash on empty input"), test_description("prevent crash on empty input"),
@@ -448,7 +499,7 @@ mod tests {
/// Test real-world commit message example: documentation /// Test real-world commit message example: documentation
#[test] #[test]
fn real_world_docs() { fn real_world_docs() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Docs, CommitType::Docs,
test_scope("README"), test_scope("README"),
test_description("add installation instructions"), test_description("add installation instructions"),
@@ -462,7 +513,7 @@ mod tests {
/// Test real-world commit message example: refactoring /// Test real-world commit message example: refactoring
#[test] #[test]
fn real_world_refactor() { fn real_world_refactor() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Refactor, CommitType::Refactor,
test_scope("core"), test_scope("core"),
test_description("extract validation logic"), test_description("extract validation logic"),
@@ -473,7 +524,7 @@ mod tests {
/// Test real-world commit message example: CI change /// Test real-world commit message example: CI change
#[test] #[test]
fn real_world_ci() { fn real_world_ci() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Ci, CommitType::Ci,
test_scope("github"), test_scope("github"),
test_description("add release workflow"), test_description("add release workflow"),
@@ -481,29 +532,179 @@ mod tests {
assert_eq!(commit.format(), "ci(github): add release workflow"); assert_eq!(commit.format(), "ci(github): add release workflow");
} }
/// Test commit message with maximum length description (72 chars) /// Test commit message with maximum description length (50 chars)
#[test] #[test]
fn format_with_max_length_description() { fn format_with_max_length_description() {
let long_desc = "a".repeat(72); let long_desc = "a".repeat(50);
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
Scope::empty(), Scope::empty(),
Description::parse(&long_desc).unwrap(), Description::parse(&long_desc).unwrap(),
); );
// Format should be "feat: " + 72 chars = 78 total chars // Format should be "feat: " + 50 chars = 56 total chars
let formatted = commit.format(); let formatted = commit.format();
assert!(formatted.starts_with("feat: ")); assert!(formatted.starts_with("feat: "));
assert_eq!(formatted.len(), 78); // "feat: " (6) + 72 = 78 assert_eq!(formatted.len(), 56); // "feat: " (6) + 50 = 56
} }
/// Test commit message with scope containing all valid special chars /// Test commit message with scope containing all valid special chars
#[test] #[test]
fn format_with_complex_scope() { fn format_with_complex_scope() {
let commit = ConventionalCommit::new( let commit = test_commit(
CommitType::Feat, CommitType::Feat,
test_scope("my-scope_v2/feature"), test_scope("my-scope_v2/feature"),
test_description("add support"), test_description("add support"),
); );
assert_eq!(commit.format(), "feat(my-scope_v2/feature): add support"); assert_eq!(commit.format(), "feat(my-scope_v2/feature): add support");
} }
// =========================================================================
// Line Length Validation Tests
// =========================================================================
/// Test FIRST_LINE_MAX_LENGTH constant is 72
#[test]
fn first_line_max_length_constant_is_72() {
assert_eq!(ConventionalCommit::FIRST_LINE_MAX_LENGTH, 72);
}
/// Test first_line_len() calculates correctly without scope
#[test]
fn first_line_len_without_scope() {
let commit = test_commit(
CommitType::Feat,
Scope::empty(),
test_description("add login"),
);
// "feat: add login" = 4 + 2 + 9 = 15
assert_eq!(commit.first_line_len(), 15);
}
/// Test first_line_len() calculates correctly with scope
#[test]
fn first_line_len_with_scope() {
let commit = test_commit(
CommitType::Feat,
test_scope("auth"),
test_description("add login"),
);
// "feat(auth): add login" = 4 + 4 + 4 + 9 = 21
assert_eq!(commit.first_line_len(), 21);
}
/// Test exactly 72 characters is accepted (boundary)
#[test]
fn exactly_72_characters_accepted() {
// Build a commit that's exactly 72 chars:
// feat(20chars): 44chars = 4 + 20 + 4 + 44 = 72
let scope_20 = "a".repeat(20);
let desc_44 = "b".repeat(44);
let result = ConventionalCommit::new(
CommitType::Feat,
Scope::parse(&scope_20).unwrap(),
Description::parse(&desc_44).unwrap(),
);
assert!(result.is_ok());
let commit = result.unwrap();
assert_eq!(commit.first_line_len(), 72);
}
/// Test 73 characters is rejected (boundary)
#[test]
fn seventy_three_characters_rejected() {
// Build a commit that's 73 chars:
// "refactor: " = 10 chars, so we need 63 chars of description for 73 total
// But wait, we need to account for commit type and potentially scope
// Let's use "feat" (4) + ": " (2) + 67 chars = 73
// However Description MAX_LENGTH is 50, so we need a different approach
// Use scope to pad: "refactor(scope): desc"
// refactor = 8, (scope) = 7, : = 1, space = 1, desc needed for 73
// 8 + 7 + 2 + desc = 73, so desc = 56, but max is 50
// Let's use a longer type: "refactor" (8) + longer scope
// Actually, let me use max scope (30) and appropriate description:
// type(scope): desc
// refactor(30chars): X = 8 + 30 + 4 + X = 73
// X = 73 - 42 = 31
let scope_30 = "a".repeat(30);
let desc_31 = "b".repeat(31);
let result = ConventionalCommit::new(
CommitType::Refactor,
Scope::parse(&scope_30).unwrap(),
Description::parse(&desc_31).unwrap(),
);
assert!(result.is_err());
assert_eq!(
result.unwrap_err(),
CommitMessageError::FirstLineTooLong {
actual: 73,
max: 72
}
);
}
/// Test that valid components can still exceed 72 chars when combined
#[test]
fn valid_components_can_exceed_limit() {
// Use maximum valid scope (30 chars) and a 40-char description
// refactor(30): 40 = 8 + 30 + 4 + 40 = 82 chars (exceeds 72)
let scope_30 = "a".repeat(30);
let desc_40 = "b".repeat(40);
let result = ConventionalCommit::new(
CommitType::Refactor,
Scope::parse(&scope_30).unwrap(),
Description::parse(&desc_40).unwrap(),
);
assert!(result.is_err());
assert_eq!(
result.unwrap_err(),
CommitMessageError::FirstLineTooLong {
actual: 82,
max: 72
}
);
}
/// Test short commit without scope is accepted
#[test]
fn short_commit_without_scope_accepted() {
let result = ConventionalCommit::new(
CommitType::Fix,
Scope::empty(),
test_description("quick fix"),
);
assert!(result.is_ok());
}
/// Test short commit with scope is accepted
#[test]
fn short_commit_with_scope_accepted() {
let result = ConventionalCommit::new(
CommitType::Feat,
test_scope("cli"),
test_description("add feature"),
);
assert!(result.is_ok());
}
/// Test CommitMessageError::FirstLineTooLong displays correctly
#[test]
fn first_line_too_long_error_display() {
let err = CommitMessageError::FirstLineTooLong {
actual: 80,
max: 72,
};
let msg = format!("{}", err);
assert!(msg.contains("too long"));
assert!(msg.contains("80"));
assert!(msg.contains("72"));
}
/// Test new() returns Result type
#[test]
fn new_returns_result() {
let result =
ConventionalCommit::new(CommitType::Feat, Scope::empty(), test_description("test"));
// Just verify it's a Result by using is_ok()
assert!(result.is_ok());
}
} }

View File

@@ -8,3 +8,4 @@ mod description;
pub use description::{Description, DescriptionError}; pub use description::{Description, DescriptionError};
mod message; mod message;
pub use message::{CommitMessageError, ConventionalCommit};

View File

@@ -6,7 +6,7 @@ pub struct Scope(String);
impl Scope { impl Scope {
/// Maximum allowed length for a scope /// Maximum allowed length for a scope
pub const MAX_LENGTH: usize = 50; pub const MAX_LENGTH: usize = 30;
/// Parse and validate a scope string /// Parse and validate a scope string
/// ///
@@ -14,7 +14,7 @@ impl Scope {
/// - Trims leading/trailing whitespace /// - Trims leading/trailing whitespace
/// - Empty/whitespace-only input returns empty Scope /// - Empty/whitespace-only input returns empty Scope
/// - Validates character set /// - Validates character set
/// - Validates maximum length (50 chars) /// - Validates maximum length (30 chars)
pub fn parse(value: impl Into<String>) -> Result<Self, ScopeError> { pub fn parse(value: impl Into<String>) -> Result<Self, ScopeError> {
let value: String = value.into().trim().to_owned(); let value: String = value.into().trim().to_owned();
if value.is_empty() { if value.is_empty() {
@@ -285,26 +285,26 @@ mod tests {
assert_eq!(result.unwrap_err(), ScopeError::InvalidCharacter('.')); assert_eq!(result.unwrap_err(), ScopeError::InvalidCharacter('.'));
} }
/// Test that exactly 50 characters is accepted (boundary) /// Test that exactly 30 characters is accepted (boundary)
#[test] #[test]
fn fifty_characters_accepted() { fn thirty_characters_accepted() {
let scope_50 = "a".repeat(50); let scope_30 = "a".repeat(30);
let result = Scope::parse(&scope_50); let result = Scope::parse(&scope_30);
assert!(result.is_ok()); assert!(result.is_ok());
assert_eq!(result.unwrap().as_str().len(), 50); assert_eq!(result.unwrap().as_str().len(), 30);
} }
/// Test that 51 characters is rejected /// Test that 31 characters is rejected
#[test] #[test]
fn fifty_one_characters_rejected() { fn thirty_one_characters_rejected() {
let scope_51 = "a".repeat(51); let scope_31 = "a".repeat(31);
let result = Scope::parse(&scope_51); let result = Scope::parse(&scope_31);
assert!(result.is_err()); assert!(result.is_err());
assert_eq!( assert_eq!(
result.unwrap_err(), result.unwrap_err(),
ScopeError::TooLong { ScopeError::TooLong {
actual: 51, actual: 31,
max: 50 max: 30
} }
); );
} }
@@ -319,7 +319,7 @@ mod tests {
result.unwrap_err(), result.unwrap_err(),
ScopeError::TooLong { ScopeError::TooLong {
actual: 100, actual: 100,
max: 50 max: 30
} }
); );
} }
@@ -327,17 +327,17 @@ mod tests {
/// Test that length is checked after trimming /// Test that length is checked after trimming
#[test] #[test]
fn length_checked_after_trimming() { fn length_checked_after_trimming() {
// 50 chars + leading/trailing spaces = should be valid after trim // 30 chars + leading/trailing spaces = should be valid after trim
let scope_with_spaces = format!(" {} ", "a".repeat(50)); let scope_with_spaces = format!(" {} ", "a".repeat(30));
let result = Scope::parse(&scope_with_spaces); let result = Scope::parse(&scope_with_spaces);
assert!(result.is_ok()); assert!(result.is_ok());
assert_eq!(result.unwrap().as_str().len(), 50); assert_eq!(result.unwrap().as_str().len(), 30);
} }
/// Test MAX_LENGTH constant is 50 /// Test MAX_LENGTH constant is 30
#[test] #[test]
fn max_length_constant_is_50() { fn max_length_constant_is_30() {
assert_eq!(Scope::MAX_LENGTH, 50); assert_eq!(Scope::MAX_LENGTH, 30);
} }
/// Test that empty() creates an empty Scope /// Test that empty() creates an empty Scope
@@ -432,12 +432,12 @@ mod tests {
#[test] #[test]
fn too_long_error_display() { fn too_long_error_display() {
let err = ScopeError::TooLong { let err = ScopeError::TooLong {
actual: 51, actual: 31,
max: 50, max: 30,
}; };
let msg = format!("{}", err); let msg = format!("{}", err);
assert!(msg.contains("too long")); assert!(msg.contains("too long"));
assert!(msg.contains("51")); assert!(msg.contains("31"));
assert!(msg.contains("50")); assert!(msg.contains("30"));
} }
} }