Skip to content

Commit

Permalink
fix(cli): unexpected error on "" (empty) scope or type input (#281)
Browse files Browse the repository at this point in the history 
* fix(cli): scope and scope-empty for empty scopes

Signed-off-by: KeisukeYamashita <19yamashita15@gmail.com>

* fix(cli): type and type-empty combination

Signed-off-by: KeisukeYamashita <19yamashita15@gmail.com>

---------

Signed-off-by: KeisukeYamashita <19yamashita15@gmail.com>
  • Loading branch information
@KeisukeYamashita
KeisukeYamashita authored Feb 16, 2024
1 parent 0177ec6 commit f53203d
Show file tree
Hide file tree
Showing 4 changed files with 347 additions and 226 deletions.
36 changes: 21 additions & 15 deletions src/git.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,19 @@ pub fn parse_commit_message(
/// Note that exclamation mark is not respected as the existing commitlint
/// does not have any rules for it.
/// See: https://commitlint.js.org/#/reference-rules
pub fn parse_subject(subject: &str) -> (String, Option<String>, String) {
pub fn parse_subject(subject: &str) -> (Option<String>, Option<String>, Option<String>) {
let re =
regex::Regex::new(r"^(?P<type>\w+)(?:\((?P<scope>[^\)]+)\))?(!)?\:\s(?P<description>.+)$")
.unwrap();
if let Some(captures) = re.captures(subject) {
let r#type = captures.name("type").unwrap().as_str().to_string();
let r#type = captures.name("type").map(|m| m.as_str().to_string());
let scope = captures.name("scope").map(|m| m.as_str().to_string());
let description = captures.name("description").unwrap().as_str().to_string();
let description = captures.name("description").map(|m| m.as_str().to_string());

return (r#type, scope, description);
}
// Fall back to the description.
("".to_string(), None, subject.to_string())
(None, None, Some(subject.to_string()))
}

#[cfg(test)]
Expand Down Expand Up @@ -200,9 +200,9 @@ Name: Keke";
assert_eq!(
parse_subject(input),
(
"feat".to_string(),
Some("feat".to_string()),
Some("cli".to_string()),
"add dummy option".to_string()
Some("add dummy option".to_string())
)
);
}
Expand All @@ -213,9 +213,9 @@ Name: Keke";
assert_eq!(
parse_subject(input),
(
"feat".to_string(),
Some("feat".to_string()),
Some("cli".to_string()),
"add dummy option".to_string()
Some("add dummy option".to_string())
)
);
}
Expand All @@ -225,29 +225,35 @@ Name: Keke";
let input = "feat: add dummy option";
assert_eq!(
parse_subject(input),
("feat".to_string(), None, "add dummy option".to_string())
(
Some("feat".to_string()),
None,
Some("add dummy option".to_string())
)
);
}


#[test]
fn test_parse_subject_with_emphasized_type_without_scope() {
let input = "feat!: add dummy option";
assert_eq!(
parse_subject(input),
("feat".to_string(), None, "add dummy option".to_string())
(
Some("feat".to_string()),
None,
Some("add dummy option".to_string())
)
);
}
#[test]
fn test_parse_subject_without_message() {
let input = "";
assert_eq!(parse_subject(input), ("".to_string(), None, "".to_string()));
assert_eq!(parse_subject(input), (None, None, Some("".to_string())));
}
#[test]
fn test_parse_subject_with_error_message() {
let input = "test";
assert_eq!(
parse_subject(input),
("".to_string(), None, "test".to_string())
);
assert_eq!(parse_subject(input), (None, None, Some("test".to_string())));
}
}
4 changes: 2 additions & 2 deletions src/message.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ impl Message {
let (r#type, scope, description) = parse_subject(&subject);
Self {
body,
description: Some(description),
description,
footers,
raw,
r#type: Some(r#type),
r#type,
scope,
subject: Some(subject),
}
Expand Down
248 changes: 163 additions & 85 deletions src/rule/scope.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,21 @@ impl Rule for Scope {
}

fn validate(&self, message: &Message) -> Option<Violation> {
if let Some(scope) = &message.scope {
if self.options.contains(scope) {
match &message.scope {
None => {
if self.options.is_empty() {
return None;
}
}
Some(scope) if scope.is_empty() => {
if self.options.is_empty() {
return None;
}
}
Some(scope) if self.options.contains(scope) => {
return None;
}
_ => {}
}

Some(Violation {
Expand All @@ -62,92 +73,159 @@ impl Default for Scope {
mod tests {
use super::*;

#[test]
fn test_empty_scope() {
let mut rule = Scope::default();
rule.options = vec!["api".to_string(), "web".to_string()];

let message = Message {
body: None,
description: None,
footers: None,
r#type: None,
raw: "".to_string(),
scope: None,
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(
violation.unwrap().message,
"scope is not allowed. Only [\"api\", \"web\"] are allowed"
);
}
mod empty_options {
use super::*;

#[test]
fn test_empty_scope() {
let rule = Scope::default();

let message = Message {
body: None,
description: None,
footers: None,
r#type: None,
raw: "".to_string(),
scope: Some("".to_string()),
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_none());
}

#[test]
fn test_valid_scope() {
let mut rule = Scope::default();
rule.options = vec!["api".to_string(), "web".to_string()];

let message = Message {
body: None,
description: None,
footers: None,
r#type: Some("feat".to_string()),
raw: "feat(web): broadcast $destroy event on scope destruction".to_string(),
scope: Some("web".to_string()),
subject: None,
};

assert!(rule.validate(&message).is_none());
}
#[test]
fn test_none_scope() {
let rule = Scope::default();

let message = Message {
body: None,
description: None,
footers: None,
r#type: None,
raw: "".to_string(),
scope: None,
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_none());
}

#[test]
fn test_invalid_scope() {
let mut rule = Scope::default();
rule.options = vec!["api".to_string(), "web".to_string()];

let message = Message {
body: None,
description: None,
footers: None,
r#type: Some("feat".to_string()),
raw: "feat(invalid): broadcast $destroy event on scope destruction".to_string(),
scope: Some("invalid".to_string()),
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(
violation.unwrap().message,
"scope invalid is not allowed. Only [\"api\", \"web\"] are allowed".to_string()
);
#[test]
fn test_scope() {
let rule = Scope::default();

let message = Message {
body: None,
description: None,
footers: None,
r#type: Some("feat".to_string()),
raw: "feat(web): broadcast $destroy event on scope destruction".to_string(),
scope: Some("web".to_string()),
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(
violation.unwrap().message,
"scopes are not allowed".to_string()
);
}
}

#[test]
fn test_no_options() {
let rule = Scope::default();

let message = Message {
body: None,
description: None,
footers: None,
r#type: Some("feat".to_string()),
raw: "feat(invalid): broadcast $destroy event on scope destruction".to_string(),
scope: Some("invalid".to_string()),
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(
violation.unwrap().message,
"scopes are not allowed".to_string()
);
mod scopes {
use super::*;
#[test]
fn test_empty_scope() {
let mut rule = Scope::default();
rule.options = vec!["api".to_string(), "web".to_string()];

let message = Message {
body: None,
description: None,
footers: None,
r#type: None,
raw: "".to_string(),
scope: Some("".to_string()),
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(
violation.unwrap().message,
"scope is not allowed. Only [\"api\", \"web\"] are allowed"
);
}

#[test]
fn test_none_scope() {
let mut rule = Scope::default();
rule.options = vec!["api".to_string(), "web".to_string()];

let message = Message {
body: None,
description: None,
footers: None,
r#type: None,
raw: "".to_string(),
scope: None,
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(
violation.unwrap().message,
"scope is not allowed. Only [\"api\", \"web\"] are allowed".to_string()
);
}

#[test]
fn test_valid_scope() {
let mut rule = Scope::default();
rule.options = vec!["api".to_string(), "web".to_string()];

let message = Message {
body: None,
description: None,
footers: None,
r#type: Some("feat".to_string()),
raw: "feat(web): broadcast $destroy event on scope destruction".to_string(),
scope: Some("web".to_string()),
subject: None,
};

assert!(rule.validate(&message).is_none());
}

#[test]
fn test_invalid_scope() {
let mut rule = Scope::default();
rule.options = vec!["api".to_string(), "web".to_string()];

let message = Message {
body: None,
description: None,
footers: None,
r#type: Some("feat".to_string()),
raw: "feat(invalid): broadcast $destroy event on scope destruction".to_string(),
scope: Some("invalid".to_string()),
subject: None,
};

let violation = rule.validate(&message);
assert!(violation.is_some());
assert_eq!(violation.clone().unwrap().level, Level::Error);
assert_eq!(
violation.unwrap().message,
"scope invalid is not allowed. Only [\"api\", \"web\"] are allowed".to_string()
);
}
}
}
Loading

0 comments on commit f53203d

Please sign in to comment.