tidy: add unit tests for alphabetical markers.

This requires introducing `tidy_error_ext!` as an alternative to
`tidy_error!`. It is passed a closure that is called for an error. This
lets tests capture tidy error messages in a buffer instead of printing
them to stderr.

It also requires pulling part of `check` out into a new function
`check_lines`, so that tests can pass in an iterator over some strings
instead of a file.

src/tools/tidy/src/alphabetical.rs

@@ -24,6 +24,9 @@ use std::path::Path;
 
 use crate::walk::{filter_dirs, walk};
 
+#[cfg(test)]
+mod tests;
+
 fn indentation(line: &str) -> usize {
     line.find(|c| c != ' ').unwrap_or(0)
 }
@@ -38,6 +41,7 @@ const END_MARKER: &str = "tidy-alphabetical-end";
 fn check_section<'a>(
     file: impl Display,
     lines: impl Iterator<Item = (usize, &'a str)>,
+    err: &mut dyn FnMut(&str) -> std::io::Result<()>,
     bad: &mut bool,
 ) {
     let mut prev_line = String::new();
@@ -50,7 +54,12 @@ fn check_section<'a>(
         }
 
         if line.contains(START_MARKER) {
-            tidy_error!(bad, "{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`", idx + 1);
+            tidy_error_ext!(
+                err,
+                bad,
+                "{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`",
+                idx + 1
+            );
             return;
         }
 
@@ -93,32 +102,44 @@ fn check_section<'a>(
         let prev_line_trimmed_lowercase = prev_line.trim_start_matches(' ').to_lowercase();
 
         if trimmed_line.to_lowercase() < prev_line_trimmed_lowercase {
-            tidy_error!(bad, "{file}:{}: line not in alphabetical order", idx + 1);
+            tidy_error_ext!(err, bad, "{file}:{}: line not in alphabetical order", idx + 1);
         }
 
         prev_line = line;
     }
 
-    tidy_error!(bad, "{file}: reached end of file expecting `{END_MARKER}`")
+    tidy_error_ext!(err, bad, "{file}: reached end of file expecting `{END_MARKER}`")
 }
 
