Sort compact/detailed/markdown error output by file path (#1622)

sudfud · web-flow · commit 50687175d17a · 2025-02-15T00:10:59.000+01:00
* Sort compact/detailed/markdown error output by file path

* - Modify sort_stats_map to sort HashMap values
- Add unit/integration tests for sort_stats_map
- Add human-sort dependency for natural sorting

* Fix warnings reported by GitHub checks

* Fix clippy warning

- Fix clippy warning
- Make entry sorting case-insensitive in sort_stat_map

* Fix clippy warning
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/lychee-bin/Cargo.toml b/lychee-bin/Cargo.toml
@@ -55,6 +55,7 @@ tokio = { version = "1.42.0", features = ["full"] }
 tokio-stream = "0.1.17"
 toml = "0.8.19"
 url = "2.5.4"
+human-sort = "0.2.2"
 
 [dev-dependencies]
 assert_cmd = "2.0.16"
diff --git a/lychee-bin/src/formatters/stats/compact.rs b/lychee-bin/src/formatters/stats/compact.rs
@@ -37,7 +37,7 @@ impl Display for CompactResponseStats {
 
         let response_formatter = get_response_formatter(&self.mode);
 
-        for (source, responses) in &stats.error_map {
+        for (source, responses) in super::sort_stat_map(&stats.error_map) {
             color!(f, BOLD_YELLOW, "[{}]:\n", source)?;
             for response in responses {
                 writeln!(
@@ -47,9 +47,16 @@ impl Display for CompactResponseStats {
                 )?;
             }
 
-            if let Some(suggestions) = &stats.suggestion_map.get(source) {
+            if let Some(suggestions) = stats.suggestion_map.get(source) {
+                // Sort suggestions
+                let mut sorted_suggestions: Vec<_> = suggestions.iter().collect();
+                sorted_suggestions.sort_by(|a, b| {
+                    let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
+                    human_sort::compare(&a, &b)
+                });
+
                 writeln!(f, "\n\u{2139} Suggestions")?;
-                for suggestion in *suggestions {
+                for suggestion in sorted_suggestions {
                     writeln!(f, "{suggestion}")?;
                 }
             }
diff --git a/lychee-bin/src/formatters/stats/detailed.rs b/lychee-bin/src/formatters/stats/detailed.rs
@@ -49,7 +49,7 @@ impl Display for DetailedResponseStats {
 
         let response_formatter = get_response_formatter(&self.mode);
 
-        for (source, responses) in &stats.error_map {
+        for (source, responses) in super::sort_stat_map(&stats.error_map) {
             // Using leading newlines over trailing ones (e.g. `writeln!`)
             // lets us avoid extra newlines without any additional logic.
             write!(f, "\n\nErrors in {source}")?;
@@ -60,12 +60,19 @@ impl Display for DetailedResponseStats {
                     "\n{}",
                     response_formatter.format_detailed_response(response)
                 )?;
+            }
 
-                if let Some(suggestions) = &stats.suggestion_map.get(source) {
-                    writeln!(f, "\nSuggestions in {source}")?;
-                    for suggestion in *suggestions {
-                        writeln!(f, "{suggestion}")?;
-                    }
+            if let Some(suggestions) = stats.suggestion_map.get(source) {
+                // Sort suggestions
+                let mut sorted_suggestions: Vec<_> = suggestions.iter().collect();
+                sorted_suggestions.sort_by(|a, b| {
+                    let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
+                    human_sort::compare(&a, &b)
+                });
+
+                writeln!(f, "\nSuggestions in {source}")?;
+                for suggestion in sorted_suggestions {
+                    writeln!(f, "{suggestion}")?;
                 }
             }
         }
diff --git a/lychee-bin/src/formatters/stats/markdown.rs b/lychee-bin/src/formatters/stats/markdown.rs
@@ -127,7 +127,7 @@ where
 {
     if !&map.is_empty() {
         writeln!(f, "\n## {name} per input")?;
-        for (source, responses) in map {
+        for (source, responses) in super::sort_stat_map(map) {
             writeln!(f, "\n### {name} in {source}\n")?;
             for response in responses {
                 writeln!(f, "{}", write_stat(response)?)?;
diff --git a/lychee-bin/src/formatters/stats/mod.rs b/lychee-bin/src/formatters/stats/mod.rs
@@ -10,10 +10,110 @@ pub(crate) use json::Json;
 pub(crate) use markdown::Markdown;
 pub(crate) use raw::Raw;
 
+use std::{
+    collections::{HashMap, HashSet},
+    fmt::Display,
+};
+
 use crate::stats::ResponseStats;
 use anyhow::Result;
+use lychee_lib::InputSource;
 
 pub(crate) trait StatsFormatter {
     /// Format the stats of all responses and write them to stdout
     fn format(&self, stats: ResponseStats) -> Result<Option<String>>;
 }
+
+/// Convert a `ResponseStats` `HashMap` to a sorted Vec of key-value pairs
+/// The returned keys and values are both sorted in natural, case-insensitive order
+fn sort_stat_map<T>(stat_map: &HashMap<InputSource, HashSet<T>>) -> Vec<(&InputSource, Vec<&T>)>
+where
+    T: Display,
+{
+    let mut entries: Vec<_> = stat_map
+        .iter()
+        .map(|(source, responses)| {
+            let mut sorted_responses: Vec<&T> = responses.iter().collect();
+            sorted_responses.sort_by(|a, b| {
+                let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
+                human_sort::compare(&a, &b)
+            });
+
+            (source, sorted_responses)
+        })
+        .collect();
+
+    entries.sort_by(|(a, _), (b, _)| {
+        let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
+        human_sort::compare(&a, &b)
+    });
+
+    entries
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use lychee_lib::{ErrorKind, Response, Status, Uri};
+    use url::Url;
+
+    fn make_test_url(url: &str) -> Url {
+        Url::parse(url).expect("Expected valid Website URI")
+    }
+
+    fn make_test_response(url_str: &str, source: InputSource) -> Response {
+        let uri = Uri::from(make_test_url(url_str));
+
+        Response::new(uri, Status::Error(ErrorKind::InvalidUrlHost), source)
+    }
+
+    #[test]
+    fn test_sorted_stat_map() {
+        let mut test_stats = ResponseStats::default();
+
+        // Sorted list of test sources
+        let test_sources = vec![
+            InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/404"))),
+            InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/home"))),
+            InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/page/1"))),
+            InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/page/10"))),
+        ];
+
+        // Sorted list of test responses
+        let test_response_urls = vec![
+            "https://example.com/",
+            "https://github.com/",
+            "https://itch.io/",
+            "https://youtube.com/",
+        ];
+
+        // Add responses to stats
+        // Responses are added to a HashMap, so the order is not preserved
+        for source in &test_sources {
+            for response in &test_response_urls {
+                test_stats.add(make_test_response(response, source.clone()));
+            }
+        }
+
+        // Sort error map and extract the sources
+        let sorted_errors = sort_stat_map(&test_stats.error_map);
+        let sorted_sources: Vec<InputSource> = sorted_errors
+            .iter()
+            .map(|(source, _)| (*source).clone())
+            .collect();
+
+        // Check that the input sources are sorted
+        assert_eq!(test_sources, sorted_sources);
+
+        // Check that the responses are sorted
+        for (_, response_bodies) in sorted_errors {
+            let response_urls: Vec<&str> = response_bodies
+                .into_iter()
+                .map(|response| response.uri.as_str())
+                .collect();
+
+            assert_eq!(test_response_urls, response_urls);
+        }
+    }
+}
diff --git a/lychee-bin/tests/cli.rs b/lychee-bin/tests/cli.rs
@@ -1943,4 +1943,51 @@ mod cli {
 
         Ok(())
     }
+
+    #[test]
+    fn test_sorted_error_output() -> Result<()> {
+        let test_files = ["TEST_GITHUB_404.md", "TEST_INVALID_URLS.html"];
+
+        let test_urls = [
+            "https://httpbin.org/status/404",
+            "https://httpbin.org/status/500",
+            "https://httpbin.org/status/502",
+        ];
+
+        let cmd = &mut main_command()
+            .arg("--format")
+            .arg("compact")
+            .arg(fixtures_path().join(test_files[1]))
+            .arg(fixtures_path().join(test_files[0]))
+            .assert()
+            .failure()
+            .code(2);
+
+        let output = String::from_utf8_lossy(&cmd.get_output().stdout);
+        let mut position: usize = 0;
+
+        // Check that the input sources are sorted
+        for file in test_files {
+            assert!(output.contains(file));
+
+            let next_position = output.find(file).unwrap();
+
+            assert!(next_position > position);
+            position = next_position;
+        }
+
+        position = 0;
+
+        // Check that the responses are sorted
+        for url in test_urls {
+            assert!(output.contains(url));
+
+            let next_position = output.find(url).unwrap();
+
+            assert!(next_position > position);
+            position = next_position;
+        }
+
+        Ok(())
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,7 @@ where`
`127`	`127`	`{`
`128`	`128`	`if !&map.is_empty() {`
`129`	`129`	`writeln!(f, "\n## {name} per input")?;`
`130`		`- for (source, responses) in map {`
	`130`	`+ for (source, responses) in super::sort_stat_map(map) {`
`131`	`131`	`writeln!(f, "\n### {name} in {source}\n")?;`
`132`	`132`	`for response in responses {`
`133`	`133`	`writeln!(f, "{}", write_stat(response)?)?;`