Bug #1760: Methods Expecting Paths Should Prefer #to_path Over #to_str

• Status: Closed
• Assignee: matz (Yukihiro Matsumoto)
• Target version: 1.9.2
• ruby -v: ruby 1.9.2dev (2009-07-08 trunk 23995) [i686-linux]

[runpaint (Run Paint Run Run)]

=begin
Methods that expect paths usually call #to_path on their argument. They may also try #to_str. When an argument responds to both of these methods, I suggest that #to_path is preferred as it is the more specific, thus more likely to be correct.

Dir.chdir and File.open, at the least, prefer #to_str over #to_path.

$ cat /tmp/path.rb
p = Class.new do
def to_str; 'str' ; end
def to_path; 'path'; end
end

File.open(p.new)

$ ruby -v /tmp/path.rb
ruby 1.9.2dev (2009-07-08 trunk 23995) [i686-linux]
/tmp/path.rb:6:in initialize': No such file or directory - str (Errno::ENOENT) from /tmp/path.rb:6:in open'
from /tmp/path.rb:6:in `'
=end

[yugui (Yuki Sonoda)]

• Assignee set to matz (Yukihiro Matsumoto)
• Priority changed from Normal to 5
• Target version set to 1.9.2

=begin

=end

[yugui (Yuki Sonoda)]

• Priority changed from 5 to 6

=begin
We need your decision. Decide it. < matz.
=end

[naruse (Yui NARUSE)]

• Status changed from Open to Assigned

=begin

=end

[mame (Yusuke Endoh)]

=begin
Hi,

Methods that expect paths usually call #to_path on their argument. They may also try #to_str. When an argument responds to both of these methods, I suggest that #to_path is preferred as it is the more specific, thus more likely to be correct.

Seems reasonable.

I'll apply the patch unless anyone expresses opposition.

diff --git a/file.c b/file.c
index 36b0b06..9b5d380 100644
--- a/file.c
+++ b/file.c
@@ -140,16 +140,16 @@ rb_get_path_check(VALUE obj, int level)
if (insecure_obj_p(obj, level)) {
rb_insecure_operation();
}

• tmp = rb_check_string_type(obj);

• if (!NIL_P(tmp)) goto exit;

  CONST_ID(to_path, "to_path");

• if (rb_respond_to(obj, to_path)) {

• if (!rb_special_const_p(obj) && RBASIC(obj)->klass != 0 && rb_respond_to(obj, to_path)) {
  tmp = rb_funcall(obj, to_path, 0, 0);
  }
  else {
  tmp = obj;
  }
• tmp = rb_check_string_type(tmp);
• if (!NIL_P(tmp)) goto exit;
  StringValue(tmp);
  exit:
  tmp = file_path_convert(tmp);

A patch for rubyspec:

diff --git a/core/dir/chdir_spec.rb b/core/dir/chdir_spec.rb
index ec82759..068833d 100644
--- a/core/dir/chdir_spec.rb
+++ b/core/dir/chdir_spec.rb
@@ -56,10 +56,10 @@ describe "Dir.chdir" do
Dir.chdir(obj)
end

• it "prefers #to_str over #to_path" do

• it "prefers #to_path over #to_str" do
  obj = Class.new do

•    def to_path; DirSpecs.mock_dir; end
  

•    def to_str;  Dir.pwd; end
  

•    def to_path; Dir.pwd; end
  

•    def to_str;  DirSpecs.mock_dir; end
   end
   Dir.chdir(obj.new)
   Dir.pwd.should == @original
  

diff --git a/core/kernel/shared/require.rb b/core/kernel/shared/require.rb
index 1b35fd4..956fa72 100644
--- a/core/kernel/shared/require.rb
+++ b/core/kernel/shared/require.rb
@@ -108,10 +108,11 @@ describe :kernel_require_basic, :shared => true do
ScratchPad.recorded.should == [:loaded]
end

•  it "does not call #to_path on a String" do
  

•  it "calls #to_path on a String" do
     path = File.expand_path "load_fixture.rb", CODE_LOADING_DIR
  

•    path.should_not_receive(:to_path)
  

•    @object.send(@method, path).should be_true
  

•    str = mock("load_fixture.rb mock")
  

•    str.should_receive(:to_path).and_return(path)
  

•    @object.send(@method, str).should be_true
     ScratchPad.recorded.should == [:loaded]
   end
  

--
Yusuke ENDOH [e-mail]
=end

[mame (Yusuke Endoh)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

=begin
This issue was solved with changeset r27045.
Run Paint, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end