Skip to content

Commit 04d7e1b

Browse files
Add new optional tidy check to check if ftl error messages are unused
1 parent 8d72d3e commit 04d7e1b

File tree

3 files changed

+109
-0
lines changed

3 files changed

+109
-0
lines changed
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
use std::collections::HashMap;
2+
use std::path::Path;
3+
4+
use fluent_syntax::ast::{Entry, Expression, InlineExpression, PatternElement};
5+
use fluent_syntax::parser;
6+
use regex::Regex;
7+
8+
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
9+
10+
pub fn check(root_path: &Path, diag_ctx: DiagCtx) {
11+
let path = &root_path.join("compiler");
12+
let mut check = diag_ctx.start_check(CheckId::new("error_messages").path(path));
13+
let regex = Regex::new(r#"(?:(?:#\[(diag|label|suggestion|note|help|help_once|multipart_suggestion)\()|(?:fluent(_generated)?::))\s*(?<id>[a-z_][a-zA-Z0-9_]+)\s*[\n,\)};]"#).unwrap();
14+
15+
for dir in std::fs::read_dir(path).unwrap() {
16+
let Ok(dir) = dir else { continue };
17+
let dir = dir.path();
18+
let messages_file = dir.join("messages.ftl");
19+
20+
if messages_file.is_file() {
21+
check_if_messages_are_used(&mut check, &dir.join("src"), &messages_file, &regex);
22+
}
23+
}
24+
}
25+
26+
fn check_fluent_ast<'a>(ids: &mut HashMap<&'a str, bool>, elem: &PatternElement<&'a str>) {
27+
if let PatternElement::Placeable { expression } = elem {
28+
match expression {
29+
Expression::Inline(InlineExpression::MessageReference { id, .. }) => {
30+
*ids.entry(&id.name).or_default() = true;
31+
}
32+
Expression::Select { variants, .. } => {
33+
for variant in variants {
34+
for elem in &variant.value.elements {
35+
check_fluent_ast(ids, elem);
36+
}
37+
}
38+
}
39+
_ => {}
40+
}
41+
}
42+
}
43+
44+
fn check_if_messages_are_used(
45+
check: &mut RunningCheck,
46+
src_path: &Path,
47+
messages_file: &Path,
48+
regex: &Regex,
49+
) {
50+
// First we retrieve all error messages ID.
51+
let content = std::fs::read_to_string(messages_file).expect("failed to read file");
52+
let resource =
53+
parser::parse(content.as_str()).expect("Errors encountered while parsing a resource.");
54+
55+
let mut ids: HashMap<&str, bool> = HashMap::new();
56+
for entry in &resource.body {
57+
if let Entry::Message(msg) = entry {
58+
let id: &str = &msg.id.name;
59+
if !ids.contains_key(&id) {
60+
ids.insert(id, false);
61+
}
62+
if let Some(value) = &msg.value {
63+
for elem in &value.elements {
64+
check_fluent_ast(&mut ids, elem);
65+
}
66+
}
67+
for attr in &msg.attributes {
68+
for elem in &attr.value.elements {
69+
check_fluent_ast(&mut ids, elem);
70+
}
71+
}
72+
}
73+
}
74+
75+
assert!(!ids.is_empty());
76+
77+
let skip = |f: &Path, is_dir: bool| !is_dir && !f.extension().is_some_and(|ext| ext == "rs");
78+
crate::walk::walk(src_path, skip, &mut |_path: &_, content: &str| {
79+
for cap in regex.captures_iter(content) {
80+
let id = &cap["id"];
81+
if let Some(found) = ids.get_mut(id) {
82+
// Error message IDs can be used more than once.
83+
*found = true;
84+
}
85+
}
86+
});
87+
const TO_IGNORE: &[&str] = &[
88+
// FIXME: #114050
89+
"hir_typeck_option_result_asref",
90+
];
91+
for (id, found) in ids {
92+
if !found && !TO_IGNORE.iter().any(|to_ignore| *to_ignore == id) {
93+
check.error(format!("unused message ID `{id}` from `{}`", messages_file.display()));
94+
}
95+
}
96+
}

src/tools/tidy/src/extra_checks/mod.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ pub fn check(
7070
bless,
7171
extra_checks,
7272
pos_args,
73+
diag_ctx,
7374
) {
7475
check.error(e);
7576
}
@@ -86,6 +87,7 @@ fn check_impl(
8687
bless: bool,
8788
extra_checks: Option<&str>,
8889
pos_args: &[String],
90+
diag_ctx: DiagCtx,
8991
) -> Result<(), Error> {
9092
let show_diff =
9193
std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1");
@@ -138,6 +140,7 @@ fn check_impl(
138140
let shell_lint = extra_check!(Shell, Lint);
139141
let cpp_fmt = extra_check!(Cpp, Fmt);
140142
let spellcheck = extra_check!(Spellcheck, None);
143+
let fluent_check = extra_check!(Ftl, None);
141144
let js_lint = extra_check!(Js, Lint);
142145
let js_typecheck = extra_check!(Js, Typecheck);
143146

@@ -346,6 +349,11 @@ fn check_impl(
346349
rustdoc_js::typecheck(outdir, librustdoc_path)?;
347350
}
348351

352+
if fluent_check {
353+
eprintln!("checking ftl files");
354+
crate::error_messages::check(&root_path, diag_ctx);
355+
}
356+
349357
Ok(())
350358
}
351359

@@ -766,6 +774,7 @@ impl ExtraCheckArg {
766774
}
767775
&[]
768776
}
777+
ExtraCheckLang::Ftl => &[".ftl"],
769778
};
770779
exts.iter().any(|ext| filepath.ends_with(ext))
771780
}
@@ -782,6 +791,7 @@ impl ExtraCheckArg {
782791
ExtraCheckLang::Shell => &[Lint],
783792
ExtraCheckLang::Spellcheck => &[],
784793
ExtraCheckLang::Js => &[Lint, Typecheck],
794+
ExtraCheckLang::Ftl => &[],
785795
};
786796
supported_kinds.contains(&kind)
787797
}
@@ -823,6 +833,7 @@ enum ExtraCheckLang {
823833
Cpp,
824834
Spellcheck,
825835
Js,
836+
Ftl,
826837
}
827838

828839
impl FromStr for ExtraCheckLang {
@@ -835,6 +846,7 @@ impl FromStr for ExtraCheckLang {
835846
"cpp" => Self::Cpp,
836847
"spellcheck" => Self::Spellcheck,
837848
"js" => Self::Js,
849+
"ftl" => Self::Ftl,
838850
_ => return Err(ExtraCheckParseError::UnknownLang(s.to_string())),
839851
})
840852
}

src/tools/tidy/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ pub mod deps;
229229
pub mod diagnostics;
230230
pub mod edition;
231231
pub mod error_codes;
232+
pub mod error_messages;
232233
pub mod extdeps;
233234
pub mod extra_checks;
234235
pub mod features;

0 commit comments

Comments
 (0)