-pub fn check(path: &Path, bad: &mut bool) {
-    walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
-        let file = &entry.path().display();
+fn check_lines<'a>(
+    file: &impl Display,
+    mut lines: impl Iterator<Item = (usize, &'a str)>,
+    err: &mut dyn FnMut(&str) -> std::io::Result<()>,
+    bad: &mut bool,
+) {
+    while let Some((idx, line)) = lines.next() {
+        if line.contains(END_MARKER) {
+            tidy_error_ext!(
+                err,
+                bad,
+                "{file}:{} found `{END_MARKER}` expecting `{START_MARKER}`",
+                idx + 1
+            )
+        }
 
-        let mut lines = contents.lines().enumerate();
-        while let Some((idx, line)) = lines.next() {
-            if line.contains(END_MARKER) {
-                tidy_error!(
-                    bad,
-                    "{file}:{} found `{END_MARKER}` expecting `{START_MARKER}`",
-                    idx + 1
-                )
-            }
-
-            if line.contains(START_MARKER) {
-                check_section(file, &mut lines, bad);
-            }
+        if line.contains(START_MARKER) {
+            check_section(file, &mut lines, err, bad);
         }
+    }
+}
+
+pub fn check(path: &Path, bad: &mut bool) {
+    let skip =
+        |path: &_, _is_dir| filter_dirs(path) || path.ends_with("tidy/src/alphabetical/tests.rs");
+
+    walk(path, skip, &mut |entry, contents| {
+        let file = &entry.path().display();
+        let lines = contents.lines().enumerate();
+        check_lines(file, lines, &mut crate::tidy_error, bad)
     });
 }

src/tools/tidy/src/alphabetical/tests.rs

@@ -0,0 +1,188 @@
+use super::*;
+use std::io::Write;
+use std::str::from_utf8;
+
+fn test(lines: &str, name: &str, expected_msg: &str, expected_bad: bool) {
+    let mut actual_msg = Vec::new();
+    let mut actual_bad = false;
+    let mut err = |args: &_| {
+        write!(&mut actual_msg, "{args}")?;
+        Ok(())
+    };
+    check_lines(&name, lines.lines().enumerate(), &mut err, &mut actual_bad);
+    assert_eq!(expected_msg, from_utf8(&actual_msg).unwrap());
+    assert_eq!(expected_bad, actual_bad);
+}
+
+fn good(lines: &str) {
+    test(lines, "good", "", false);
+}
+
+fn bad(lines: &str, expected_msg: &str) {
+    test(lines, "bad", expected_msg, true);
+}
+
+#[test]
+fn test_no_markers() {
+    let lines = "\
+        def
+        abc
+        xyz
+    ";
+    good(lines);
+}
+
+#[test]
+fn test_rust_good() {
+    let lines = "\
+        // tidy-alphabetical-start
+        abc
+        def
+        xyz
+        // tidy-alphabetical-end"; // important: end marker on last line
+    good(lines);
+}
+
+#[test]
+fn test_complex_good() {
+    let lines = "\
+        zzz
+
+        // tidy-alphabetical-start
+        abc
+        // Rust comments are ok
+        def
+        # TOML comments are ok
+        xyz
+        // tidy-alphabetical-end
+
+        # tidy-alphabetical-start
+        foo(abc);
+        // blank lines are ok
+
+        // split line gets joined
+        foo(
+            def
+        );
+
+        foo(xyz);
+        # tidy-alphabetical-end
+
+        % tidy-alphabetical-start
+        abc
+            ignored_due_to_different_indent
+        def
+        % tidy-alphabetical-end
+
+        aaa
+    ";
+    good(lines);
+}
+
+#[test]
+fn test_rust_bad() {
+    let lines = "\
+        // tidy-alphabetical-start
+        abc
+        xyz
+        def
+        // tidy-alphabetical-end
+    ";
+    bad(lines, "bad:4: line not in alphabetical order");
+}
+
+#[test]
+fn test_toml_bad() {
+    let lines = "\
+        # tidy-alphabetical-start
+        abc
+        xyz
+        def
+        # tidy-alphabetical-end
+    ";
+    bad(lines, "bad:4: line not in alphabetical order");
+}
+
+#[test]
+fn test_features_bad() {
+    // Even though lines starting with `#` are treated as comments, lines
+    // starting with `#!` are an exception.
+    let lines = "\
+        tidy-alphabetical-start
+        #![feature(abc)]
+        #![feature(xyz)]
+        #![feature(def)]
+        tidy-alphabetical-end
+    ";
+    bad(lines, "bad:4: line not in alphabetical order");
+}
+
+#[test]
+fn test_indent_bad() {
+    // All lines are indented the same amount, and so are checked.
+    let lines = "\
+        $ tidy-alphabetical-start
+            abc
+            xyz
+            def
+        $ tidy-alphabetical-end
+    ";
+    bad(lines, "bad:4: line not in alphabetical order");
+}
+
+#[test]
+fn test_split_bad() {
+    let lines = "\
+        || tidy-alphabetical-start
+        foo(abc)
+        foo(
+            xyz
+        )
+        foo(
+            def
+        )
+        && tidy-alphabetical-end
+    ";
+    bad(lines, "bad:7: line not in alphabetical order");
+}
+
+#[test]
+fn test_double_start() {
+    let lines = "\
+        tidy-alphabetical-start
+        abc
+        tidy-alphabetical-start
+    ";
+    bad(lines, "bad:3 found `tidy-alphabetical-start` expecting `tidy-alphabetical-end`");
+}
+
+#[test]
+fn test_missing_start() {
+    let lines = "\
+        abc
+        tidy-alphabetical-end
+        abc
+    ";
+    bad(lines, "bad:2 found `tidy-alphabetical-end` expecting `tidy-alphabetical-start`");
+}
+
+#[test]
+fn test_missing_end() {
+    let lines = "\
+        tidy-alphabetical-start
+        abc
+    ";
+    bad(lines, "bad: reached end of file expecting `tidy-alphabetical-end`");
+}
+
+#[test]
+fn test_double_end() {
+    let lines = "\
+        tidy-alphabetical-start
+        abc
+        tidy-alphabetical-end
+        def
+        tidy-alphabetical-end
+    ";
+    bad(lines, "bad:5 found `tidy-alphabetical-end` expecting `tidy-alphabetical-start`");
+}

src/tools/tidy/src/lib.rs

@@ -3,8 +3,6 @@
 //! This library contains the tidy lints and exposes it
 //! to be used by tools.
 
-use std::fmt::Display;
-
 use termcolor::WriteColor;
 
 /// A helper macro to `unwrap` a result except also print out details like:
@@ -31,16 +29,22 @@ macro_rules! t {
 
 macro_rules! tidy_error {
     ($bad:expr, $($fmt:tt)*) => ({
-        $crate::tidy_error($bad, format_args!($($fmt)*)).expect("failed to output error");
+        $crate::tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error");
+        *$bad = true;
     });
 }
 
-fn tidy_error(bad: &mut bool, args: impl Display) -> std::io::Result<()> {
+macro_rules! tidy_error_ext {
+    ($tidy_error:path, $bad:expr, $($fmt:tt)*) => ({
+        $tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error");
+        *$bad = true;
+    });
+}
+
+fn tidy_error(args: &str) -> std::io::Result<()> {
     use std::io::Write;
     use termcolor::{Color, ColorChoice, ColorSpec, StandardStream};
 
-    *bad = true;
-
     let mut stderr = StandardStream::stdout(ColorChoice::Auto);
     stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?;