Skip to content

build: cap function size, complexity and method surface - #226

Merged
OmarAlJarrah merged 1 commit into
mainfrom
feat/method-and-size-caps
Aug 2, 2026
Merged

build: cap function size, complexity and method surface#226
OmarAlJarrah merged 1 commit into
mainfrom
feat/method-and-size-caps

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Closes #83. Closes #178. Stacked on #225#223#222#221#220#219#218.

Summary

Three shapes produced the god object, and none of them was measured while it was growing. This enforces all three, now that the restructuring has finished and the caps can be calibrated against the shape they are meant to hold rather than the one that failed.

funlen adopts the styleguide's 70-line cap, which until now was declared in CLAUDE.md and enforced by nothing. gocognit caps nesting-weighted complexity — the reason the line cap is 70 and not 200. Both skip test files: a table-driven test is long because its table is long, and the table is data; splitting one to satisfy a size cap moves cases away from the assertion that reads them, which is the opposite of what the cap is for.

The method cap is the one that would have caught the actual failure. No function in compilers/openapi ever came near 70 lines — #178 measured that when it was filed. What grew was type surface, from one method to 159, and nothing measured type surface. A new AST rule in internal/archtest caps methods per type under compilers/ at 20, against a finished tree whose widest type is compile.Types at 14.

gocognit's first finding was a real one, and I took it rather than tuning it away. irverify's reflect walk scored 28 — twice anything else in the repo, the next being 14. Setting the bar at 30 would have made the tree pass while encoding exactly the shape the cap exists to prevent, which is the failure mode #178 names in its own sequencing note. So the walk is split: a small valueWalk value carrying the seen-set, the visitor and the truncation flag, with walk, descend and children as methods. Behaviour is unchanged; the tree's worst is now 14 and the cap sits at 20.

Test plan

  • gofmt, go vet, golangci-lint run (0 issues), go build ./..., ./scripts/check-coverage.sh — all pass. Statement count moves 4188 → 4185: the three closure assignments in the reflect walk became method declarations.
  • The diagnostic stream is byte-identical to main over all 163 fixtures, and the whole suite passes, which is what holds the walk split to "behaviour unchanged".
  • Both caps proven by planting, per archtest: cap methods per type so the god object cannot regrow #178's acceptance. A 75-line function fails funlen at 74 > 70. Giving lowering.Ctx a twenty-first method fails the new rule with lowering.Ctx has 21 methods. Both restored, both green again.
  • The counter has its own planted tests, because the live tree has nothing near the limit and a counter that always returned zero would pass the live check forever: a package past the cap, two same-named types in different packages counted apart, and value, pointer and generic receivers counted as one type — plus a method in a _test.go file that must not count.
  • The live check requires a non-empty sweep, so a walk that stopped reaching compilers/ fails rather than passing vacuously.

Notes

  • build: the documented golangci-lint gate cannot run against go 1.26.3 #61 (the gate could not run against go 1.26.3) is no longer blocking: golangci-lint 2.12.2 runs clean here. This PR does not close it — that issue is about the toolchain pin, not about these linters.
  • statements is left off in funlen. Two limits on the same axis means the tighter one is the rule and the other is noise.

@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 0ac57f8 to 1eead21 Compare August 1, 2026 22:41
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 38c945a to 73f7a6b Compare August 1, 2026 22:41
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 1eead21 to 53cd85d Compare August 1, 2026 23:04
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 73f7a6b to 1632cd0 Compare August 1, 2026 23:04
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 53cd85d to 627f6dc Compare August 1, 2026 23:06
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 1632cd0 to 10a5b23 Compare August 1, 2026 23:06
@OmarAlJarrah

Copy link
Copy Markdown
Member Author

Force-pushed after a deeper review pass.

Splitting the reflect walk moved its cycle guard into a struct field, and nothing held that guard. Removing if w.seen[p] { return } changed nothing any test observed — no document in the corpus shares a pointer, because a compiled Document is a flat registry referenced by ID rather than by aliasing. A planted document that does share one now holds it, and removing the guard fails that test. Visiting a shared pointer once is also what makes the walk's paths deterministic, which invariant 7 rests on.

The rest of the PR was re-probed rather than re-read: gocognit fires at the configured 20 (not merely at the 28 it found on the old walk), funlen fires at 71 lines, the method cap fires on a twenty-first method, and the byte-sequence skip in the walk is still held by its own test.

@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 627f6dc to 233cc84 Compare August 1, 2026 23:11
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 10a5b23 to 2ccc7cd Compare August 1, 2026 23:11
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 233cc84 to 279cb9a Compare August 1, 2026 23:40
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 2ccc7cd to a7adffa Compare August 1, 2026 23:41
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 279cb9a to e5a931d Compare August 1, 2026 23:45
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from a7adffa to 6dbe9a4 Compare August 1, 2026 23:45
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from e5a931d to 0670aa0 Compare August 1, 2026 23:56
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 6dbe9a4 to 3c143ce Compare August 1, 2026 23:56
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 0670aa0 to 3d80f64 Compare August 2, 2026 00:08
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 3c143ce to 0fdb553 Compare August 2, 2026 00:08
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from 3d80f64 to aebda3e Compare August 2, 2026 00:20
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 0fdb553 to f73c29c Compare August 2, 2026 00:20
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from aebda3e to a072c1f Compare August 2, 2026 00:30
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from f73c29c to 860239c Compare August 2, 2026 00:30
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/recursion-pin-value-edges branch from a072c1f to f71cddb Compare August 2, 2026 00:40
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from 860239c to dfdfdd6 Compare August 2, 2026 00:40
Base automatically changed from fix/recursion-pin-value-edges to main August 2, 2026 00:44
Closes #83. Closes #178.

