From a7e6ca4172349892cc1053302b55cfdb4ce67ad2 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 22 Sep 2024 11:06:35 +0100 Subject: [PATCH] fix: Only look for issue number within the comment Closes kemitix/forgejo-todo-checker#7 --- README.md | 11 +++++------ src/init.rs | 8 +++----- src/model/config.rs | 4 ---- src/model/tests/config.rs | 21 +-------------------- src/patterns/mod.rs | 7 +++++-- src/patterns/tests/issue.rs | 4 ++-- src/tests/init.rs | 7 +------ src/tests/mod.rs | 3 +-- src/tests/scanner.rs | 3 +-- 9 files changed, 19 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index aa14325..029b5e4 100644 --- a/README.md +++ b/README.md @@ -43,13 +43,13 @@ This Action looks for comments in the following formats: These are all considered valid comments. Pick the format that fits your language or file best. -Comments are found by matching them against this regular expression: `(#|//)\s*(TODO|FIXME)` +Comments are found by matching them against this regular expression: `(#|//)\s*(TODO|FIXME):?` -i.e.: be a comment by starting with either '#' or '//', then the word `TODO` or `FIXME` in all caps. +i.e.: be a comment by starting with either '#' or '//', then the word `TODO` or `FIXME` in all caps, with or without a trailing '`:`'. Once we have a line with such a comment we look for the Issue Number with: `\(#?(?P\d+)\)` -i.e.: a number in `()`, with or without a leading `#` (inside the braces). +i.e.: a number in '`()`', with or without a leading '`#`' (inside the braces) immediately after the '`TODO`' or '`FIXME`'. The `ISSUE_NUMBER` must correspond to an **OPEN** Issue in the repo that the Action is running against. @@ -62,9 +62,8 @@ The output will be similar to the following if there are any errors: ``` Forgejo TODO Checker! -Repo: kemitix/my-project -Prefix: (#|//)\s*(TODO|FIXME) -Issues: \(#?(?P\d+)\) +Repo : kemitix/my-project +Regex: (#|//)\s*(TODO|FIXME):?\s*\(#?(?P\d+)\) - Issue number missing: src/main.rs#38: // TODO: implement this cool feature and get rich! diff --git a/src/init.rs b/src/init.rs index 85a1cbb..1cfda9e 100644 --- a/src/init.rs +++ b/src/init.rs @@ -1,6 +1,6 @@ // use crate::model::Config; -use crate::patterns::{issue_pattern, marker_pattern}; +use crate::patterns::issue_pattern; use crate::printer::Printer; use anyhow::{Context, Result}; use kxio::fs; @@ -16,15 +16,13 @@ pub fn init_config(printer: &impl Printer, net: Network) -> Result { )) .repo(std::env::var("GITHUB_REPOSITORY").context("GITHUB_REPOSITORY")?) .server(std::env::var("GITHUB_SERVER_URL").context("GITHUB_SERVER_URL")?) - .prefix_pattern(marker_pattern()?) .issue_pattern(issue_pattern()?) .maybe_auth_token(std::env::var("REPO_TOKEN").ok()) .build(); printer.println(""); - printer.println(format!("Repo: {}", config.repo())); - printer.println(format!("Prefix: {}", config.prefix_pattern())); - printer.println(format!("Issues: {}", config.issue_pattern())); + printer.println(format!("Repo : {}", config.repo())); + printer.println(format!("Regex: {}", config.issue_pattern())); printer.println(""); Ok(config) diff --git a/src/model/config.rs b/src/model/config.rs index 84b72cb..3c29743 100644 --- a/src/model/config.rs +++ b/src/model/config.rs @@ -9,7 +9,6 @@ pub struct Config { repo: String, server: String, auth_token: Option, - prefix_pattern: Regex, issue_pattern: Regex, } impl Config { @@ -28,9 +27,6 @@ impl Config { pub fn auth_token(&self) -> Option<&str> { self.auth_token.as_deref() } - pub fn prefix_pattern(&self) -> &Regex { - &self.prefix_pattern - } pub fn issue_pattern(&self) -> &Regex { &self.issue_pattern } diff --git a/src/model/tests/config.rs b/src/model/tests/config.rs index 3a9ec5b..9e0779a 100644 --- a/src/model/tests/config.rs +++ b/src/model/tests/config.rs @@ -1,10 +1,7 @@ // use anyhow::Result; -use crate::{ - patterns::{issue_pattern, marker_pattern}, - tests::a_config, -}; +use crate::{patterns::issue_pattern, tests::a_config}; #[tokio::test] async fn with_config_get_net() -> Result<()> { @@ -37,22 +34,6 @@ fn with_config_get_fs() -> Result<()> { Ok(()) } -#[test] -fn with_config_get_prefix_pattern() -> Result<()> { - //given - let net = kxio::network::Network::new_mock(); - let fs = kxio::fs::temp()?; - let config = a_config(net, fs)?; - - //when - let result = config.prefix_pattern(); - - //then - assert_eq!(result.to_string(), marker_pattern()?.to_string()); - - Ok(()) -} - #[test] fn with_config_get_issue_pattern() -> Result<()> { //given diff --git a/src/patterns/mod.rs b/src/patterns/mod.rs index 592a9bd..873d9ca 100644 --- a/src/patterns/mod.rs +++ b/src/patterns/mod.rs @@ -5,12 +5,15 @@ mod tests; use anyhow::{Context as _, Result}; use regex::Regex; +const MARKER_RE: &str = r"(#|//)\s*(TODO|FIXME):?"; +const ISSUE_RE: &str = r"\(#?(?P\d+)\)"; + /// The pattern to find a TODO or FIXME comment pub fn marker_pattern() -> Result { - regex::Regex::new(r"(#|//)\s*(TODO|FIXME)").context("prefix regex") + regex::Regex::new(MARKER_RE).context("prefix regex") } /// The pattern to find an issue number on an already found TODO or FIXME comment pub fn issue_pattern() -> Result { - regex::Regex::new(r"\(#?(?P\d+)\)").context("issue regex") + regex::Regex::new(format!(r"{MARKER_RE}\s*{ISSUE_RE}").as_str()).context("issue regex") } diff --git a/src/patterns/tests/issue.rs b/src/patterns/tests/issue.rs index 869fe16..d8d5704 100644 --- a/src/patterns/tests/issue.rs +++ b/src/patterns/tests/issue.rs @@ -21,8 +21,8 @@ fn when_issue_should_find_number() -> Result<()> { } #[rstest::rstest] -#[case("(#13)")] -#[case("(13)")] +#[case("(41) // TODO: (#13)")] // should ignore the 41 +#[case("(#52) // FIXME (13)")] // should ignore the 52 fn find_issue_thirteen(#[case] input: &str) -> Result<()> { assert_eq!( issue_pattern()? diff --git a/src/tests/init.rs b/src/tests/init.rs index 487c9ab..750cc5a 100644 --- a/src/tests/init.rs +++ b/src/tests/init.rs @@ -4,7 +4,7 @@ use super::*; use assert2::let_assert; use kxio::network::Network; use model::Config; -use patterns::{issue_pattern, marker_pattern}; +use patterns::issue_pattern; use printer::TestPrinter; #[test] @@ -22,7 +22,6 @@ fn init_when_all_valid() -> anyhow::Result<()> { .fs(kxio::fs::new(fs.base().to_path_buf())) .repo("repo".to_string()) .server("server".to_string()) - .prefix_pattern(marker_pattern()?) .issue_pattern(issue_pattern()?) .maybe_auth_token(Some("auth".to_string())) .build(); @@ -34,10 +33,6 @@ fn init_when_all_valid() -> anyhow::Result<()> { assert_eq!(result.fs().base(), expected.fs().base()); assert_eq!(result.repo(), expected.repo()); // assert_eq!(result.server(), expected.server()); - assert_eq!( - result.prefix_pattern().to_string(), - expected.prefix_pattern().to_string() - ); assert_eq!( result.issue_pattern().to_string(), expected.issue_pattern().to_string() diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 35ea1f9..80ffef0 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -4,7 +4,7 @@ use super::*; use std::sync::{LazyLock, Mutex}; use model::Config; -use patterns::{issue_pattern, marker_pattern}; +use patterns::issue_pattern; mod init; mod printer; @@ -20,7 +20,6 @@ pub fn a_config(net: kxio::network::Network, fs: kxio::fs::FileSystem) -> Result .server("https://git.kemitix.net".to_string()) .repo("kemitix/test".to_string()) .auth_token("secret".to_string()) - .prefix_pattern(marker_pattern()?) .issue_pattern(issue_pattern()?) .build()) } diff --git a/src/tests/scanner.rs b/src/tests/scanner.rs index b73bec2..b6a30c0 100644 --- a/src/tests/scanner.rs +++ b/src/tests/scanner.rs @@ -7,7 +7,7 @@ use std::{cell::RefCell, collections::HashSet, fs::File, io::Write, path::PathBu use issues::Issue; use model::Config; -use patterns::{issue_pattern, marker_pattern}; +use patterns::issue_pattern; use pretty_assertions::assert_eq; use printer::TestPrinter; @@ -30,7 +30,6 @@ fn find_markers_in_dir() -> anyhow::Result<()> { .fs(fs.clone()) .server("".to_string()) .repo("".to_string()) - .prefix_pattern(marker_pattern()?) .issue_pattern(issue_pattern()?) .build(); let issues = HashSet::from_iter(vec![Issue::new(23), Issue::new(43)]);