refactor(metrics): unify average-divisor guard, cyclomatic per-function#527
Merged
Conversation
Extract a single `crate::metrics::average(sum, count)` helper that applies the `.max(1)` divide-by-zero guard (#428) in one place, and route every metric average through it. This removes the former reliance on a counter that merely defaulted to 1 for cyclomatic and nom, and adds the guard to the previously-unguarded per-space averages (loc, abc, tokens). Make `cyclomatic.average` / `cyclomatic.modified.average` per-function: the divisor is now the count of function/closure spaces in the subtree (`function_spaces`), the per-function convention cognitive/exit/nargs use, instead of the per-space count (which also divided by classes, structs, and the file unit). The count is seeded in `FuncSpace::new` from the space kind and summed on merge, so it is sourced independently of the Nom metric and a cyclomatic-only selection still divides per function without pulling a nom block into the output. Metric values change for cyclomatic.average and modified.average only; sum/min/max and every other metric (including MI and WMC, which consume the cyclomatic sum) are unchanged. The denominator equals nom.total() wherever a closure opens its own space; a spaceless closure such as a Python lambda is counted by cognitive but not as a separate cyclomatic divisor unit (pinned by a regression test). Snapshots — including the big-code-analysis-output submodule — are re-baselined accordingly. Part of #505. Folded into the 2.0 re-baseline. Fixes #512
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #527 +/- ##
==========================================
+ Coverage 86.01% 86.04% +0.03%
==========================================
Files 74 75 +1
Lines 56528 56666 +138
Branches 56488 56626 +138
==========================================
+ Hits 48621 48759 +138
Misses 7401 7401
Partials 506 506
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #512 (part of #505; folded into the 2.0 re-baseline).
Unifies the "average over a count" divisor guarding behind one helper and
makes cyclomatic's average per function, reconciling it with the
cognitive/exit/nargsconvention.Changes
crate::metrics::average(sum, count)applies the.max(1)divide-by-zero guard (fix(metrics): Cognitive/Exit/NArgs averages depend on undeclared Nom dependency → inf/NaN #428) in one place. Every metricaverage now routes through it —
cognitive,exit,nargs,nom,and the previously-unguarded per-space averages
loc/abc/tokens(which divided byspace_countwith no guard at all).cyclomatic.average/cyclomatic.modified.averagedivide by a newfunction_spacesaccumulator (count of
SpaceKind::Functionspaces in the subtree),seeded in
FuncSpace::newfrom the space kind and summed on merge.It is sourced independently of the
Nommetric, sometrics=["cyclomatic"]still emits exactly{cyclomatic}— nonomblock leaks into a cyclomatic-only selection.
Denominator convention (documented)
cognitive,cyclomatic,exit,nargs): divide bythe function/closure count.
nom,loc,abc,tokens): divide by the total spacecount (per-function would be circular for
nom).Value impact
Only
cyclomatic.averageandcyclomatic.modified.averagechange value(larger for files with classes/structs/units).
sum/min/maxandevery other metric — including the Maintainability Index and WMC, which
consume the cyclomatic sum — are unchanged. All 25,088 changed
submodule snapshot lines are under
cyclomatic:.Known nuance (pinned, intentional)
function_spaces == nom.total()wherever a closure opens its own space(verified across Ruby / Go / Kotlin / C# / Java). A Python
lambdaiscounted by
nombut opens no space, so it is not a separate cyclomaticdivisor unit — the divisor counts the spaces that actually carry a
cyclomatic value. Pinned by
cyclomatic_python_lambda_divisor_excludes_spaceless_closure. Exactlambda-level reconciliation with
cognitivewould require nom-coupling orindependent lambda-node counting and is left as a deliberate follow-up.
Tests / validation
nomcontract, zero-divisor guard,and the Python-lambda edge case. Verified via production-mutation
(revert → tests fail → restore).
code-review, review); findings resolved.
tests/snapshots, and thebig-code-analysis-outputsubmodule.(all features), doc, snapshot-anchors, self-scan + headroom, rumdl.
Submodule
Requires
dekobon/big-code-analysis-outputat65f5dccf(pushed tomain); the parent records the bumped SHA in this PR.