check: a locale file nobody listed is not silently ignored

CHANGELOG.md

@@ -4,6 +4,8 @@ All notable changes to polygo. Versions follow [SemVer](https://semver.org); dat
 
 ## Unreleased
 
+- `locale` (warning): a locale file sitting next to the ones being checked whose locale is not in `target_locales` — a translator's whole language, silently unchecked. Found the same way as the `syntax` gap: a test fixture added `locales/it.json`, forgot to list `it`, and the reviewer reported a clean pull request. Only real language tags count, so a `README.json` or an Android `values-night` is not mistaken for a locale, and a file that does not parse keeps its `syntax` error instead of being reported twice.
+
 - `polygo-bot` runs on Cloud Run, where its always-free tier covers roughly 18,000 pull requests a month: `bot/deploy-cloud-run.sh` takes a project and the App's key and does the rest (APIs, Secret Manager, Cloud Build, deploy), then prints the webhook URL to paste into the App. `POLYGO_BOT_SYNC=1` reviews before answering GitHub, since Cloud Run stops a container's CPU once the response goes out and a background thread would be frozen mid-review.
 - The bot refuses what an App anyone can install will eventually be pointed at: a repository over `POLYGO_BOT_MAX_REPO_MB` (default 500, asked of the API before anything is fetched) and an installation asking for more than `POLYGO_BOT_MAX_REVIEWS` reviews in ten minutes (default 20).
 

README.md

@@ -53,6 +53,7 @@ Ten seconds, no setup. It reads the string files your app already has and report
 
 | code | severity | what it catches |
 |---|:-:|---|
+| `locale` | warning | a locale file sitting next to the others that nothing checks, because its locale is not in `target_locales` |
 | `syntax` | error | a string file that does not parse (a dropped comma, an unclosed `<string>`) — reported instead of the locale silently vanishing from the run |
 | `placeholders` | error | `%1$@`, `{{name}}`, `%(count)s`, `{0}`, `{n, plural, …}` missing, added, retyped or reordered — and a translated *name* (`{{ modelli }}` for `{{ models }}`) named as such |
 | `plural` | error | a CLDR form the locale needs and doesn't have: `few`/`many` for Polish, six for Arabic; ICU, `.xcstrings`, `.stringsdict`, `<plurals>`, `msgstr[n]`, i18next `_few` |

src/check/code.rs

@@ -30,6 +30,7 @@ impl std::fmt::Display for Severity {
 #[serde(rename_all = "lowercase")]
 pub enum Code {
     Syntax,
+    Locale,
     Placeholders,
     Plural,
     Markup,
@@ -55,8 +56,9 @@ pub enum Code {
 }
 
 impl Code {
-    pub const ALL: [Code; 23] = [
+    pub const ALL: [Code; 24] = [
         Code::Syntax,
+        Code::Locale,
         Code::Placeholders,
         Code::Plural,
         Code::Markup,
@@ -84,6 +86,7 @@ impl Code {
     pub fn as_str(self) -> &'static str {
         match self {
             Code::Syntax => "syntax",
+            Code::Locale => "locale",
             Code::Placeholders => "placeholders",
             Code::Plural => "plural",
             Code::Markup => "markup",
@@ -142,6 +145,7 @@ impl Code {
     pub fn title(self) -> &'static str {
         match self {
             Code::Syntax => "Unparsable file",
+            Code::Locale => "Locale not checked",
             Code::Placeholders => "Placeholder mismatch",
             Code::Plural => "Missing plural form",
             Code::Markup => "Markup mismatch",
@@ -173,6 +177,9 @@ impl Code {
             Code::Syntax => {
                 "A string file cannot be parsed: a comma missing from a JSON catalog, an unclosed <string>, a .po entry without its msgstr. Reported with the line the parser stopped at. It is an error on its own because of what it hides: a file polygo cannot read has no keys, so every other check would find nothing wrong with it and the locale would quietly vanish from the run."
             }
+            Code::Locale => {
+                "A locale file sits next to the ones being checked, but its locale is not in target_locales, so nothing in it is looked at: a translator's whole language, silently unchecked. Add the locale to polygo.toml (or `polygo locales add`), or delete the file. Only names that are real language tags count, so a README.json or an Android values-night directory is not mistaken for a locale."
+            }
             Code::Placeholders => {
                 "A format placeholder (%1$@, {{name}}, %(count)s, {0}, {n, plural, …}) is missing, added, retyped or reordered in the translation. At runtime the value lands in the wrong place, shows raw, or crashes (Android: %d where %s was expected). A placeholder whose name was translated ({{ models }} → {{ modelli }}, %(count)s → %(anzahl)s) is the most common placeholder bug of all; it is named as such, and `polygo check --fix` puts the source name back with no model involved. Unnumbered printf arguments are numbered by position, so reordering them needs explicit %2$s %1$s. In zero/one/two plural forms the count may be left out where those are exact counts; not where `one` also covers 21, 31 (ru, uk, be, hr, sr, bs, lt, lv)."
             }

src/check/formats/mod.rs

@@ -24,6 +24,7 @@ pub fn check(cx: &mut Cx) -> Result<()> {
             Format::Arb | Format::Po | Format::Resx => {}
         }
         files::check(cx, idx)?;
+        syntax::unlisted(cx, idx);
     }
     Ok(())
 }

src/check/formats/syntax.rs

@@ -1,3 +1,6 @@
+//! Files the project has but does not read: one that cannot be parsed, and one whose locale
+//! nobody listed.
+//!
 //! A string file that does not parse. Without this pass a locale file someone broke — a
 //! comma dropped from a JSON catalog, an unclosed `<string>` — is simply not there: the
 //! detector cannot read it, so the locale disappears from the run and `check` says the
@@ -81,6 +84,46 @@ pub fn check(cx: &mut crate::check::report::Cx, broken: &[Broken]) -> bool {
     fatal
 }
 
+/// A locale file the project is not checking, because its locale is not in
+/// `target_locales`. Someone added the language and forgot to say so, and everything in it
+/// — every placeholder, every plural — goes unlooked at.
+pub fn unlisted(cx: &mut crate::check::report::Cx, idx: usize) {
+    let (root, cfg) = (cx.root.to_path_buf(), cx.cfg);
+    let spec = &cfg.files[idx];
+    let source = cfg.source_locale.clone();
+    let targets = cfg.target_locales.clone();
+    let found: Vec<(String, PathBuf)> = locale_files(&root, spec)
+        .into_iter()
+        .filter(|(locale, path)| {
+            locale != &source
+                && !targets.contains(locale)
+                && crate::init::is_locale(locale)
+                // A file that does not parse is already reported as such, and that is why
+                // the detector left its locale out; saying so twice helps nobody.
+                && parse_error(path, spec.format).is_none()
+        })
+        .collect();
+    for (locale, path) in found {
+        let file = path
+            .strip_prefix(&root)
+            .unwrap_or(&path)
+            .to_string_lossy()
+            .replace('\\', "/");
+        cx.emit_at(
+            idx,
+            file.clone(),
+            None,
+            &file,
+            &locale,
+            Code::Locale,
+            Severity::Warning,
+            format!(
+                "nothing in this file is checked: `{locale}` is not in target_locales (`polygo locales add {locale}`, or delete the file)"
+            ),
+        );
+    }
+}
+
 /// `(line, message)` when the file does not parse, `None` when it does.
 fn parse_error(path: &Path, format: Format) -> Option<(Option<usize>, String)> {
     let text = match formats::read_text(path) {

tests/check_syntax.rs

@@ -142,3 +142,93 @@ fn the_reviewer_comments_on_the_broken_line() {
         c["body"]
     );
 }
+
+#[test]
+fn a_locale_file_nobody_listed_is_not_silently_ignored() {
+    let dir = tempfile::tempdir().unwrap();
+    let root = dir.path();
+    project(root);
+    // Someone added Italian and forgot to say so. Everything in it — every placeholder,
+    // every plural — would go unchecked without a word.
+    fs::write(
+        root.join("locales/it.json"),
+        "{\n  \"a\": \"Ciao {{utente}}\",\n  \"b\": \"Arrivederci\"\n}\n",
+    )
+    .unwrap();
+    // A file that matches the template but is not a locale must not be mistaken for one.
+    fs::write(
+        root.join("locales/README.json"),
+        "{\n  \"x\": \"notes\"\n}\n",
+    )
+    .unwrap();
+    fs::write(
+        root.join("polygo.toml"),
+        "source_locale = \"en\"\ntarget_locales = [\"de\"]\n\n[[files]]\nformat = \"json\"\npath = \"locales/en.json\"\nlocale_path = \"locales/{locale}.json\"\n\n[provider]\nkind = \"mock\"\n",
+    )
+    .unwrap();
+    let (code, out, _) = check(root, &["--json"]);
+    assert_eq!(
+        code, 0,
+        "an unlisted locale is a warning, not an error: {out}"
+    );
+    let v: serde_json::Value = serde_json::from_str(&out).unwrap();
+    let f: Vec<&serde_json::Value> = v["findings"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .filter(|f| f["code"] == "locale")
+        .collect();
+    assert_eq!(f.len(), 1, "{out}");
+    assert_eq!(f[0]["locale"], "it");
+    assert_eq!(f[0]["file"], "locales/it.json");
+    assert_eq!(f[0]["severity"], "warning");
+    assert!(
+        f[0]["message"]
+            .as_str()
+            .unwrap()
+            .contains("`it` is not in target_locales"),
+        "{}",
+        f[0]["message"]
+    );
+    // Listing it makes the warning go and the real bug in it appear.
+    fs::write(
+        root.join("polygo.toml"),
+        "source_locale = \"en\"\ntarget_locales = [\"de\", \"it\"]\n\n[[files]]\nformat = \"json\"\npath = \"locales/en.json\"\nlocale_path = \"locales/{locale}.json\"\n\n[provider]\nkind = \"mock\"\n",
+    )
+    .unwrap();
+    let (code, out, _) = check(root, &[]);
+    assert_eq!(code, 1, "{out}");
+    assert!(!out.contains("locale:"), "{out}");
+    assert!(out.contains("placeholder name translated"), "{out}");
+}
+
+#[test]
+fn an_android_qualifier_directory_is_not_a_locale() {
+    let dir = tempfile::tempdir().unwrap();
+    let root = dir.path();
+    for d in ["values", "values-de", "values-night", "values-w600dp"] {
+        fs::create_dir_all(root.join("res").join(d)).unwrap();
+        fs::write(
+            root.join("res").join(d).join("strings.xml"),
+            "<resources><string name=\"a\">Hi</string></resources>\n",
+        )
+        .unwrap();
+    }
+    fs::write(
+        root.join("polygo.toml"),
+        "source_locale = \"en\"\ntarget_locales = [\"de\"]\n\n[[files]]\nformat = \"android\"\npath = \"res/values/strings.xml\"\nlocale_path = \"res/values-{android_locale}/strings.xml\"\n\n[provider]\nkind = \"mock\"\n",
+    )
+    .unwrap();
+    let (_, out, _) = check(root, &["--json"]);
+    let v: serde_json::Value = serde_json::from_str(&out).unwrap();
+    let locales: Vec<&serde_json::Value> = v["findings"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .filter(|f| f["code"] == "locale")
+        .collect();
+    assert!(
+        locales.is_empty(),
+        "night and w600dp are qualifiers, not languages: {out}"
+    );
+}