[GR-18163] Fix File.open and handle the flags option

PullRequest: truffleruby/3620

CHANGELOG.md

@@ -64,6 +64,7 @@ Compatibility:
 * Implement `Enumerable#compact` and `Enumerator::Lazy#compact` (#2733, @andrykonchin).
 * Implement `Array#intersect?` (#2831, @nirvdrum).
 * Record the source location in the constant for the `module`/`class` keywords (#2833, @eregon).
+* Fix `File.open` and support `flags` option (#2820, @andrykonchin).
 
 Performance:
 

lib/truffle/pathname.rb

@@ -812,14 +812,14 @@ def each_line(*args, **kw, &block) # :yield: line
 
   # See <tt>IO.read</tt>.  Returns all data from the file, or the first +N+ bytes
   # if specified.
-  def read(*args) IO.read(@path, *args) end
+  def read(*args, **kw) IO.read(@path, *args, **kw) end
 
   # See <tt>IO.binread</tt>.  Returns all the bytes from the file, or the first +N+
   # if specified.
   def binread(*args) IO.binread(@path, *args) end
 
   # See <tt>IO.write</tt>.  Returns the number of bytes written to the file.
-  def write(*args) IO.write(@path, *args) end
+  def write(*args, **kw) IO.write(@path, *args, **kw) end
 
   # See <tt>IO.binwrite</tt>.  Returns the number of bytes written to the file.
   def binwrite(*args) IO.binwrite(@path, *args) end
@@ -870,8 +870,8 @@ def ftype() File.ftype(@path) end
   def make_link(old) File.link(old, @path) end
 
   # See <tt>File.open</tt>.  Opens the file for reading or writing.
-  def open(*args, &block) # :yield: file
-    File.open(@path, *args, &block)
+  def open(*args, **kw, &block) # :yield: file
+    File.open(@path, *args, **kw, &block)
   end
 
   # See <tt>File.readlink</tt>.  Read symbolic link.

spec/ruby/core/file/new_spec.rb

@@ -78,6 +78,20 @@
     File.should.exist?(@file)
   end
 
+  it "returns a new read-only File when mode is not specified" do
+    @fh = File.new(@file)
+
+    -> { @fh.puts("test") }.should raise_error(IOError)
+    @fh.read.should == ""
+  end
+
+  it "returns a new read-only File when mode is not specified but flags option is present" do
+    @fh = File.new(@file, flags: File::CREAT)
+
+    -> { @fh.puts("test") }.should raise_error(IOError)
+    @fh.read.should == ""
+  end
+
   it "creates a new file when use File::EXCL mode" do
     @fh = File.new(@file, File::EXCL)
     @fh.should be_kind_of(File)
@@ -112,7 +126,6 @@
     File.should.exist?(@file)
   end
 
-
   it "creates a new file when use File::WRONLY|File::TRUNC mode" do
     @fh = File.new(@file, File::WRONLY|File::TRUNC)
     @fh.should be_kind_of(File)
@@ -133,6 +146,25 @@
     File.should.exist?(@file)
   end
 
+  it "accepts options as a keyword argument" do
+    @fh = File.new(@file, 'w', 0755, flags: @flags)
+    @fh.should be_kind_of(File)
+
+    -> {
+      @fh = File.new(@file, 'w', 0755, {flags: @flags})
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 4, expected 1..3)")
+  end
+
+  it "bitwise-ORs mode and flags option" do
+    -> {
+      @fh = File.new(@file, 'w', flags: File::EXCL)
+    }.should raise_error(Errno::EEXIST, /File exists/)
+
+    -> {
+      @fh = File.new(@file, mode: 'w', flags: File::EXCL)
+    }.should raise_error(Errno::EEXIST, /File exists/)
+  end
+
   it "raises a TypeError if the first parameter can't be coerced to a string" do
     -> { File.new(true) }.should raise_error(TypeError)
     -> { File.new(false) }.should raise_error(TypeError)

spec/ruby/core/file/open_spec.rb

@@ -565,6 +565,15 @@
     File.open(@file, 'wb+') {|f| f.external_encoding.should == Encoding::BINARY}
   end
 
+  it "accepts options as a keyword argument" do
+    @fh = File.open(@file, 'w', 0755, flags: File::CREAT)
+    @fh.should be_an_instance_of(File)
+
+    -> {
+      File.open(@file, 'w', 0755, {flags: File::CREAT})
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 4, expected 1..3)")
+  end
+
   it "uses the second argument as an options Hash" do
     @fh = File.open(@file, mode: "r")
     @fh.should be_an_instance_of(File)

spec/ruby/core/io/initialize_spec.rb

@@ -27,6 +27,17 @@
     @io.fileno.should == fd
   end
 
+  it "accepts options as keyword arguments" do
+    fd = new_fd @name, "w:utf-8"
+
+    @io.send(:initialize, fd, "w", flags: File::CREAT)
+    @io.fileno.should == fd
+
+    -> {
+      @io.send(:initialize, fd, "w", {flags: File::CREAT})
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 3, expected 1..2)")
+  end
+
   it "raises a TypeError when passed an IO" do
     -> { @io.send :initialize, STDOUT, 'w' }.should raise_error(TypeError)
   end

spec/ruby/core/io/read_spec.rb

@@ -23,6 +23,14 @@
     IO.read(p)
   end
 
+  it "accepts options as keyword arguments" do
+    IO.read(@fname, 3, 0, mode: "r+").should == @contents[0, 3]
+
+    -> {
+      IO.read(@fname, 3, 0, {mode: "r+"})
+    }.should raise_error(ArgumentError, /wrong number of arguments/)
+  end
+
   it "accepts an empty options Hash" do
     IO.read(@fname, **{}).should == @contents
   end
@@ -55,6 +63,19 @@
     IO.read(@fname, mode: "a+").should == @contents
   end
 
+  it "uses an :open_args option" do
+    string = IO.read(@fname, nil, 0, open_args: ["r", nil, {encoding: Encoding::US_ASCII}])
+    string.encoding.should == Encoding::US_ASCII
+
+    string = IO.read(@fname, nil, 0, open_args: ["r", nil, {}])
+    string.encoding.should == Encoding::UTF_8
+  end
+
+  it "disregards other options if :open_args is given" do
+    string = IO.read(@fname,mode: "w", encoding: Encoding::UTF_32LE, open_args: ["r", encoding: Encoding::UTF_8])
+    string.encoding.should == Encoding::UTF_8
+  end
+
   it "treats second nil argument as no length limit" do
     IO.read(@fname, nil).should == @contents
     IO.read(@fname, nil, 5).should == IO.read(@fname, @contents.length, 5)

spec/ruby/core/io/shared/new.rb

@@ -64,6 +64,15 @@
     @io.should be_an_instance_of(IO)
   end
 
+  it "accepts options as keyword arguments" do
+    @io = IO.send(@method, @fd, "w", flags: File::CREAT)
+    @io.write("foo").should == 3
+
+    -> {
+      IO.send(@method, @fd, "w", {flags: File::CREAT})
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 3, expected 1..2)")
+  end
+
   it "accepts a :mode option" do
     @io = IO.send(@method, @fd, mode: "w")
     @io.write("foo").should == 3

spec/ruby/core/io/write_spec.rb

@@ -61,10 +61,6 @@
     File.binread(@filename).should == "h\u0000\u0000\u0000i\u0000\u0000\u0000"
   end
 
-  it "uses an :open_args option" do
-    IO.write(@filename, 'hi', open_args: ["w", nil, {encoding: Encoding::UTF_32LE}]).should == 8
-  end
-
   it "raises a invalid byte sequence error if invalid bytes are being written" do
     # pack "\xFEhi" to avoid utf-8 conflict
     xFEhi = ([254].pack('C*') + 'hi').force_encoding('utf-8')

spec/ruby/core/kernel/open_spec.rb

@@ -72,6 +72,15 @@
     -> { open }.should raise_error(ArgumentError)
   end
 
+  it "accepts options as keyword arguments" do
+    @file = open(@name, "r", 0666, flags: File::CREAT)
+    @file.should be_kind_of(File)
+
+    -> {
+      open(@name, "r", 0666, {flags: File::CREAT})
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 4, expected 1..3)")
+  end
+
   describe "when given an object that responds to to_open" do
     before :each do
       ScratchPad.clear
@@ -109,11 +118,21 @@
 
     it "passes its arguments onto #to_open" do
       obj = mock('to_open')
-      obj.should_receive(:to_open).with(1,2,3)
-
+      obj.should_receive(:to_open).with(1, 2, 3)
       open(obj, 1, 2, 3)
     end
 
+    it "passes keyword arguments onto #to_open as keyword arguments if to_open accepts them" do
+      obj = Object.new
+      def obj.to_open(*args, **kw)
+        ScratchPad << {args: args, kw: kw}
+      end
+
+      ScratchPad.record []
+      open(obj, 1, 2, 3, a: "b")
+      ScratchPad.recorded.should == [args: [1, 2, 3], kw: {a: "b"}]
+    end
+
     it "passes the return value from #to_open to a block" do
       obj = mock('to_open')
       obj.should_receive(:to_open).and_return(:value)

spec/tags/core/file/open_tags.txt

@@ -1,7 +1,5 @@
 fails:File.open with a block propagates StandardErrors produced by close
 slow:File.open on a FIFO opens it as a normal file
-fails:File.open accepts extra flags as a keyword argument and combine with a string mode
-fails:File.open accepts extra flags as a keyword argument and combine with an integer mode
 fails:File.open 'x' flag does nothing if the file doesn't exist
 fails:File.open 'x' flag throws a Errno::EEXIST error if the file exists
 fails:File.open 'x' flag can't be used with 'r' and 'a' flags

spec/tags/core/io/read_tags.txt

@@ -1,3 +1,5 @@
 fails:IO.read from a pipe opens a pipe to a fork if the rest is -
 fails(hangs):IO#read raises IOError when stream is closed by another thread
 fails:IO#read raises ArgumentError when length is less than 0
+fails:IO.read uses an :open_args option
+fails:IO.read disregards other options if :open_args is given

src/main/ruby/truffleruby/core/file.rb

@@ -1183,40 +1183,17 @@ class << self
     alias_method :fnmatch?, :fnmatch
   end
 
-  def initialize(path_or_fd, mode=nil, perm=undefined, options=undefined)
+  def initialize(path_or_fd, mode=nil, perm=nil, **options)
     if Primitive.object_kind_of?(path_or_fd, Integer)
-      super(path_or_fd, mode, options)
+      super(path_or_fd, mode, **options)
       @path = nil
     else
       path = Truffle::Type.coerce_to_path path_or_fd
-
-      # TODO: fix normalize_options
-      case mode
-      when String, Integer
-        # do nothing
-      when nil
-        mode = 'r'
-      when Hash
-        options = mode
-        mode = nil
-      else
-        options = Truffle::Type.coerce_to mode, Hash, :to_hash
-        mode = nil
-      end
-
-      if Primitive.undefined?(options) and !Primitive.undefined?(perm)
-        options = Truffle::Type.try_convert(perm, Hash, :to_hash)
-        perm = undefined if options
-      end
-
-      perm = nil if Primitive.undefined? perm
       nmode, _binary, _external, _internal, _autoclose, perm = IO.normalize_options(mode, perm, options)
-      nmode ||= 'r'
-
       fd = IO.sysopen(path, nmode, perm)
 
       @path = path
-      super(fd, mode, options)
+      super(fd, mode, **options)
     end
   end