Project

General

Profile

Bug #1760

Methods Expecting Paths Should Prefer #to_path Over #to_str

Added by runpaint (Run Paint Run Run) about 10 years ago. Updated over 8 years ago.

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
ruby 1.9.2dev (2009-07-08 trunk 23995) [i686-linux]
Backport:
[ruby-core:24257]

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


Related issues

Has duplicate Backport191 - Backport #3604: File.path, File.open, etc call to_str twice on the path argumentClosedActions

History

#1

Updated by yugui (Yuki Sonoda) about 10 years ago

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

=begin

=end

#2

Updated by yugui (Yuki Sonoda) about 10 years ago

  • Priority changed from 5 to 6

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

#3

Updated by naruse (Yui NARUSE) almost 10 years ago

  • Status changed from Open to Assigned

=begin

=end

#4

Updated by mame (Yusuke Endoh) over 9 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

#5

Updated by mame (Yusuke Endoh) over 9 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

Also available in: Atom PDF