Skip to content

Commit 89a1cb9

Browse files
committed
Add codegen tidy check enforcing FIXME over TODO
1 parent 50db919 commit 89a1cb9

4 files changed

Lines changed: 67 additions & 0 deletions

File tree

src/tools/tidy/src/codegen.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//! Tidy check for codegen backend TODO policy.
2+
3+
use std::ffi::OsStr;
4+
use std::path::Path;
5+
6+
use crate::diagnostics::{CheckId, TidyCtx};
7+
use crate::walk::walk;
8+
9+
fn is_codegen_repo_path(path: &Path) -> bool {
10+
const CODEGEN_REPO_PATHS: &[&str] =
11+
&["compiler/rustc_codegen_cranelift", "compiler/rustc_codegen_gcc"];
12+
13+
CODEGEN_REPO_PATHS.iter().any(|repo| path.ancestors().any(|p| p.ends_with(Path::new(repo))))
14+
}
15+
16+
fn has_supported_extension(path: &Path) -> bool {
17+
const EXTENSIONS: &[&str] =
18+
&["rs", "py", "js", "sh", "c", "cpp", "h", "md", "css", "ftl", "toml", "yml", "yaml"];
19+
path.extension().is_some_and(|ext| EXTENSIONS.iter().any(|e| ext == OsStr::new(e)))
20+
}
21+
22+
pub fn check(path: &Path, tidy_ctx: TidyCtx) {
23+
let mut check = tidy_ctx.start_check(CheckId::new("codegen").path(path));
24+
25+
fn skip(path: &Path, is_dir: bool) -> bool {
26+
if path.file_name().is_some_and(|name| name.to_string_lossy().starts_with(".#")) {
27+
// Editor temp file.
28+
return true;
29+
}
30+
31+
if is_dir {
32+
return false;
33+
}
34+
35+
// Scan only selected text file types.
36+
!has_supported_extension(path)
37+
}
38+
39+
walk(path, skip, &mut |entry, contents| {
40+
let file = entry.path();
41+
42+
if !is_codegen_repo_path(file) {
43+
return;
44+
}
45+
46+
for (i, line) in contents.split('\n').enumerate() {
47+
let trimmed = line.trim();
48+
49+
// TODO policy for codegen-only trees.
50+
if trimmed.contains("TODO") && !trimmed.contains("ignore-tidy-todo") {
51+
check.error(format!(
52+
"{}:{}: TODO is used for tasks that should be done before merging a PR; \
53+
if you want to leave a message in the codebase use FIXME",
54+
file.display(),
55+
i + 1
56+
));
57+
}
58+
}
59+
});
60+
}

src/tools/tidy/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ pub fn files_modified(
104104
pub mod alphabetical;
105105
pub mod arg_parser;
106106
pub mod bins;
107+
pub mod codegen;
107108
pub mod debug_artifacts;
108109
pub mod deps;
109110
pub mod diagnostics;

src/tools/tidy/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ fn main() {
109109
check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path]);
110110
check!(target_policy, &root_path);
111111
check!(gcc_submodule, &root_path, &compiler_path);
112+
check!(codegen, &compiler_path);
112113

113114
// Checks that only make sense for the std libs.
114115
check!(pal, &library_path);

src/tools/tidy/src/style.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,11 +378,16 @@ pub fn check(path: &Path, tidy_ctx: TidyCtx) {
378378
// In some cases, a style check would be triggered by its own implementation
379379
// or comments. A simple workaround is to just allowlist this file.
380380
let this_file = Path::new(file!());
381+
let codegen_file = Path::new("src/tools/tidy/src/codegen.rs");
381382

382383
walk(path, skip, &mut |entry, contents| {
383384
let file = entry.path();
384385
let path_str = file.to_string_lossy();
385386
let filename = file.file_name().unwrap().to_string_lossy();
387+
let is_codegen_tidy_file = file.ends_with(codegen_file);
388+
if is_codegen_tidy_file {
389+
return;
390+
}
386391

387392
let is_css_file = filename.ends_with(".css");
388393
let under_rustfmt = filename.ends_with(".rs") &&

0 commit comments

Comments
 (0)