fix: batch of 73 bug/doc fixes (#739–#823)

CHANGELOG.md

@@ -23,6 +23,15 @@ for historical reference.
 
 ### Added
 
+- `MetricSet::resolved()` returns the set closed under
+  `Metric::dependencies` (idempotent), the set-in/set-out counterpart of
+  `from_slice_with_deps` (#743).
+- `defang_formula` is now public (re-exported from the crate root) so the
+  CLI's VCS-report CSV writer can share the lib's CWE-1236 spreadsheet
+  formula-injection mitigation rather than duplicating it (#794).
+- `AuthorId::has_identity()` reports whether a VCS author carries any
+  usable name or email key (#817).
+
 - `LANG::Objc` (slug `objc`) and the `objc` Cargo feature: dedicated
   Objective-C support backed by upstream `tree-sitter-objc` `=3.0.2`,
   owning the `.m` extension and the `objc` / `objective-c` emacs modes
@@ -1041,6 +1050,10 @@ for historical reference.
 
 ### Changed
 
+- VCS JIT scoring diffs each touched blob once (computing added/deleted
+  counts and hunk count from a single `Diff::compute`) instead of twice,
+  with bit-identical results (#815).
+
 - The HTML and Markdown report's headline **Average MI** is now the
   **SLOC-weighted mean of the *unclamped* Visual Studio MI** and is
   relabelled `Average MI (SLOC-weighted)`. Previously it averaged the
@@ -2363,6 +2376,110 @@ for historical reference.
 
 ### Fixed
 
+- Cross-language metric consistency: several per-language metrics were
+  brought into line with their siblings. ABC now counts numeric-truthy
+  operands in Python/JS/TS boolean slots (`if 5:`, `while (5)`, `x && 5`)
+  (#772) and counts a Kotlin bare `if`/`while`/`do-while` predicate as
+  one condition (#773); cognitive complexity resets nesting and applies
+  the function-depth surcharge at nested PHP function/method boundaries
+  (#775); cyclomatic no longer counts the head clause of a single-clause
+  Elixir anonymous function (#776); JS-family LLOC no longer counts
+  `statement_block` brace groupings as logical lines (#777); multi-line
+  string interior rows now count as PLOC (not blank) across all languages
+  with multi-line strings, matching Python (#778); `nexits` now counts Go
+  `panic` and Lua `error`/`os.exit` as abrupt exits (#779); `npa` no
+  longer counts C# interface fields with explicit `private`/`protected`
+  as public (#780) nor PHP enum cases as attributes (#781); and `npm` no
+  longer counts a C# property's narrowed (`private`/`protected`) accessor
+  as a public method (#783). These shift the affected languages' metric
+  values; integration snapshots are re-baselined.
+- `loc.sloc` under `--exclude-tests` now drops the lines of `#[test]`
+  functions nested in retained `impl`/`trait`/closure spaces, not only
+  top-level `#[cfg(test)] mod`, matching `loc.ploc` and the MI SLOC term
+  (#741).
+- `MetricsOptions::with_metric_set` now resolves the supplied
+  `MetricSet`'s dependency closure before storing it, so a derived metric
+  (`Mi`/`Wmc`) selected via a hand-built set no longer computes from
+  zero-valued prerequisites (#743).
+- The `nargs` text (`Display`) output now reports `function_args` /
+  `closure_args` as the cross-space sum, matching the JSON/YAML/TOML/CBOR
+  serializers (#782).
+- Markdown VCS bus-factor directory cells are now GFM-escaped, so a `|`
+  in a directory path no longer corrupts the table (#739). `bca preproc`
+  recovers its worker accumulator without panicking on a poisoned mutex
+  or un-joined `Arc` (#740).
+- Output edge cases: Checkstyle clamps a `Some(0)` column to `column="1"`
+  (#784); SARIF neutralizes a scheme-ambiguous colon in a relative path's
+  first segment with a `./` prefix (#798); and the human-readable number
+  formatter renders a tiny negative that rounds to zero as `0`, not `-0`
+  (#800).
+- `read_file_with_eol` now skips UTF-16 BE/LE BOM files (`Ok(None)`)
+  instead of stripping the BOM and parsing the interleaved-NUL body as
+  garbage, and propagates a genuine probe-read I/O error as `Err` instead
+  of swallowing it as `Ok(None)` (#803, #804).
+- Comment removal preserves the source file's existing line-ending
+  convention: stripping comments from a CRLF file no longer emits LF in
+  the removed-comment region (#767).
+- `Ast::ops` no longer invents a synthetic `<anonymous>` name for unnamed
+  `Unit` spaces (their `Ops::name` is `None`), and now wraps an
+  ERROR-root parse in a synthetic `Unit` space instead of returning
+  `Err(EmptyRoot)`, matching `metrics()` (#755, #789).
+- `CountCollector::into_count` trips a `debug_assert!` when called while
+  the collector is still shared (a worker failed to join) instead of
+  silently returning a non-final snapshot (#757).
+- A `cfg(not(X), …)`-led top-level comma list ending in `)` no longer
+  swallows a trailing `test` operand, so such items are correctly
+  classified test-only under `--exclude-tests` (#763).
+- The C/C++ macro-masking prepass treats a C++14/C23 digit-separator `'`
+  (`1'000`, `0xDEAD'BEEF`) as numeric rather than a char-literal opener,
+  so macros after such a literal are still masked (#765).
+- The predefined-macro and special-token tables no longer list the
+  nonexistent `UINT*_MIN` macros (#760) or the non-type `char64_t` /
+  `charptr_t` specials (#762).
+- Bash `heredoc_body` and Perl `heredoc_body_statement` now flatten in
+  the AST dump, matching `Checker::is_string` (#761).
+- VCS correctness: rollback revert-detection is now subject-anchored so a
+  body-prose "rollback" no longer flags a commit as a revert (#806); the
+  security-fix classifier requires a qualifier for `injection`/`overflow`
+  and drops the bare ambiguous terms (#808); a persistent history-cache
+  entry written from a shallow clone is no longer reused after the repo
+  is deepened (and vice versa) (#810); `Co-authored-by:` is only honoured
+  in a commit's final trailer block, so a body-quoted co-author is no
+  longer counted (#812); authors with no name and no email are dropped
+  from the participant set instead of collapsing into one phantom
+  identity (#817); and a rename-only file with a space in its new path is
+  no longer truncated, taking the path from the authoritative `rename to`
+  line (#813).
+- VCS arithmetic is now saturating/checked at the blame line-number,
+  window-cutoff, days-rounding, and cross-author edit-sum sites,
+  restoring the subsystem's no-panic invariant on extreme/degenerate
+  inputs (#742, #809, #814, #820, #821).
+- `Node::child_by_field_name` returns `Node<'a>` (the tree lifetime)
+  instead of a borrow-scoped `Node<'_>`, so callers can hold a child past
+  the parent borrow (#786).
+- Documentation/comment accuracy: the crate-level Supported Languages
+  rustdoc now lists Objective-C and three corrected slugs, guarded by a
+  drift test (#769); and numerous stale or misleading comments/docs were
+  corrected across the CSV/dump/parser/output modules, the suppression
+  hint (now derived from `Metric::suppressible()`), the VCS jit/score/
+  trend/wire/error/identity modules, and the book (#764, #766, #771,
+  #774, #788, #790, #791, #792, #793, #795, #796, #797, #799, #801, #802,
+  #805, #807, #816, #818, #819, #822, #823). The `tokens` metric gained
+  smoke tests for C/Objc/Elixir/Ruby/iRules and a strengthened C++
+  attribution test (#785, #787).
+- `read_file_with_eol` now validates its 64-byte UTF-8 probe at the
+  byte level with `std::str::from_utf8` instead of the previous
+  `from_utf8_lossy` + unconditional `pop` + `U+FFFD` scan (#746, #758).
+  The old heuristic dropped the probe's final character unconditionally,
+  which both hid a genuinely invalid trailing byte (wrongly accepting
+  non-UTF-8 input) and rejected files that legitimately contained the
+  `U+FFFD` replacement scalar within the probe window. A trailing
+  incomplete multibyte sequence is now tolerated only when the file
+  continues past the probe (so the split character is completed by later
+  bytes); when the probe is the whole file, an incomplete tail is
+  treated as genuine corruption. The public `Ok(None)` /
+  `Ok(Some(..))` contract is unchanged.
+
 - `--exclude-tests` (and `MetricsOptions::exclude_tests`) now prunes
   unit- and function-level `loc.sloc` in step with the other loc
   sub-metrics (#722). `sloc` is the lone loc metric computed by span
@@ -3617,6 +3734,17 @@ for historical reference.
 
 ### Security
 
+- The VCS-report CSV writer (`bca vcs --format csv`) now defangs the
+  free-text `path` column against spreadsheet formula injection
+  (CWE-1236), closing the second emission path that bypassed the #703 fix
+  in the standard CSV output (#794).
+- The `--emit-author-details` author hash is now documented honestly as a
+  stable pseudonym (it avoids emitting plaintext emails and deters casual
+  disclosure) rather than "irreversible": an unsalted SHA-256 of a
+  low-entropy, enumerable email is recoverable against a candidate email
+  set (the Gravatar weakness). Opt-in keyed/salted hardening is tracked
+  as a follow-up (#811).
+
 - CSV output now defangs spreadsheet formula injection (CWE-1236): a
   `path` or `space_name` cell beginning with `=` `+` `-` `@` tab or CR is
   prefixed with `'` so a spreadsheet app treats it as literal text rather

STABILITY.md

@@ -81,8 +81,12 @@ section.
     is the opaque bitfield consumed by
     `MetricsOptions::with_only(&[Metric])` and read back through
     `CodeMetrics::selected()`. Its constructors (`empty`, `all`,
-    `with`, `union`) and inspectors (`contains`) are stable; the
-    underlying integer representation is not. `Metric` is the single
+    `with`, `union`), the dependency-closure operations
+    (`from_slice_with_deps`, `resolved`), and inspectors
+    (`contains`) are stable; the underlying integer representation
+    is not. `MetricsOptions::with_metric_set` closes the supplied
+    set under `Metric::dependencies` before storing it (#743), so a
+    derived metric always reaches the walker with its inputs. `Metric` is the single
     metric vocabulary for both selection and suppression: it now
     derives `Ord`/`PartialOrd` (declaration order, so a
     `BTreeSet<Metric>` iterates deterministically) and implements

big-code-analysis-book/src/commands/nodes.md

@@ -20,7 +20,15 @@ bca find -t ERROR -I "*.ext" /path/to/your/file/or/directory
   all files when given a directory). Paths are given positionally or via
   `--paths`; both are unioned. Flags follow the subcommand.
 - `-t, --type`: the node type to match. Repeat the flag for several
-  types (`-t function_item -t struct_item`); at least one is required.
+  types (`-t function_item -t struct_item`); at least one is required. A
+  *string* value matches the node-type name exactly (for example
+  `function_item`). A purely *numeric* value is instead interpreted as a
+  raw tree-sitter `kind_id` and matches nodes whose internal symbol id
+  equals that number (so `-t 0` matches the end/`ERROR` sentinel). The
+  numeric form is an escape hatch for grammar inspection and is unstable:
+  a `kind_id` is an index into the grammar's symbol table, so the same
+  number names a different node after a grammar-version bump. Prefer the
+  string form unless you specifically need a kind that has no stable name.
 - `-I, --include`: glob filter for selecting files by extension (e.g.
   `*.js`, `*.rs`). Each `-I` takes exactly one value, so a following
   positional path is never swallowed.

big-code-analysis-book/src/commands/vcs.md

@@ -236,9 +236,11 @@ they reuse the same cached walk.)
 By default the cache lives under
 `$XDG_CACHE_HOME/big-code-analysis/vcs` (`%LOCALAPPDATA%` on Windows,
 `~/.cache` otherwise). Author identities are stored only as their
-irreversible SHA-256 digests — never plaintext — so the cache is not a
-side channel for raw author emails. The same cache transparently
-accelerates `bca metrics --vcs` and `bca report --vcs`.
+SHA-256 digests — never plaintext — so the cache holds no raw author
+emails. Note this is pseudonymization, not anonymization: the digests
+are recoverable against a candidate email set (see
+[`--emit-author-details`](#author-detail-privacy)). The same cache
+transparently accelerates `bca metrics --vcs` and `bca report --vcs`.
 
 ```bash
 # First run primes the cache; the second replays it.
@@ -586,6 +588,22 @@ only for the dedicated `bca vcs` / `bca report --vcs` reports and the REST
 / Python endpoints; the per-file `bca metrics --vcs` injection path does
 not pay for it.
 
+### Author-detail privacy
+
+The `key_author_ids` digests are a **stable pseudonym, not
+anonymization**. Hashing keeps plaintext emails out of the report and the
+cache and deters casual disclosure, but the hash is *not* cryptographically
+irreversible. The pre-image is an email — low-entropy and enumerable — and
+commit histories are public, so anyone with a candidate set of emails can
+recover which digest belongs to whom by hashing each candidate or with a
+precomputed email→hash table. This is the same weakness that broke
+Gravatar's email hashing.
+
+Treat published `key_author_ids` as pseudonymization that avoids emitting
+plaintext emails, **not** as a guarantee that authors cannot be
+re-identified by a determined attacker. If you need that guarantee, do not
+publish the digests.
+
 ## Dogfooding in this repo
 
 This project runs `bca vcs` on its own source. `make vcs` prints the

big-code-analysis-book/src/metrics.md

@@ -618,7 +618,7 @@ The three values nest under the `mi` object as the keys `original`,
 
 ```text
 mi.original      = 171 − 5.2·ln(HV) − 0.23·CC − 16.2·ln(SLOC)
-mi.sei           = 171 − 5.2·log2(HV) − 0.23·CC − 16.2·log2(SLOC) + 50·sin(√(2.4·comment_ratio))
+mi.sei           = 171 − 5.2·log2(HV) − 0.23·CC − 16.2·log2(SLOC) + 50·sin(√(2.4·comment_percentage))
 mi.visual_studio = max(0, mi.original · 100 / 171)
 ```
 
@@ -627,6 +627,10 @@ mi.visual_studio = max(0, mi.original · 100 / 171)
 - `mi.sei` is the Software Engineering Institute's refinement, which
   adds a comment-density term — the `sin(√(...))` shape was chosen so
   that *some* comments help, but adding more after a point does not.
+  `comment_percentage` is the comment-line share expressed as a
+  percentage in `[0, 100]` (not a ratio in `[0, 1]`); the code feeds
+  this percentage straight into the SEI term (see `src/metrics/mi.rs`
+  and issue #241).
 - `mi.visual_studio` is the linear rescaling Microsoft chose for
   Visual Studio, where the score is clamped to `[0, 100]` and shown
   to developers traffic-light style: green ≥ 20, yellow ≥ 10, red

big-code-analysis-book/src/python/vcs.md

@@ -214,8 +214,10 @@ vcs.rank("path/to/repo", cache_dir="/tmp/bca")     # override the directory
 ```
 
 By default the cache lives under the platform cache directory. Author
-identities are stored only as their irreversible SHA-256 digests, never
-plaintext.
+identities are stored only as their SHA-256 digests, never plaintext.
+Note that hashing is pseudonymization, not anonymization: the digests are
+recoverable against a candidate email set — see
+[Author-detail privacy](../commands/vcs.md#author-detail-privacy).
 
 ## Releasing the GIL
 

big-code-analysis-cli/src/commands.rs

@@ -2171,6 +2171,33 @@ fn run_command_strip_comments(
     run_walk_resolved(resolved.files, num_jobs, cfg);
 }
 
+/// Recovers the accumulated [`PreprocResults`] from the shared worker
+/// accumulator once every worker has joined.
+///
+/// Mirrors the panic-free recovery of [`CountCollector::into_count`]
+/// (issue #445): a worker that panicked mid-update poisons the inner
+/// mutex, and a worker that failed to join leaves the `Arc` shared.
+/// Both failure modes degrade to the recovered data rather than
+/// panicking (issue #740). The recovered guard still holds the
+/// fully-applied append-collections, since each worker inserts a
+/// distinct file's entry.
+fn into_preproc_data(preproc_lock: Arc<Mutex<PreprocResults>>) -> PreprocResults {
+    match Arc::try_unwrap(preproc_lock) {
+        Ok(mutex) => mutex
+            .into_inner()
+            .unwrap_or_else(std::sync::PoisonError::into_inner),
+        Err(shared) => {
+            let mut guard = shared
+                .lock()
+                .unwrap_or_else(std::sync::PoisonError::into_inner);
+            // The `Arc` is still shared (a worker failed to join), so we
+            // cannot move the data out by value; take it from behind the
+            // guard, leaving an empty default in its place.
+            std::mem::take(&mut *guard)
+        }
+    }
+}
+
 fn run_command_preproc(globals: GlobalOpts, args: PreprocArgs) {
     let preproc_lock = Arc::new(Mutex::new(PreprocResults::default()));
     let output = args.output;
@@ -2189,10 +2216,7 @@ fn run_command_preproc(globals: GlobalOpts, args: PreprocArgs) {
     // `bca preproc` include resolution — see #495.
     let all_files = group_files_by_basename(paths);
 
-    let mut data = Arc::try_unwrap(preproc_lock)
-        .expect("all worker threads have joined; Arc refcount is 1")
-        .into_inner()
-        .expect("mutex not poisoned");
+    let mut data = into_preproc_data(preproc_lock);
     // Include-resolution diagnostics (self-inclusion, cycles, non-UTF-8
     // paths, un-preprocessed files) are returned rather than written to
     // stderr by the library, so the CLI surfaces them here.

big-code-analysis-cli/src/commands_tests.rs

@@ -1045,3 +1045,69 @@ fn resolve_provenance_maps_each_tier_branch() {
         baseline::Provenance::soft_headroom(DEFAULT_SOFT_HEADROOM)
     );
 }
+
+// Regression test for issue #740: `bca preproc` finalization must not
+// panic when a worker poisoned the shared accumulator's mutex. The
+// recovered guard still holds every joined worker's file entry, so
+// `into_preproc_data` degrades to the accumulated data rather than
+// re-panicking on `.expect("mutex not poisoned")`. Verified by revert
+// per `.claude/rules/testing.md`: restoring the `.expect()` chain makes
+// this test panic instead of recovering.
+#[test]
+fn into_preproc_data_degrades_on_poisoned_mutex() {
+    use big_code_analysis::PreprocFile;
+    use std::thread;
+
+    let mut results = PreprocResults::default();
+    results
+        .files
+        .insert(PathBuf::from("a.c"), PreprocFile::default());
+    let preproc_lock = Arc::new(Mutex::new(results));
+
+    // Poison the mutex: panic while holding the guard on a helper thread.
+    let poisoner = preproc_lock.clone();
+    let handle = thread::spawn(move || {
+        let _guard = poisoner.lock().expect("fresh mutex is unpoisoned");
+        panic!("intentional panic to poison the preproc mutex");
+    });
+    assert!(
+        handle.join().is_err(),
+        "poisoner thread should have panicked"
+    );
+    assert!(
+        preproc_lock.is_poisoned(),
+        "test setup failed to poison the mutex"
+    );
+
+    // Finalization must recover the accumulated data, not panic. The
+    // `Arc` here is sole-owned (the poisoner dropped its clone on
+    // unwind), so this exercises the `Ok` + poisoned-`into_inner` arm.
+    let data = into_preproc_data(preproc_lock);
+    assert!(
+        data.files.contains_key(&PathBuf::from("a.c")),
+        "poison recovery must preserve the accumulated file entries"
+    );
+}
+
+// `into_preproc_data` must also degrade when the `Arc` is still shared
+// (a worker failed to join), taking the data from behind the guard
+// rather than panicking on the un-joined-`Arc` `.expect()` (#740).
+#[test]
+fn into_preproc_data_degrades_on_shared_arc() {
+    use big_code_analysis::PreprocFile;
+
+    let mut results = PreprocResults::default();
+    results
+        .files
+        .insert(PathBuf::from("b.c"), PreprocFile::default());
+    let preproc_lock = Arc::new(Mutex::new(results));
+
+    // Hold a second clone so `Arc::try_unwrap` returns `Err(shared)`.
+    let _still_shared = preproc_lock.clone();
+
+    let data = into_preproc_data(preproc_lock);
+    assert!(
+        data.files.contains_key(&PathBuf::from("b.c")),
+        "shared-Arc recovery must surface the accumulated file entries"
+    );
+}