Skip to content

refactor(getter): declare Modifier, DeclKeyword and Primitive through role tables #1693

Closed

Description

@dekobon

Objective

Pilot the mechanism on the Halstead keyword roles — Modifier, DeclKeyword, Primitive — through the C-family template first, then the JS-family macro, then every remaining getter. Zero drift. Address #1598 and #1484 and record the policy decision for #1600.

Umbrella: R00: #1690 · Planning ID: R03: #1693
Completion prerequisites: R02: #1692

Status: implemented on branch refactor/1691-role-table-proof at bef8600d (fourteen commits on R02's 92d7ff74; not pushed). Zero drift measured; see the completion comment for the evidence table, the keyword-residue disposition, and the exemption list.

Adjustments to this body, recorded before implementation (from a plan review by astra-review; each is a scope or interface fact, not a behaviour change):

  • Interface. R01's DSL is Role { owns [Kind, Kind(gated_by f)] excludes [K "why"] }, not Excluded(role, reason); a row answers whether a node is a member, and the getter beside the call decides the TokenRole. That matters for Primitive: C-family, Java and TypeScript bill primitives as operators, PHP bills them as operands (getter/php.rs), so the consumer expression is per language, never a universal pays → Operator.
  • Where the role call lives. get_op_type is the base classifier; the default get_op_type_with_code and five language overrides (Java var, Groovy text primitives, C# aliased modifiers, Kotlin strings, JS JSX) fall back to it, and Tcl/iRules' braced_word_op_type builds on it. Reversing that direction would recurse. The three role calls therefore go into a new internal keyword classifier called from get_op_type_with_code (the default forwarder, each override, and braced_word_op_type, which gains code) ahead of the fallback; get_op_type stops knowing keywords. A byte-less compatibility test and Tcl/iRules braced-word tests pin it. This is the first step toward retiring the byte-less spelling, which R06 owns.
  • Family templates and the gates. utils/role_tables.py reads a literal role_table!; the C-family and JS-family templates are one bounded shape it learns to expand (shared list from the template definition, extras from the invocation), pinned by the existing Rust differential test per dialect. The C-family shared set is the intersection of the four lists — C's Auto is not C++'s — with per-dialect extras; nothing is promoted into a dialect that did not bill it.
  • Acceptance criteria, amended. "No keyword-shaped coverage residue" and "no keyword kind matched outside a role call" are scoped to the three roles this package declares: Modifier, DeclKeyword and Primitive. Every other keyword-shaped id receives a reviewed disposition in the completion comment — owner package per class (if/for/switch/catch/return → R07; &&/?:/case/when → R05; new/call forms → R08) or "Halstead operand, no role" (this, super, true, false, null) — and stays baselined debt for that owner; no excludes is written to empty the baseline. "No new exemptions" means none for membership; a claimed kind's retained operand-classification, key-selection (Checker::is_primitive, which selects the vocabulary key after classification and is not recognition) and naming sites take reasoned // role-bypass: allow(...) exemptions, each listed in the completion comment.
  • design(getter/java): bill directive keywords like C# or keep the import-static guard #1600. The directive guard stays (Java and Groovy bill no directive keyword; C# bills using static in full); changing it moves output and is a separate fix. Both options are measured with bca ops --no-config in the completion comment, the decision sits on the Java and Groovy Modifier rows, and the book's Halstead rules name C# as the outlier, which is what design(getter/java): bill directive keywords like C# or keep the import-static guard #1600 asks of this choice.
  • docs(kind_sets): Float2 is never a (float) cast #1484. The false (float)-cast sentence no longer exists: kind_sets.rs was rewritten in 22a10d7a and the correct claim lives on getter/php.rs's Float2 arm. Closed as superseded with that evidence.
  • refactor(getter/c-family): one kind table for the four copied modifier lists #1598. The shared kind list lives in the C-family template rather than kind_sets.rs (rows are now the home for membership); the two test helpers merge behind the existing Role enum with every fixture row kept verbatim.

Why this exists, and why first

The modifier-keyword class is the cleanest instance of the problem: five issues, five commits, no shared table (#1558 → #1563 → #1565 → #1578 → #1579), then review found Volatile2 missing from all four hand-copied C-family arms (#1598). The declaration-keyword class went the same way after a genuinely measured nine-language sweep (#1552 → #1556 → #1557 → #1564): the sweep's boundary was the issue's own table, so Kotlin was absent and typealias/namespace/typedef arrived as follow-ups within a day.

It goes first because it is the lowest-risk proof of the mechanism: one consumer (Getter::get_op_type), a finite candidate set that can be fully classified (keywords, from grammar.json resolved to ids), and a byte-comparable oracle (bca ops vocabularies). Volatile2 is already in all four getters at HEAD, so there is no behaviour change to disentangle. It is not the proof that matters for the ABC/cyclomatic trails — that is R04: #1694, where the go/no-go sits.

Implementation steps

  • roles/c_family.rs: a c_family_roles! template taking the dialect enum and per-dialect extras; Modifier, DeclKeyword, Primitive rows for C, Cpp, Mozcpp, Objc. The shared set is the current getter/c.rs list; extras are per dialect. Row comments move with their rows; a section comment that sits inside a pattern is hoisted above the row.
  • For every language (not only the four): classify each keyword-shaped candidate as a row, Excluded(role, reason) or operand, reviewed by hand; the ratchet residue for keyword kinds must be empty per language at close.
  • Replace each get_op_type keyword arm with pays_Modifier || pays_DeclKeyword || pays_Primitive; delete the copied lists. kind_sets.rs alias macros fold into the rows that use them; correct the Float2 note (docs(kind_sets): Float2 is never a (float) cast #1484) in passing.
  • Decide design(getter/java): bill directive keywords like C# or keep the import-static guard #1600 (import static / requires static bill nothing in Java vs C#'s using static billed) before writing Java's Modifier row; record the decision on the row and close or re-file design(getter/java): bill directive keywords like C# or keep the import-static guard #1600.
  • Mozcpp has no file extension and no corpus: pin it to Cpp through tests/parity/cpp_mozcpp_parity.rs, extended for keyword vocabularies.
  • Recheck every item of refactor(getter/c-family): one kind table for the four copied modifier lists #1598, including its requested consolidation of two test helpers; implement it or explicitly supersede it with rationale. Preserve independent fixture expectations when sharing test support.

Tests and edge cases

  • Existing getter and Halstead tests unchanged and passing.
  • Per language with corpus files: bca ops on every integration-corpus file before and after, vocabularies diffed byte-for-byte; metric JSON and space trees likewise. Per language without corpus files (Mozcpp, any helper-grammar case): an explicit fixture set diffed the same way, named in the completion comment.
  • INSTA_FORCE_PASS=1 cargo test --workspace --all-features --no-fail-fast; count .snap.new and .pending-snap (both zero); then a plain run.
  • Perturb the C-family template (drop Volatile2); the R02: #1692 coverage gate fails naming it.

Acceptance criteria

Verification and completion evidence

  • Per-language command + output table for the bca ops diff; snapshot counts; BCA_GATE: line; self-scan baseline refreshed if a gated file moved.

Activity

  1. added
    refactorCode maintainability / tech-debt cleanup (no behaviour change)
    on Oct 23, 2026
  2. dekobon commented on Oct 23, 2026

    @dekobon
    OwnerAuthor

    Tracking is ready

    The body now links the master issue and the work packages it references. Native parent and blocked-by relationships match the listed completion prerequisites. Implementation and acceptance checkboxes remain open; prerequisite closure and the R04: #1694 go decision govern execution order.

  3. dekobon commented on Oct 23, 2026

    @dekobon
    OwnerAuthor

    R03 is implemented on refactor/1691-role-table-proof (not pushed), twelve commits on top of R02's 92d7ff74, 7fbef3b6 (fragment splicing) through 18a2a6ae: the C-family migration (db082a94), the JS family (6852e142), every remaining language (b8e2e52c), and the review follow-ups (a6de85bd, 66ac2d6a, ec2e38e5, 18a2a6ae).

    Mechanism. Getter::get_keyword_role is the new classifier a keyword row feeds; keyword_or_op_type::<G> (a free function, so no language can override the fallback) prefers its answer and falls back to the byte-less get_op_type — a synthetic conflicting getter pins that order. Every spelling of get_op_type_with_code ends in it: the default forwarder, the five language overrides (Java, Groovy, C#, Kotlin, JS family) and Tcl/iRules' braced_word_op_type. keyword_roles!(lang) emits the C-family / Java / TypeScript reading (every row keyword is an operator) for 23 languages; keyword_roles!(php, primitive: Operand) is PHP's. The byte-less get_op_type no longer knows a keyword in any language, and every gate those arms carried is a gated_by / text_gated_by function beside its row. Shared lists are ..name!() fragments (roles/c_family.rs, roles/js_family.rs), one arm each, read by both the compiler and utils/role_tables.py.

    Zero drift, measured. /tmp/opencode/1693/oracle.py ran bca ops --no-config --language L -O json and bca metrics likewise on every corpus file plus one keyword fixture per language, before (92d7ff74) and after each of the three migration commits: 2,938 files across 25 languages (Bash 144, Cpp 1049, Mozcpp 1049 over the Cpp corpus, Javascript 388, Rust 173, Python 73, Csharp 22, Java 12, Typescript 8, Groovy 5, one fixture each for C, Ccomment, Elixir, Go, Irules, Kotlin, Lua, Mozjs, Objc, Perl, Php, Preproc, Ruby, Tcl, Tsx). diff -r of the two output trees: 0 differences, each time. The corpus-less languages are covered by the fixtures named above; Mozcpp additionally by tests/parity/cpp_mozcpp_parity.rs::cpp_and_mozcpp_agree_on_keyword_vocabularies, whose expected vocabulary is written out independently (dropping a kind from Mozcpp's row alone, from both rows, or from the shared fragment each fails by name — verified).

    Volatile2 perturbation. Dropping Volatile2 from c_family_modifiers! fails make role-coverage naming kind __volatile__ in C, Cpp, Mozcpp and Objc as claimed by no role, and fails c_modifier_spellings_bill_1578, cpp_modifier_spellings_bill_1578 and objc_modifier_spellings_bill_1579. Nothing masks it.

    Row perturbations. Twenty-two single-edit perturbations of the migrated rows and gates (Java directive and class-literal gates, Groovy contextual / text / import gates, C# alias entry, Kotlin soft-keyword gate and Companion, TS export { type } and void gates, PHP operand reading, and one kind dropped from each DeclKeyword-only language) each fail at least one existing Halstead test, except three: PHP bottom_type and Elixir fn now have a test each; C#'s aliased this is held by call order rather than by the row's word list, and the row's doc says so. The review pass then found eleven row kinds no corpus file or other fixture spells (Bash declare / typeset / readonly, Ruby undef, Rust dyn / async, PHP global / var, Tcl and iRules global, C# unsafe / record); each now has a per-language spelling test verified to fail by name. Rust's sixteen suffixed primitive_type ids are canonicalised by the parser and unreachable from written source (rust_aliased_primitive_type_classification records this).

    Gates. Census baseline −50 entries, coverage baseline −489, bypass baseline unchanged (125), every update remove-only. 29 new role-bypass: allow exemptions, none for membership: Checker::is_primitive ×8 (key selection after classification), access-label / which-modifier reads in npm/npa/abc ×9, naming (get_operator_id_as_str, ObjC declarator) ×3, operand classification of a gated-out spelling (Java class-literal class, Groovy names, ObjC identifiers, PHP leaves under primitive_type) ×5, the C# alias shape test ×1, Kotlin property-parameter test ×1, and the ABC const/final/val binding walks ×3. Rust's is_primitive is the one that was membership and now reads roles::rust::primitive::OWNERS.

    Keyword-shaped coverage residue, by owner, for the roles this package does not declare (1,095 anon visible entries remain across the 25 languages; none is a Modifier, DeclKeyword or Primitive):

    • R07 (Branch / Switch / Loop / Catch / ElseIf / Exit): if else elif elsif elseif then fi switch case default when match for foreach while until do done loop repeat select range try catch except finally rescue ensure after return break continue next last redo retry goto fallthrough throw raise yield co_return co_yield defer go and the Tcl-family if elseif else while for foreach switch try catch finally on error trap, Bash esac done, PHP end*, ObjC @try @catch @finally @throw @synchronized @autoreleasepool.
    • R05 (Comparison / ShortCircuit / EagerLogical / Ternary / NullSafe / CaseArm / Guard): and or xor not and_eq or_eq xor_eq bitand bitor compl not_eq eq ne lt gt le ge cmp isa in ni notin is instanceof starts_with ends_with contains equals matches matches_regex matches_glob when (Elixir/Kotlin/C#), Perl unless.
    • R08 (Call / StringLit): new delete sizeof typeof decltype alignof offsetof _Generic asm __asm__ __asm va_arg @selector @encode __builtin_available print echo exit unset include require use no require_once q qq qx qw m qr s tr y and the JS import export from as of with using await async target meta, TS satisfies keyof infer is asserts, C# query keywords (from into join on equals let orderby ascending descending group by select) and checked unchecked stackalloc lock with __makeref __reftype __refvalue.
    • Halstead operand, no role: this super base self parent true false nil null NULL nullptr undefined __FILE__ __LINE__ __PACKAGE__ __SUB__, ObjC BOOL IMP SEL Class id, Rust macro fragment specifiers (block expr ident …), Bash @ U u L Q E P A K a k (parameter-expansion operators), Ruby i ri r (numeric suffixes).
    • Annotations / attributes / pragmas, no role by decision recorded on the rows: __attribute__ __declspec and the MS calling conventions, Gecko MOZ_* / *_TRY_* macros, Apple SDK macros (NS_*, CF_*, IB*, API_*, __deprecated_*), ObjC availability words (ios tvos macos watchos), C# #pragma words (hidden warning disable restore checksum enable annotations warnings) and attribute targets (alias global assembly module field event method param property return type typevar), Kotlin use-site targets (file property receiver param setparam delegate field), Java module directives (open module requires transitive exports to opens uses provides with), Perl phase blocks (BEGIN UNITCHECK CHECK INIT END), PHP declare words (ticks encoding strict_types), Rust raw gen ref _.

    Issues #1598, #1484, #1600: see their own comments.

    Performance: criterion corpus/walk/halstead before/after, interleaved on one target dir, six rounds: 13.6/12.6, 12.43/12.48, 14.0/13.5, 14.4/15.8, 13.2/13.1, 12.1/11.9 ms (before/after) — inside the run-to-run noise; corpus/walk/all 21.4 → 21.3 ms. make bench-scaling passes, every halstead/ probe at exponent 0.98–1.08. The row predicates compile to one switch i16 per language with gate calls reached only on the gated kinds (LLVM IR inspected for Cpp, ObjC, Groovy, Kotlin).

    Verification on 18a2a6ae: make pre-commit → BCA_GATE: pass (gate=pre-commit); INSTA_FORCE_PASS=1 cargo test --workspace --all-features --no-fail-fast → 0 .snap.new, 0 .pending-snap, output submodule clean; make chain-audit passes; make check-test-lang-gates passes; rustfmt-bail baseline unchanged. Reviews: astra-review ×3 (plan, C-family, full) and a confirmation pass, the review skill checklist, a performance pass, simplify-rust and audit-tests; every finding is fixed in the branch except two recorded as out of scope — Perl's auto-quoted hash key spelled as a declarator ($h{sub}) bills the keyword, pre-existing and output-moving, and the TS/TSX and Cpp/Mozcpp row copies, which contract 5 of #1690 assigns to R04.

    Not pushed; stopping before R04 (#1694).

  4. dekobon commented on Oct 23, 2026

    @dekobon
    OwnerAuthor

    Final review round (the review skill, astra-review, sol-review, the security agent and a claims-vs-code audit) on 18a2a6ae. Two more commits landed from it, and the completion comment above needs four corrections it found.

    Fixes in the branch:

    • bcc46ae1: an absent row now emits every predicate a declared row has (owns, pays_at_leaf, pays_at_wrapper as well as the _kind pair), with a test over JavaScript's absent Primitive; the Python reader's #[cfg(test)] match admits whitespace and rejects a predicate that is not exactly test, both self-tested; fragment comments that still described the two-arm form are rewritten; the conflict test names the shipped nodes where row and arm really disagree (Groovy int, ObjC property-attribute words); grammar-dispatch.md now points a shared kind list at the roles directory rather than kind_sets.rs.
    • The next commit: spelling rows for the five row kinds the earlier sweep missed (C# required / fixed / implicit, Perl state, Rust move), each failing by name when dropped; a lesson-34 drift marker for type_specifier, the hidden supertype the C-family Primitive fragment lists defensively, in all four dialects; two comments that still pointed at is_primitive and the Cpp arm as the home of a list.

    Corrections to the completion comment:

    • The branch is thirteen commits on 92d7ff74 after this round, HEAD de773579 (it was eleven, not twelve, when the comment said twelve).
    • "1,095 anon visible entries remain" should read "1,094 keyword-shaped anon visible entries" — the filter is an identifier-shaped spelling (letters, _, @, -), not every anonymous visible kind, of which there are 4,017.
    • The exemption buckets: Checker::is_primitive is nine sites (Kotlin's is allow(Modifier) for reified), and the ABC binding walks are four (C#, Groovy, JS family, Kotlin); the total of 29 stands.
    • The design(getter/java): bill directive keywords like C# or keep the import-static guard #1600 Groovy measurement quoted n1 9 / N1 13 for a fixture "the same minus the module"; those numbers are for the newline-separated, semicolon-free spelling. With Java's semicolons kept the Groovy fixture reads n1 10 / N1 17. Either way no directive keyword bills, which is the finding.

    Two things the audit flagged that stay as they are, as judgement calls recorded here rather than in a row: Groovy var and Python global / nonlocal are left in the keyword-shaped residue — var because the pinned Groovy grammar emits it only as a type placeholder, the Python pair because they scope a name rather than declare one; a later package may claim them under DeclKeyword. The security pass found no new panic, out-of-bounds, or unbounded path for analysed source (every byte read goes through node_text, every walk is bounded by the tree), and noted only that a malformed roles file referencing a large fragment from many rows materialises the product in memory before validation — a first-party repository file, not an input.

  5. dekobon commented on Oct 23, 2026

    @dekobon
    OwnerAuthor

    Follow-up bef8600d (fourteen commits on 92d7ff74; make pre-commit → BCA_GATE: pass; output still byte-identical): the two judgement calls the previous comment recorded only here now sit on their rows, where the next reader of roles/groovy.rs and roles/python.rs will find them.

    • Groovy var: DeclKeyword now carries excludes [Var "…"] pointing at fix(getter/groovy): bill var as a declaration keyword #1704. One correction to the previous comment — I wrote that the grammar emits var only as a type placeholder; it does not. It is a keyword leaf under local_variable_declaration, the same shape as Kotlin's var and Groovy's own def, and it belongs in the row. The reason it is not there is that no arm ever billed it, so claiming it moves n1 (measured: def f() { var x = 1 } is n1 4 today, 5 with var), which is a fix, not a refactor. fix(getter/groovy): bill var as a declaration keyword #1704 is that fix.
    • Python global / nonlocal: excludes [Global "…", Nonlocal "…"] on DeclKeyword — each rebinds the scope of a name declared elsewhere rather than declaring one, so they stay with the statement keywords in the getter. Claiming them would move no output; the exclusion records the classification rather than leaving it to omission.

    Coverage baseline −3 (exactly those entries); census and bypass unchanged.

  6. 14 remaining items

  7. dekobon commented on Oct 23, 2026

    @dekobon
    OwnerAuthor

    Merged to main in #1705 at bdd70eb9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorCode maintainability / tech-debt cleanup (no behaviour change)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions