diff --git a/Cargo.toml b/Cargo.toml index 9d5a7ad..5477e5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,3 +16,4 @@ serde_json = "1.0" [dev-dependencies] assert2 = "0.3" +pretty_assertions = "1.4" diff --git a/src/issues/fetch.rs b/src/issues/fetch.rs index bec0ad7..63bcba6 100644 --- a/src/issues/fetch.rs +++ b/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> { +pub async fn fetch_open_issues(config: &Config) -> Result> { 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> { } .build(); - let issues = config + let issues: HashSet = config .net() .get::>(request) .await? .response_body() - .unwrap_or_default(); + .unwrap_or_default() + .into_iter() + .collect(); Ok(issues) } diff --git a/src/issues/mod.rs b/src/issues/mod.rs index 2ad6ec0..aa1e6a5 100644 --- a/src/issues/mod.rs +++ b/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 = ::Err; + + fn try_from(value: &str) -> Result { + 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) } } diff --git a/src/issues/tests.rs b/src/issues/tests.rs index f0b7bcc..25beeea 100644 --- a/src/issues/tests.rs +++ b/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(()) } diff --git a/src/main.rs b/src/main.rs index bda3679..81668de 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,16 +23,13 @@ async fn run(net: kxio::network::Network) -> Result<()> { let config = init_config(net)?; - let markers = find_markers(&config)?; + let issues = fetch_open_issues(&config).await?; + + let markers = find_markers(&config, issues)?; 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 + // TODO: loop over markers, logging any that are invalid or closed. + // TODO: if we logged any, then exit as and error Ok(()) } diff --git a/src/model/line.rs b/src/model/line.rs index edea0f5..b1e01f8 100644 --- a/src/model/line.rs +++ b/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)), diff --git a/src/model/markers/mod.rs b/src/model/markers/mod.rs index afe1f2c..24d7de6 100644 --- a/src/model/markers/mod.rs +++ b/src/model/markers/mod.rs @@ -3,21 +3,32 @@ #[cfg(test)] mod tests; -use crate::model::Line; +use crate::{issues::Issue, model::Line}; #[derive(Debug)] pub enum Marker { Unmarked, Invalid(Line), #[allow(dead_code)] - Valid(Line, String), + 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 { - Marker::Unmarked => Ok(()), - Marker::Invalid(line) => write!(f, "- Invalid: {line}"), - Marker::Valid(line, _) => write!(f, "- Valid : {line}"), + 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}"), } } } diff --git a/src/model/markers/tests.rs b/src/model/markers/tests.rs index 41268c8..454986c 100644 --- a/src/model/markers/tests.rs +++ b/src/model/markers/tests.rs @@ -21,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()?; @@ -45,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() ] ); diff --git a/src/scanner.rs b/src/scanner.rs index f9fbfb0..d4bc6b0 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -1,22 +1,30 @@ // -use std::path::Path; +use std::{collections::HashSet, path::Path}; -use crate::model::{Config, Line, Marker, Markers}; +use crate::{ + issues::Issue, + model::{Config, Line, Marker, Markers}, +}; use anyhow::Result; use ignore::Walk; -pub fn find_markers(config: &Config) -> Result { +pub fn find_markers(config: &Config, issues: HashSet) -> Result { 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 Markers) -> Result<()> { +fn scan_file( + file: &Path, + config: &Config, + found_markers: &mut Markers, + issues: &HashSet, +) -> 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 Markers) -> Resul 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) -> 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 + } +} diff --git a/src/tests/data/file_with_invalids.txt b/src/tests/data/file_with_invalids.txt index 4d43b2d..eceb58e 100644 --- a/src/tests/data/file_with_invalids.txt +++ b/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 diff --git a/src/tests/run.rs b/src/tests/run.rs index 1dbf6a8..b94d260 100644 --- a/src/tests/run.rs +++ b/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 diff --git a/src/tests/scanner.rs b/src/tests/scanner.rs index 4e7470e..adf3360 100644 --- a/src/tests/scanner.rs +++ b/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" ] );