feat: log any invalid/closed markers and exit if any found

Cargo.toml

@@ -16,3 +16,4 @@ serde_json = "1.0"
 
 [dev-dependencies]
 assert2 = "0.3"
+pretty_assertions = "1.4"

renovate.json

@@ -0,0 +1,6 @@
+{
+  "$schema": "https://docs.renovatebot.com/renovate-schema.json",
+  "extends": [
+    "config:recommended"
+  ]
+}

src/issues/fetch.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 //
 use crate::model::Config;
 
@@ -6,7 +8,7 @@ use kxio::network::{NetRequest, NetUrl};
 
 use super::Issue;
 
-pub async fn fetch_open_issues(config: &Config) -> Result<Vec<Issue>> {
+pub async fn fetch_open_issues(config: &Config) -> Result<HashSet<Issue>> {
     let server_url = config.server();
     let repo = config.repo();
     let url = format!("{server_url}/api/v1/repos/{repo}/issues?state=open");
@@ -18,12 +20,14 @@ pub async fn fetch_open_issues(config: &Config) -> Result<Vec<Issue>> {
     }
     .build();
 
-    let issues = config
+    let issues: HashSet<Issue> = config
         .net()
         .get::<Vec<Issue>>(request)
         .await?
         .response_body()
-        .unwrap_or_default();
+        .unwrap_or_default()
+        .into_iter()
+        .collect();
 
     Ok(issues)
 }

src/issues/mod.rs

@@ -7,17 +7,29 @@ mod tests;
 pub use fetch::fetch_open_issues;
 use serde::Deserialize;
 
-#[derive(Debug, Deserialize, PartialEq, Eq)]
+#[derive(Debug, Deserialize, Hash, PartialEq, Eq)]
 pub struct Issue {
     number: u64,
 }
 impl Issue {
-    #[cfg(test)]
     pub fn new(number: u64) -> Self {
         Self { number }
     }
-    #[cfg(test)]
-    pub fn number(&self) -> u64 {
-        self.number
+    // #[cfg(test)]
+    // pub fn number(&self) -> u64 {
+    //     self.number
+    // }
+}
+impl TryFrom<&str> for Issue {
+    type Error = <u64 as std::str::FromStr>::Err;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        let n: u64 = value.parse()?;
+        Ok(Issue::new(n))
+    }
+}
+impl std::fmt::Display for Issue {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.number)
     }
 }

src/issues/tests.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use crate::tests::a_config;
 
 //
@@ -21,9 +23,10 @@ async fn fetch_lists_issues() -> Result<()> {
     let result = fetch_open_issues(&config).await?;
 
     //then
-    assert_eq!(result, vec![Issue::new(13), Issue::new(64)]);
-    assert_eq!(result[0].number(), 13);
-    assert_eq!(result[1].number(), 64);
+    assert_eq!(
+        result,
+        HashSet::from_iter(vec![Issue::new(13), Issue::new(64)])
+    );
 
     Ok(())
 }

src/main.rs

@@ -1,5 +1,5 @@
 //
-use anyhow::Result;
+use anyhow::{bail, Result};
 use init::init_config;
 use issues::fetch_open_issues;
 use scanner::find_markers;
@@ -22,17 +22,22 @@ async fn run(net: kxio::network::Network) -> Result<()> {
     println!("Forgejo TODO Checker!");
 
     let config = init_config(net)?;
+    let issues = fetch_open_issues(&config).await?;
+    let markers = find_markers(&config, issues)?;
 
-    let markers = find_markers(&config)?;
-    println!("{markers}");
-
-    let _issues = fetch_open_issues(&config).await;
-
-    // TODO: loop over list of expected issues and drop any where they do exist and are open
-    // TODO: if remaining list is not empty - add all to error list
-    //
-    // TODO: if error list is empty - exit okay
-    // TODO: if error list is not empty - log erros and exit not okay
+    let mut errors = false;
+    for marker in (*markers).iter() {
+        match marker {
+            model::Marker::Closed(_, _) | model::Marker::Invalid(_) => {
+                println!("{marker}");
+                errors = true;
+            }
+            model::Marker::Unmarked | model::Marker::Valid(_, _) => {}
+        }
+    }
 
+    if errors {
+        bail!("Invalid or closed TODO/FIXMEs found")
+    }
     Ok(())
 }

