style_lint: STYLE040 finds extractable duplicated statement regions, and extract every one in daslib + utils

[aleksisch]

What

Adds STYLE040 — a lint rule that finds runs of statements duplicated elsewhere in the same module that a helper function could absorb verbatim — then acts on what it reports in daslib.

Two commits, deliberately separate:

1. style_lint: STYLE040 … — the engine (daslib/dupe_detect.das) + rule wiring + fixture + docs.
2. daslib: extract the duplicated regions … — the extractions the rule asked for.

This is not utils/detect-dupe. That tool clusters whole functions across a corpus by canonical token stream; this works inside one module and reports extractable regions — the sub-function granularity the corpus matcher cannot see.

How it works

A preorder index of every own-module function body, one record per AST node carrying {payload hash, merkle hash, size, nchild}. A node's subtree is exactly the interval [id, id+size), its first child is id+1, the next sibling is c + size[c] — so full tree navigation needs no child-list storage, and every containment / overlap / nesting question is an integer interval test.

Discovery buckets equal-hash statements and extends sibling runs in O(1) per step, then processes candidates widest-first with a claim set so the largest region wins.

Hashes only ever pick candidates. Every reported pair is confirmed by subtree_equal, an exact lockstep walk comparing node class, payload text and memoized type text under a variable bijection — so no hash collision can become a finding. A class absent from KNOWN_CLASSES is opaque and never matches: an omission costs recall, never correctness. payload_key is the single reader of per-node payload (names, ops, literal values, flag bitfields) shared by hashing and verification, so the two cannot drift.

Variable names may differ (they map through the bijection and become parameters); structure, types, called functions and literal values may not.

Zero-false-positive gates

The rule fires only when the extraction is mechanically valid. It stays silent for:

• a local declared inside the region and read after it (needs a return value — a different refactor)
• an escaping return / break / continue / goto / yield / label
• an assume alias the rest of the block consumes
• a scoped or finalizable declaration whose finalization point would move
• a macro-generated statement, or a defer residue (nada())
• a written free variable that is not a ref or ref type — the write would not survive the call
• a region nested inside one already reported (the same duplication one level down)

Cost, and why it ships on by default

Measured, min-of-3, on the two largest modules in the tree:

module	lines	rule off	rule on
daslib/linq_fold_common.das	6223	6.15s	5.68s (below noise)
daslib/debug.das	2403	5.54s	5.58s (+0.7%)

Under 1% of a lint pass; compile time dominates. Thresholds default to 20 AST nodes / 2 statements, per-module overrides options _dupe_min_nodes / _dupe_min_statements, suppression via a // nolint:STYLE040 tail comment on the reported line.

Also checked: a normal compile with optimizations and auto-inlining on does not invent duplicates from two call sites of the same inlined helper — the generated-statement gate covers it.

What it found, and what commit 2 does

30 regions across daslib (20) and utils (10), spanning 2–9 lines each, 137 duplicated lines per copy. I read all 30 by hand; every one is genuine — no false positives. Commit 2 fixes all 30, leaving zero STYLE040 reports in daslib and utils:

file	helper extracted
perf_lint	pop_loop_scope — shared by visitExprFor / visitExprWhile
debug	finish_structure_preview, finish_leaf_value, wait_for_resume, push_dap_variable (4 regions, 3 of them 3-site)
decs	store_entity_row — the two call sites differed only in the row index
rst_comment	begin_decl(next_state) — 5 sites collapse into one
aot_cpp	write_field_access_suffix
ast_print	write_variable_decl
match	bind_match_rules
das_source_formatter	mark_type_after_separator
toml	take_time_of_day — the tail of both a date-time and a local-time
rst	collect_field_rows
sql_boost	index_ddl, quoted_column_list
sql_linq	reversed_json_path, reserve_projection, push_source_column
ast_match	guarded_block — the if (cond) { body } every match arm appends
mcp/tools/common	setup_introspection_policies
mcp/subtools/list_types	write_type_body
daspkg/commands	rollback_partial_install — 4 sites
dasHerd watcher	send_changelist_action_result (4 sites), close_current_hunk, clear_attachment, scroll_to_current_hunk, sync_diff_scroll, setup_commit_table_columns

