From fe57f4203f55fc2975fb07fe0b08420e1543be6a Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 6 Aug 2026 21:57:55 +0200 Subject: [PATCH] Remove rubyvm: keyword argument from Prism.find * This does not scale well with more find implementations. * RubyVM is CRuby-specific and experimental so not something usually exposed in public APIs. * Tests can use the specific class directly, which is more reliable. * Unify the Line*Find implementations for Proc, Method and UnboundMethod since they have similar logic and are easier to test that way. --- lib/prism.rb | 6 +- lib/prism/node_find.rb | 119 +++++++++++------------------- rbi/generated/prism.rbi | 4 +- rbi/generated/prism/node_find.rbi | 24 +----- sig/generated/prism.rbs | 4 +- sig/generated/prism/node_find.rbs | 30 ++------ test/prism/ruby/find_test.rb | 16 ++-- 7 files changed, 69 insertions(+), 134 deletions(-) diff --git a/lib/prism.rb b/lib/prism.rb index 8dc8050cc4..0ddf668735 100644 --- a/lib/prism.rb +++ b/lib/prism.rb @@ -90,9 +90,9 @@ def self.load(source, serialized, freeze = false) # an exact match. On other implementations, it falls back to best-effort # matching by source location line number. #-- - #: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable, ?rubyvm: bool) -> Node? - def self.find(callable, rubyvm: !!defined?(RubyVM)) - NodeFind.find(callable, rubyvm) + #: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable) -> Node? + def self.find(callable) + NodeFind.find(callable) end # @rbs! diff --git a/lib/prism/node_find.rb b/lib/prism/node_find.rb index 697ee430e8..6650984a6e 100644 --- a/lib/prism/node_find.rb +++ b/lib/prism/node_find.rb @@ -14,25 +14,17 @@ module Prism module NodeFind # :nodoc: # Find the node for the given callable or backtrace location. #-- - #: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable, bool rubyvm) -> Node? - def self.find(callable, rubyvm) + #: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable) -> Node? + def self.find(callable) case callable - when Proc - if rubyvm + when Proc, Method, UnboundMethod + if defined?(::RubyVM) RubyVMCallableFind.new.find(callable) - elsif callable.lambda? - LineLambdaFind.new.find(callable) else - LineProcFind.new.find(callable) - end - when Method, UnboundMethod - if rubyvm - RubyVMCallableFind.new.find(callable) - else - LineMethodFind.new.find(callable) + LineCallableFind.new.find(callable) end when Thread::Backtrace::Location - if rubyvm + if defined?(::RubyVM) RubyVMBacktraceLocationFind.new.find(callable) else LineBacktraceLocationFind.new.find(callable) @@ -91,77 +83,54 @@ def find(location) end end - # Finds the AST node for a Method or UnboundMethod using best-effort line + # Finds the AST node for a Method, UnboundMethod, or Proc using best-effort line # matching. Used on non-CRuby implementations. - class LineMethodFind < Find + class LineCallableFind < Find # Find the node for the given method by matching on name and line. #-- - #: (Method | UnboundMethod callable) -> Node? - def find(callable) - return unless (source_location = callable.source_location) - return unless (result = parse_file(source_location[0])) - - name = callable.name - start_line = source_location[1] - - result.value.find do |node| - case node - when DefNode - node.name == name && node.location.start_line == start_line - when CallNode - node.block.is_a?(BlockNode) && node.location.start_line == start_line - else - false - end - end - end - end - - # Finds the AST node for a lambda using best-effort line matching. Used - # on non-CRuby implementations. - class LineLambdaFind < Find - # Find the node for the given lambda by matching on line. - #-- - #: (Proc callable) -> Node? + #: (Method | UnboundMethod | Proc callable) -> Node? def find(callable) return unless (source_location = callable.source_location) return unless (result = parse_file(source_location[0])) - start_line = source_location[1] - result.value.find do |node| - case node - when LambdaNode - node.location.start_line == start_line - when CallNode - node.block.is_a?(BlockNode) && node.location.start_line == start_line - else - false + case callable + when Method, UnboundMethod + name = callable.name + + result.value.find do |node| + case node + when DefNode + node.name == name && node.location.start_line == start_line + when CallNode + node.block.is_a?(BlockNode) && node.location.start_line == start_line + else + false + end end - end - end - end - - # Finds the AST node for a non-lambda Proc using best-effort line - # matching. Used on non-CRuby implementations. - class LineProcFind < Find - # Find the node for the given proc by matching on line. - #-- - #: (Proc callable) -> Node? - def find(callable) - return unless (source_location = callable.source_location) - return unless (result = parse_file(source_location[0])) - - start_line = source_location[1] - - result.value.find do |node| - case node - when ForNode - node.location.start_line == start_line - when CallNode - node.block.is_a?(BlockNode) && node.location.start_line == start_line + when Proc + if callable.lambda? + result.value.find do |node| + case node + when LambdaNode + node.location.start_line == start_line + when CallNode + node.block.is_a?(BlockNode) && node.location.start_line == start_line + else + false + end + end else - false + result.value.find do |node| + case node + when ForNode + node.location.start_line == start_line + when CallNode + node.block.is_a?(BlockNode) && node.location.start_line == start_line + else + false + end + end end end end diff --git a/rbi/generated/prism.rbi b/rbi/generated/prism.rbi index 963c2fb851..af27809728 100644 --- a/rbi/generated/prism.rbi +++ b/rbi/generated/prism.rbi @@ -27,8 +27,8 @@ module Prism # returns the Prism node representing it. On CRuby, this uses node_id for # an exact match. On other implementations, it falls back to best-effort # matching by source location line number. - sig { params(callable: ::T.any(Method, UnboundMethod, Proc, Thread::Backtrace::Location), rubyvm: T::Boolean).returns(::T.nilable(Node)) } - def self.find(callable, rubyvm: T.unsafe(nil)); end + sig { params(callable: ::T.any(Method, UnboundMethod, Proc, Thread::Backtrace::Location)).returns(::T.nilable(Node)) } + def self.find(callable); end VERSION = T.let(nil, String) BACKEND = T.let(nil, Symbol) diff --git a/rbi/generated/prism/node_find.rbi b/rbi/generated/prism/node_find.rbi index 17afe9cc40..3d59cd5be3 100644 --- a/rbi/generated/prism/node_find.rbi +++ b/rbi/generated/prism/node_find.rbi @@ -10,8 +10,8 @@ module Prism # pay for its definition. module NodeFind # Find the node for the given callable or backtrace location. - sig { params(callable: ::T.any(Method, UnboundMethod, Proc, Thread::Backtrace::Location), rubyvm: T::Boolean).returns(::T.nilable(Node)) } - def self.find(callable, rubyvm); end + sig { params(callable: ::T.any(Method, UnboundMethod, Proc, Thread::Backtrace::Location)).returns(::T.nilable(Node)) } + def self.find(callable); end # Base class that handles parsing a file. class Find @@ -36,30 +36,14 @@ module Prism def find(location); end end - # Finds the AST node for a Method or UnboundMethod using best-effort line + # Finds the AST node for a Method, UnboundMethod, or Proc using best-effort line # matching. Used on non-CRuby implementations. - class LineMethodFind < Find + class LineCallableFind < Find # Find the node for the given method by matching on name and line. sig { params(callable: ::T.any(Method, UnboundMethod)).returns(::T.nilable(Node)) } def find(callable); end end - # Finds the AST node for a lambda using best-effort line matching. Used - # on non-CRuby implementations. - class LineLambdaFind < Find - # Find the node for the given lambda by matching on line. - sig { params(callable: Proc).returns(::T.nilable(Node)) } - def find(callable); end - end - - # Finds the AST node for a non-lambda Proc using best-effort line - # matching. Used on non-CRuby implementations. - class LineProcFind < Find - # Find the node for the given proc by matching on line. - sig { params(callable: Proc).returns(::T.nilable(Node)) } - def find(callable); end - end - # Finds the AST node for a Thread::Backtrace::Location using best-effort # line matching. Used on non-CRuby implementations. class LineBacktraceLocationFind < Find diff --git a/sig/generated/prism.rbs b/sig/generated/prism.rbs index e9f4ca2b85..8db7b3bc60 100644 --- a/sig/generated/prism.rbs +++ b/sig/generated/prism.rbs @@ -37,8 +37,8 @@ module Prism # an exact match. On other implementations, it falls back to best-effort # matching by source location line number. # -- - # : (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable, ?rubyvm: bool) -> Node? - def self.find: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable, ?rubyvm: bool) -> Node? + # : (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable) -> Node? + def self.find: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable) -> Node? VERSION: String diff --git a/sig/generated/prism/node_find.rbs b/sig/generated/prism/node_find.rbs index 9924ff1452..cbd9a77329 100644 --- a/sig/generated/prism/node_find.rbs +++ b/sig/generated/prism/node_find.rbs @@ -11,8 +11,8 @@ module Prism module NodeFind # Find the node for the given callable or backtrace location. # -- - # : (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable, bool rubyvm) -> Node? - def self.find: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable, bool rubyvm) -> Node? + # : (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable) -> Node? + def self.find: (Method | UnboundMethod | Proc | Thread::Backtrace::Location callable) -> Node? # Base class that handles parsing a file. class Find @@ -42,31 +42,13 @@ module Prism def find: (Thread::Backtrace::Location location) -> Node? end - # Finds the AST node for a Method or UnboundMethod using best-effort line + # Finds the AST node for a Method, UnboundMethod, or Proc using best-effort line # matching. Used on non-CRuby implementations. - class LineMethodFind < Find + class LineCallableFind < Find # Find the node for the given method by matching on name and line. # -- - # : (Method | UnboundMethod callable) -> Node? - def find: (Method | UnboundMethod callable) -> Node? - end - - # Finds the AST node for a lambda using best-effort line matching. Used - # on non-CRuby implementations. - class LineLambdaFind < Find - # Find the node for the given lambda by matching on line. - # -- - # : (Proc callable) -> Node? - def find: (Proc callable) -> Node? - end - - # Finds the AST node for a non-lambda Proc using best-effort line - # matching. Used on non-CRuby implementations. - class LineProcFind < Find - # Find the node for the given proc by matching on line. - # -- - # : (Proc callable) -> Node? - def find: (Proc callable) -> Node? + # : (Method | UnboundMethod | Proc callable) -> Node? + def find: (Method | UnboundMethod | Proc callable) -> Node? end # Finds the AST node for a Thread::Backtrace::Location using best-effort diff --git a/test/prism/ruby/find_test.rb b/test/prism/ruby/find_test.rb index 5b59113d30..53ef07c81a 100644 --- a/test/prism/ruby/find_test.rb +++ b/test/prism/ruby/find_test.rb @@ -143,42 +143,42 @@ def test_multiple_methods_on_same_line assert_def_node Prism.find(Fixtures::MultipleOnLine.method(:second)), :second end - # === Fallback (line-based) tests via rubyvm: false === + # === Fallback (line-based) tests === def test_fallback_simple_method - assert_def_node Prism.find(Fixtures::Methods.instance_method(:simple_method), rubyvm: false), :simple_method + assert_def_node NodeFind::LineCallableFind.new.find(Fixtures::Methods.instance_method(:simple_method)), :simple_method end def test_fallback_singleton_method - assert_def_node Prism.find(Fixtures::Methods.method(:singleton_method_fixture), rubyvm: false), :singleton_method_fixture + assert_def_node NodeFind::LineCallableFind.new.find(Fixtures::Methods.method(:singleton_method_fixture)), :singleton_method_fixture end def test_fallback_lambda - node = Prism.find(Fixtures::Procs::SIMPLE_LAMBDA, rubyvm: false) + node = NodeFind::LineCallableFind.new.find(Fixtures::Procs::SIMPLE_LAMBDA) assert_instance_of LambdaNode, node end def test_fallback_proc - node = Prism.find(Fixtures::Procs::SIMPLE_PROC, rubyvm: false) + node = NodeFind::LineCallableFind.new.find(Fixtures::Procs::SIMPLE_PROC) assert_instance_of CallNode, node assert node.block.is_a?(BlockNode) end def test_fallback_define_method - node = Prism.find(Fixtures::DefineMethod.instance_method(:dynamic), rubyvm: false) + node = NodeFind::LineCallableFind.new.find(Fixtures::DefineMethod.instance_method(:dynamic)) assert_instance_of CallNode, node assert node.block.is_a?(BlockNode) end def test_fallback_for_loop - node = Prism.find(Fixtures::ForLoop::FOR_PROC, rubyvm: false) + node = NodeFind::LineCallableFind.new.find(Fixtures::ForLoop::FOR_PROC) assert_instance_of ForNode, node end def test_fallback_backtrace_location location = zero_division_location assert_not_nil location - node = Prism.find(location, rubyvm: false) + node = NodeFind::LineBacktraceLocationFind.new.find(location) assert_not_nil node assert_equal location.lineno, node.location.start_line end