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
2 changes: 1 addition & 1 deletion .bca-baseline.toml
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ value = 854.0
path = "big-code-analysis-ast/src/roles/mod.rs"
qualified = "<file>"
metric = "loc.sloc"
value = 1789.0
value = 2014.0

[[entry]]
path = "big-code-analysis-ast/src/tools.rs"
Expand Down
109 changes: 91 additions & 18 deletions .claude/rules/grammar-dispatch.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,33 @@
# Grammar Dispatch Rule

Every per-language metric impl, `Checker` predicate, and `Getter`
classifier is a `match` over `node.kind_id()`. **The arm list is a
coverage claim, not a specification** — the compiler cannot tell you it
is incomplete, and a metric that scores a valid construct as zero looks
exactly like a construct that legitimately scores zero.

Nothing in the standard gates catches any failure below. There is no
compile error, no clippy warning, and no test failure unless a fixture
Every per-language `Checker` predicate, `Getter` classifier and metric
arm asks a question of a node's kind. Since the role-table series
(#1690, R01–R10, closed by #1701) the *membership* half of that
question — which kinds own a construct, which tokens pay for it — is
declared once per
language in `big-code-analysis-ast/src/roles/<lang>.rs` and read
through the predicates `role_table!` emits; the module doc of
`roles/mod.rs` is the contributor reference for the row grammar, what
a table emits, how a consumer reads a row, and **what the mechanism
cannot see**. Read that last section before this file. Where a gate
now catches part of an item, the item says which (§1, §5, §7); the
rest is manual and stays for that reason.

**A kind list is a coverage claim, not a specification** — whether it
sits on a row or in a surviving `match`. The compiler cannot tell you
it is incomplete, and a metric that scores a valid construct as zero
looks exactly like a construct that legitimately scores zero. Three
gates (`make role-census`, `role-coverage`, `role-bypass`) and the
derived parity suite see a *new, unaccounted-for* kind, a restated
list inside the consumer boundary, and two consumers of one row
disagreeing on a corpus node. None of them judges whether a row's
membership is right, which role should claim a kind, or whether a
consumer reads the right projection. There is no compile error, no
clippy warning, and no test failure for those unless a fixture
exercises the exact shape *and* asserts a value above the base.

The checklist applies whenever you add a language, bump a grammar pin,
add an arm to a dispatch table, or copy an arm from a sibling module.
add a kind to a row, or copy a row or a gate from a sibling module.

## 1. Match every aliased variant

Expand All @@ -34,13 +50,28 @@ The bug class reaches every match on a grammar rule — `alterator.rs`,
`checker.rs`. A kind list a role owns belongs on its row in
`big-code-analysis-ast/src/roles/<lang>.rs`, and a list several
languages share is a `..name!()` fragment there (`roles/c_family.rs`,
`roles/js_family.rs`); the alias unions no row owns yet live in
`big-code-analysis-ast/src/macros/kind_sets.rs`. Prefer extending either
over open-coding a list. When a rule has many
aliases, prefer one `node.kind()` string comparison over enumerating
`roles/js_family.rs`, `roles/tcl_family.rs`); what is left in
`big-code-analysis-ast/src/macros/kind_sets.rs` is five alias unions
no row owns — C#'s parenthesised, prefix-unary and variable-declaration
shapes and Go's boolean terminals, read by the ABC and NPA walkers
only. Prefer extending either over open-coding a list. When a rule has
many aliases, prefer one `node.kind()` string comparison over enumerating
seventeen variants — pay the small runtime cost for forward
compatibility.

