Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .bca-baseline.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ headroom = 0.95
path = "big-code-analysis-ast/src/alterator.rs"
qualified = "<file>"
metric = "loc.ploc"
value = 561.0
value = 550.0

[[entry]]
path = "big-code-analysis-ast/src/alterator.rs"
Expand Down Expand Up @@ -267,7 +267,7 @@ value = 120805.48245794396
path = "big-code-analysis-ast/src/node.rs"
qualified = "Node<'a>"
metric = "nom"
value = 35.0
value = 36.0

[[entry]]
path = "big-code-analysis-ast/src/parser.rs"
Expand Down Expand Up @@ -561,7 +561,7 @@ value = 59351.805921378844
path = "big-code-analysis-cli/src/markdown_report.rs"
qualified = "<file>"
metric = "loc.ploc"
value = 588.0
value = 575.0

[[entry]]
path = "big-code-analysis-cli/src/markdown_report.rs"
Expand Down Expand Up @@ -591,7 +591,7 @@ value = 6.0
path = "big-code-analysis-cli/src/markdown_report/hotspot.rs"
qualified = "<file>"
metric = "loc.ploc"
value = 696.0
value = 694.0

[[entry]]
path = "big-code-analysis-cli/src/markdown_report/hotspot.rs"
Expand Down Expand Up @@ -627,7 +627,7 @@ value = 66737.04848699087
path = "big-code-analysis-cli/src/thresholds.rs"
qualified = "<file>"
metric = "loc.ploc"
value = 562.0
value = 549.0

[[entry]]
path = "big-code-analysis-cli/src/thresholds.rs"
Expand All @@ -639,7 +639,7 @@ value = 5.0
path = "big-code-analysis-cli/src/vcs_command.rs"
qualified = "<file>"
metric = "loc.ploc"
value = 532.0
value = 531.0

[[entry]]
path = "big-code-analysis-cli/src/vcs_command.rs"
Expand Down Expand Up @@ -819,7 +819,7 @@ value = 6.0
path = "big-code-analysis-py/src/types_codegen.rs"
qualified = "<file>"
metric = "loc.ploc"
value = 748.0
value = 747.0

