[ruby-core:125265] [Ruby Feature#21998] Add {Method,UnboundMethod,Proc}#source_range
Issue #21998 has been reported by Eregon (Benoit Daloze). ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998 * 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/
Issue #21998 has been updated by matz (Yukihiro Matsumoto). Thank you for the proposal. Introducing a new method rather than extending `source_location` is the right direction, given the compatibility issues we hit. I approve the shape of this feature, including: - The class name `Ruby::SourceRange` (matches the method name). - The four accessors `start_line`, `end_line`, `start_column`, `end_column`. - Byte-based columns, as long as this is clearly documented. - Excluding heredocs from `end_line`/`end_column` with documentation of the workaround. - Not adding `Binding#source_range`. Matz. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117058 * 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/
Issue #21998 has been updated by mame (Yusuke Endoh). @matz What should the following return? ```ruby f = proc { <<END } xxx END f.source_range #=> Start position: from the `p` of `proc`? Or from the `{`? #=> End position: up to the `}`? Or up to the `D` of `END`? ``` I don't understand the use cases for this feature, so I'm not sure what it should return. A related feature that Eregon also mentioned in #6012, `show_source` in irb, behaves as follows, so perhaps the heredoc should be included? ``` $ irb -rmethod_source irb(main):001" def foo = (<<END) irb(main):002" xxx irb(main):003> END => :foo irb(main):004> show_source foo From: (irb):1 def foo = (<<END) xxx END ``` ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117059 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). For the start position I think either is fine. I think from the `p` of `proc` is more useful because it gives extra context to what the block is given (a block can't exist on its own, it's always part of something bigger syntax-wise). For the end position, as the description says:
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.
There are use cases for both. We can easily compute the end of heredoc if the API returns `}`, but it's impossible (or at least very complicated & inefficient) to do the reverse. So the API returns what makes it possible to have both. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117064 * 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/
Issue #21998 has been updated by headius (Charles Nutter). Eregon (Benoit Daloze) wrote in #note-7:
For the start position I think either is fine. I think from the `p` of `proc` is more useful because it gives extra context to what the block is given (a block can't exist on its own, it's always part of something bigger syntax-wise).
The `proc` part is irrelevant to the source of the block, and is only a method call receiver for a block argument. We wouldn't include any other calls that receive a block as part of the source of the block, so proc should be no different. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117073 * 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/
Issue #21998 has been updated by mame (Yusuke Endoh). Eregon (Benoit Daloze) wrote in #note-7:
We can easily compute the end of heredoc if the API returns `}`
How do you determine whether a block contains a heredoc? Consider: - `{ <<X }`: heredoc - `{ x<<X }`: shift operator - `{ x <<X }`: heredoc Of course, you also have to handle `<<"X"`, and `"<<X"` is a string literal, and you have to consider cases like `{ <<"X" + <<"Y" }`. Even once you have identified a heredoc, finding its end delimiter is not trivial either: ``` <<"X" #{ X } X ``` Here the end delimiter is the `X` on line 5, because `#{` opens an interpolation. ``` <<'X' #{ X } X ``` Here the end delimiter is the `X` on line 3, because single-quoted heredocs do not interpolate, so `#{` is literal text. I cannot see how any of this is "easily computed". ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117074 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). I discussed this in detail with @mame and @matz. @mame (to reply here too) what I meant is using Prism with the `Method|UnboundMethod|Proc` and the `source_range` to find the Prism node, and then compute the max end offset by walking the child nodes (recursively). Matz said for the example in #6: `End position: up to the }`. I'm not sure if @matz made his mind on: Start position: from the `p` of `proc`? Or from the `{`? ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117087 * 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/
Issue #21998 has been updated by kddnewton (Kevin Newton). We should make sure start line respects the line passed into eval (not sure if that was implicit, but wanted to be sure to call it out). For the heredocs, I agree that they should be omitted. We made that decision early with Prism because of the following code just being super confusing: ```ruby define_method(:foo) { <<~FOO }; define_method(:bar) { <<~BAR } foo content FOO bar content BAR ``` The reality is in order to represent this in a way that even comes close we would need source range to not necessarily be contiguous. I think instead we should document the limitation, and explicitly call out that eval-ing or trusting that the source returned by the source range is not and never expected to be 1:1 with the original. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117117 * 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/
Issue #21998 has been updated by headius (Charles Nutter).
The example of define_method(:foo) { ... } was given
define_method is just a method call like any other. It should not be included in the range for the syntactic construct that is the block. What about do..end with a huge piece of code attached? ``` foo( # large amount of code ) do ... end ``` It makes no sense for the range of the syntactic block here to include the entire expression of the call and its arguments. None of that is syntactically part of the block and it would parse if the block were removed.
Also foo { break } the break there breaks out of foo, so IMO a block isn't complete without the method call to which receives it.
The target of the break is a semantic detail, not a syntactic one, and it is not determined at parse time. For that matter, arguments to a method would not be arguments without the call, but we you don't consider their syntactic range to include the entire call. The block is just an argument of another form. I'll turn this question around: how can I get just the range of the block syntactic element if it includes all this other stuff unrelated to the block? I think what you really want is for the call's range to include the block, not the other way around. A nested AST element's range should not include source ranges of parent nodes. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117121 * 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/
Issue #21998 has been updated by kddnewton (Kevin Newton).
It makes no sense for the range of the syntactic block here to include the entire expression of the call and its arguments.
If I'm understanding you correctly, then what you would like for source_range for the example for `foo` and `bar` methods is then: ```ruby define_method(:foo) { <<~FOO }; define_method(:bar) { <<~BAR } foo content FOO bar content BAR method(:foo).source_range # => # { <<~FOO }; define_method(:bar) { <<~BAR } # foo content # FOO method(:bar).source_range # => # { <<~BAR } # foo content # FOO # bar content # BAR ``` as opposed to not including the heredoc, where it would instead be: ```ruby define_method(:foo) { <<~FOO }; define_method(:bar) { <<~BAR } foo content FOO bar content BAR method(:foo).source_range # => # { <<~FOO } method(:bar).source_range # => # { <<~BAR } ``` is that correct? ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117123 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). headius (Charles Nutter) wrote in #note-12:
The target of the break is a semantic detail, not a syntactic one, and it is not determined at parse time.
It does, it is determined at parse time. Break is a syntactic construct.
I'll turn this question around: how can I get just the range of the block syntactic element if it includes all this other stuff unrelated to the block?
We can easily use `Prism.find(Method|UnboundMethod|Proc).block` for this. Similar if one wants to get the end position including the heredoc, `Prism.find` is the way too. BTW another interesting case is: ```ruby for i in enum p i abc end ``` There we must include at least from the `i` just after `for ` as that's the block's parameters. What both Prism and RubyVM::AST do in that case is include from the beginning of `for`, which makes sense. If one just wants the body without parameters, they can use `Prism.find(...).statements`. The only choice for `Prism.find` there is to return the `ForNode`, and so it is consistent to return a `CallNode` for `proc/lambda/foo { ... }` and a `LambdaNode` for `-> { ... }`. Another reason is returning the CallNode there is useful for many cases, and having to walk the AST again to find it seems needlessly inefficient. It's the same philosophy as the heredoc end: make it easy to locate the node with Prism, and access sub-parts as wanted from there. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117126 * 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/
Issue #21998 has been updated by headius (Charles Nutter).
The only choice for Prism.find there is to return the ForNode
`for` syntax does not parse as a ForNode without the block, so the two are not separable. Calls do parse without the block and remain CallNode. A block is an argument to a call. Therefore the block and the call are separate constructs that should have separate ranges. Why include the CallNode range in the block's range but not in the range of other arguments? It is even less bound to the syntax of the call than the arguments are. It seems arbitrary to include the call's range in the block when it is not included in the range of any other argument or in the range of the call's receiver expression. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117133 * 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/
Issue #21998 has been updated by headius (Charles Nutter).
If I'm understanding you correctly, then what you would like for source_range for the example for foo and bar methods is then:
I don't claim to have any suggestion for HERE docs because of the disjoint problem you mentioned. But clearly the range should not include the first character of the call to which the HERE doc is being passed or the block of code in which it is contained. My point is that including the call range in the source range of a block passed to it doesn't make any more sense syntactically than including the call range in the source range of its other arguments. In both cases, the argument or block could be removed from the call and it would still parse as a call. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117134 * 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/
Issue #21998 has been updated by tompng (tomoya ishida). Proc's source in the code below consists of 3 parts. `do`, `inside\n<<B; end`, and `exclave\nB`. ```ruby <<A; tap do outside injected A inside <<B; end; outside exclave B ``` Even if heredocs are used, I think `do` is the start position and `end` is the end position. Source ranges may have several segments and may have exclave part that exceeds end position. Some code outside of proc may be injected into `start_pos...end_pos` with heredocs. Injected part may contain any characters(parentheses, keywords, operators, etc) that prevents analyzing source string as a code. By the way, IRB's show-source is not working well with the code above because IRB just simply finds syntax-valid `lines[start_line..start_line+i]` lines. It's impossible to correctly implement show-source only from `start_line` for code like this: ```ruby def f; end; def f; def f end end ``` ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117135 * 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/
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/
Issue #21998 has been updated by Eregon (Benoit Daloze). For ```ruby l = -> x, y { x + y } l.call(1) ``` 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 = -> x, y { ^^ from test.rb:5:in '<main>' ``` So in this case it highlights the `->`, so maybe the strategy is to highlight the token before block parameters. For consistency with the `lambda { |x, y|` case maybe it'd make sense to highlight `lambda {`, but I think either way it's clear enough. I think both `{`/`do` or the start of the call which receives the block (`proc {`) are valid start position and each have pros & cons. We just need to make a choice and we should be consistent for the start position of a node from an arity ArgumentError with both `--parser=prism` and `--parser=parse.y`. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117142 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). Eregon (Benoit Daloze) wrote in #note-10:
I'm not sure if [@matz (Yukihiro Matsumoto)](/users/13) made his mind on: Start position: from the `p` of `proc`? Or from the `{`?
I confirmed with both @mame and @matz, and matz said it should be from `p` of `proc`. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117153 * 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/
Issue #21998 has been updated by headius (Charles Nutter). Eregon (Benoit Daloze) wrote in #note-21:
I confirmed with both @mame and @matz, and matz said it should be from `p` of `proc`.
So that means it will include the original call and all arguments in all cases, including multi-line calls with large expressions, correct? ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117154 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). Yes ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117155 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). PR is ready: https://github.com/ruby/ruby/pull/16835 When implementing I realized it's not a good idea to have the start of the `Ruby::SourceRange` to be the `p` of `proc { foo }`. It should be the `{`/`do` for a bunch of reasons: * That's what is currently used by `{ methods: true }` coverage in `test_method_coverage_for_define_method`. * That's what the extended `source_location` used. * That's what the compilers (compile.c, prism_compile.c) expect, and changing that is awkward (`rb_iseq_compile_node()` expects a `NODE_SCOPE`, which is the block in this context, and the `ISEQ_BODY(block_iseq)->location.code_location` should rather clearly refer to the block only, not including the call, because `rb_iseq_compile_node()` compiles the block, not the call, so it seems weird to give it a wider `code_location`). * When talking about the "source location/range" of "a block" I believe most people will expect from the `{`/`do`, not from the call to which the block is given. * The many reasons @headius mentioned. There are a few benefits to include the call, but I don't think they can compare to these, so let's not include the call. --- More details: You can see the complications in compile.c, prism_compile.c as reverted in [this commit](https://github.com/ruby/ruby/pull/16835/changes/9ec948fa7bacbf0f409dcd83c83e...). So to include the call we actually have to overwrite the `code_location` of the block iseq, which doesn't feel nice and conceptually introduces separate compile & location nodes, which seems clearly not worth it. If we decide to include the call we'd also need to change Coverage information, which I don't think is a good idea. It would mean for `methods: true` Coverage we'd get `define_method(:a) do; end` for method `a` instead of `do; end`. If it was something like `define_method((sleep(1); :a)) do; end` it'd be misleading to include the call, because the `sleep(1)` is not executed in that method but outside. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117407 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). @matz Could you approve these specific tweaks to the original proposal? * `Proc#source_range` starts from the `{`/`do` for `proc { foo }`/`proc do foo; end`, for the reasons mentioned in the previous comment * I added `Ruby::SourceRange#absolute_path`, which is like `Thread::Backtrace::Location#absolute_path`. This is otherwise not available, and it's necessary to find the file from the main script on the filesystem (if the script uses `chdir`). * I added `Ruby::SourceRange#path`, this is the same as `proc_or_method.source_location[0]` but that way the `Ruby::SourceRange` has all the information contained, and it pairs with `absolute_path`. * `Ruby::SourceRange#inspect` is `"#<Ruby::SourceRange #{absolute_path || path}:(#{start_line},#{start_column})-(#{end_line},#{end_column})>"`. Example: `ruby -e 'p proc { 1 }.source_range'` => `#<Ruby::SourceRange -e:(1,7)-(1,12)>` ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117408 * 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/
Issue #21998 has been updated by Eregon (Benoit Daloze). From https://github.com/ruby/dev-meeting-log/blob/master/2026/DevMeeting-2026-06-...
matz: I want to confirm his intention. Let me respond.
@matz What can I clarify? My main motivation here is to be able to implement `Prism.find(Method|UnboundMethod|Proc)` precisely and cleanly on any Ruby implementation, in a way which does not depend on implementation details like `node_id`. For that we need the start/end line/column and the `absolute_path`, which is exactly what `Ruby::SourceRange` provides. Whether for a Proc we start at `{`/`do` or the method call does not matter much for `Prism.find`, we can handle either case. However it's clear that method Coverage, the previous extended `source_location` and more (see #note-24) expect to start at `{`/`do` so I see too little reason to change that. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117737 * 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/
Issue #21998 has been updated by matz (Yukihiro Matsumoto). Thank you, that clarifies my concerns. If the primary purpose is `Prism.find(Method|UnboundMethod|Proc)`, then the range is a key to locate the node in a full parse, and it does not need to be an evaluable fragment by itself. That also settles my hesitation about starting at `{`/`do`; given Coverage and the compiler structure already expect it, I agree. I approve the proposal with the tweaks in note-25. Please add the `Prism.find` motivation to the ticket description and mention it in the documentation of `Ruby::SourceRange`, so future readers can see why this API exists without digging through #6012. Matz. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-118007 * 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/
participants (6)
-
Eregon (Benoit Daloze) -
headius (Charles Nutter)
-
kddnewton (Kevin Newton)
-
mame (Yusuke Endoh)
-
matz (Yukihiro Matsumoto)
-
tompng (tomoya ishida)