**What `make role-coverage` buys here, precisely.** It fails on a kind
id that is claimed by *no* row, excluded by none, and not in
`.role-coverage-baseline.txt` — so a grammar bump that mints a
`PrimitiveType4` is caught the moment the enum is regenerated. It does
**not** catch an alias omitted from the *intended* row: that id is
either still in the baseline as debt, or claimed by some other role,
and the gate passes either way (`utils/check-role-coverage.py`, "What
the ratchet cannot see: … an incomplete row"). So the inventory above
— every numeric-suffix variant listed on the row or excluded with a
reason — stays a manual check, for row declarations exactly as for
the consumers outside the boundary. Do not read "the gate is green" as
"every alias is on its row".

**After bumping a grammar pin, regenerate the enum first.** Bump both
manifests in lockstep (root `Cargo.toml` and `enums/Cargo.toml` — the
excluded crate cannot inherit the workspace pin), then:
Expand Down Expand Up @@ -123,6 +154,12 @@ as an owner that pays for itself. And Elixir's `absent` is a declared
answer with its reason on the row, not a stub: the language has no
`else if` chain to recognise.

**Not every structural fact is a row.** A consumer's projection of an
owner's *child* (the `switch_block` a `Switch` owner nests its arms in)
lives as a named `fn` beside the table with a row test pinning the
relation; the module doc's "Reading a row from a consumer" states the
convention and its two kind-accounting forms (#1701).

Dump the AST for the construct in the *target* grammar before reusing a
sibling's predicate, and add a fixture that exercises the gated branch —
a suite of only unlabelled inputs proves nothing about a label gate.
Expand All @@ -146,6 +183,17 @@ container and a kind it can contain double-counts. Ruby's
`is_closure` matching `Lambda | Block | DoBlock` scored one stabby
lambda as two closures for the same reason.

On a row the two halves are declared apart and the macro keeps them
apart: a kind in both `owns` and `pays` of one row fails a `const`
assertion at expansion time, so declared owners and declared payers
are disjoint by construction. A consumer that wants the wrapper *and*
the leaf reads two projections — `pays_at_wrapper` and `pays_at_leaf` — so a
deliberate double count is a visible choice at the consumer, never an
accident on the row. The compound-operator case above is still the
consumer's: the row owns `not in` and pays its token, and returning
`Unknown` for the inner `not` *under* the compound is a parent guard
in the classifier, which a row cannot spell.

## 6. Narrow a dispatch set with a gate, never by deletion

When a construct over-counts because its node sits in `is_func` /
Expand Down Expand Up @@ -256,14 +304,26 @@ complete as the fixtures it was read from.

## 7. Walk the sibling predicates for parity

This is the manual check for a consumer reading the wrong projection,
and for an incomplete row seen from the consumer's side; a wrong-role
claim is caught by the dump-and-spelling-test check in the module doc's
"What the mechanism cannot see", and this cross-walk stays after the
series for the other two. The derived parity suite
(`roles/derived_parity_tests.rs`, `src/metrics/derived_parity_tests.rs`,
`make role-parity-corpus`) now
asks every consumer of every claimed kind on a corpus node, which
catches two consumers *disagreeing*; it cannot catch two consumers
agreeing on a wrong row, because its expected verdict comes from the
same declaration.

`Checker::is_string`, `Getter::get_op_type`, `Checker::is_call`,
`Checker::is_func_space`, and the per-metric body walkers all classify
the same nodes through parallel `matches!()`. Two that disagree drift
silently: `find string` counts a node that Halstead calls `Unknown`.
Each predicate reads as internally consistent, so the disagreement is
invisible from either one alone.
the same nodes, most through one row now and some through a surviving
`matches!()`. Two that disagree drift silently: `find string` counts a
node that Halstead calls `Unknown`. Each predicate reads as internally
consistent, so the disagreement is invisible from either one alone.

Minimum cross-walk when you edit one:
Minimum cross-walk when you edit one — or edit the row they read:

- `is_string` ↔ `get_op_type` operand classification of string kinds
- `is_call` ↔ `get_op_type` operator classification of call kinds
Expand Down Expand Up @@ -318,6 +378,19 @@ spelling over-approximates there exactly as the pre-row kind list did.
A byte-less call site is therefore wrong on Elixir *and* Ruby today, and
on whichever language next gates an opener.

Elixir's byte-less pair is also the worked example of a kind list the
bypass gate *exempts* on purpose: `is_func_space` and `get_space_kind`
spell `Source | AnonymousFunction` under `// role-bypass: allow(…)` —
the ungated subset of the `FuncOpener` / `Container` owners, which is
all a spelling with no bytes can answer, and not a forward to
`opens_space`, because the walk's promotion calls the byte-less checker
spelling per node *before* its single keyword read and the forward
would scan each `call`'s target twice more for a known `None`. The
exemption hides a kind later added inside that `matches!`; the derived
suite's `KNOWN_DISAGREEMENTS` names exactly `call` for both, so any
other kind added there fails as an unnamed disagreement, and `call`
itself fails `promotion_reads_each_calls_keyword_once` (#1701).

This has already landed once. `ops_inner` opened spaces on the byte-less
`is_func || is_func_space`, so `bca ops` returned a bare file-level space
for every Elixir input while `bca metrics` returned a full
Expand Down
26 changes: 26 additions & 0 deletions .claude/rules/macro-comments.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,29 @@ Lesson #34 documents the related case of hidden-rule variants;
this rule is the call-site discipline that makes those defensive
arms self-explanatory at the call site even after another round
of macro consolidation.

## Row comments in a `role_table!`

A `role_table!` invocation (`big-code-analysis-ast/src/roles/<lang>.rs`)
is the largest macro call site in the tree, and the same rule applies
one row at a time. The module doc of `roles/mod.rs` ("Reading a row
from a consumer") states what a row's comment may and may not say —
the grammar, not the consumer's policy; cite it rather than restating
it. Where each kind of rationale goes:

| what it explains | where it lives |
| --- | --- |
| the grammar fact a row encodes — what the construct's nodes are, how they nest, why an alias is listed or left off | a `//` comment on the line above the row, inside the invocation |
| why a gate exists and what it reads (`gated_by f`, `text_gated_by f`) | the `///` doc on the gate `fn` beside the table |
| why a kind the author considered is not on the row | the `excludes [K "why"]` string itself — it is read by the coverage gate and printed with the residue |
| which consumer reads which projection, and why it counts there | at the consumer, in `big-code-analysis` |
| the shape of the macro — what `owns`, `pays`, `opens` mean | `roles/mod.rs` only, never a row comment |

Groovy's `Switch` row after #1701 is the model: the `//` comment above
it states the grammar fact, the `excludes` string the rejection, and
the consumer's reading sits in `src/metrics/cognitive/groovy.rs`.

The failure mode is the same as above: a row comment that explains a
consumer's reading is wrong the day that consumer changes, and the
reader deciding whether the row is right has no way to tell the stale
policy sentence from the grammar fact beside it.
4 changes: 0 additions & 4 deletions .role-bypass-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,3 @@
# One entry per line, tab-separated and sorted:
# <file> <item> <language> <kind> <shape>#<n>
# where <n> numbers the sites of one kind and shape within the item.
big-code-analysis-ast/src/checker/elixir.rs fn is_func_space Elixir AnonymousFunction matches#1
big-code-analysis-ast/src/checker/elixir.rs fn is_func_space Elixir Source matches#1
big-code-analysis-ast/src/getter/elixir.rs fn get_space_kind Elixir AnonymousFunction arm#1
big-code-analysis-ast/src/getter/elixir.rs fn get_space_kind Elixir Source arm#1
1 change: 0 additions & 1 deletion .role-coverage-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,6 @@
210 named visible 'annotation'
212 named visible 'expression_statement'
213 anon hidden '_expression'
215 named visible 'switch_block'
217 named visible 'switch_arrow_case'
219 named visible 'cast_expression'
220 anon hidden '_cast_type'
Expand Down
27 changes: 23 additions & 4 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ and `cargo run -p big-code-analysis-web --`.
mirror each other, as do the per-language `checker/`, `getter/` and
`src/metrics/<metric>/` siblings. A bug in one language module
typically exists in several — fix all affected siblings together.
- `big-code-analysis-ast/src/roles/` — one `role_table!` per language
(`<lang>.rs`), declaring for every role which kinds own a construct
and which tokens pay for it, read by the `Checker`, the `Getter` and
the root crate's metric arms instead of restated kind lists (#1690).
`mod.rs` holds the macro and the **module doc that is the
contributor reference**: the role set, the row grammar, what a table
emits, how a consumer reads a row, the gates, and the one list of
what the mechanism cannot see — link to it, do not copy it.
`c_family.rs`, `js_family.rs` and `tcl_family.rs` are the shared
`..fragment!()` kind lists; `control_flow.rs` the shared `ElseIf`
gates; `*_tests.rs`, `doc_tests/` and `derived_parity_tests.rs` the
per-package, documentation and derived parity suites.
- `big-code-analysis-ast/src/` — `langs.rs` (`LANG`, `AnyParser`),
`node.rs`, `parser.rs`, `traits.rs` (`ParserTrait`), `checker.rs`,
`getter.rs`, `alterator.rs`, `lang_helpers/`, `preproc.rs`,
Expand Down Expand Up @@ -799,16 +811,23 @@ over the three repository corpora, and `make role-corpus-refresh` runs
it with the observation — the recipe for a moved corpus (#1700). `make role-bypass`: inside the consumer boundary
(the per-language dispatch files, their dispatcher modules and
macros), a kind a row claims must not be restated as classification;
`.role-bypass-baseline.txt` holds the pre-migration sites, and a site
that is not membership — naming, traversal — takes a `// role-bypass:
`.role-bypass-baseline.txt` is empty since #1701, so a new finding
fails outright, and a site that is not membership — naming, traversal,
a consumer-chosen subset of a row's owners — takes a `// role-bypass:
allow(<Role>) — <reason>` exemption on the line above, which must name
the role that claims the kind. All three baselines are exact record
a role that claims the kind and covers the whole statement or arm
after it (so a kind of the *same* role later added inside that span
is the derived suite's to catch, not the gate's; a kind only another
role claims is still reported). All three baselines are exact record
sets (the bypass one numbers repeated sites within an item):
`--update` only removes, `--bootstrap` refuses a file that exists or is
tracked at `HEAD`, so a swap cannot hide behind an unchanged count. Each
gate prints what it cannot see with every run: none judges whether a
row's membership is right, which role should claim a kind, or whether
a consumer reads the right projection.
a consumer reads the right projection. The full list, with the manual
check that covers each item, is the "What the mechanism cannot see"
section of `big-code-analysis-ast/src/roles/mod.rs`'s module doc —
the one copy; the rules under `.claude/rules/` link to it.

**`cargo fmt --check` is not the whole formatting gate.** A comment
inside a match *pattern* makes rustfmt emit the enclosing match
Expand Down
41 changes: 41 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,47 @@ for historical reference.

### Changed

- **The role-bypass baseline is empty** (#1701, part of #1690). The
four pre-migration sites it still held were Elixir's byte-less
`is_func_space` and `get_space_kind`, which list `source` and
`anonymous_function` — the ungated subset of the `FuncOpener` and
`Container` rows' owners, the one a spelling with no bytes can answer,
since the rows' third owner is the text-gated `call`. They are now
`// role-bypass: allow` exemptions under the lint's documented
subset clause rather than forwards to `opens_space`: the walk's
promotion calls the byte-less checker spelling before its single
keyword read, and the forward would scan each `call`'s target twice
more for a known `None`. The derived forward-parity test pins both
lists against the rows on every corpus node. Groovy's `switch_block`
is recorded under `excludes` on its `Switch` row instead of as
coverage debt — the owner's child, where cognitive opens its
nesting level — and the Java row comment no longer says no row
claims its twin, which `HoldsDocInBody` has since #1699. The
convention both name: a consumer's structural projection of an
owner's child lives as a named `fn` beside the table, with a row
test pinning the parent/child relation. Zero drift: no emitted
predicate changed.
- **The role-table series is closed** (#1701, closing #1690). The
module doc of `big-code-analysis-ast/src/roles/mod.rs` is rewritten
as the contributor reference — the role set, the row grammar with
its three `pays` modes, what a table emits, how a consumer reads a
row, the three gates and the derived suite, and the one list of
what the mechanism cannot see, which the rules under
`.claude/rules/` and the book now link to rather than restate.
`grammar-dispatch.md` says which of its items the gates catch and
which stay manual (an alias omitted from its intended row passes
`role-coverage`; the §7 cross-walk is the check for a consumer
reading the wrong projection); `macro-comments.md` gains the
row-comment placement table.
Lesson 115 records what the series found: sharing a rule left a
kind list at every caller, and sharing the list leaves the
judgement — wrong-role claims, incomplete rows, text-keyed policy
and dead walker paths — to review. Seven DSL follow-ups are filed
outside the series: #1792 (per-role emission profile), #1793
(`child_gated_by` / `CHAIN_GATED`), #1794 (owner `under` /
`not_under`), #1795 (`preproc_conditional` by id), #1796 (a
`Parameter` row or the kept bypass), #1797 (`HoldsDocInBody`'s
name), #1798 (role test-file house style).
- **The history walk no longer rebuilds gix's blob-diff platform for
every commit it visits** (#1761). `for_each_to_obtain_tree` builds a
platform per call for its rename tracker, whether or not rename
Expand Down
8 changes: 8 additions & 0 deletions big-code-analysis-ast/src/checker/elixir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ impl Checker for ElixirCode {
// `promotes_to_func_space_with_code` below, which reads the one
// keyword lookup both rows share rather than each row in turn.
fn is_func_space(node: &Node) -> bool {
// role-bypass: allow(FuncOpener, Container) — the ungated subset of the
// two rows' owners, which is all a byte-less spelling can answer: the
// rows' third owner is the text-gated `Call`. Not a forward to
// `opens_space` with no bytes: `promotes_to_func_space_with_code`
// calls this per node before its one keyword read, and the forward
// would scan the target twice more per `call` for a known `None`
// (`promotion_reads_each_calls_keyword_once`). The derived forward
// parity test pins this list against the rows on every corpus node.
matches!(
node.kind_id().into(),
Elixir::Source | Elixir::AnonymousFunction
Expand Down
7 changes: 6 additions & 1 deletion big-code-analysis-ast/src/getter/elixir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,17 @@ impl Getter for ElixirCode {
// function and the file root. `defmodule` / `def` / `defp` /
// `defmacro` / `defmacrop` are `call`s told apart by their target
// text, so only the `_with_code` spelling — the `opens` annotations
// of `roles/elixir.rs` (#275, #310, #1696) — labels those.
// of `roles/elixir.rs` (#275, #310, #1696) — labels those. Each arm
// is the ungated subset of its row's owners, the one a byte-less
// spelling can answer; the derived forward parity test pins both
// against the rows on every corpus node.
fn get_space_kind(node: &Node) -> SpaceKind {
use Elixir as E;

match node.kind_id().into() {
// role-bypass: allow(FuncOpener, Closure) — the `FuncOpener` owner with no gate
E::AnonymousFunction => SpaceKind::Function,
// role-bypass: allow(Container) — the `Container` owner with no gate
E::Source => SpaceKind::Unit,
_ => SpaceKind::Unknown,
}
Expand Down
Loading
Loading