Skip to content

Commit e09936e

Browse files
committed
Add codegen tidy check enforcing FIXME over TODO
1 parent b91901d commit e09936e

4 files changed

Lines changed: 65 additions & 1 deletion

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") {
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: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,7 @@ 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();
@@ -449,6 +450,7 @@ pub fn check(path: &Path, tidy_ctx: TidyCtx) {
449450
let is_this_file = file.ends_with(this_file) || this_file.ends_with(file);
450451
let is_test_for_this_file =
451452
is_test && file.parent().unwrap().ends_with(this_file.with_extension(""));
453+
let is_codegen_tidy_file = file.ends_with(codegen_file);
452454
// scanning the whole file for multiple needles at once is more efficient than
453455
// executing lines times needles separate searches.
454456
let any_problematic_line =
@@ -516,7 +518,7 @@ pub fn check(path: &Path, tidy_ctx: TidyCtx) {
516518
if line.contains('\r') {
517519
suppressible_tidy_err!(err, skip_cr, "CR character");
518520
}
519-
if !is_this_file {
521+
if !is_this_file && !is_codegen_tidy_file {
520522
let directive_line_starts = ["// ", "# ", "/* ", "<!-- "];
521523
let possible_line_start =
522524
directive_line_starts.into_iter().any(|s| line.starts_with(s));

0 commit comments

Comments
 (0)