fix(rules): flag only deliberate TODO markers, skip eprintln and invariant expect

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - **`cora.scan_finalize` reports how much of the project the verdict covers (#685).** New optional input `batches_judged`; the result has `coverage` (`complete`, `partial`, or `unverified` when not declared), `total_batches`, `batches_judged`, `files_judged` and `files_total`. A partial run says `Judged 1/11 batches (3/23 files)` and `should_block` is null instead of false. The `/cora-review` skill now passes `batches_judged`.
+- **Three built-in rules stop flagging idioms (#681, part of #682).** `bug-todo` fires only on a deliberate marker: upper-case `TODO`, `FIXME`, `HACK` or `XXX` inside a comment (not `pub mod todo;`, `todo_state` or the word "todo" in prose). `bug-println` no longer matches `eprintln!` / `eprint!` (stderr diagnostics). `bug-expect` flags only `.expect("")`; a non-empty message states an invariant and is the recommended alternative to `unwrap()`. Treating every module of a binary crate as program code is still open in #682.
 - **Custom rules are validated and the docs match the schema (#684, #678).** `cora config validate` and config load now reject an invalid regex, an empty or duplicate id, an id that shadows a built-in rule, an empty message and an unknown `languages` value, naming the rule. `languages` accepts language names (`"rust"`) as well as extensions (`"rs"`); `languages` and `exclude` are optional. `cora config validate --dry-run [--path DIR]` runs each custom rule and prints how many lines it matched. `docs/configuration.md` now shows the real schema (`rules_engine.custom`); the 0.19.0 entry no longer says `custom_rules`.
 - **`cora-ignore` no longer fails silently (#679).** A marker on a line with findings that names none of them (a typo, an unknown rule, or explanation text that became part of the rule name) now prints a warning on stderr naming the marker and the rule ids on that line. ` -- ` ends the rule list, so `cora-ignore: sec-hardcoded-secret -- env var name` suppresses the finding and keeps the reason in the source.
 - The `Cora Review` workflow now fails with an error naming each empty secret (`CORA_API_KEY`, `CORA_BASE_URL`, `CORA_MODEL`) instead of running without them.

src/engine/rules/builtin.rs

@@ -69,7 +69,9 @@ pub fn builtin_rules() -> Vec<CustomRule> {
         },
         CustomRule {
             id: "bug-println".to_string(),
-            pattern: r"(?:println!|dbg!|print!\s*\()".to_string(),
+            // `eprintln!` / `eprint!` write to stderr (diagnostics) and are not matched:
+            // the macro name must not be preceded by an identifier character (#682).
+            pattern: r"(?:^|[^A-Za-z0-9_])(?:println!|dbg!|print!\s*\()".to_string(),
             severity: Severity::Minor,
             message: "Debug output macro found. Remove `println!`/`dbg!`/`print!` before merging.".to_string(),
             languages: vec!["rs".to_string()],
@@ -567,6 +569,8 @@ pub fn post_match_filter_for_path(rule_id: &str, line: &str, path: &str) -> bool
     match rule_id {
         "sec-hardcoded-secret" | "crypto/hardcoded-secret" => is_false_positive_secret(line, path),
         "bug-println" => is_cli_output_path(path),
+        "bug-todo" => !is_deliberate_todo_marker(line),
+        "bug-expect" => is_invariant_expect(line),
         "sec-hardcoded-url" => is_false_positive_url(line),
         "config/cors-wildcard" => is_false_positive_cors(line),
         "injection/sql-concat" | "sec-sql-concat" => is_false_positive_sql_concat(line),
@@ -578,6 +582,33 @@ pub fn post_match_filter_for_path(rule_id: &str, line: &str, path: &str) -> bool
     }
 }
 
+/// `bug-todo` fires only on a deliberate marker: an upper-case `TODO`, `FIXME`,
+/// `HACK` or `XXX` whole word inside a comment (#681). The word "todo" in an
+/// identifier, a module name or prose (`pub mod todo;`, "the todo list") is not
+/// a marker.
+fn is_deliberate_todo_marker(line: &str) -> bool {
+    static COMMENT_START: std::sync::LazyLock<regex::Regex> = std::sync::LazyLock::new(|| {
+        regex::Regex::new(r"//|/\*|<!--|#|--|%|^\s*\*").expect("comment start regex")
+    });
+    static MARKER: std::sync::LazyLock<regex::Regex> = std::sync::LazyLock::new(|| {
+        regex::Regex::new(r"\b(?:TODO|FIXME|HACK|XXX)\b").expect("marker regex")
+    });
+    let Some(start) = COMMENT_START.find(line) else {
+        return false;
+    };
+    MARKER.is_match(&line[start.start()..])
+}
+
+/// `.expect("<message>")` with a non-empty message documents an invariant, the
+/// panic style the Rust API guidelines recommend over a bare `unwrap()`; only
+/// `.expect("")` is flagged (#682). Returns `true` to suppress.
+fn is_invariant_expect(line: &str) -> bool {
+    let Some(i) = line.find(".expect(\"") else {
+        return false;
+    };
+    !line[i + ".expect(\"".len()..].starts_with('"')
+}
+
 /// `println!` is the intended output of a program's entry point: `main.rs`,
 /// `src/bin/`, `examples/`, `benches/` (#663). Other files keep the rule; a
 /// crate's own `commands/` modules can be silenced with `ignore.rules` or

src/engine/rules/mod.rs

@@ -844,6 +844,52 @@ mod tests {
             .collect()
     }
 
+    const TODO_SRC: &str = "\
+pub mod todo;
+fn f() { let todo_state = 1; let _ = todo_state; }
+/// the todo list is per session
+fn g() {}
+// TODO(me): real marker
+// todo lower case is prose
+// FIXME: also real
+# HACK in a hash comment
+";
+
+    #[test]
+    fn bug_todo_flags_only_deliberate_markers() {
+        let entry = file_entry("src/a.rs", TODO_SRC);
+        let ids: Vec<String> = rule_ids(&entry, &default_config())
+            .into_iter()
+            .filter(|i| i.starts_with("bug-todo"))
+            .collect();
+        assert_eq!(
+            ids,
+            vec!["bug-todo@5", "bug-todo@7", "bug-todo@8"],
+            "{ids:?}"
+        );
+    }
+
+    #[test]
+    fn bug_println_ignores_stderr_macros_but_keeps_stdout_ones() {
+        let src = "fn a(){ println!(\"x\"); }\nfn b(){ eprintln!(\"ok\"); }\nfn c(){ print!(\"y\"); }\nfn d(){ eprint!(\"z\"); }\n";
+        let entry = file_entry("src/lib.rs", src);
+        let ids: Vec<String> = rule_ids(&entry, &default_config())
+            .into_iter()
+            .filter(|i| i.starts_with("bug-println"))
+            .collect();
+        assert_eq!(ids, vec!["bug-println@1", "bug-println@3"], "{ids:?}");
+    }
+
+    #[test]
+    fn bug_expect_flags_an_empty_message_but_not_a_stated_invariant() {
+        let src = "fn a(){ c.stdout.take().expect(\"stdout piped above\"); }\nfn b(){ x.expect(\"\"); }\nfn c(){ y.unwrap(); }\n";
+        let entry = file_entry("src/lib.rs", src);
+        let ids = rule_ids(&entry, &default_config());
+        assert!(!ids.contains(&"bug-expect@1".to_string()), "{ids:?}");
+        assert!(ids.contains(&"bug-expect@2".to_string()), "{ids:?}");
+        assert!(ids.contains(&"bug-unwrap@3".to_string()), "{ids:?}");
+    }
+
     const RUST_WITH_TESTS: &str = "\
 fn real() {
     let a = x.unwrap();