Three shapes produced the god object, and none of them was measured while
it was growing. This enforces all three.

funlen adopts the styleguide's 70-line cap, which until now was declared
and enforced by nothing. gocognit caps nesting-weighted complexity, which
is the reason the line cap is 70 rather than 200. Both skip test files: a
table-driven test is long because its table is long, and its table is
data — splitting one to satisfy a size cap moves cases away from the
assertion that reads them.

The method cap is the one that would have caught the actual failure. No
function in that package ever came near 70 lines; the surface that grew
was the type's, from one method to 159, and no rule in force measured
type surface at all. An AST rule in internal/archtest now caps methods
per type under compilers/ at 20, against a finished tree whose widest
type carries 14.

gocognit's first finding was a real one: irverify's reflect walk scored
28, twice anything else in the repo. Adopting a cap above it would have
encoded exactly the shape the cap exists to prevent, so the walk is split
into a small value carrying the walk's state and three methods — visit,
descend, and the per-kind children. Behaviour is unchanged; the tree's
worst is now 14, and the cap is set at 20.

Both caps are proven by planting, not by reading: a 75-line function and
a type given a twenty-first method each fail, and the counter's own tests
plant a package past the cap, two same-named types in different packages,
and all three receiver spellings of one type.

Splitting the walk moved its cycle guard into a struct field, and nothing
held that guard: no document in the corpus shares a pointer, because a
compiled Document is a flat registry referenced by ID, so removing the
guard changed nothing any test observed. A planted document that does
share one holds it — visiting once is also what makes the walk's paths
deterministic, which invariant 7 rests on.

What the method cap measures is now stated and pinned: methods declared
on a type, not the type's method set. That is the dimension the failure
grew in — 159 methods written across 11 files, one at a time — and it is
what an AST rule can measure without a type checker. It also means a type
can stay under the cap by embedding, which is the refactoring the cap
exists to prompt rather than a way around it; leaving that unsaid would
have let the first reader assume otherwise.
@OmarAlJarrah
OmarAlJarrah force-pushed the feat/method-and-size-caps branch from dfdfdd6 to d943957 Compare August 2, 2026 00:48
@OmarAlJarrah

Copy link
Copy Markdown
Member Author

Rebased onto the new main (the #225 squash), which clears the conflict. Then a deeper pass. The caps all still fire — funlen on a 74-line function, gocognit on a planted 48, the method cap with lowering.Ctx has 21 methods — so this is about what they measure.

The method cap counts declared methods, not the method set, and nothing said so. A type declaring 15 methods and embedding another with 15 has a surface of 30 and passes. I planted exactly that to confirm it.

That is the right measure, not a hole, and the const now says why: the dimension the failure actually grew in was declarations — 159 of them, written across 11 files, one at a time — and it is what an AST rule can measure without standing up a type checker, which is what #178 asked for. It also means the cap can be satisfied by embedding, and that is the refactoring it exists to prompt rather than a way around it: two types each under the cap, each with a coherent set, is the outcome you want. A type reaching for embedding to duck the number is visible in review in a way that one more method on one more struct never was. There is a planted case pinning the behaviour so the choice cannot change silently.

What was already sound, probed rather than assumed:

  • Cross-file aggregation is held — the thing that matters most here, since the god object was spread over 11 files. Making the counter reset per file fails the planted tests.
  • The test-file exclusion, and value/pointer/generic receivers counting as one type, are each held by their own planted case.
  • The linter exclusion is scoped to _test\.go only: internal/testspec/spec.go is still linted, so a production file with "test" in its path does not slip through.
  • The reflect-walk split is behaviour-neutral and its invariants have teeth: removing the cycle guard fails (the test added in the earlier pass), removing the byte-sequence skip fails, and dropping the truncation flag fails four tests. The depth cap's test derives its document from maxWalkDepth rather than restating 4096, so the constant cannot drift away from the test that exercises it.

Unrelated, filed on #59: the coverage gate reported two different statement counts for the same commit while I was running it repeatedly across this stack — cover.out is a fixed path and concurrent runs can duplicate blocks. It cannot cause a false pass (verified), so it is a reporting defect.

@OmarAlJarrah
OmarAlJarrah merged commit bf15ab3 into main Aug 2, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the feat/method-and-size-caps branch August 2, 2026 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

archtest: cap methods per type so the god object cannot regrow lint: enforce the function-size and complexity caps in golangci-lint

1 participant