src/model/line.rs

@@ -6,7 +6,10 @@ use std::path::PathBuf;
 use anyhow::{Context, Result};
 use bon::Builder;
 
-use crate::patterns::{issue_pattern, marker_pattern};
+use crate::{
+    issues::Issue,
+    patterns::{issue_pattern, marker_pattern},
+};
 
 use super::Marker;
 
@@ -32,11 +35,11 @@ impl Line {
         if marker_pattern()?.find(&self.value).is_some() {
             match issue_pattern()?.captures(&self.value) {
                 Some(capture) => {
-                    let issue = capture
+                    let issue: Issue = capture
                         .name("ISSUE_NUMBER")
                         .context("ISSUE_NUMBER")?
                         .as_str()
-                        .to_owned();
+                        .try_into()?;
                     Ok(Marker::Valid(self, issue))
                 }
                 None => Ok(Marker::Invalid(self)),

src/model/markers/found.rs

@@ -1,22 +0,0 @@
-use super::Marker;
-
-//
-#[derive(Debug, Default)]
-pub struct FoundMarkers {
-    markers: Vec<Marker>,
-}
-impl FoundMarkers {
-    pub fn add_marker(&mut self, marker: Marker) {
-        self.markers.push(marker);
-    }
-}
-impl std::fmt::Display for FoundMarkers {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        for marker in self.markers.iter() {
-            // if !matches!(marker, Marker::Unmarked) {
-            write!(f, "{marker}")?;
-            // }
-        }
-        Ok(())
-    }
-}

src/model/markers/issue.rs

@@ -1,15 +0,0 @@
-//
-#![allow(dead_code)]
-
-use super::Marker;
-
-use bon::Builder;
-
-#[derive(Debug, Builder)]
-pub struct IssueMarker {
-    /// The marker
-    marker: Marker,
-
-    /// The issue number
-    issue: usize,
-}

src/model/markers/marker.rs

@@ -1,20 +0,0 @@
-//
-#![allow(dead_code)]
-
-use crate::model::Line;
-
-#[derive(Debug)]
-pub enum Marker {
-    Unmarked,
-    Invalid(Line),
-    Valid(Line, String),
-}
-impl std::fmt::Display for Marker {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Marker::Unmarked => Ok(()),
-            Marker::Invalid(line) => write!(f, "- Invalid: {line}"),
-            Marker::Valid(line, _) => write!(f, "- Valid  : {line}"),
-        }
-    }
-}

src/model/markers/mod.rs

@@ -1,10 +1,61 @@
 //
-mod found;
-mod issue;
-mod marker;
 
 #[cfg(test)]
 mod tests;
 
-pub use found::FoundMarkers;
-pub use marker::Marker;
+use std::ops::Deref;
+
+use crate::{issues::Issue, model::Line};
+
+#[derive(Debug)]
+pub enum Marker {
+    Unmarked,
+    Invalid(Line),
+    #[allow(dead_code)]
+    Valid(Line, Issue),
+    Closed(Line, Issue),
+}
+impl Marker {
+    pub fn into_closed(self) -> Self {
+        match self {
+            Self::Valid(line, issue) => Self::Closed(line, issue),
+            _ => self,
+        }
+    }
+}
+
+impl std::fmt::Display for Marker {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Unmarked => Ok(()),
+            Self::Invalid(line) => write!(f, "- Invalid: {line}"),
+            Self::Valid(line, issue) => write!(f, "- Valid  : ({issue}) {line}"),
+            Self::Closed(line, issue) => write!(f, "- Closed : ({issue}) {line}"),
+        }
+    }
+}
+
+#[derive(Debug, Default)]
+pub struct Markers {
+    markers: Vec<Marker>,
+}
+impl Markers {
+    pub fn add_marker(&mut self, marker: Marker) {
+        self.markers.push(marker);
+    }
+}
+impl Deref for Markers {
+    type Target = Vec<Marker>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.markers
+    }
+}
+impl std::fmt::Display for Markers {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        for marker in self.markers.iter() {
+            write!(f, "{marker}")?;
+        }
+        Ok(())
+    }
+}

src/model/markers/tests.rs

