check: a finding about a file sits on line 1, so a review can carry it

CHANGELOG.md

@@ -4,6 +4,8 @@ All notable changes to polygo. Versions follow [SemVer](https://semver.org); dat
 
 ## Unreleased
 
+- A finding about a whole file (`locale`, and a `syntax` error whose parser gave no position) now sits on line 1 instead of nowhere. A review can only comment on a line, so those findings never reached the pull request that introduced them: the bot reported a pull request adding an unlisted locale as clean, which is the failure the `locale` code exists to end.
+
 - `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.

src/check/formats/syntax.rs

@@ -18,6 +18,9 @@ pub struct Broken {
     pub idx: usize,
     /// Path relative to the checked root, as findings name files.
     pub file: String,
+    /// Where to put it. A finding about the file rather than a line in it goes on line 1:
+    /// a reviewer can only comment on a line, so a finding without one never reaches the
+    /// pull request that introduced it.
     pub line: Option<usize>,
     /// The locale the path stands for, or the source locale for a source file.
     pub locale: String,
@@ -112,7 +115,7 @@ pub fn unlisted(cx: &mut crate::check::report::Cx, idx: usize) {
         cx.emit_at(
             idx,
             file.clone(),
-            None,
+            Some(1),
             &file,
             &locale,
             Code::Locale,
@@ -141,7 +144,7 @@ fn parse_error(path: &Path, format: Format) -> Option<(Option<usize>, String)> {
     };
     let e = parsed.err()?;
     let message = one_line(&format!("{e:#}"));
-    Some((line_of(&message, &text), message))
+    Some((Some(line_of(&message, &text).unwrap_or(1)), message))
 }
 
 /// Where the fix goes. Parsers say either "at line 584 column 2" or "at byte 34886"; a
@@ -244,6 +247,6 @@ mod tests {
         assert_eq!(line_of("expected ',' or '}' at byte 5", text), Some(3));
         assert_eq!(line_of("expected ',' at byte 0", text), Some(1));
         assert_eq!(line_of("beyond the end at byte 9999", text), Some(3));
-        assert_eq!(line_of("no numbers here", text), None);
+        assert_eq!(line_of("no numbers here", text), None); // the caller falls back to 1
     }
 }

tests/check_syntax.rs

@@ -232,3 +232,39 @@ fn an_android_qualifier_directory_is_not_a_locale() {
         "night and w600dp are qualifiers, not languages: {out}"
     );
 }
+
+/// A finding about a whole file has to sit on a line, or the reviewer cannot say it: a
+/// pull request that adds an unlisted locale would come back clean, which is the very
+/// failure the `locale` code exists to end.
+#[test]
+fn a_file_level_finding_lands_on_line_one_so_a_review_can_carry_it() {
+    let dir = tempfile::tempdir().unwrap();
+    let root = dir.path();
+    project(root);
+    fs::write(
+        root.join("locales/it.json"),
+        "{\n  \"a\": \"Ciao {{name}}\",\n  \"b\": \"Arrivederci\"\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();
+    // The diff that adds the file, as GitHub would give it.
+    fs::write(
+        root.join("pr.diff"),
+        "--- a/locales/it.json\n+++ b/locales/it.json\n@@ -0,0 +1,4 @@\n+{\n+  \"a\": \"Ciao {{name}}\",\n+  \"b\": \"Arrivederci\"\n+}\n",
+    )
+    .unwrap();
+    let (_, out, _) = check(root, &["--review", "--diff", "pr.diff"]);
+    let v: serde_json::Value = serde_json::from_str(&out).unwrap();
+    let c = &v["comments"][0];
+    assert_eq!(c["path"], "locales/it.json", "{out}");
+    assert_eq!(c["line"], 1);
+    assert!(
+        c["body"].as_str().unwrap().contains("Locale not checked"),
+        "{}",
+        c["body"]
+    );
+}