Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/tools/tidy/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ fn eee() {}
fn z() {}
// tidy-alphabetical-end
```
<!--ignore-tidy-todo-->While not exactly a tidy directive, // TODO will fail tidy and make sure you can't merge a PR with unfinished work.

@jdonszelmann jdonszelmann Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the tidy TODO lint previously implemented ignoring themselves, rather stupidly. And it worked different than all other tidy ignores, and had to be on the same line, instead of a previous line. With this lint using the standard ignore system now, the ignore comment needs to go the the previous line.

View changes since the review

<!-- ignore-tidy-todo -->
While not exactly a tidy directive, // TODO will fail tidy and make sure you can't merge a PR with unfinished work.

### Test Specific Directives

Expand Down
37 changes: 29 additions & 8 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,9 @@ fn long_line_is_ok(extension: &str, is_error_code: bool, max_columns: usize, lin
}

macro_rules! suppressible_tidy_err {
($err:ident, $skip:expr, $msg:literal) => {
($err:ident, $skip:expr, $msg:literal $($args: tt)*) => {
if let Err(()) = $skip.check() {
$err(&format!($msg));
$err(&format!($msg $($args)*));
}
};
}
Expand Down Expand Up @@ -455,10 +455,22 @@ fn check_file_style(base_path: &Path, check: &mut RunningCheck, file: &Path, con
})
&& filename != "tests.rs"
{
let without_macro_call =
trimmed.split_once("todo!").expect("todo in line because of previous check").1;
let without_start =
without_macro_call.split_once("(").map_or(without_macro_call, |(_, s)| s);
let without_end =
without_start.rsplit_once(")").map_or(without_start, |(s, _)| s).trim();
let message = if without_end.is_empty() {
format_args!("")
} else {
format_args!("\n >> TODO: {}", without_end)
};

suppressible_tidy_err!(
err,
ignore.todo,
"the `todo!` macro is used for tasks that should be done before merging a PR. If you want to panic here, use `panic!`, `unimplemented!`, `unreachable!`, `rustc_middle::bug!` or an assertion"
"the `todo!` macro is used for tasks that should be done before merging a PR.\nIf you want to panic here, use `panic!`, `unimplemented!`, `unreachable!`, `rustc_middle::bug!` or an assertion{message}"
)
}
Comment thread
jdonszelmann marked this conversation as resolved.

Expand Down Expand Up @@ -508,11 +520,20 @@ fn check_file_style(base_path: &Path, check: &mut RunningCheck, file: &Path, con
if contains_potential_directive && (!has_recognized_directive) {
err("Unrecognized tidy directive")
}
// Allow using TODO in diagnostic suggestions by marking the
// relevant line with `ignore-tidy-todo`.
if trimmed.contains("TODO") && !trimmed.contains("ignore-tidy-todo") {
err(
"TODO is used for tasks that should be done before merging a PR; If you want to leave a message in the codebase use FIXME",
if trimmed.contains("TODO") {
let without_todo =
trimmed.split_once("TODO").expect("TODO in line because of previous check").1;
let without_colon = without_todo.trim().trim_start_matches(":").trim();

let message = if without_colon.is_empty() {
format_args!("")
} else {
format_args!("\n >> TODO: {}", without_colon)
};
suppressible_tidy_err!(
err,
ignore.todo,
"TODO is used for tasks that should be done before merging a PR;\nIf you want to leave a message in the codebase use FIXME{message}",
)
}
if trimmed.contains("//") && trimmed.contains(" XXX") {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/style/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ todo!()
),
vec![
"/test.rs:1: ignoring todo usage unnecessarily".to_string(),
"/test.rs:3: the `todo!` macro is used for tasks that should be done before merging a PR. If you want to panic here, use `panic!`, `unimplemented!`, `unreachable!`, `rustc_middle::bug!` or an assertion".to_string()
"/test.rs:3: the `todo!` macro is used for tasks that should be done before merging a PR.\nIf you want to panic here, use `panic!`, `unimplemented!`, `unreachable!`, `rustc_middle::bug!` or an assertion".to_string()
]
);
}
Expand Down
Loading