Repository navigation
fix(npm/ruby): a def wrapped in a non-visibility call is not counted #1401
Description
Summary
#1255 fixed private def x by teaching the Ruby Npm walk to look
inside a visibility call's argument list. The root cause it names is
more general: a method node that is not a direct child of the class
body is not counted at all, so class_nm disagrees with
nom.functions_sum on the same space. Three shapes still hit it.
All measured with bca metrics --no-config -O json at fix/issue-1255.
1 — A def wrapped in a non-visibility call
class Memo
memoize def cached; end
def plain; end
endclass_nm = 1, nom.functions = 2. memoize def / deprecate def /
sig { ... }; def are real idioms; the method is simply absent from
nm.
2 — A doubly-wrapped visibility call
class N
private public def x; end
endclass_nm = 0, class_npm = 0, nom.functions = 1. This is the same
keyword family #1255 handles, one nesting level deeper:
RubyClassBody::apply_visibility_call sees a Call argument, falls to
its symbol arm, resolves no name, and drops the declaration exactly as
before the fix. Ruby's answer is that x is private — public(def x)
returns :x and the outer private :x then demotes it — so a fix has
to declare through the inner keyword and then let the outer one refile
by name.
3 — A def inside a conditional or a begin
class Cond
if RUBY_VERSION > "3.0"
def modern; end
else
def legacy; end
end
private
begin
def wrapped; end
end
def plain; end
endclass_nm = 1, nom.functions = 4.
Direction
The general rule would be "any Method / SingletonMethod reachable
from a body-level statement counts, taking its visibility from the
innermost recognised keyword, or from the body-wide flag when there is
none". That is a bigger change than #1255's arm and needs a decision on
how deep to descend — a def inside a def must not be counted at
class scope, and neither should one inside a do block that opens its
own space.
Until then, the class_nm ↔ nom.functions_sum parity holds only for
declarations at body level or wrapped in a visibility call. Found by
the independent review of the #1255 fix.
Status after R06 (#1696)
The walk reads the FuncOpener row under each argument's own chain (PR #1717); shapes 1 and 2 are two places it does not yet ask — see the comment of that date.
Resolution
Status: Fixed (pending merge)
Commit: 6b1dbd1 on branch fix/issue-1401 (integration branch fix/batch-2026-10-25)
Root cause: the member walk read the body's direct children and a visibility call's arguments only. Fixed with an explicit-stack walk that reads every statement position at class scope (conditionals, case arms, loop bodies, begin/rescue/ensure, modifiers, parentheses, assignment right sides) and every call's argument list, composing nested visibility calls per Ruby 3.1+, and stops at a nested def / block / lambda. The attribute walk's identical gap is #1719.
Activity
dekobon commentedon Oct 26, 2026
Status after R06 (#1696, PR #1717)
Still open on the PR's head: both repros measure as before —
memoize def cachedgivesclass_nm = 1againstnom.functions = 2, andprivate public def xgivesclass_nm = 0againstnom.functions = 1.What changed is the code the fix lands in.
RubyClassBody::visitandapply_visibility_call(src/metrics/npm/ruby.rs) now read theFuncOpenerrow throughfunc_opener::is_callableand ask it under each argument's own chain (Ancestors::of_childrenon the call, then on itsargument_list), so "amethodnode that is not a direct child of the body" is already answered by the row wherever the walk asks. The two gaps are therefore just two places the walk does not yet ask:memoize def):visit'sCall | Call2 | Call3 | Call4arm hands only visibility calls toapply_visibility_call; a non-visibility call's named arguments are never asked. Askingis_callableof them with the same chain, and declaring under the body-wide flag, is the whole fix.private public def x):apply_visibility_callmeets aCallargument, finds it is not callable, and hands it torefile_argument, which resolves no symbol. Recursing into a nested visibility call — declare through the inner keyword, then let the outer one refile by name, as the issue says Ruby does — is the fix, and the chain for the inner call's arguments isof_childrenone level further down.Shape 3 (
sig { … }; def) is unchanged. Theclass_nm↔nom.functions_sumparity test in the issue would make a good perturbation target for both.dekobon commentedon Oct 29, 2026
Fix Summary
Root cause: RubyClassBody (src/metrics/npm/ruby.rs) read the class body's direct children and a visibility call's arguments, and nothing else. All three shapes in the issue — and several more the reviews found — were therefore invisible to class_nm while nom counted them.
Changes:
Decision on the issue's open question ("how deep to descend"): every statement position at class scope, every call's argument list, and no further — a def inside another def, a block or a lambda belongs to the space nom opens for it. The class_nm == nom.functions_sum parity the issue proposes therefore holds on a body with no nested space (pinned by ruby_class_nm_agrees_with_nom_on_a_flat_body) and not in general.
Review findings taken (astra-review, review, audit-tests, algorithmic-performance): the recursive first draft overflowed the release binary at 5,000 nested calls — now iterative, pinned on a 512 KiB thread; re-filing was quadratic on private def × W — now indexed; rescue modifier, parenthesized statements, assignment right sides, unless/until and the if/until modifiers were missing; nested-keyword semantics are Ruby 3.1+ and documented as such; the rescue_modifier aliases the grammar declares but never emits are pinned unreachable; wmc and halstead.effort headroom on the new struct were met by hoisting the three stateless statement predicates (which #1719 should share) and the frame-close bookkeeping.
Not fixed, filed: the attribute walk ruby_walk_class_body in npa/shared.rs still reads direct children only — #1719, with a FIXME at the predicates it should take over.
Tests: 27-perturbation sweep, 25 discriminating; the two that do not (when-pattern / rescue-class positions, assignment left side) are defence in depth and say so in the code. Patch coverage 99.74% (387/388; project 98.78%). make chain-audit and make bench-scaling pass. No integration-snapshot movement (the corpora carry no Ruby).
Evidence: ~/bca-evidence/1401/ (fixtures, ruby 3.0.2 oracle outputs, dumps, perturbation driver and log, bench log, coverage-llvm.json, the first draft's overflow and width probes under review-astra/).
Commit: 6b1dbd1 on branch fix/issue-1401 (integration branch fix/batch-2026-10-25).
dekobon commentedon Oct 31, 2026
Evidence supplement (2026-10-31)
The fix comment above cites a local evidence directory, a path on one machine. Three source comments in the tree rested on two files there. The ruby 3.0.2 oracle script and its output are now
docs/development/ruby_visibility_oracle.md(re-run against the installed 3.0.2 today, identical). The other is the perturbation sweep, recorded here.The fix comment says "27-perturbation sweep, 25 discriminating". That was the sweep at review time. The retained log is a later, larger run against the merged
src/metrics/npm/ruby.rs(6b1dbd14): 35 perturbations, 29 discriminating, 6 that fail no test. The driver removed one arm or inverted one guard per row from an in-memory copy and restored infinally; it is not in the tree because its anchors are tied to that revision's text.npm.rsandnpm/ruby.rscite this for the six0 -> []rows, the position exclusions whose widening no fixture can observe: a widened arm that reads a condition, loop header,casevalue orwhenpattern as a statement finds the same shadowed local or bare marker. The comments say those exclusions are chosen by role and unpinned rather than claiming coverage.