fix(erlang): don't count comments as arguments when computing arity - #1619
Open
Dshuishui wants to merge 1 commit into
Open
fix(erlang): don't count comments as arguments when computing arity#1619Dshuishui wants to merge 1 commit into
Dshuishui wants to merge 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
tree-sitter reports `comment` as a named child, so a comment written at the top level of a parameter or argument list inflated the arity taken from `namedChildCount`: `plain( % note\n 1, 2)` read as plain/3 and the edge was dropped, and `f(X, % note\n Y) ->` was indexed as f/3. The MFA list in `spawn(?MODULE, f, [A, % note\n B])` had the same problem. Arity only became part of a function's identity in colbymchenry#1610, so the miscount was harmless before that. The three counts now share one `namedArgCount` helper so the invariant is stated once.
Dshuishui
force-pushed
the
fix/erlang-arity-comments
branch
from
August 26, 2026 15:48
d54be5c to
9bf68eb
Compare
Author
|
Rebased onto Re-verified on current |
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
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.
Follow-up to #1615, rebased onto
mainnow that it has landed.Arity is computed with
namedChildCount, but tree-sitter reportscommentas a named child, so a comment at the top level of a parameter or argument list inflates it:Same for the MFA list in
spawn(?MODULE, f, [A, % note\n B]). A comment nested one level deeper (inside#state{...}, a tuple, a list) is unaffected — only the top level of the list counts.This surfaced with #1615: before it, arity took no part in resolution, so the miscount was harmless. Now arity is identity, so one extra unit silently drops the edge.
Small blast radius. Across cowboy, ranch, jsx, rebar3 and fast_xml only rebar3 is affected, losing two real edges (
rebar_compiler.erl:403,rebar_release_SUITE.erl:247); re-indexing those five with this patch changes nothing else — no nodes added or removed, no edges removed, exactly those two edges restored.The three counts now go through one
namedArgCounthelper so the invariant is stated once.The new test in
erlang-arity-resolution.test.tscovers the four shapes above. I checked it is not vacuous: onmainwithout this patch it fails, and it passes with it. The full suite passes (3,053).No CHANGELOG entry — #1615's entry already describes the user-visible behaviour, and the trigger here is rare; this just keeps that rare case from silently dropping edges.