fix: Only look for issue number within the comment
All checks were successful
Test / checks (map[name:nightly]) (push) Successful in 3m58s
Test / checks (map[name:stable]) (push) Successful in 1m24s

Closes kemitix/forgejo-todo-checker#7
This commit is contained in:
Paul Campbell 2024-09-22 11:06:35 +01:00
parent 1bd6d1adb0
commit a7e6ca4172
9 changed files with 19 additions and 49 deletions

View file

@ -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. 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<ISSUE_NUMBER>\d+)\)` Once we have a line with such a comment we look for the Issue Number with: `\(#?(?P<ISSUE_NUMBER>\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. 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! Forgejo TODO Checker!
Repo: kemitix/my-project Repo : kemitix/my-project
Prefix: (#|//)\s*(TODO|FIXME) Regex: (#|//)\s*(TODO|FIXME):?\s*\(#?(?P<ISSUE_NUMBER>\d+)\)
Issues: \(#?(?P<ISSUE_NUMBER>\d+)\)
- Issue number missing: src/main.rs#38: - Issue number missing: src/main.rs#38:
// TODO: implement this cool feature and get rich! // TODO: implement this cool feature and get rich!

View file

@ -1,6 +1,6 @@
// //
use crate::model::Config; use crate::model::Config;
use crate::patterns::{issue_pattern, marker_pattern}; use crate::patterns::issue_pattern;
use crate::printer::Printer; use crate::printer::Printer;
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use kxio::fs; use kxio::fs;
@ -16,15 +16,13 @@ pub fn init_config(printer: &impl Printer, net: Network) -> Result<Config> {
)) ))
.repo(std::env::var("GITHUB_REPOSITORY").context("GITHUB_REPOSITORY")?) .repo(std::env::var("GITHUB_REPOSITORY").context("GITHUB_REPOSITORY")?)
.server(std::env::var("GITHUB_SERVER_URL").context("GITHUB_SERVER_URL")?) .server(std::env::var("GITHUB_SERVER_URL").context("GITHUB_SERVER_URL")?)
.prefix_pattern(marker_pattern()?)
.issue_pattern(issue_pattern()?) .issue_pattern(issue_pattern()?)
.maybe_auth_token(std::env::var("REPO_TOKEN").ok()) .maybe_auth_token(std::env::var("REPO_TOKEN").ok())
.build(); .build();
printer.println(""); printer.println("");
printer.println(format!("Repo: {}", config.repo())); printer.println(format!("Repo : {}", config.repo()));
printer.println(format!("Prefix: {}", config.prefix_pattern())); printer.println(format!("Regex: {}", config.issue_pattern()));
printer.println(format!("Issues: {}", config.issue_pattern()));
printer.println(""); printer.println("");
Ok(config) Ok(config)

View file

