fix: eliminate dead-code false positives from optimistic effect classification - #37
Open
gilbertwong96 wants to merge 5 commits into
Open
fix: eliminate dead-code false positives from optimistic effect classification#37gilbertwong96 wants to merge 5 commits into
gilbertwong96 wants to merge 5 commits into
Conversation
Dependency beam inference classifies Phoenix.PubSub.broadcast/3 as :pure, so callers that fire-and-forget the :ok result (plus wrapper functions and __aliases__ arguments) show up as dead code. Same for Task.Supervisor.start_child/2. Add both to the built-in messaging classification so classification doesn't depend on beam inference.
gilbertwong96
force-pushed
the
fix/pubsub-broadcast-effect
branch
from
August 1, 2026 19:30
5d703a0 to
0fbd5d0
Compare
:if/:unless/:cond desugar to :case nodes and :for to :comprehension, so their branch bodies are not direct block children. connect_heex_parts only wired direct children to the block output, leaving events and component calls inside conditionals (including EEx <%= if %> branches, whose bodies are plain blocks without a HEEx origin) with no output edges — dead-code false positives. Recursively connect branch bodies: every child inside a branch is wired to the branch output, and the branch output to the enclosing block output. Nested control flow is handled recursively.
gilbertwong96
force-pushed
the
fix/pubsub-broadcast-effect
branch
from
August 2, 2026 05:32
2fc8515 to
4c69a6b
Compare
classify_from_spec / classify_from_inferred marked functions whose @SPEC or ExCk-inferred signature returns {:ok, _} | {:error, _} as :pure, because result tuples look like plain data. But that signature is the canonical shape of side-effecting operations (Repo, GenServer, HTTP, File) — fire-and-forget calls like record_read/2 were reported as dead code. Treat any return type whose leading element is an :ok/:error atom as non-pure in both the typespec and ExCk inference paths. This is a conservative fix: functions that can't be proven pure are no longer reported as dead, which is the right trade-off for an advisory check. Alternative approaches were evaluated and rejected: - Project-level effect inference prewarm: the ExCk heuristic still contaminates it — inference order determines whether a function or its callers get classified first, and the optimistic :pure wins either way. - Signature heuristics alone for unions were insufficient: ExCk encodes {:ok, nil} as a tuple, not a union.
gilbertwong96
force-pushed
the
fix/pubsub-broadcast-effect
branch
from
August 2, 2026 06:22
4c69a6b to
e2bd412
Compare
The Phoenix plugin treats every call in Phoenix.Controller / Plug.Conn as :pure via @pure_remote_modules. delete_csrf_token/0 (delegated to Plug.CSRFProtection) mutates process state to drop the CSRF token — the standard fire-and-forget call in phx.gen.auth login flows — so its discarded :ok result was reported as dead code. Exclude it explicitly before the module-level pure classification.
Two buttons with the same phx-click event name but different phx-value-* attributes (e.g. clear-filter vs set-filter in a tag strip) were flagged as duplicate calls, because __live_event__ nodes only carry the event name string — the distinguishing values live in separate dynamic-attr nodes. Event registrations are compile-time template artifacts with no runtime cost, so duplicate registration is never a real redundancy. Exclude __live_event__ alongside the other compiler-generated nodes (:__aliases__) already in @excluded_fns.
gilbertwong96
changed the title
gilbertwong96
force-pushed
the
fix/pubsub-broadcast-effect
branch
from
August 2, 2026 07:25
b628ced to
6dce258
Compare
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.
Problem
mix reach.check --dead-code --smellsreported 26 false positives. They share one root cause: the effect classifier optimistically marks side-effecting calls as:pure— from beam inference, from@spec/ExCk return types, or from module-level classification — so fire-and-forget calls (whose:okresults are intentionally discarded) get flagged as dead code.Fixes
Phoenix.PubSub.broadcast/Task.Supervisor.start_child→:send(0fbd5d0)Beam-based dependency inference can't see the message/process side effects; classify them in the built-in messaging table instead.
HEEx events/components inside conditionals connect to template output (
9ee97e0):if/:fordesugar to:case/:comprehensionnodes whose branch bodies were never wired to the block output, sophx-clickevents and component calls inside conditionals (including EEx<%= if %>branches) looked unused.{:ok, _}result-wrapper return types are not:pure(e2bd412)@spec/ExCk inference treated{:ok, _} | {:error, _}as plain data — but that is the canonical signature of side-effecting operations (Repo, GenServer, HTTP, File). Conservative: unprovable functions are no longer reported dead.Phoenix.Controller.delete_csrf_tokenis not:pure(ae00b18)The Phoenix plugin classified the whole
Phoenix.Controllermodule as pure; the token-drop call mutates process state (standardphx.gen.authlogin flow).__live_event__excluded from the redundant-computation smell (b628ced)Two buttons sharing an event name but differing in
phx-value-*were flagged as duplicates — event registrations are compile-time template artifacts with no runtime cost.Verification