feat(cli): make_rule! macros
#404
Conversation
…)] to all rules & config
… handle #[doc = ...], but let's see with v1 https://graham.cool/schemars/examples/6-doc_comments/
… of #[doc] attributes
…can not convert to dash-case only snake_case
…ng incorrect docs
feat(cli): `make_rule!` macro
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on updating the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (17)
cli/src/rule/description_format.rs (2)
Line range hint
15-18: Potential panic in message() methodThe
unwrap()call onself.formatcould panic ifformatisNone. Consider usingformat.as_deref().unwrap_or("no format specified")or similar to provide a safe fallback.- "description format does not match format: {}", - self.format.as_ref().unwrap() + "description format does not match format: {}", + self.format.as_deref().unwrap_or("no format specified")
Line range hint
67-143: Add test coverage for edge casesWhile the current tests cover the main functionality well, consider adding tests for:
- Default behavior when format is None
- Case when description is None
- Empty description string
Here's a suggested test case:
#[test] fn test_default_format() { let rule = DescriptionFormat::default(); let message = Message { description: Some("Any description".to_string()), // ... other fields }; assert!(rule.validate(&message).is_none()); } #[test] fn test_none_description() { let rule = DescriptionFormat { format: Some(r"^[a-z].*".to_string()), ..Default::default() }; let message = Message { description: None, // ... other fields }; let violation = rule.validate(&message); assert!(violation.is_some()); assert_eq!(violation.unwrap().message, "found no description"); }Cargo.toml (1)
16-18: Consider the impact of settingopt-level = 3for development dependencies.Setting a higher optimization level in the development profile can increase compilation times, which may slow down the development cycle. If not strictly necessary for debugging or performance testing, you might prefer to keep the default optimization levels to maintain faster build times.
cli/src/rule/type_empty.rs (1)
4-6: Add Documentation for Macro UsageWhile the macro simplifies the code, it's important to document:
- The structure generated by the macro
- Any constraints or requirements
- The relationship with the
Ruletrait implementationConsider adding a doc comment explaining the macro's behavior and generated code.
cli/src/rule/subject_empty.rs (1)
Line range hint
18-27: Consider Enhancing Error MessagesThe validation logic is correct, but consider making the error message more descriptive by including:
- The actual value received (if any)
- Guidance on how to fix the issue
fn message(&self, _message: &Message) -> String { - "subject is empty".to_string() + "subject is empty - commit message must include a subject line".to_string() }cli/src/rule/scope_empty.rs (2)
Line range hint
44-93: Rename test methods to match their purposeThe test method names refer to "subject" but they're actually testing the scope field:
test_non_empty_subjecttests non-empty scopetest_no_subjecttests missing scopetest_empty_subjecttests empty scopeConsider renaming for clarity:
- fn test_non_empty_subject() { + fn test_non_empty_scope() { - fn test_no_subject() { + fn test_missing_scope() { - fn test_empty_subject() { + fn test_empty_scope() {
1-1: Consider additional macro improvementsThe
make_rule!macro successfully reduces boilerplate across rule implementations. Consider these potential enhancements:
- Auto-generate the
Defaultimplementation within the macro to eliminate redundant code- Include documentation generation in the macro to ensure consistent doc patterns
- Add derive macros for common traits like Debug, Clone if needed
Would you like help implementing any of these suggestions?
cli/src/rule/type_max_length.rs (1)
Line range hint
20-38: Fix incorrect handling of None type in validate methodThe current implementation returns a violation when
message.r#typeisNone, which seems incorrect. The rule should only validate the length when a type is present.Here's the suggested fix:
fn validate(&self, message: &Message) -> Option<Violation> { match &message.r#type { Some(t) => { if t.len() >= self.length { return Some(Violation { level: self.level.unwrap_or(Self::LEVEL), message: self.message(message), }); } } - None => { - return Some(Violation { - level: self.level.unwrap_or(Self::LEVEL), - message: self.message(message), - }) - } + None => None, } None }cli/src/rule/body_max_length.rs (2)
Line range hint
47-96: Add test case for None bodyThe test suite covers the cases for long and short bodies, but it would be beneficial to add a test case for when the body is None.
Here's a suggested additional test:
#[test] fn test_none_body() { let rule = BodyMaxLength::default(); let message = Message { body: None, description: Some("desc".to_string()), footers: None, r#type: Some("feat".to_string()), raw: "feat(scope): desc".to_string(), scope: Some("scope".to_string()), subject: Some("feat(scope): desc".to_string()), }; assert!(rule.validate(&message).is_none()); }
1-1: Consider creating a trait for length validationThere's a common pattern across all these length rules where they:
- Check for the presence of an optional field
- Validate its length if present
- Return None if the field is None
Consider extracting this pattern into a trait to ensure consistent behavior across all length rules and prevent the current issue of incorrect None handling.
Would you like me to provide an example implementation of such a trait?
cli/src/rule/description_empty.rs (1)
4-6: Consider adding documentation attributes to the macro invocationWhile the macro simplifies the struct definition, we should ensure documentation is preserved. Consider adding doc comments within or around the macro invocation to maintain code documentation standards.
cli/src/rule/description_max_length.rs (1)
5-8: Consider using a constant for the description parameterThe string literal "description" is used as a parameter to the macro. Consider defining this as a constant to maintain consistency and ease future updates.
+const DESCRIPTION_FIELD: &str = "description"; make_length_rule! { DescriptionMaxLength, - "description" + DESCRIPTION_FIELD }cli/src/rule/scope_format.rs (2)
4-7: Consider enhancing error handling for format validationThe macro generates a struct with format validation, but the regex compilation error handling could be improved by providing more specific error messages.
Consider updating the error message to include the invalid format pattern:
fn validate(&self, message: &Message) -> Option<Violation> { if let Some(format) = &self.format { let regex = match regex::Regex::new(format) { Ok(regex) => regex, Err(err) => { return Some(Violation { level: self.level.unwrap_or(Self::LEVEL), - message: err.to_string(), + message: format!("Invalid format pattern '{}': {}", format, err), }); } };
1-1: Architecture Review: Macro Implementation StrategyThe introduction of specialized macros (
make_rule!,make_length_rule!,make_format_rule!) effectively reduces boilerplate while maintaining clear separation between different rule types. This approach:
- Improves maintainability by centralizing common struct definitions
- Preserves type safety and compile-time checks
- Maintains clear documentation and testing patterns
Consider documenting the macro usage patterns in a central location to help future contributors.
cli/src/rule/type.rs (1)
Line range hint
25-41: Consider simplifying the validation logic.The validation logic could be more concise by combining similar conditions.
Consider this alternative implementation:
- match &message.r#type { - None => { - if self.options.is_empty() { - return None; - } - } - Some(r#type) if r#type.is_empty() => { - if self.options.is_empty() { - return None; - } - } - Some(r#type) if self.options.contains(r#type) => { - return None; - } - _ => {} - } + match &message.r#type { + None | Some(r#type) if r#type.is_empty() => { + if self.options.is_empty() { + return None; + } + } + Some(r#type) if self.options.contains(r#type) => return None, + _ => {} + }cli/json-schema/config.json (1)
15-518: Consider adding examples to rule definitions.While the schema is comprehensive, adding examples would improve the documentation.
Consider adding an
examplesfield to each rule definition, showing common configurations. For instance:"Type": { "description": "[Type] represents the [`type`](https://keisukeyamashita.github.io/commitlint-rs/rules/type) rule.", + "examples": [ + { + "level": "error", + "options": ["feat", "fix", "docs", "style", "refactor", "test", "chore"] + } + ], "type": "object",cspell.yml (1)
1-1: Improve YAML formattingLet's enhance readability and fix the missing newline at the end of file.
-words: [commitlint, schemars, commitlintrc, Keke, insta, binstall, serde, Keisuke, astrojs, splitn, insta, schemars] +words: + - commitlint + - schemars + - commitlintrc + - Keke + - insta + - binstall + - serde + - Keisuke + - astrojs + - splitn +🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockschema/src/snapshots/schema__tests__snap_json_schema.snapis excluded by!**/*.snap
📒 Files selected for processing (22)
Cargo.toml(1 hunks)cli/Cargo.toml(1 hunks)cli/json-schema/config.json(1 hunks)cli/src/rule.rs(2 hunks)cli/src/rule/body_empty.rs(1 hunks)cli/src/rule/body_max_length.rs(1 hunks)cli/src/rule/description_empty.rs(1 hunks)cli/src/rule/description_format.rs(1 hunks)cli/src/rule/description_max_length.rs(1 hunks)cli/src/rule/footers_empty.rs(1 hunks)cli/src/rule/scope.rs(1 hunks)cli/src/rule/scope_empty.rs(1 hunks)cli/src/rule/scope_format.rs(1 hunks)cli/src/rule/scope_max_length.rs(1 hunks)cli/src/rule/subject_empty.rs(1 hunks)cli/src/rule/type.rs(1 hunks)cli/src/rule/type_empty.rs(1 hunks)cli/src/rule/type_format.rs(1 hunks)cli/src/rule/type_max_length.rs(1 hunks)cspell.yml(1 hunks)schema/Cargo.toml(1 hunks)schema/src/main.rs(1 hunks)
🧰 Additional context used
🪛 GitHub Check: clippy
cli/src/rule/scope_max_length.rs
[warning] 8-8:
empty line after doc comment
[warning] 8-8:
empty line after doc comment
cli/src/rule.rs
[warning] 248-248:
crate references the macro call's crate
[warning] 262-262:
crate references the macro call's crate
[warning] 281-281:
crate references the macro call's crate
[warning] 248-248:
crate references the macro call's crate
[warning] 262-262:
crate references the macro call's crate
[warning] 281-281:
crate references the macro call's crate
🪛 yamllint (1.35.1)
cspell.yml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (22)
cli/src/rule/description_format.rs (2)
Line range hint 58-65: LGTM: Default implementation is sound
The default implementation correctly initializes the struct with appropriate default values, maintaining consistency with the Rule trait's LEVEL constant.
4-7: Verify macro expansion maintains all required fields
The use of make_format_rule! macro aligns with the PR objectives to reduce boilerplate. However, we should verify that the macro expansion generates all necessary fields that were previously defined explicitly.
✅ Verification successful
Macro expansion correctly maintains required functionality
The make_format_rule! macro expands to use make_rule! internally, generating a struct with a format: Option<String> field and appropriate documentation. This matches the standard pattern used across all format rules (TypeFormat, ScopeFormat, DescriptionFormat) and maintains the necessary functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the macro definition to understand what fields it generates
ast-grep --pattern 'macro_rules! make_format_rule {
$$$
}'
# Search for other usages to compare implementation patterns
rg -p "make_format_rule!" -A 5
Length of output: 1097
cli/src/rule/type_format.rs (1)
1-6: LGTM! The use of make_format_rule! macro simplifies the code.
Replacing the explicit TypeFormat struct definition with the make_format_rule! macro reduces boilerplate and enhances maintainability.
cli/src/rule/scope.rs (1)
4-8: LGTM! Transition to make_options_rule! macro enhances consistency.
Utilizing the make_options_rule! macro for Scope aligns it with other rule definitions, promoting uniformity and reducing repetitive code.
schema/Cargo.toml (2)
17-17: Verify the stability when upgrading to an alpha version of schemars.
Updating schemars to version 1.0.0-alpha.16 introduces an alpha release into the project, which may have unstable features or breaking changes. Please ensure this version is compatible with the existing codebase and consider potential impacts on stability.
21-21: LGTM! Adding insta as a dev-dependency for snapshot testing.
Including insta enhances the testing capabilities by allowing snapshot assertions, which can improve test coverage and reliability.
schema/src/main.rs (1)
19-28: LGTM! Addition of snapshot tests with insta improves test coverage.
The new test module effectively uses insta to create YAML snapshots of the JSON schema, ensuring that any changes to the schema are tracked and validated.
cli/Cargo.toml (1)
23-23: Caution: Using Alpha Version of schemars
The schemars dependency is set to version 1.0.0-alpha.16, which may introduce breaking changes or instability. Consider:
- Documenting any known issues or limitations
- Adding integration tests to catch potential breaking changes
- Having a rollback plan if issues arise
✅ Verification successful
Let me analyze the repository issues and gather more information about the current usage of schemars in the codebase.
Alpha Version of schemars is Appropriately Used
The alpha version of schemars (1.0.0-alpha.16) is being used in a controlled manner:
- It's an optional dependency in cli/Cargo.toml
- Used only for JSON schema generation in a separate schema crate
- Limited to deriving JsonSchema for config and rule types
- No critical runtime dependencies on its functionality
While the repository has some open issues, they are primarily feature requests and don't indicate stability concerns for the current usage pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any reported issues with the alpha version
gh api graphql -f query='
{
repository(owner: "GREsau", name: "schemars") {
issues(first: 5, states: OPEN) {
nodes {
title
body
createdAt
}
}
}
}'
Length of output: 4523
Script:
#!/bin/bash
# Check how schemars is being used in the codebase
rg "schemars" -B2 -A2
# Check if there are any test files related to schemars
fd "test.*\.(rs|toml)" --exec rg "schemars" {}
Length of output: 2985
cli/src/rule/type_empty.rs (1)
1-6: Verify Macro Implementation Consistency
The macro implementation looks good, but let's ensure consistent behavior across all rule types.
✅ Verification successful
Let me gather more information about the macro implementation and its usage pattern.
Let me check the macro definition since it wasn't found in the expected location.
Let me check the complete macro definition and its usage pattern.
Macro Implementation is Consistent Across All Rules
The make_rule! macro and its variants (make_length_rule!, make_format_rule!, make_options_rule!) are consistently implemented across all rule types in the codebase. The TypeEmpty rule follows the same pattern as other empty-check rules like SubjectEmpty, BodyEmpty, etc. Each rule correctly:
- Uses the appropriate macro variant
- Implements the
Ruletrait - Includes proper documentation
- Has consistent struct generation with
levelfield
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent macro usage across rule files
echo "Checking macro usage patterns..."
rg -l "make_rule!" cli/src/rule/
# Verify consistent struct generation
echo "Checking generated struct patterns..."
ast-grep --pattern 'impl Rule for $name {
const NAME: &'\''static str = $_;
const LEVEL: Level = $_;
$$$
}'
Length of output: 486
Script:
#!/bin/bash
# Check the macro definition and its implementation
echo "Checking macro definition:"
cat cli/src/rule/mod.rs
# Check one of the rule implementations to understand the pattern
echo -e "\nChecking TypeEmpty rule implementation:"
cat cli/src/rule/type_empty.rs
# Check another rule implementation for comparison
echo -e "\nChecking SubjectEmpty rule implementation:"
cat cli/src/rule/subject_empty.rs
Length of output: 4606
Script:
#!/bin/bash
# Search for macro definition in the entire project
echo "Searching for macro definition:"
rg "macro_rules!\s+make_rule" -A 10
# Also check if it might be in lib.rs
echo -e "\nChecking lib.rs for macro definition:"
cat cli/src/lib.rs
Length of output: 903
Script:
#!/bin/bash
# Get the complete macro definition
echo "Complete macro definition:"
cat cli/src/rule.rs
# Check all rule implementations to verify consistency
echo -e "\nChecking all rule implementations:"
for file in $(find cli/src/rule -name "*.rs" -type f); do
echo -e "\n=== $file ==="
head -n 10 "$file"
done
Length of output: 16626
cli/src/rule/subject_empty.rs (1)
5-7: LGTM: Consistent Macro Implementation
The macro usage is consistent with other rule implementations and maintains the same validation logic. The test coverage adequately verifies the behavior.
cli/src/rule/body_empty.rs (2)
4-6: LGTM! Macro implementation looks clean.
The make_rule! macro usage effectively reduces boilerplate while maintaining the rule's structure.
Line range hint 31-37: Consider removing redundant Default implementation
The Default implementation appears to be setting the same fields that should be handled by the make_rule! macro. If the macro is managing the struct fields, this implementation might be redundant.
cli/src/rule/footers_empty.rs (1)
4-6: LGTM! Consistent with other rule implementations.
The macro usage maintains consistency with other rules while preserving the specific footer validation logic.
cli/src/rule/scope_empty.rs (1)
4-6: LGTM! Consistent macro implementation.
The make_rule! macro usage aligns with the project's goal of reducing boilerplate.
cli/src/rule/scope_max_length.rs (1)
Line range hint 20-38: Fix incorrect handling of None scope in validate method
Similar to the type_max_length rule, this implementation incorrectly returns a violation when message.scope is None. The rule should only validate the length when a scope is present.
Apply the same fix as suggested for type_max_length.rs:
fn validate(&self, message: &Message) -> Option<Violation> {
match &message.scope {
Some(scope) => {
if scope.len() >= self.length {
return Some(Violation {
level: self.level.unwrap_or(Self::LEVEL),
message: self.message(message),
});
}
}
- None => {
- return Some(Violation {
- level: self.level.unwrap_or(Self::LEVEL),
- message: self.message(message),
- })
- }
+ None => None,
}
None
}🧰 Tools
🪛 GitHub Check: clippy
[warning] 8-8:
empty line after doc comment
[warning] 8-8:
empty line after doc comment
cli/src/rule/body_max_length.rs (1)
Line range hint 20-38: Fix incorrect handling of None body in validate method
This is the same issue present in the other length rules. The implementation incorrectly returns a violation when message.body is None. The rule should only validate the length when a body is present.
Apply the same fix as suggested for the other rules:
fn validate(&self, message: &Message) -> Option<Violation> {
match &message.body {
Some(body) => {
if body.len() >= self.length {
return Some(Violation {
level: self.level.unwrap_or(Self::LEVEL),
message: self.message(message),
});
}
}
- None => {
- return Some(Violation {
- level: self.level.unwrap_or(Self::LEVEL),
- message: self.message(message),
- })
- }
+ None => None,
}
None
}cli/src/rule/description_empty.rs (1)
1-1: Verify macro implementation details
The macro usage looks clean, but we should verify:
- That the generated struct maintains the same fields and visibility
- That derive macros are properly handled
Also applies to: 4-6
✅ Verification successful
Let me generate additional scripts to verify the macro usage and implementation details.
Based on the macro definition and usage evidence gathered, I can now provide a final response:
Macro implementation is correctly used and maintains consistency
The make_rule! macro generates a struct with:
- Proper derive macros (
Clone,Debug,serde::Deserialize,serde::Serialize) - Consistent visibility (pub)
- Standard
levelfield with correct type - Correct implementation of the
Ruletrait
The usage in description_empty.rs follows the expected pattern and maintains all the required structural elements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the macro definition to verify implementation
ast-grep --pattern 'macro_rules! make_rule {
$$$
}'
# Check if any other files use different macro patterns
rg -l 'make_rule!' | xargs rg 'make_rule!.*{.*}'
Length of output: 1717
Script:
#!/bin/bash
# Check how the macro is used in description_empty.rs
rg -A 10 "make_rule!" cli/src/rule/description_empty.rs
# Check the struct fields and visibility in the implementation
ast-grep --pattern 'impl Rule for DescriptionEmpty {
$$$
}'
# Check if there are any derive macros on the struct
ast-grep --pattern '#[derive($$$)]
struct DescriptionEmpty'
Length of output: 2102
cli/src/rule/scope_format.rs (1)
1-1: Verify macro consistency across rule types
Let's verify that all format rule implementations follow the same pattern and generate consistent structures.
Also applies to: 4-7
✅ Verification successful
Format rules follow consistent implementation pattern
After analyzing the codebase, all format rules (scope, type, and description) demonstrate consistent implementation patterns:
- They all use the
make_format_rule!macro with identical structure - Their
validatemethods share the same error handling pattern for regex compilation - They maintain uniform structure for violation reporting
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistency in format rule implementations
rg -l 'make_format_rule!' | xargs rg 'make_format_rule!.*{.*}'
# Verify error handling patterns across format rules
ast-grep --pattern 'impl Rule for $_ {
$$$
fn validate(&self, $_) -> Option<Violation> {
$$$
Err(err) => {
$$$
}
$$$
}
}'
Length of output: 342
Script:
#!/bin/bash
# Find files containing make_format_rule macro
rg -l "make_format_rule"
# Check the implementation pattern of make_format_rule
rg -A 10 "make_format_rule!"
# Look for Rule implementations with validate method
ast-grep --pattern 'impl Rule for $_ {
fn validate(&self, $_) -> Option<Violation> {
$$$
}
}'
Length of output: 2136
Script:
#!/bin/bash
# Let's check the macro definition and its usage pattern
rg -A 5 "macro_rules!\s+make_format_rule" cli/src/rule.rs
# Check validate method implementations in format rules
rg -A 10 "fn validate\(&self,.*\)" cli/src/rule/scope_format.rs cli/src/rule/type_format.rs cli/src/rule/description_format.rs
Length of output: 2690
cli/src/rule/type.rs (1)
4-6: LGTM! Clean macro implementation.
The make_options_rule! macro usage effectively reduces boilerplate while maintaining the same functionality. The macro generates the struct with appropriate fields and derives.
cli/src/rule.rs (2)
241-325: Well-structured macro hierarchy.
The macro design follows good practices:
- Clear hierarchy with
make_rule!as the base - Specific macros for common patterns (length, format, options)
- Consistent documentation generation
🧰 Tools
🪛 GitHub Check: clippy
[warning] 248-248:
crate references the macro call's crate
[warning] 262-262:
crate references the macro call's crate
[warning] 281-281:
crate references the macro call's crate
[warning] 248-248:
crate references the macro call's crate
[warning] 262-262:
crate references the macro call's crate
[warning] 281-281:
crate references the macro call's crate
306-307: Verify the forked paste library dependency.
The PR mentions using a forked version of the paste library for dash-case conversion.
✅ Verification successful
Forked paste dependency verified and properly configured
The codebase correctly uses the forked version of the paste library from https://github.com/elcoosp/paste with the 'feat/dash' branch, which is specifically configured for dash-case conversion. The dependency is properly declared in cli/Cargo.toml and locked to a specific commit in Cargo.lock.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the paste dependency in Cargo.toml
# Look for paste dependency
rg -A 2 'paste.*='
Length of output: 228
Script:
#!/bin/bash
# Check if there are any other references to paste crate or its features in the codebase
rg -i 'paste' -g 'Cargo.*'
# Check if there are any other macro usages of paste in the codebase
ast-grep --pattern 'paste::paste!'
Length of output: 319
cli/json-schema/config.json (1)
1-14: LGTM! Well-structured schema definition.
The schema follows JSON Schema best practices:
- Clear title and description
- Required fields properly specified
- Proper schema version reference
| [[bin]] | ||
| name = "commitlint" | ||
| path = "src/main.rs" | ||
|
|
||
| [dependencies] | ||
| paste = { git = "https://github.com/elcoosp/paste", branch = "feat/dash" } |
There was a problem hiding this comment.
Security & Maintenance Risk: Dependency on Forked Repository
Using a forked version of the paste crate introduces several risks:
- Maintenance burden if upstream changes aren't regularly merged
- Security concerns as the fork might miss security updates
- Potential build issues if the fork becomes unavailable
Consider:
- Contributing the dash-case conversion feature upstream to the official
pastecrate - Implementing the dash-case conversion locally if the upstream contribution isn't feasible
| make_length_rule! { | ||
| TypeMaxLength, | ||
| "type" |
There was a problem hiding this comment.
💡 Codebase verification
Bug: Incorrect handling of None type in validate method
The current implementation returns a violation when message.r#type is None, which is incorrect. The length validation should only be performed when a type is present. Other length rules in the codebase (body, description, scope) follow this pattern correctly.
cli/src/rule/type_max_length.rs: Remove theNonecase in thevalidatemethod's match expression as it incorrectly reports a violation when type is absent.
The macro usage itself is correct and consistent with other length rules in the codebase, but the implementation of the validate method needs to be fixed to handle the None case properly by simply returning None when no type is present.
🔗 Analysis chain
LGTM! Macro usage looks correct.
The make_length_rule! macro is used appropriately to define the TypeMaxLength rule structure, reducing boilerplate while maintaining functionality.
Let's verify the macro expansion and its usage across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of make_length_rule! macro
# and find any potential issues in macro expansion
# Find all rule files using make_length_rule
rg -l 'make_length_rule!' 'cli/src/rule/*.rs'
# Look for any potential macro expansion issues
rg -A 2 'make_length_rule!' 'cli/src/rule/*.rs'
Length of output: 213
Script:
#!/bin/bash
# Let's first find the correct path structure
fd -t f "type_max_length.rs"
# Then search for make_length_rule macro usage
rg -l "make_length_rule!"
# Look for macro definition
rg -l "macro_rules!\s+make_length_rule"
Length of output: 282
Script:
#!/bin/bash
# Let's examine the macro definition and its usage pattern
cat cli/src/rule.rs | rg -A 20 "macro_rules!\s+make_length_rule"
# Check implementation of other rules using this macro
rg -A 5 "make_length_rule!" cli/src/rule/body_max_length.rs cli/src/rule/description_max_length.rs cli/src/rule/scope_max_length.rs
# Check the complete implementation of type_max_length
cat cli/src/rule/type_max_length.rs
Length of output: 4510
Why
Remove boilerplate needed to create rule struct by using
make_rule!, this macro predefine thelevelfield and ensure the documentation is consistent.schemarsis updated to v1-alpha to allow retrieval of#[doc = ...]attrs during the json schema generation (for which a snapshot test was added viainsta).Also added
make_length_rule!,make_format_rule!&make_options_rule!Warning
This rely on a forked version of
pastesupporting dash-case conversion to get the correct documentation url of a ruleSummary by CodeRabbit
New Features
Dependency Updates
schemarsdependency version.insta.Refactor
Bug Fixes
Documentation