Net −86 lines (22 files, +372 / −458).

Where a region had more sites than the rule reported, the helper takes the differing value and absorbs them all: watcher_server had 4 changelist_action_result sends of which only 2 were byte-identical (the rule was right); the helper takes the action name and covers all four.

Two deliberate non-changes:

• utf8_utils keeps its repetition under a nolint — the continuation-byte tail really is shared between the 3- and 4-byte arms, but those branches mirror the UTF-8 spec table and a helper would obscure it. The rule reports extractable, not advisable.
• debug.das keeps one asymmetry: the stepRequestedStack arm runs afterPause() before clearing continueRequested, where the other three clear it first. STYLE040 correctly did not merge it; the ordering is left alone rather than silently normalized.

Bugs the audit surfaced (all in the engine, all fixed here)

Hand-auditing early output found three false-positive mechanisms, which is the reason the gates above exist:

1. Non-adjacent statements looked adjacent — any statement containing a block was dropped from its block's statement list, so its neighbours became "consecutive" (aot_cpp offered three write_sep() calls separated by if blocks). Cause: scalar pending-statement state clobbered by inner blocks → now a PendFrame stack.
2. The escaping-local gate could never fire — declare_var never stamped the vid on the declaring node, so locals declared inside a region were invisible (bool_array offered to extract let offs + let mask, both read afterwards).
3. assume aliases — matched as ordinary statements though the rest of the block consumes the alias.

Plus one silent hashing bug: the Merkle combine was FNV's (h ^ v) * prime, which annihilates to 0 when the accumulator equals the child hash — constant here — so a differing literal (+7 vs +8) never reached the statement hash. Replaced with a murmur3-finalizer combine. The exact-verification pass had been catching it, which is precisely why hashes only pick candidates.

Verification

• 1141 tests pass: tests/ast_match 392, tests/daslib 262, tests/decs 245, utils/daspkg 219, utils/lint/tests 64, tests/match 61, tests/debug_agent 52, tests/lint 38, tests/mcp 14, tests/ast 13
• utils/lint/tests and tests/lint assert exact warning counts, so default-on causing no regression there is meaningful
• das-fmt --verify clean over the whole tracked tree (3791 files) — the real gate for the das_source_formatter change
• list_types output byte-identical before/after (its two branches merged into one helper with a flag)
• lint clean on every changed file except pre-existing STYLE037/038/PERF030, each confirmed against baseline with git stash
• new fixture utils/lint/tests/style040_duplicate_region.das pins one hit plus three negatives (differing literal, escaping local, non-ref scalar write)

Two deliberate non-changes: utf8_utils keeps its repetition under a nolint (the 3-/4-byte arms mirror the UTF-8 spec table and a helper would obscure it — the rule reports extractable, not advisable), and debug.das keeps one asymmetry where the stepRequestedStack arm runs afterPause() before clearing continueRequested; STYLE040 correctly did not merge it and the ordering is left alone rather than silently normalized.

Scope note

daslib already carries 28 STYLE037 + 32 STYLE038 warnings today, so STYLE040 lands in the same advisory band as the existing default-on metric rules — but unlike those, it starts at zero outstanding reports across daslib and utils.

Not swept: tests/, modules/*/daslib/, examples/, and the 106 utils/ files whose deps don't resolve standalone. Those may report on their next touch.

Not run — please weigh these in review

• Full preflight was not run. core.hooksPath is not enabled in this clone, so the pre-push token gate did not apply.
• tests/dasSQLITE and tests/sql_conformance cannot run in this tree — the sqlite native module is not built (missing prerequisite 'sqlite', same on baseline). The sql_boost / sql_linq extractions therefore rest on compile + lint + CI, not on executed tests.
• The aot_cpp.das emitter change is behaviour-identical by construction (same three writes, same order, same conditions), but the nightly full test_aot is its real gate.

🤖 Generated with Claude Code