@ -9,7 +9,6 @@ pub struct Config {
repo: String, repo: String,
server: String, server: String,
auth_token: Option<String>, auth_token: Option<String>,
prefix_pattern: Regex,
issue_pattern: Regex, issue_pattern: Regex,
} }
impl Config { impl Config {
@ -28,9 +27,6 @@ impl Config {
pub fn auth_token(&self) -> Option<&str> { pub fn auth_token(&self) -> Option<&str> {
self.auth_token.as_deref() self.auth_token.as_deref()
} }
pub fn prefix_pattern(&self) -> &Regex {
&self.prefix_pattern
}
pub fn issue_pattern(&self) -> &Regex { pub fn issue_pattern(&self) -> &Regex {
&self.issue_pattern &self.issue_pattern
} }

View file

@ -1,10 +1,7 @@
// //
use anyhow::Result; use anyhow::Result;
use crate::{ use crate::{patterns::issue_pattern, tests::a_config};
patterns::{issue_pattern, marker_pattern},
tests::a_config,
};
#[tokio::test] #[tokio::test]
async fn with_config_get_net() -> Result<()> { async fn with_config_get_net() -> Result<()> {
@ -37,22 +34,6 @@ fn with_config_get_fs() -> Result<()> {
Ok(()) 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] #[test]
fn with_config_get_issue_pattern() -> Result<()> { fn with_config_get_issue_pattern() -> Result<()> {
//given //given

View file

@ -5,12 +5,15 @@ mod tests;
use anyhow::{Context as _, Result}; use anyhow::{Context as _, Result};
use regex::Regex; use regex::Regex;
const MARKER_RE: &str = r"(#|//)\s*(TODO|FIXME):?";
const ISSUE_RE: &str = r"\(#?(?P<ISSUE_NUMBER>\d+)\)";
/// The pattern to find a TODO or FIXME comment /// The pattern to find a TODO or FIXME comment
pub fn marker_pattern() -> Result<Regex> { pub fn marker_pattern() -> Result<Regex> {
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 /// The pattern to find an issue number on an already found TODO or FIXME comment
pub fn issue_pattern() -> Result<Regex> { pub fn issue_pattern() -> Result<Regex> {
regex::Regex::new(r"\(#?(?P<ISSUE_NUMBER>\d+)\)").context("issue regex") regex::Regex::new(format!(r"{MARKER_RE}\s*{ISSUE_RE}").as_str()).context("issue regex")
} }

View file

@ -21,8 +21,8 @@ fn when_issue_should_find_number() -> Result<()> {
} }
#[rstest::rstest] #[rstest::rstest]
#[case("(#13)")] #[case("(41) // TODO: (#13)")] // should ignore the 41
#[case("(13)")] #[case("(#52) // FIXME (13)")] // should ignore the 52
fn find_issue_thirteen(#[case] input: &str) -> Result<()> { fn find_issue_thirteen(#[case] input: &str) -> Result<()> {
assert_eq!( assert_eq!(
issue_pattern()? issue_pattern()?

View file

@ -4,7 +4,7 @@ use super::*;
use assert2::let_assert; use assert2::let_assert;
use kxio::network::Network; use kxio::network::Network;
use model::Config; use model::Config;
use patterns::{issue_pattern, marker_pattern}; use patterns::issue_pattern;
use printer::TestPrinter; use printer::TestPrinter;
#[test] #[test]
@ -22,7 +22,6 @@ fn init_when_all_valid() -> anyhow::Result<()> {
.fs(kxio::fs::new(fs.base().to_path_buf())) .fs(kxio::fs::new(fs.base().to_path_buf()))
.repo("repo".to_string()) .repo("repo".to_string())
.server("server".to_string()) .server("server".to_string())
.prefix_pattern(marker_pattern()?)
.issue_pattern(issue_pattern()?) .issue_pattern(issue_pattern()?)
.maybe_auth_token(Some("auth".to_string())) .maybe_auth_token(Some("auth".to_string()))
.build(); .build();
@ -34,10 +33,6 @@ fn init_when_all_valid() -> anyhow::Result<()> {
assert_eq!(result.fs().base(), expected.fs().base()); assert_eq!(result.fs().base(), expected.fs().base());
assert_eq!(result.repo(), expected.repo()); assert_eq!(result.repo(), expected.repo());
// assert_eq!(result.server(), expected.server()); // assert_eq!(result.server(), expected.server());
assert_eq!(
result.prefix_pattern().to_string(),
expected.prefix_pattern().to_string()
);
assert_eq!( assert_eq!(
result.issue_pattern().to_string(), result.issue_pattern().to_string(),
expected.issue_pattern().to_string() expected.issue_pattern().to_string()

View file

@ -4,7 +4,7 @@ use super::*;
use std::sync::{LazyLock, Mutex}; use std::sync::{LazyLock, Mutex};
use model::Config; use model::Config;
use patterns::{issue_pattern, marker_pattern}; use patterns::issue_pattern;
mod init; mod init;
mod printer; 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()) .server("https://git.kemitix.net".to_string())
.repo("kemitix/test".to_string()) .repo("kemitix/test".to_string())
.auth_token("secret".to_string()) .auth_token("secret".to_string())
.prefix_pattern(marker_pattern()?)
.issue_pattern(issue_pattern()?) .issue_pattern(issue_pattern()?)
.build()) .build())
} }

View file

@ -7,7 +7,7 @@ use std::{cell::RefCell, collections::HashSet, fs::File, io::Write, path::PathBu
use issues::Issue; use issues::Issue;
use model::Config; use model::Config;
use patterns::{issue_pattern, marker_pattern}; use patterns::issue_pattern;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use printer::TestPrinter; use printer::TestPrinter;
@ -30,7 +30,6 @@ fn find_markers_in_dir() -> anyhow::Result<()> {
.fs(fs.clone()) .fs(fs.clone())
.server("".to_string()) .server("".to_string())
.repo("".to_string()) .repo("".to_string())
.prefix_pattern(marker_pattern()?)
.issue_pattern(issue_pattern()?) .issue_pattern(issue_pattern()?)
.build(); .build();
let issues = HashSet::from_iter(vec![Issue::new(23), Issue::new(43)]); let issues = HashSet::from_iter(vec![Issue::new(23), Issue::new(43)]);