From d82fc3360d7cfa7e1e1a4dddb668b4c38808538a Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 16 Dec 2025 09:48:41 +0100 Subject: [PATCH] Reapply "[Feature #6012] Extend `source_location` for end position * This reverts commit 065c48cdf11a1c4cece84db44ed8624d294f8fd5. * This functionality is very valuable and has already taken 14 years to agree on the API. * Let's just document it's byte columns (in the next commit). * See https://bugs.ruby-lang.org/issues/21783#note-9 --- NEWS.md | 10 ++++ proc.c | 14 +++-- spec/ruby/core/method/source_location_spec.rb | 16 ++++-- spec/ruby/core/proc/source_location_spec.rb | 51 ++++++++++++------- .../unboundmethod/source_location_spec.rb | 18 ++++--- test/ruby/test_lambda.rb | 12 ++--- test/ruby/test_proc.rb | 18 ++++--- 7 files changed, 92 insertions(+), 47 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8ac808c2e7..e37e4ebf5b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,6 +11,15 @@ Note that each entry is kept to a minimum, see links for details. Note: We're only listing outstanding class updates. +* Method + + * `Method#source_location`, `Proc#source_location`, and + `UnboundMethod#source_location` now return extended location + information with 5 elements: `[path, start_line, start_column, + end_line, end_column]`. The previous 2-element format `[path, + line]` can still be obtained by calling `.take(2)` on the result. + [[Feature #6012]] + * Set * A deprecated behavior, `Set#to_set`, `Range#to_set`, and @@ -66,4 +75,5 @@ A lot of work has gone into making Ractors more stable, performant, and usable. ## JIT +[Feature #6012]: https://bugs.ruby-lang.org/issues/6012 [Feature #21390]: https://bugs.ruby-lang.org/issues/21390 diff --git a/proc.c b/proc.c index a0fc3a7e08..715f346853 100644 --- a/proc.c +++ b/proc.c @@ -1513,14 +1513,20 @@ proc_eq(VALUE self, VALUE other) static VALUE iseq_location(const rb_iseq_t *iseq) { - VALUE loc[2]; + VALUE loc[5]; + int i = 0; if (!iseq) return Qnil; rb_iseq_check(iseq); - loc[0] = rb_iseq_path(iseq); - loc[1] = RB_INT2NUM(ISEQ_BODY(iseq)->location.first_lineno); + loc[i++] = rb_iseq_path(iseq); + const rb_code_location_t *cl = &ISEQ_BODY(iseq)->location.code_location; + loc[i++] = RB_INT2NUM(cl->beg_pos.lineno); + loc[i++] = RB_INT2NUM(cl->beg_pos.column); + loc[i++] = RB_INT2NUM(cl->end_pos.lineno); + loc[i++] = RB_INT2NUM(cl->end_pos.column); + RUBY_ASSERT_ALWAYS(i == numberof(loc)); - return rb_ary_new4(2, loc); + return rb_ary_new_from_values(i, loc); } VALUE diff --git a/spec/ruby/core/method/source_location_spec.rb b/spec/ruby/core/method/source_location_spec.rb index c5b296f6e2..1b175ebaba 100644 --- a/spec/ruby/core/method/source_location_spec.rb +++ b/spec/ruby/core/method/source_location_spec.rb @@ -11,23 +11,23 @@ describe "Method#source_location" do end it "sets the first value to the path of the file in which the method was defined" do - file = @method.source_location.first + file = @method.source_location[0] file.should be_an_instance_of(String) file.should == File.realpath('fixtures/classes.rb', __dir__) end it "sets the last value to an Integer representing the line on which the method was defined" do - line = @method.source_location.last + line = @method.source_location[1] line.should be_an_instance_of(Integer) line.should == 5 end it "returns the last place the method was defined" do - MethodSpecs::SourceLocation.method(:redefined).source_location.last.should == 13 + MethodSpecs::SourceLocation.method(:redefined).source_location[1].should == 13 end it "returns the location of the original method even if it was aliased" do - MethodSpecs::SourceLocation.new.method(:aka).source_location.last.should == 17 + MethodSpecs::SourceLocation.new.method(:aka).source_location[1].should == 17 end it "works for methods defined with a block" do @@ -108,7 +108,13 @@ describe "Method#source_location" do c = Class.new do eval('def self.m; end', nil, "foo", 100) end - c.method(:m).source_location.should == ["foo", 100] + location = c.method(:m).source_location + ruby_version_is(""..."4.0") do + location.should == ["foo", 100] + end + ruby_version_is("4.0") do + location.should == ["foo", 100, 0, 100, 15] + end end describe "for a Method generated by respond_to_missing?" do diff --git a/spec/ruby/core/proc/source_location_spec.rb b/spec/ruby/core/proc/source_location_spec.rb index a8b99287d5..69b4e2fd82 100644 --- a/spec/ruby/core/proc/source_location_spec.rb +++ b/spec/ruby/core/proc/source_location_spec.rb @@ -17,57 +17,64 @@ describe "Proc#source_location" do end it "sets the first value to the path of the file in which the proc was defined" do - file = @proc.source_location.first + file = @proc.source_location[0] file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) - file = @proc_new.source_location.first + file = @proc_new.source_location[0] file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) - file = @lambda.source_location.first + file = @lambda.source_location[0] file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) - file = @method.source_location.first + file = @method.source_location[0] file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) end - it "sets the last value to an Integer representing the line on which the proc was defined" do - line = @proc.source_location.last + it "sets the second value to an Integer representing the line on which the proc was defined" do + line = @proc.source_location[1] line.should be_an_instance_of(Integer) line.should == 4 - line = @proc_new.source_location.last + line = @proc_new.source_location[1] line.should be_an_instance_of(Integer) line.should == 12 - line = @lambda.source_location.last + line = @lambda.source_location[1] line.should be_an_instance_of(Integer) line.should == 8 - line = @method.source_location.last + line = @method.source_location[1] line.should be_an_instance_of(Integer) line.should == 15 end it "works even if the proc was created on the same line" do - proc { true }.source_location.should == [__FILE__, __LINE__] - Proc.new { true }.source_location.should == [__FILE__, __LINE__] - -> { true }.source_location.should == [__FILE__, __LINE__] + ruby_version_is(""..."4.0") do + proc { true }.source_location.should == [__FILE__, __LINE__] + Proc.new { true }.source_location.should == [__FILE__, __LINE__] + -> { true }.source_location.should == [__FILE__, __LINE__] + end + ruby_version_is("4.0") do + proc { true }.source_location.should == [__FILE__, __LINE__, 11, __LINE__, 19] + Proc.new { true }.source_location.should == [__FILE__, __LINE__, 15, __LINE__, 23] + -> { true }.source_location.should == [__FILE__, __LINE__, 8, __LINE__, 17] + end end it "returns the first line of a multi-line proc (i.e. the line containing 'proc do')" do - ProcSpecs::SourceLocation.my_multiline_proc.source_location.last.should == 20 - ProcSpecs::SourceLocation.my_multiline_proc_new.source_location.last.should == 34 - ProcSpecs::SourceLocation.my_multiline_lambda.source_location.last.should == 27 + ProcSpecs::SourceLocation.my_multiline_proc.source_location[1].should == 20 + ProcSpecs::SourceLocation.my_multiline_proc_new.source_location[1].should == 34 + ProcSpecs::SourceLocation.my_multiline_lambda.source_location[1].should == 27 end it "returns the location of the proc's body; not necessarily the proc itself" do - ProcSpecs::SourceLocation.my_detached_proc.source_location.last.should == 41 - ProcSpecs::SourceLocation.my_detached_proc_new.source_location.last.should == 51 - ProcSpecs::SourceLocation.my_detached_lambda.source_location.last.should == 46 + ProcSpecs::SourceLocation.my_detached_proc.source_location[1].should == 41 + ProcSpecs::SourceLocation.my_detached_proc_new.source_location[1].should == 51 + ProcSpecs::SourceLocation.my_detached_lambda.source_location[1].should == 46 end it "returns the same value for a proc-ified method as the method reports" do @@ -86,6 +93,12 @@ describe "Proc#source_location" do it "works for eval with a given line" do proc = eval('-> {}', nil, "foo", 100) - proc.source_location.should == ["foo", 100] + location = proc.source_location + ruby_version_is(""..."4.0") do + location.should == ["foo", 100] + end + ruby_version_is("4.0") do + location.should == ["foo", 100, 2, 100, 5] + end end end diff --git a/spec/ruby/core/unboundmethod/source_location_spec.rb b/spec/ruby/core/unboundmethod/source_location_spec.rb index 5c2f14362c..85078ff34e 100644 --- a/spec/ruby/core/unboundmethod/source_location_spec.rb +++ b/spec/ruby/core/unboundmethod/source_location_spec.rb @@ -7,23 +7,23 @@ describe "UnboundMethod#source_location" do end it "sets the first value to the path of the file in which the method was defined" do - file = @method.source_location.first + file = @method.source_location[0] file.should be_an_instance_of(String) file.should == File.realpath('fixtures/classes.rb', __dir__) end - it "sets the last value to an Integer representing the line on which the method was defined" do - line = @method.source_location.last + it "sets the second value to an Integer representing the line on which the method was defined" do + line = @method.source_location[1] line.should be_an_instance_of(Integer) line.should == 5 end it "returns the last place the method was defined" do - UnboundMethodSpecs::SourceLocation.method(:redefined).unbind.source_location.last.should == 13 + UnboundMethodSpecs::SourceLocation.method(:redefined).unbind.source_location[1].should == 13 end it "returns the location of the original method even if it was aliased" do - UnboundMethodSpecs::SourceLocation.instance_method(:aka).source_location.last.should == 17 + UnboundMethodSpecs::SourceLocation.instance_method(:aka).source_location[1].should == 17 end it "works for define_method methods" do @@ -54,6 +54,12 @@ describe "UnboundMethod#source_location" do c = Class.new do eval('def m; end', nil, "foo", 100) end - c.instance_method(:m).source_location.should == ["foo", 100] + location = c.instance_method(:m).source_location + ruby_version_is(""..."4.0") do + location.should == ["foo", 100] + end + ruby_version_is("4.0") do + location.should == ["foo", 100, 0, 100, 10] + end end end diff --git a/test/ruby/test_lambda.rb b/test/ruby/test_lambda.rb index 7738034240..3cbb54306c 100644 --- a/test/ruby/test_lambda.rb +++ b/test/ruby/test_lambda.rb @@ -276,27 +276,27 @@ class TestLambdaParameters < Test::Unit::TestCase end def test_do_lambda_source_location - exp_lineno = __LINE__ + 3 + exp = [__LINE__ + 1, 12, __LINE__ + 5, 7] lmd = ->(x, y, z) do # end - file, lineno = lmd.source_location + file, *loc = lmd.source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(exp_lineno, lineno, "must be at the beginning of the block") + assert_equal(exp, loc) end def test_brace_lambda_source_location - exp_lineno = __LINE__ + 3 + exp = [__LINE__ + 1, 12, __LINE__ + 5, 5] lmd = ->(x, y, z) { # } - file, lineno = lmd.source_location + file, *loc = lmd.source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(exp_lineno, lineno, "must be at the beginning of the block") + assert_equal(exp, loc) end def test_not_orphan_return diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb index f74342322f..959ea87f25 100644 --- a/test/ruby/test_proc.rb +++ b/test/ruby/test_proc.rb @@ -513,7 +513,7 @@ class TestProc < Test::Unit::TestCase file, lineno = method(:source_location_test).to_proc.binding.source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(@@line_of_source_location_test, lineno, 'Bug #2427') + assert_equal(@@line_of_source_location_test[0], lineno, 'Bug #2427') end def test_binding_error_unless_ruby_frame @@ -1499,15 +1499,19 @@ class TestProc < Test::Unit::TestCase assert_include(EnvUtil.labeled_class(name, Proc).new {}.to_s, name) end - @@line_of_source_location_test = __LINE__ + 1 + @@line_of_source_location_test = [__LINE__ + 1, 2, __LINE__ + 3, 5] def source_location_test a=1, b=2 end def test_source_location - file, lineno = method(:source_location_test).source_location + file, *loc = method(:source_location_test).source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(@@line_of_source_location_test, lineno, 'Bug #2427') + assert_equal(@@line_of_source_location_test, loc, 'Bug #2427') + + file, *loc = self.class.instance_method(:source_location_test).source_location + assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) + assert_equal(@@line_of_source_location_test, loc, 'Bug #2427') end @@line_of_attr_reader_source_location_test = __LINE__ + 3 @@ -1540,13 +1544,13 @@ class TestProc < Test::Unit::TestCase end def test_block_source_location - exp_lineno = __LINE__ + 3 - file, lineno = block_source_location_test(1, + exp_loc = [__LINE__ + 3, 49, __LINE__ + 4, 49] + file, *loc = block_source_location_test(1, 2, 3) do end assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(exp_lineno, lineno) + assert_equal(exp_loc, loc) end def test_splat_without_respond_to