From ea681ef2814b321177d1eee437b8867d31141754 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 21 Jun 2024 18:20:41 -0400 Subject: [PATCH] Add a tidy rule to make sure that diagnostics don't end in periods --- src/tools/tidy/Cargo.toml | 1 + src/tools/tidy/src/fluent_period.rs | 84 +++++++++++++++++++++++++++++ src/tools/tidy/src/lib.rs | 1 + src/tools/tidy/src/main.rs | 1 + 4 files changed, 87 insertions(+) create mode 100644 src/tools/tidy/src/fluent_period.rs diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml index 63963b0bd1ced..f39438bd9ace2 100644 --- a/src/tools/tidy/Cargo.toml +++ b/src/tools/tidy/Cargo.toml @@ -13,6 +13,7 @@ ignore = "0.4.18" semver = "1.0" termcolor = "1.1.3" rustc-hash = "1.1.0" +fluent-syntax = "0.11.1" [[bin]] name = "rust-tidy" diff --git a/src/tools/tidy/src/fluent_period.rs b/src/tools/tidy/src/fluent_period.rs new file mode 100644 index 0000000000000..3a1fb6daf4854 --- /dev/null +++ b/src/tools/tidy/src/fluent_period.rs @@ -0,0 +1,84 @@ +//! Checks that no Fluent messages or attributes end in periods (except ellipses) + +use fluent_syntax::ast::{Entry, PatternElement}; + +use crate::walk::{filter_dirs, walk}; +use std::path::Path; + +fn filter_fluent(path: &Path) -> bool { + if let Some(ext) = path.extension() { ext.to_str() != Some("ftl") } else { true } +} + +/// Messages allowed to have `.` at their end. +/// +/// These should probably be reworked eventually. +const ALLOWLIST: &[&str] = &[ + "const_eval_long_running", + "const_eval_validation_failure_note", + "driver_impl_ice", + "incremental_corrupt_file", + "mir_build_pointer_pattern", +]; + +fn check_period(filename: &str, contents: &str, bad: &mut bool) { + if filename.contains("codegen") { + // FIXME: Too many codegen messages have periods right now... + return; + } + + let (Ok(parse) | Err((parse, _))) = fluent_syntax::parser::parse(contents); + for entry in &parse.body { + if let Entry::Message(m) = entry { + if ALLOWLIST.contains(&m.id.name) { + continue; + } + + if let Some(pat) = &m.value { + if let Some(PatternElement::TextElement { value }) = pat.elements.last() { + // We don't care about ellipses. + if value.ends_with(".") && !value.ends_with("...") { + let ll = find_line(contents, *value); + let name = m.id.name; + tidy_error!(bad, "{filename}:{ll}: message `{name}` ends in a period"); + } + } + } + + for attr in &m.attributes { + // Teach notes usually have long messages. + if attr.id.name == "teach_note" { + continue; + } + + if let Some(PatternElement::TextElement { value }) = attr.value.elements.last() { + if value.ends_with(".") && !value.ends_with("...") { + let ll = find_line(contents, *value); + let name = attr.id.name; + tidy_error!(bad, "{filename}:{ll}: attr `{name}` ends in a period"); + } + } + } + } + } +} + +/// Evil cursed bad hack. Requires that `value` be a substr (in memory) of `contents`. +fn find_line(haystack: &str, needle: &str) -> usize { + for (ll, line) in haystack.lines().enumerate() { + if line.as_ptr() > needle.as_ptr() { + return ll; + } + } + + 1 +} + +pub fn check(path: &Path, bad: &mut bool) { + walk( + path, + |path, is_dir| filter_dirs(path) || (!is_dir && filter_fluent(path)), + &mut |ent, contents| { + check_period(ent.path().to_str().unwrap(), contents, bad); + }, + ); +} diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index ae9d2b8b8dc87..ecd32727fa058 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -72,6 +72,7 @@ pub mod ext_tool_checks; pub mod extdeps; pub mod features; pub mod fluent_alphabetical; +pub mod fluent_period; mod fluent_used; pub(crate) mod iter_header; pub mod known_bug; diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 1d2b2e4d03472..ec6fc1f07f11e 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -115,6 +115,7 @@ fn main() { // Checks that only make sense for the compiler. check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose); check!(fluent_alphabetical, &compiler_path, bless); + check!(fluent_period, &compiler_path); check!(target_policy, &root_path); // Checks that only make sense for the std libs.