Issue #21998 has been updated by Eregon (Benoit Daloze). I was curious what's the reported location for `ArgumentError`s caused by lambda parameters and how ErrorHighlight highlights them. For ```ruby l = lambda { |x, y| x + y } l.call(1) ``` With `parse.y`, the node (`RubyVM::AbstractSyntaxTree.of(e.backtrace_locations[0])`) is actually: ``` (ITER@1:4-3:1 (FCALL@1:4-1:10 :lambda nil) (SCOPE@1:11-3:1 tbl: [:x, :y] args: (ARGS@1:14-1:18 ... ``` which covers from `lambda { |x, y|` to `}`. With Prism, the node (Prism node with node_id == `RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(e.backtrace_locations[0])`) is actually: ``` BlockNode (location: (1,11)-(3,1)) ├── locals: [:x, :y] ├── parameters: │ @ BlockParametersNode (location: (1,13)-(1,19)) │ ├── parameters: │ │ @ ParametersNode (location: (1,14)-(1,18)) │ │ ├── requireds: (length: 2) │ │ │ ├── @ RequiredParameterNode (location: (1,14)-(1,15)) │ │ │ │ └── name: :x │ │ │ └── @ RequiredParameterNode (location: (1,17)-(1,18)) │ │ │ └── name: :y │ │ ├── ... │ ├── opening_loc: (1,13)-(1,14) = "|" │ └── closing_loc: (1,18)-(1,19) = "|" ... ``` which covers from `{ |x, y|` to `}`. ErrorHighlight shows: ``` test.rb:1:in 'block in <main>': wrong number of arguments (given 1, expected 2) (ArgumentError) caller: test.rb:5 | l.call(1) ^^^^^ callee: test.rb:1 | l = lambda { |x, y| ^ ``` ErrorHighlight makes the choice (uses `node.children[1]` with `RubyVM::AST`) to highlight the `{` and not `lambda {` and I agree that makes sense since it's the block (parameters) causing the exception, not the call to `lambda`. Maybe it could even highlight `|x, y|` or `x, y` to pinpoint that's it about the wrong arguments vs parameters by using the `ARGS`/`BlockParametersNode`/`ParametersNode`. --- Similarly, for `define_method` with a block: ```ruby define_method :define_method_test do |x, y| x + y end define_method_test(1) ``` The `parse.y` node is: ``` #<RubyVM::AbstractSyntaxTree::Node:ITER@1:0-3:3> ``` which covers from `define_method :define_method_test do |x, y|` to `end`. The Prism node is: ``` BlockNode (location: (1,34)-(3,3)) ``` which covers from `do |x, y|` to `end`. ErrorHighlight shows: ``` test.rb:1:in 'block in <main>': wrong number of arguments (given 1, expected 2) (ArgumentError) caller: test.rb:5 | define_method_test(1) ^^^^^^^^^^^^^^^^^^ callee: test.rb:1 | define_method :define_method_test do |x, y| ^^ ``` --- In conclusion if we look at the returned node start/end line/column we see RubyVM::AST returns the full call and Prism returns just the block. Both work. From the experiment in https://github.com/eregon/error_highlight/pull/1 it would be best if both match. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117141 * Author: Eregon (Benoit Daloze) * Status: Open ---------------------------------------- I'm using matz's suggestion almost as-is from https://bugs.ruby-lang.org/issues/6012#note-53. The only change is the proposed class name. ## Use Cases Use cases have been discussed extensively and matz said:
The use cases are real and I want to support them
So I think we don't need to discuss that anymore :) ## Background Adding column and last line information to `#source_location` is deemed too incompatible given the usages of `obj.source_location.last` which expect the start line (they would get the end column instead). ## Proposal So instead we add a new method, `{Method,UnboundMethod,Proc}#source_range`, which returns a `Ruby::SourceRange` and has these methods: * `start_line`: 1-indexed (same as `source_location.first`) * `end_line`: 1-indexed * `start_column`: in bytes, 0-indexed, I think `start_byte_column` could be good for extra clarity * `end_column`: in bytes, 0-indexed, I think `end_byte_column` could be good for extra clarity * `inspect` which shows something like `#<Ruby::SourceRange (1,0)-(2,10)>`. For the edge case of a heredoc spanning further than the end of a method/block, we would not include it in the `end_line` & `end_column` methods, but instead document it and provide a small code snippet in the docs to compute that if desired with Prism. ## Alternative An alternative could be a `Ruby::SourceLocation` class and `obj.source_location(object: true)`/`obj.source_location(extended: true)` but it feels less nice. Since we are designing something new I think it's best to go for the cleanest design. ## Consistency The above methods match the name of methods on `Prism::Node` and have the same semantics, which is good for consistency and to avoid confusion. ## Scope The scope is intentionally minimal to keep the discussion focused. ## Implementation I'm happy to implement this, it should be pretty trivial and very similar to the extended `source_location`. I would prefer to get an approval for this feature before implementing. -- https://bugs.ruby-lang.org/