[ruby-core:114074] [Ruby master Feature#19755] Module#class_eval and Binding#eval use caller location by default
Issue #19755 has been reported by byroot (Jean Boussier). ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by Eregon (Benoit Daloze). `#<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10>` sounds great to me, +1. The `(eval` makes it clear it's an eval in that file and so the line of an exception inside might not be exact. ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103747 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by Dan0042 (Daniel DeLorme). +1 Just be careful about the implementation, because that monkey patch doesn't work if another module is in the call chain ```ruby module DebugEval def class_eval(code, ...) p debug: code super # <= source_location will report the eval comes from here end prepend_features Module end ``` Actually this is a problem I tend to have whenever I want to use caller_locations. `Thread.each_caller_location` has made it easier but it would be nice to have something like `#first_caller_location_which_is_not_self_or_super` ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103748 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by byroot (Jean Boussier).
doesn't work if another module is in the call chain
I'm not sure we can / should handle this. Decorating `class_eval / eval` should be quite rare anyways. ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103749 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by Eregon (Benoit Daloze). byroot (Jean Boussier) wrote in #note-3:
Decorating `class_eval / eval` should be quite rare anyways.
Indeed, and `class_eval`/`eval` is broken if decorated e.g. for `a = 3; class_eval "a"`. The direct caller of `Module#class_eval` should always be the code around it. Full example: ```ruby # works as-is, breaks with DebugEval module M a = 3 class_eval "p a" end ``` ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103750 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by Dan0042 (Daniel DeLorme).
Indeed, and `class_eval`/`eval` is broken if decorated e.g. for `a = 3; class_eval "a"`.
Oh yeah, good point, that one slipped my mind. ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103751 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by nobu (Nobuyoshi Nakada). ```ruby Foo.class_eval <<~RUBY def bar end RUBY ``` This code equals `Foo.class_eval " def bar\n end\n"`. Which do you expect 1 or 2 as `__LINE__` at the `def` line? ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103806 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by byroot (Jean Boussier).
Which do you expect 1 or 2 as __LINE__ at the def line?
I don't feel strongly about either, as it is assumed that the line number can't always be correct anyway. I think `1` seem more logical to me, as it would be weird to assume a heredoc was used. ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103807 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by Dan0042 (Daniel DeLorme).
I think `1` seem more logical to me, as it would be weird to assume a heredoc was used.
Ah, but with #19458 it would be *possible* to know that a heredoc was used, and use 2 for the def line. :-D ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103808 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by matz (Yukihiro Matsumoto). Accepted. I prefer the last format `#<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1>`. Matz. ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103838 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by nobu (Nobuyoshi Nakada). matz (Yukihiro Matsumoto) wrote in #note-9:
Accepted. I prefer the last format `#<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1>`.
In that case, what should `__FILE__` and `__dir__` be? `"(eval at /tmp/foo.rb:10)"` as `__FILE__` may be ok, but `"(eval at /tmp"` as `__dir__`? ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103844 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by byroot (Jean Boussier). Oh, `__dir__` is a good point, I haven't thought of it. Currently, it's `nil`: ```ruby
eval("__dir__") => nil
I suppose we should just keep that?
----------------------------------------
Feature #19755: Module#class_eval and Binding#eval use caller location by default
https://bugs.ruby-lang.org/issues/19755#change-103846
* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
----------------------------------------
### Background
In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code.
However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location:
```ruby
Foo.class_eval <<~RUBY
def bar
end
RUBY
p Foo.instance_method(:bar).source_location # => ["(eval)", 1]
The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by byroot (Jean Boussier). I just finished a PR for it, but I agree we need to handle `__dir__`: https://github.com/ruby/ruby/pull/8070 ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103847 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by byroot (Jean Boussier).
we need to handle `__dir__`
So the way it was handled until now was simply: ```c if (path == eval_default_path) { return Qnil; } ``` So how I'm handling it right now is basically `path.start_with?("(eval at ") && path.end_with?(")")`. It doesn't feel great, but I can't think of another solution, and I think it's good enough. The last blocker is that this change breaks `syntax_suggest` test suite, so I need https://github.com/ruby/syntax_suggest/pull/200 merged first. Other than that I think the PR is good to go. ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103917 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
Issue #19755 has been updated by schneems (Richard Schneeman). I've merged the PR. Thanks for your work here! ---------------------------------------- Feature #19755: Module#class_eval and Binding#eval use caller location by default https://bugs.ruby-lang.org/issues/19755#change-103933 * Author: byroot (Jean Boussier) * Status: Open * Priority: Normal ---------------------------------------- ### Background In Ruby we're very reliant on `Method#source_location` as well as `caller_locations` to locate source code. However, code generated via `Binding#eval`, `Module#class_eval` etc defeat this if called without a location: ```ruby Foo.class_eval <<~RUBY def bar end RUBY p Foo.instance_method(:bar).source_location # => ["(eval)", 1] ``` The overwhelming majority of open source code properly pass a `filename` and `lineno`, however a small minority doesn't and make locating the code much harder than it needs to be. Here's some example of anonymous eval uses I fixed in the past: - https://github.com/ruby/mutex_m/pull/11 - https://github.com/rails/execjs/pull/120 - https://github.com/jnunemaker/httparty/pull/776 - https://github.com/SiftScience/sift-ruby/pull/76 - https://github.com/activemerchant/active_merchant/pull/4675 - https://github.com/rails/thor/pull/807 - https://github.com/dry-rb/dry-initializer/pull/104 - https://github.com/rmosolgo/graphql-ruby/pull/4288 ### Proposal I suggest that instead of defaulting to `"(eval)"`, the optional `filename` argument of the eval family of methods should instead default to: `"(eval in #{caller_locations(1, 1).first.path})"` and `lineno` to `caller_locations(1, 1).first.lineno`. Which can pretty much be monkey patched as this: ```ruby module ModuleEval def class_eval(code, location = "(eval in #{caller_locations(1, 1).first.path})", lineno = caller_locations(1, 1).first.lineno) super end end Module.prepend(ModuleEval) module Foo class_eval <<~RUBY def foo end RUBY end p Foo.instance_method(:foo) ``` before: ``` #<UnboundMethod: Foo#foo() (eval):1> ``` after: ``` #<UnboundMethod: Foo#foo() (eval in /tmp/foo.rb):10> ``` Of course the `lineno` part is likely to not be fully correct, but the reasoning is that it's better than defaulting to 0 or 1. Another possiblity would be to include the caller `lineo` inside the `filename` part, and leave the actual lineo default to `1`: ``` #<UnboundMethod: Foo#foo() (eval at /tmp/foo.rb:10):1> ``` -- https://bugs.ruby-lang.org/
participants (6)
-
byroot (Jean Boussier) -
Dan0042 (Daniel DeLorme)
-
Eregon (Benoit Daloze)
-
matz (Yukihiro Matsumoto)
-
nobu (Nobuyoshi Nakada)
-
schneems (Richard Schneeman)