Bug #1760
closedMethods Expecting Paths Should Prefer #to_path Over #to_str
Description
=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
Updated by yugui (Yuki Sonoda) about 15 years ago
- Assignee set to matz (Yukihiro Matsumoto)
- Priority changed from Normal to 5
- Target version set to 1.9.2
=begin
=end
Updated by yugui (Yuki Sonoda) about 15 years ago
- Priority changed from 5 to 6
=begin
We need your decision. Decide it. < matz.
=end
Updated by mame (Yusuke Endoh) over 14 years ago
=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 mame@tsg.ne.jp
=end
Updated by mame (Yusuke Endoh) over 14 years ago
- 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