feat: Log errors as they are found

Closes kemitix/forgejo-todo-checker#5

src/main.rs

@@ -29,20 +29,9 @@ async fn run(printer: &impl Printer, net: Network) -> Result<()> {
 
     let config = init_config(printer, net)?;
     let issues = fetch_open_issues(&config).await?;
-    let markers = find_markers(&config, issues, &DefaultFileScanner)?;
+    let errors = find_markers(printer, &config, issues, &DefaultFileScanner)?;
 
-    let mut errors = false;
-    for marker in (*markers).iter() {
-        match marker {
-            model::Marker::Closed(_, _) | model::Marker::Invalid(_) => {
-                printer.println("{marker}");
-                errors = true;
-            }
-            model::Marker::Unmarked | model::Marker::Valid(_, _) => {}
-        }
-    }
-
-    if errors {
+    if errors > 0 {
         bail!("Invalid or closed TODO/FIXMEs found")
     }
     Ok(())

src/model/config.rs

@@ -1,6 +1,4 @@
 //
-#![allow(dead_code)]
-
 use bon::Builder;
 use regex::Regex;
 

src/model/line.rs

@@ -1,6 +1,4 @@
 //
-#![allow(dead_code)]
-
 use std::path::PathBuf;
 
 use anyhow::{Context, Result};
@@ -15,22 +13,11 @@ use super::Marker;
 
 #[derive(Debug, Builder)]
 pub struct Line {
-    file: PathBuf,
     relative_path: PathBuf,
     num: usize,
     value: String,
 }
 impl Line {
-    // pub fn file(&self) -> &Path {
-    //     &self.file
-    // }
-    // pub fn num(&self) -> usize {
-    //     self.num
-    // }
-    // pub fn value(&self) -> &str {
-    //     &self.value
-    // }
-
     pub fn into_marker(self) -> Result<Marker> {
         if marker_pattern()?.find(&self.value).is_some() {
             match issue_pattern()?.captures(&self.value) {

src/model/markers.rs

@@ -0,0 +1,29 @@
+//
+use crate::{issues::Issue, model::Line};
+
+#[derive(Debug)]
+pub enum Marker {
+    Unmarked,
+    Invalid(Line),
+    Valid(Line, Issue),
+    Closed(Line, Issue),
+}
+impl Marker {
+    pub fn into_closed(self) -> Self {
+        match self {
+            Self::Valid(line, issue) => Self::Closed(line, issue),
+            #[cfg(not(tarpaulin_include))] // only ever called when is a Valid
+            _ => self,
+        }
+    }
+}
+
+impl std::fmt::Display for Marker {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Invalid(line) => write!(f, "- Invalid: {}", line),
+            Self::Closed(line, issue) => write!(f, "- Closed : ({issue}) {line}"),
+            Self::Valid(_, _) | Self::Unmarked => Ok(()), // tarpaulin uncovered okay
+        }
+    }
+}

src/model/markers/mod.rs

@@ -1,62 +0,0 @@
-//
-
-#[cfg(test)]
-mod tests;
-
-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),
-            #[cfg(not(tarpaulin_include))] // only ever called when is a Valid
-            _ => 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,55 +0,0 @@
-//
-use crate::model::{markers::Markers, Line};
-
-#[test]
-fn found_when_displayed() -> anyhow::Result<()> {
-    //given
-    let fs = kxio::fs::temp()?;
-    let file = fs.base().join("file-name");
-    let relative = file.strip_prefix(fs.base())?.to_path_buf();
-
-    let mut found = Markers::default();
-
-    let marker_unmarked = Line::builder()
-        .file(file.clone())
-        .relative_path(relative.clone())
-        .num(10)
-        .value("line with no comment".to_owned())
-        .build()
-        .into_marker()?;
-    let marker_invalid = Line::builder()
-        .file(file.clone())
-        .relative_path(relative.clone())
-        .num(10)
-        .value(format!("line // {}: comment", "TODO"))
-        .build()
-        .into_marker()?;
-    let marker_valid = Line::builder()
-        .file(file)
-        .relative_path(relative)
-        .num(11)
-        .value(format!("line // {}: (#13) do this", "TODO"))
-        .build()
-        .into_marker()?;
-
-    found.add_marker(marker_unmarked);
-    found.add_marker(marker_invalid);
-    found.add_marker(marker_valid);
-
-    //when
-    let markers_as_string = found.to_string();
-    let result = markers_as_string.lines().collect::<Vec<&str>>();
-
-    //then
-    assert_eq!(
-        result,
-        vec![
-            "- Invalid: file-name#10:",
-            format!(" line // {}: comment", "TODO").as_str(),
-            "- Valid  : (13) file-name#11:",
-            format!(" line // {}: (#13) do this", "TODO").as_str()
-        ]
-    );
-
-    Ok(())
-}

src/model/mod.rs

@@ -9,4 +9,3 @@ mod tests;
 pub use config::Config;
 pub use line::Line;
 pub use markers::Marker;
-pub use markers::Markers;

src/printer.rs

@@ -5,6 +5,7 @@ pub trait Printer {
 
 pub struct StandardPrinter;
 impl Printer for StandardPrinter {
+    #[cfg(not(tarpaulin_include))]
     fn println(&self, message: impl Into<String>) {
         println!("{}", message.into());
     }

src/scanner.rs

@@ -3,36 +3,37 @@ use std::{collections::HashSet, path::Path};
 
 use crate::{
     issues::Issue,
-    model::{Config, Line, Marker, Markers},
+    model::{Config, Line, Marker},
+    printer::Printer,
 };
 use anyhow::Result;
 use file_format::FileFormat;
 use ignore::Walk;
 
-//<'a> = dyn Fn(&'a Path, &'a Config, &'a mut Markers, &'a HashSet<Issue>, Output=Result<()>);
 pub trait FileScanner {
     fn scan_file(
         &self,
         path: &Path,
         config: &Config,
-        markers: &mut Markers,
+        printer: &impl Printer,
         issues: &HashSet<Issue>,
-    ) -> Result<()>;
+    ) -> Result<u32>;
 }
 
 pub fn find_markers(
+    printer: &impl Printer,
     config: &Config,
     issues: HashSet<Issue>,
     file_scanner: &impl FileScanner,
-) -> Result<Markers, anyhow::Error> {
-    let mut markers = Markers::default();
+) -> Result<u32, anyhow::Error> {
+    let mut errors = 0;
     for file in Walk::new(config.fs().base()).flatten() {
         let path = file.path();
         if is_text_file(config, path)? {
-            file_scanner.scan_file(path, config, &mut markers, &issues)?
+            errors += file_scanner.scan_file(path, config, printer, &issues)?
         }
     }
-    Ok(markers)
+    Ok(errors)
 }
 
 fn is_text_file(config: &Config, path: &Path) -> Result<bool> {
@@ -48,10 +49,11 @@ impl FileScanner for DefaultFileScanner {
         &self,
         file: &Path,
         config: &Config,
-        markers: &mut Markers,
+        printer: &impl Printer,
         issues: &HashSet<Issue>,
-    ) -> Result<()> {
+    ) -> Result<u32> {
         let relative_path = file.strip_prefix(config.fs().base())?.to_path_buf();
+        let mut errors = 0;
         config
             .fs()
             .file_read_to_string(file)? // tarpaulin uncovered okay
@@ -59,7 +61,6 @@ impl FileScanner for DefaultFileScanner {
             .enumerate()
             .map(|(n, line)| {
                 Line::builder()
-                    .file(file.to_path_buf())
                     .relative_path(relative_path.clone())
                     .num(n + 1) // line numbers are not 0-based, but enumerate is
                     .value(line.to_owned())
@@ -68,9 +69,24 @@ impl FileScanner for DefaultFileScanner {
             .filter_map(|line| line.into_marker().ok())
             .filter(|marker| !matches!(marker, Marker::Unmarked))
             .map(|marker| has_open_issue(marker, issues))
-            .for_each(|marker| markers.add_marker(marker));
+            .for_each(|marker| match marker {
+                Marker::Invalid(_) => {
+                    errors += 1;
+                    // TODO: (#6) Better error message
+                    printer.println(marker.to_string());
+                }
+                Marker::Closed(_, _) => {
+                    errors += 1;
+                    // TODO: (#6) Better error message
+                    printer.println(marker.to_string());
+                }
+                _ => {}
+            });
+        if errors > 0 {
+            printer.println(format!(">> {errors} errors in {}", file.to_string_lossy()));
+        }
 
-        Ok(())
+        Ok(errors)
     }
 }
 

src/tests/scanner.rs

@@ -9,13 +9,15 @@ use issues::Issue;
 use model::Config;
 use patterns::{issue_pattern, marker_pattern};
 use pretty_assertions::assert_eq;
+use printer::TestPrinter;
 
 #[test]
 fn find_markers_in_dir() -> anyhow::Result<()> {
     //given
     let fs = kxio::fs::temp()?;
+    let file_with_invalids = fs.base().join("file_with_invalids.txt");
     fs.file_write(
-        &fs.base().join("file_with_invalids.txt"),
+        &file_with_invalids,
         include_str!("data/file_with_invalids.txt"),
     )?;
     fs.file_write(
@@ -32,26 +34,22 @@ fn find_markers_in_dir() -> anyhow::Result<()> {
         .issue_pattern(issue_pattern()?)
         .build();
     let issues = HashSet::from_iter(vec![Issue::new(23), Issue::new(43)]);
+    let printer = TestPrinter::default();
 
     //when
-    let markers = find_markers(&config, issues, &DefaultFileScanner)?;
+    let errors = find_markers(&printer, &config, issues, &DefaultFileScanner)?;
 
     //then
     assert_eq!(
-        markers.to_string().lines().collect::<Vec<_>>(),
+        printer.messages.take(),
         vec![
-            "- Invalid: file_with_invalids.txt#3:",
-            " It contains a todo comment: // TODO: this is it",
-            "- Invalid: file_with_invalids.txt#5:",
-            " It also contains a fix-me comment: // FIXME: and this is it",
-            "- Closed : (3) file_with_invalids.txt#9:",
-            " We also have a todo comment: // TODO: (#3) and it has an issue number, but it is closed",
-            "- Valid  : (23) file_with_valids.txt#3:",
-            " It also has a todo comment: // TODO: (#23) and it has an issue number",
-            "- Valid  : (43) file_with_valids.txt#5:",
-            " Here is a fix-me comment: // FIXME: (#43) and is also has an issue number"
+            "- Invalid: file_with_invalids.txt#3:\n It contains a todo comment: // TODO: this is it\n",
+            "- Invalid: file_with_invalids.txt#5:\n It also contains a fix-me comment: // FIXME: and this is it\n",
+            "- Closed : (3) file_with_invalids.txt#9:\n We also have a todo comment: // TODO: (#3) and it has an issue number, but it is closed\n",
+            format!(">> 3 errors in {}", file_with_invalids.to_string_lossy()).as_str() 
         ]
     );
+    assert_eq!(errors, 3);
 
     Ok(())
 }
@@ -70,9 +68,10 @@ fn skips_binary_files() -> Result<()> {
     let config = a_config(net, fs)?;
     let issues = HashSet::new();
     let file_scanner = TestFileScanner::default();
+    let printer = TestPrinter::default();
 
     //when
-    find_markers(&config, issues, &file_scanner)?;
+    find_markers(&printer, &config, issues, &file_scanner)?;
 
     //then
     assert_eq!(file_scanner.scanned.take(), vec![text_path]);
@@ -89,10 +88,10 @@ impl FileScanner for TestFileScanner {
         &self,
         path: &std::path::Path,
         _config: &Config,
-        _markers: &mut model::Markers,
+        _printer: &impl Printer,
         _issues: &HashSet<Issue>,
-    ) -> Result<()> {
+    ) -> Result<u32> {
         self.scanned.borrow_mut().push(path.to_path_buf());
-        Ok(())
+        Ok(0)
     }
 }