@@ -1,7 +1,5 @@
-use crate::model::Line;
-
 //
-use super::*;
+use crate::model::{markers::Markers, Line};
 
 #[test]
 fn found_when_displayed() -> anyhow::Result<()> {
@@ -10,7 +8,7 @@ fn found_when_displayed() -> anyhow::Result<()> {
     let file = fs.base().join("file-name");
     let relative = file.strip_prefix(fs.base())?.to_path_buf();
 
-    let mut found = FoundMarkers::default();
+    let mut found = Markers::default();
 
     let marker_unmarked = Line::builder()
         .file(file.clone())
@@ -23,14 +21,14 @@ fn found_when_displayed() -> anyhow::Result<()> {
         .file(file.clone())
         .relative_path(relative.clone())
         .num(10)
-        .value("line // TODO: comment".to_owned())
+        .value(format!("line // {}: comment", "TODO"))
         .build()
         .into_marker()?;
     let marker_valid = Line::builder()
         .file(file)
         .relative_path(relative)
         .num(11)
-        .value("line // TODO: (#13) do this".to_owned())
+        .value(format!("line // {}: (#13) do this", "TODO"))
         .build()
         .into_marker()?;
 
@@ -47,9 +45,9 @@ fn found_when_displayed() -> anyhow::Result<()> {
         result,
         vec![
             "- Invalid: file-name#10:",
-            " line // TODO: comment",
-            "- Valid  : file-name#11:",
-            " line // TODO: (#13) do this"
+            format!(" line // {}: comment", "TODO").as_str(),
+            "- Valid  : (13) file-name#11:",
+            format!(" line // {}: (#13) do this", "TODO").as_str()
         ]
     );
 

src/model/markers/tests/mod.rs

@@ -1,6 +0,0 @@
-//
-use super::*;
-
-mod found;
-mod issue;
-mod marker;

src/model/mod.rs

@@ -8,5 +8,5 @@ mod tests;
 
 pub use config::Config;
 pub use line::Line;
-pub use markers::FoundMarkers;
 pub use markers::Marker;
+pub use markers::Markers;

src/scanner.rs

@@ -1,22 +1,30 @@
 //
-use std::path::Path;
+use std::{collections::HashSet, path::Path};
 
-use crate::model::{Config, FoundMarkers, Line, Marker};
+use crate::{
+    issues::Issue,
+    model::{Config, Line, Marker, Markers},
+};
 use anyhow::Result;
 use ignore::Walk;
 
-pub fn find_markers(config: &Config) -> Result<FoundMarkers, anyhow::Error> {
-    let mut markers = FoundMarkers::default();
+pub fn find_markers(config: &Config, issues: HashSet<Issue>) -> Result<Markers, anyhow::Error> {
+    let mut markers = Markers::default();
     for file in Walk::new(config.fs().base()).flatten() {
         let path = file.path();
         if config.fs().path_is_file(path)? {
-            scan_file(path, config, &mut markers)?;
+            scan_file(path, config, &mut markers, &issues)?;
         }
     }
     Ok(markers)
 }
 
-fn scan_file(file: &Path, config: &Config, found_markers: &mut FoundMarkers) -> Result<()> {
+fn scan_file(
+    file: &Path,
+    config: &Config,
+    found_markers: &mut Markers,
+    issues: &HashSet<Issue>,
+) -> Result<()> {
     let relative_path = file.strip_prefix(config.fs().base())?.to_path_buf();
     config
         .fs()
@@ -27,13 +35,27 @@ fn scan_file(file: &Path, config: &Config, found_markers: &mut FoundMarkers) ->
             Line::builder()
                 .file(file.to_path_buf())
                 .relative_path(relative_path.clone())
-                .num(n)
+                .num(n + 1) // line numbers are not 0-based, but enumerate is
                 .value(line.to_owned())
                 .build()
         })
         .filter_map(|line| line.into_marker().ok())
         .filter(|marker| !matches!(marker, Marker::Unmarked))
+        .map(|marker| has_open_issue(marker, issues))
         .for_each(|marker| found_markers.add_marker(marker));
 
     Ok(())
 }
+
+fn has_open_issue(marker: Marker, issues: &HashSet<Issue>) -> Marker {
+    if let Marker::Valid(_, ref issue) = marker {
+        let has_open_issue = issues.contains(issue);
+        if has_open_issue {
+            marker
+        } else {
+            marker.into_closed()
+        }
+    } else {
+        marker
+    }
+}

src/tests/data/file_with_invalids.txt

@@ -5,3 +5,5 @@ It contains a todo comment: // TODO: this is it
 It also contains a fix-me comment: // FIXME: and this is it
 
 Both of these are missing an issue identifier.
+
+We also have a todo comment: // TODO: (#3) and it has an issue number, but it is closed

src/tests/run.rs

@@ -2,10 +2,17 @@
 use super::*;
 
 use anyhow::Result;
+use kxio::network::StatusCode;
 
 #[tokio::test]
 async fn run_with_some_invalids() -> Result<()> {
     //given
+    let mut net = kxio::network::MockNetwork::new();
+    net.add_get_response(
+        "https://git.kemitix.net/api/v1/repos/kemitix/test/issues?state=open",
+        StatusCode::OK,
+        r#"[{"number": 13}]"#,
+    );
     let _env = THE_ENVIRONMENT.lock();
     let fs = kxio::fs::temp()?;
     fs.file_write(
@@ -21,7 +28,7 @@ async fn run_with_some_invalids() -> Result<()> {
     std::env::set_var("GITHUB_SERVER_URL", "https://git.kemitix.net");
 
     //when
-    run(kxio::network::Network::new_mock()).await?;
+    run(net.into()).await?;
 
     //then
     // TODO: add check that run fails because file_1.txt is invalid
@@ -33,6 +40,12 @@ async fn run_with_some_invalids() -> Result<()> {
 #[tokio::test]
 async fn run_with_no_invalids() -> Result<()> {
     //given
+    let mut net = kxio::network::MockNetwork::new();
+    net.add_get_response(
+        "https://git.kemitix.net/api/v1/repos/kemitix/test/issues?state=open",
+        StatusCode::OK,
+        r#"[{"number": 13}]"#,
+    );
     let _env = THE_ENVIRONMENT.lock();
     let fs = kxio::fs::temp()?;
     fs.file_write(
@@ -44,7 +57,8 @@ async fn run_with_no_invalids() -> Result<()> {
     std::env::set_var("GITHUB_SERVER_URL", "https://git.kemitix.net");
 
     //when
-    run(kxio::network::Network::new_mock()).await?;
+
+    run(net.into()).await?;
 
     //then
     // TODO: add check that run fails because file_1.txt is invalid

src/tests/scanner.rs

@@ -1,9 +1,13 @@
-use model::Config;
-use patterns::{issue_pattern, marker_pattern};
-
 //
 use super::*;
 
+use std::collections::HashSet;
+
+use issues::Issue;
+use model::Config;
+use patterns::{issue_pattern, marker_pattern};
+use pretty_assertions::assert_eq;
+
 #[test]
 fn find_markers_in_dir() -> anyhow::Result<()> {
     //given
@@ -25,9 +29,10 @@ fn find_markers_in_dir() -> anyhow::Result<()> {
         .prefix_pattern(marker_pattern()?)
         .issue_pattern(issue_pattern()?)
         .build();
+    let issues = HashSet::from_iter(vec![Issue::new(23), Issue::new(43)]);
 
     //when
-    let markers = find_markers(&config)?;
+    let markers = find_markers(&config, issues)?;
 
     //then
     assert_eq!(
@@ -37,9 +42,11 @@ fn find_markers_in_dir() -> anyhow::Result<()> {
             " It contains a todo comment: // TODO: this is it",
             "- Invalid: file_with_invalids.txt#4:",
             " It also contains a fix-me comment: // FIXME: and this is it",
-            "- Valid  : file_with_valids.txt#2:",
+            "- Closed : (3) file_with_invalids.txt#8:",
+            " We also have a todo comment: // TODO: (#3) and it has an issue number, but it is closed",
+            "- Valid  : (23) file_with_valids.txt#2:",
             " It also has a todo comment: // TODO: (#23) and it has an issue number",
-            "- Valid  : file_with_valids.txt#4:",
+            "- Valid  : (43) file_with_valids.txt#4:",
             " Here is a fix-me comment: // FIXME: (#43) and is also has an issue number"
         ]
     );