[[entry]]
path = "big-code-analysis-py/src/vcs.rs"
Expand Down
39 changes: 39 additions & 0 deletions .claude/rules/grammar-dispatch.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,45 @@ says which; billing the leaf files all four under `1` (#1359). Bash's
`string` child is the node present in argument position, where the
grammar emits no wrapper (#1358).

**`node-types.json` cannot answer "every spelling" — it describes the
well-formed grammar only.** It says nothing about error recovery, where
a child routinely appears with its documented parent absent, so a keeper
argued from it is argued from half the evidence. #1412 routed Bash's
`heredoc_redirect` and **deleted** the two `heredoc_body` arms on the
grounds that `node-types.json` lists `heredoc_body` as a child of
`heredoc_redirect` and of nothing else. A single-line compound carrying
a heredoc — `f() { cat <<EOT; }`, and the `if` / `for` spellings, all
valid executable Bash — parses to an `{ERROR}` root whose *direct* child
is the body, no wrapper anywhere; the leaf-gated catch-all then credited
only its start row and the terminator fell to `blank`, reintroducing the
defect the fix had just repaired. Dump at least one malformed or
recovery input before deleting the other arm, and prefer keeping both:
where the wrapper's span contains the leaf's, listing both is a no-op,
because `LineSet::insert` / `insert_range` and
`check_comment_ends_on_code_line` are all idempotent.

**Decide a grammar-reachable but language-invalid shape on what the
change buys for *valid* input.** Grammars are routinely more permissive
than their languages, so "the parser can produce this" does not settle
whether to handle it. Fix it when the current behaviour is wrong in
*meaning* and valid input is unaffected: Bash's `heredoc_redirect`
credited its whole interior, so "the literal's rows" included
command-prefix rows. A *blank or comment-only* prefix row is a bash
syntax error, and the valid multi-row prefixes carry leaves the
catch-all credits, so the range now means what it says at no cost to
runnable input (#1443). Scope that claim to what you measured: the first
reading of #1443 said every multi-row prefix was invalid, which is false
— a `\`-continued one is valid — and the over-broad version hid a valid
shape the fix does move (#1445). Document the gap instead when
only invalid input can tell the two behaviours apart: C# has no
`PreprocArg` arm, and since the C# specification ends a `pp-directive`
at the new-line, a valid `preproc_arg` is always single-row and the arm
would no-op on it (#1430). In the second case add **no test** — pinning
the numbers for an invalid fixture makes the grammar's present
over-permissiveness the contract and inverts into a bug-lock when it
tightens. Check the language specification, and `bash -n` or the
compiler, rather than inferring reachability from the grammar.

## 7. Walk the sibling predicates for parity

`Checker::is_string`, `Getter::get_op_type`, `Checker::is_call`,
Expand Down
33 changes: 33 additions & 0 deletions .claude/rules/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ longer the file being edited. Before believing any perturbation result,
confirm the subject still contains the change: `rg -c <new symbol>` on
the file, or a `git diff --stat` that shows what you expect.

**For a compiled subject the rebuild must be inside the measurement
step.** `rg` on the source proves the *source* changed; a sweep that
shells out to `target/release/bca` is measuring the last binary someone
built. During #1412 a corpus sweep reported "0 of 438 files changed" —
a perfectly plausible result — because the restore ran without a
rebuild, so the before and after runs used the same stale binary. Put
`cargo build` in the same step as the run, and assert a known fixture
whose answer differs between the two states immediately after each
build; that guard is what caught it on the rewrite.

**The result *parser* is the other half of the subject.** During #1238 a
sweep drove three perturbations of one match arm and reported zero Rust
failures for all three, while the Python leg of the same sweep reported
Expand Down Expand Up @@ -212,6 +222,29 @@ Where the construct contributes to no axis once excluded — a Lua
anchor on, and the revert test is the only coverage available. Say so in
a comment, so the missing anchor is not read as an oversight.

### Never let the measured value *be* the defect's output

A fixture whose discriminating quantity is *produced by the bug* stops
measuring anything the moment the bug is fixed. It is the inverse of the
decay above, it bites benchmark probes hardest, and it fails as "the
workload scored zero on its own shape" — which reads like a broken
fixture rather than like the fix working.

`loc/wide-cfg-test-mod` (`big-code-analysis-bench/src/shapes.rs`) read
`sloc` under `exclude_tests` on a file of nothing but `#[cfg(test)]
mod m {}` repeated. Its only non-zero row was the phantom attribute row
#1431 then removed, so the probe scored zero and tripped
`probe_workload_is_exercised`. Left unnoticed it would have timed the
walk's fixed overhead and reported an excellent exponent forever. The
repair was to render a retained `fn p() {}` per item, so the reading
survives the fix.

When a change alters what a shape measures, re-check every probe or
fixture reading *that* metric on *that* shape before comparing a
before/after result. Build the measured quantity out of something the
fix does not touch — for a metric fix, usually retained content
alongside the construct under test.

## Coverage measures execution, not discrimination

A coverage report answers "did any test run this line?" It never answers
Expand Down
107 changes: 107 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,113 @@ for historical reference.

### Fixed

- **A Bash heredoc no longer bills its command-line prefix as part of
the literal** (#1443). `heredoc_redirect` spans the prefix as well as
the heredoc, and the grammar lets that prefix cross rows — a
`pipeline` is one of its children — so crediting the wrapper's whole
interior read a blank prefix row as code, and reclassified a
comment-only one as code-and-comment. The credited range now starts at
the first body row, derived from the last row any non-body child
occupies. A *blank or comment-only* prefix row — the shape this fixes
— is unreachable in runnable Bash, since bash begins the body on the
line after the `<<` and a `\` continuation splices the rows rather
than leaving one empty; `bash -n` rejects all four spellings. A prefix
that crosses rows with content is valid and unaffected, its
continuation rows carrying leaves the catch-all credits. Fixed rather
than left as a `tree-sitter-bash` divergence because `bca` is still
asked to measure malformed trees, the argument #1398 rests on. One
valid shape does move: a continuation row holding only `\` has no leaf
to credit it and now reads blank, a general Bash gap this arm's old
blanket range happened to mask in one position (#1445).
- **`--exclude-tests` now prunes the `#[cfg(test)]` / `#[test]`
attribute along with the item it marks** (#1431). An outer attribute
is an `AttributeItem` *sibling* of its item, not a child, so pruning
the item never reached it and Rust's `Loc` catch-all credited its
start row to PLOC. A file that was nothing but test code reported
`sloc 1, ploc 1` — one row of code in a file with no production code
— and every `#[cfg(test)] mod tests` at the foot of a production file
inflated that file by one row, a stacked attribute run by one row per
attribute. `Checker::should_skip_subtree` now also answers for an
attribute, looking ahead over its run to the item it decorates and
pruning only when that item is one the prune removes anyway; the
attribute on a `use`, a `struct`, or any other kind `--exclude-tests`
keeps is untouched. **Metric drift, and it is not only LOC:** the
prune arm `continue`s before the per-node computes, so the attribute
subtree leaves *every* selected metric, not just the row counts.
`loc.ploc` and `loc.sloc` fall by one per attribute row not shared
with retained code; `tokens` and the whole `halstead` block fall too,
because Rust attribute tokens are Halstead operators and operands; and
`mi` moves through both of its inputs, `ln(sloc)` and Halstead volume,
not through SLOC alone. Measured over 463 Rust files, `tokens` and
`halstead.length` change in 206 of them under `--exclude-tests`.
`--exclude-tests` off — the default — is byte-identical. A wholly
test-only file now measures `sloc 0`, which scores MI `0.0` on all
three formulas through the existing empty-input guard.
The lookahead answers **once per attribute run**, not once per row
(#1446). Every `#[…]` in a run decorates the same item and so gets the
same verdict, and re-deriving it per row read the run `run` times: a
Rust file `D` levels deep carrying one run of `3 * D` attributes — the
width `forward_attribute_scan_budget` permits at that depth, so the
shape is guaranteed onto the forward `O(children)` reading — took
5.2 s to analyse at `D = 2000` against 0.05 s with `--exclude-tests`
off. `Checker::should_skip_subtree` now reports how far its verdict
reaches (`SubtreeSkip`), the walk reuses it across the run's members,
and the same file takes 0.02 s. Metric values are unchanged; the
`nom/deep-attribute-run` scaling probe holds the class, reading 2.01
against the per-row lookahead and 1.06 against this one.
- **`--exclude-tests` no longer drops rows a pruned item shares with
retained code**, which made `sloc` fall below `ploc` (#1417).
`Sloc` accumulated each pruned subtree's whole row span as a
*count*, on the assumption — written into its `exclude_span` doc
comment — that rustfmt gives every Rust item its own rows. Anything
hand-written, generated, minified, or concatenated breaks it:
`fn a() {} #[cfg(test)] mod t { … }` reported `sloc 0, ploc 1`, one
row of code in a zero-row file, and MI's `ln(sloc)` term saw the
same zero. The count is now a row *set*, from which the space's
retained code and comment rows are subtracted at finalization, so
only rows the prune genuinely removed are subtracted; two pruned
siblings on one row also now cost that row once instead of twice.
`sloc` rises for affected spaces and `blank` rises with it; ordinary
rustfmt-shaped input is byte-identical, and the shipped
`--exclude-tests`-off default is untouched. `Loc`'s per-space
invariant `ploc <= sloc` (and `cloc <= sloc`) is now asserted in
debug builds on every space of every walk. Two observable
side effects on the `Sloc` sub-struct: its `Debug` rendering prints
the excluded rows (`excluded_rows: {6, 7, 8}`) where it printed a
count, and its `PartialEq` strengthens — two `Sloc`s that exclude
the same *number* of different rows no longer compare equal.
- **A C-family macro body ending on a dangling backslash no longer
credits the blank row below it as code** (#1423). The
`PreprocArg` arm in the C, C++, `mozcpp` and Objective-C `Loc` impls
bounded its row range with tree-sitter's raw end row, which over-reads
by one whenever a node ends at column 0 — the shape a trailing `\`
with nothing after it produces. All four arms now go through
`add_string_interior_ploc`, the helper every other multi-row PLOC path
already used, which derives the last row from `Node::end_line`.
`ploc` falls by one and `blank` rises by one for each such macro;
DeepSpeech's `left_test.cc` is one real instance. Python's `String`
arm, the last remaining open-coded copy of the same pattern, is folded
onto `add_multiline_string_ploc` — a no-op there, since a Python
`string` node always closes on a quote and so never ends at column 0.
- **A Bash heredoc whose body is empty at the top, or empty
throughout, no longer reports those rows as blank** (#1412).
`heredoc_body`'s span begins at the first body row that *has* text
and collapses to zero width when there is none, so a leading empty
row sat inside no node at all and `blank = sloc - ploc - cloc`
claimed it: `cat <<EOT\n\nEOT` reported `blank 1`. The `Loc` arm now
also routes the `heredoc_redirect` **wrapper**, the node present
whenever the heredoc parses cleanly, as #1396 did for PHP. The
`heredoc_body` symbols stay listed alongside it rather than being
replaced by it: under error recovery — a single-line compound such as
`f() { cat <<EOT; }` — tree-sitter-bash emits a body with no wrapper
at all, and an arm keyed on the wrapper alone reintroduces the very
defect on that shape. `ploc` rises and `blank` falls for Bash files
whose heredoc body opens with, or consists of, empty rows. The two
cross-language sweeps that are the home for this property now use an
**empty** interior row — with `line1\nline2\nline3` they passed
whether or not a textless interior row was credited, which is why
they saw neither #1396 nor #1412 — and gained the PHP rows they had
never had, quoted and heredoc alike.
- **Twenty-five cross-language test sweeps no longer fail spuriously
under a reduced feature set** (#1286, #1411). Each carried a loud
non-vacuity guard (`checked > 0`, `assert_fixtures_present`) without
Expand Down
Loading