Define named priority methods as real methods#115
Open
frewsxcv wants to merge 3 commits into
Open
Conversation
Replaces method_missing dispatch for named priority class methods (Delayed::Priority.eventual) and instance predicates (priority.eventual?) with real method definitions, (re)defined whenever names are assigned. Real methods are visible to runtime reflection, so documentation tools and Sorbet's tapioca can see them; method_missing methods are not, which forced Sorbet-adopting apps to shim every named priority by hand. Pre-existing methods are never overridden, matching the old method_missing behavior where a real method always won. Also fixes a test-order leak in priority_spec.rb: assign_at_midpoint was never reset by the around hook, so any example that ran after a midpoint example saw midpoint-shifted priority values.
rzane
reviewed
Jul 9, 2026
Comment on lines
+135
to
+136
| @named_priority_class_methods = names.keys.filter_map { |name| define_priority_class_method(name) } | ||
| @named_priority_predicates = names.keys.filter_map { |name| define_priority_predicate(name) } |
Contributor
There was a problem hiding this comment.
Can this be done without introducing additional pieces of state?
Member
There was a problem hiding this comment.
We do still want to call remove_method for any previously-specified names, but I agree I think this logic could be implemented with @names as the only retained state.
Author
Address review feedback: remove the @named_priority_class_methods and @named_priority_predicates tracking ivars. On reassignment, remove the methods for the previously-specified names (read from @NAMEs before it is overwritten) using method_defined?(name, false) so only methods we defined ourselves are removed, leaving any pre-existing method a colliding name never overrode (e.g. Class#superclass, Numeric#zero?) in place.
rzane
previously approved these changes
Jul 9, 2026
betterment-policy-bot-production
Bot
dismissed
rzane’s stale review
Invalidated by push of 61a7ea7
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
3 participants
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.
What
Replaces
method_missingdispatch for named priority lookups with real method definitions:Delayed::Priority.eventual,.interactive, etc.priority.eventual?,priority.interactive?, etc.Methods are defined when the class loads (for the default names) and are removed/redefined whenever
Delayed::Priority.names=is assigned. Bodies still resolve throughnames_to_priorityat call time, soassign_at_midpointbehavior is unchanged.Why
method_missingmethods are invisible to runtime reflection, so RDoc and Sorbet's tapioca can't see them. In practice this means every Sorbet-adopting app hits errors like:on every
queue_with_priority Delayed::Priority.eventualcall site, and has to hand-maintain a shim RBI. (In Betterment's retail monorepo this single pattern is currently the only thing blocking ~95 files fromtyped: true.) With real definitions,tapioca gemspicks the methods up automatically:Behavior notes
method_missing, a real method always won (e.g. a hypothetical name ofsuperclasswould never shadowClass#superclass); the new definitions skip any name/predicate that already resolves, preserving that exactly. Covered by a new spec.respond_to?semantics are unchanged, includingrespond_to?(:interactive) == falseafter names are customized (existing specs cover this).names=in an initializer still won't see their custom names in a tapioca-generated gem RBI (tapioca doesn't boot the app) — same as today; they keep their shims.Also
Fixes a pre-existing test-order leak in
priority_spec.rb: thearoundhook resetnamesandalertsbut notassign_at_midpoint, so any spec running after a midpoint example observed midpoint-shifted values. Reproducible onmainwith:Testing
spec/delayed/priority_spec.rbextended: real-method introspection, method cleanup onnames=reassignment, and collision safety.mainin my local environment (Rails 8.0.2 snapshot/error-class drift, unrelated to this change).