Skip to content

Commit

Permalink
Auto merge of rust-lang#117149 - nnethercote:tidy-alphabetical-unit-t…
Browse files Browse the repository at this point in the history
…ests, r=Nilstrieb

tidy: add unit tests for alphabetical checks

I discovered there aren't any tests while working on rust-lang#117068.

r? `@Nilstrieb`
  • Loading branch information
bors committed Oct 28, 2023
2 parents 7cc36de + a0b0ace commit e5cfc55
Show file tree
Hide file tree
Showing 3 changed files with 262 additions and 36 deletions.
94 changes: 64 additions & 30 deletions src/tools/tidy/src/alphabetical.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Checks that a list of items is in alphabetical order
//!
//! To use, use the following annotation in the code:
//! Use the following marker in the code:
//! ```rust
//! // tidy-alphabetical-start
//! fn aaa() {}
Expand All @@ -10,17 +10,23 @@
//! ```
//!
//! The following lines are ignored:
//! - Empty lines
//! - Lines that are indented with more or less spaces than the first line
//! - Lines starting with `//`, `#[`, `)`, `]`, `}` if the comment has the same indentation as
//! the first line
//! - Lines starting with `//`, `#` (except those starting with `#!`), `)`, `]`, `}` if the comment
//! has the same indentation as the first line
//! - Lines starting with a closing delimiter (`)`, `[`, `}`) are ignored.
//!
//! If a line ends with an opening bracket, the line is ignored and the next line will have
//! its extra indentation ignored.
//! If a line ends with an opening delimiter, we effectively join the following line to it before
//! checking it. E.g. `foo(\nbar)` is treated like `foo(bar)`.
use std::{fmt::Display, path::Path};
use std::fmt::Display;
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)
}
Expand All @@ -29,28 +35,36 @@ fn is_close_bracket(c: char) -> bool {
matches!(c, ')' | ']' | '}')
}

// Don't let tidy check this here :D
const START_COMMENT: &str = concat!("tidy-alphabetical", "-start");
const END_COMMENT: &str = "tidy-alphabetical-end";
const START_MARKER: &str = "tidy-alphabetical-start";
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 content_lines = lines.take_while(|(_, line)| !line.contains(END_COMMENT));

let mut prev_line = String::new();
let mut first_indent = None;
let mut in_split_line = None;

for (line_idx, line) in content_lines {
if line.contains(START_COMMENT) {
tidy_error!(
for (idx, line) in lines {
if line.is_empty() {
continue;
}

if line.contains(START_MARKER) {
tidy_error_ext!(
err,
bad,
"{file}:{} found `{START_COMMENT}` expecting `{END_COMMENT}`",
line_idx
)
"{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`",
idx + 1
);
return;
}

if line.contains(END_MARKER) {
return;
}

let indent = first_indent.unwrap_or_else(|| {
Expand All @@ -60,6 +74,7 @@ fn check_section<'a>(
});

let line = if let Some(prev_split_line) = in_split_line {
// Join the split lines.
in_split_line = None;
format!("{prev_split_line}{}", line.trim_start())
} else {
Expand All @@ -73,7 +88,7 @@ fn check_section<'a>(
let trimmed_line = line.trim_start_matches(' ');

if trimmed_line.starts_with("//")
|| trimmed_line.starts_with("#[")
|| (trimmed_line.starts_with("#") && !trimmed_line.starts_with("#!"))
|| trimmed_line.starts_with(is_close_bracket)
{
continue;
Expand All @@ -87,25 +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", line_idx + 1,);
tidy_error_ext!(err, bad, "{file}:{}: line not in alphabetical order", idx + 1);
}

prev_line = line;
}

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().peekable();
while let Some((_, line)) = lines.next() {
if line.contains(START_COMMENT) {
check_section(file, &mut lines, bad);
if lines.peek().is_none() {
tidy_error!(bad, "{file}: reached end of file expecting `{END_COMMENT}`")
}
}
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)
});
}
188 changes: 188 additions & 0 deletions src/tools/tidy/src/alphabetical/tests.rs
Original file line number Diff line number Diff line change
@@ -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`");
}
16 changes: 10 additions & 6 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)))?;

Expand Down

0 comments on commit e5cfc55

Please sign in to comment.