Feature #22520
open
{Proc,Method,UnboundMethod}#syntax_tree should return the block node, not the enclosing call
Description
Summary¶
Currently, when a Proc (or a Method defined with define_method) comes from a block, #syntax_tree returns the call that owns the block (CallNode, SuperNode, ForwardingSuperNode), not the block. I propose returning the BlockNode (prism) or the SCOPE node (parse.y) instead, i.e. the node that is actually compiled into the proc's iseq. #syntax_tree is still experimental and not yet released, so now is the time to change it.
The call includes code that is not part of the proc¶
class Response
def initialize(status) = @status = status
{ ok: 200, not_found: 404, internal_server_error: 500 }.each do |name, code|
define_method(:"#{name}?") { @status == code }
end
end
m = Response.instance_method(:ok?)
m.syntax_tree.class # => Ruby::Prism::CallNode
m.syntax_tree.slice # => define_method(:"#{name}?") { @status == code }
The code that runs when calling ok? is { @status == code }. :"#{name}?" is evaluated once, while the class body runs, and is never part of the method.
Any tool walking the returned tree gets the wrong answer. For example, collecting the local variables read in the returned node gives [:name, :code], but the method only captures code ([:code] when walking the BlockNode). The same applies to checking whether a proc is isolated (e.g. for Ractor), serializing a proc, or showing a method's source. Every such tool has to know to special-case CallNode/SuperNode/ForwardingSuperNode and call .block first.
This is the same reason given in #21998#note-24 for source_range: Coverage would otherwise report define_method((sleep(1); :a)) do; end as the method, although sleep(1) runs outside of it. Another common example is Thread.start(server.accept) do |client| ... end: server.accept runs in the accepting thread, not in the new thread's block.
It is inconsistent with #source_range¶
#21998 settled that source_range covers the block, starting at {/do (matz agreed in #21998#note-27). syntax_tree disagrees with it:
m.source_range # => #<Ruby::SourceRange response.rb:(5,31)-(5,50)>, i.e. { @status == code }
m.syntax_tree.location.start_column # => 4, i.e. define_method(...
So Prism.find(m) (which uses source_range) and m.syntax_tree return different nodes for the same method. They should agree.
The arguments made in #21998 for the block apply to syntax_tree as well. In short: a block is syntactically just another argument of the call (the call still parses and stays a CallNode without it), so including the call, its receiver and its other arguments in the block is arbitrary, and a node should not include the source of its parent (headius in #21998#note-8, #21998#note-12, #21998#note-16, #21998#note-17; tompng in #21998#note-18; and the reasons in #21998#note-24). As headius put it in #21998#note-12: "A nested AST element's range should not include source ranges of parent nodes."
I initially thought it would be convenient to include the call too (#21998#note-10, #21998#note-14), and also listed CallNode as a possible result of Proc#ast in #21795#note-2. But I changed my mind while implementing source_range (#21998#note-24): it is counter-intuitive, and it only helps a rare case while making the common case awkward.
It is inconsistent between -> {} and lambda {}¶
scope :active, -> { where(active: true) } # syntax_tree => LambdaNode: -> { where(active: true) }
scope :active, lambda { where(active: true) } # syntax_tree => CallNode: lambda { where(active: true) }
These two are semantically the same, but one returns a node with #parameters, #body and #locals, and the other returns a node without any of them. With the block node, every proc gives a BlockNode or a LambdaNode, and both have the same interface.
The lambda literal case also shows that returning the parent is not what we do elsewhere. syntax_tree for -> {} returns the LambdaNode, not the enclosing scope :active, ... call, even though it has exactly the same "cannot go up to the parent" property. Likewise Method#syntax_tree for a def returns the DefNode, not the enclosing ClassNode or the private def ... call.
The iseq's node_id already points to the BlockNode. The implementation deliberately goes up one level (ast.rb: return child.type == :block_node ? node : child). parse.y has the same distinction: today it returns the ITER node (capture { 42 }, which covers the call), but its SCOPE child covers just the block ({ 42 }). That is the node rb_iseq_compile_node() compiles and what the block iseq's code_location describes.
About getting the call node¶
The reason given for returning the call in #21795#note-27 is that a prism node cannot go up to its parent, so one can go from the call to the block but not the other way around. However:
- Most uses want the code of the proc (its parameters, body, locals), not the call it was passed to. Returning the call makes the common case harder in order to make the rare case easier.
- For the rare case, the parent can still be found from the block node: re-parse and search for the node whose
blockhas the samenode_id, or use a future generic parent-access API in prism (as discussed in #21795#note-33). That would also work for lambdas passed as arguments, which the current special case does not help with. - It is much easier to add a way to get the parent later than to change the return value once
#syntax_treeis released and tools depend on it.
Proposal¶
Proc#syntax_tree,Method#syntax_tree,UnboundMethod#syntax_treeandRubyVM::InstructionSequence#syntax_treereturn the block node for block-based procs/methods:BlockNodewith prism,SCOPEwith parse.y.- Then
syntax_tree.locationmatchessource_range, andsyntax_treematchesPrism.find. Thread::Backtrace::Location#syntax_treeis unaffected: it already returns the node at the location.
Updated by