From: Yusuke Endoh <redmine@...>
Date: 2010-03-02T00:33:12+09:00
Subject: [ruby-core:28385] [Bug #1760] Methods Expecting Paths Should Prefer #to_path Over #to_str

Issue #1760 has been updated by Yusuke Endoh.


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 <mame@tsg.ne.jp>
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1760

----------------------------------------
http://redmine.ruby-lang.org