Skip to content

fix(cli): report error message when type scope or body empty #445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 22, 2025
Merged

fix(cli): report error message when type scope or body empty #445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3cd558b
fix(rule): report error message when type scope or body empty
xieyongbin Aug 19, 2025
ed73bbe
Merge branch 'main' into fix_max_length
KeisukeYamashita Aug 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions cli/src/rule/body_max_length.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,12 @@ impl Rule for BodyMaxLength {
}

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 => {
if let Some(body) = &message.body {
if body.len() >= self.length {
return Some(Violation {
level: self.level.unwrap_or(Self::LEVEL),
message: self.message(message),
})
});
Comment on lines +30 to +35
Copy link

@coderabbitai coderabbitai bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix boundary condition: use strict “greater than” for max-length checks

Equality with the limit should pass. Using >= causes false positives and can still flag an empty body when limit is 0.

Apply this diff:

-        if let Some(body) = &message.body {
-            if body.len() >= self.length {
+        if let Some(body) = &message.body {
+            if body.len() > self.length {
                 return Some(Violation {
                     level: self.level.unwrap_or(Self::LEVEL),
                     message: self.message(message),
                 });
             }
         }
🤖 Prompt for AI Agents 
In cli/src/rule/body_max_length.rs around lines 30 to 35, the max-length check
uses a non-strict comparison (body.len() >= self.length) which incorrectly flags
messages whose body length equals the limit (including empty body when limit is
0); change the comparison to a strict greater-than (body.len() > self.length) so
only lengths strictly greater than the configured max produce a Violation.

}
}

Expand Down
14 changes: 3 additions & 11 deletions cli/src/rule/scope_max_length.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,12 @@ impl Rule for ScopeMaxLength {
}

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 => {
if let Some(scope) = &message.scope {
if scope.len() >= self.length {
return Some(Violation {
level: self.level.unwrap_or(Self::LEVEL),
message: self.message(message),
})
});
Comment on lines +30 to +35
Copy link

@coderabbitai coderabbitai bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix boundary condition: use strict “greater than” for max-length checks

Same issue as type: equality with the limit should not violate, and length 0 shouldn’t violate when max is 0.

Apply this diff:

-        if let Some(scope) = &message.scope {
-            if scope.len() >= self.length {
+        if let Some(scope) = &message.scope {
+            if scope.len() > self.length {
                 return Some(Violation {
                     level: self.level.unwrap_or(Self::LEVEL),
                     message: self.message(message),
                 });
             }
         }
🤖 Prompt for AI Agents 
In cli/src/rule/scope_max_length.rs around lines 30 to 35, the max-length check
uses >= which incorrectly flags scopes equal to the limit (including the 0
case); change the condition to use strict greater-than so it only violates when
scope.len() > self.length (i.e., replace `if scope.len() >= self.length` with
`if scope.len() > self.length`) and keep the rest of the Violation construction
unchanged.

}
}

Expand Down
14 changes: 3 additions & 11 deletions cli/src/rule/type_max_length.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,12 @@ impl Rule for TypeMaxLength {
}

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 => {
if let Some(t) = &message.r#type {
if t.len() >= self.length {
return Some(Violation {
level: self.level.unwrap_or(Self::LEVEL),
message: self.message(message),
})
});
Comment on lines +30 to +35
Copy link

@coderabbitai coderabbitai bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix boundary condition: use strict “greater than” for max-length checks

Max-length rules should pass when the length equals the limit. Using >= incorrectly flags values exactly at the configured maximum and also causes empty strings to violate when length is 0. Switch to >.

Apply this diff:

-        if let Some(t) = &message.r#type {
-            if t.len() >= self.length {
+        if let Some(t) = &message.r#type {
+            if t.len() > self.length {
                 return Some(Violation {
                     level: self.level.unwrap_or(Self::LEVEL),
                     message: self.message(message),
                 });
             }
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if let Some(t) = &message.r#type {
if t.len() >= self.length {
return Some(Violation {
level: self.level.unwrap_or(Self::LEVEL),
message: self.message(message),
})
});
if let Some(t) = &message.r#type {
if t.len() > self.length {
return Some(Violation {
level: self.level.unwrap_or(Self::LEVEL),
message: self.message(message),
});
}
}
🤖 Prompt for AI Agents 
In cli/src/rule/type_max_length.rs around lines 30 to 35, the max-length check
currently uses >= which incorrectly flags values exactly equal to the limit (and
empty strings when limit is 0); change the conditional to use > so only lengths
strictly greater than self.length produce a Violation, keeping the rest of the
Violation construction intact.

}
}

Expand Down
Loading