[GR-18163] Fix handling options by some IO methods

PullRequest: truffleruby/3628

Author: andrykonchin

lib/truffle/pathname.rb

@@ -822,7 +822,7 @@ def binread(*args) IO.binread(@path, *args) end
   def write(...) IO.write(@path, ...) end
 
   # See <tt>IO.binwrite</tt>.  Returns the number of bytes written to the file.
-  def binwrite(*args) IO.binwrite(@path, *args) end
+  def binwrite(...) IO.binwrite(@path, ...) end
 
   # See <tt>IO.readlines</tt>.  Returns all the lines from the file.
   def readlines(...) IO.readlines(@path, ...) end

spec/ruby/core/io/read_spec.rb

@@ -79,6 +79,16 @@
     string.encoding.should == Encoding::UTF_8
   end
 
+  it "doesn't require mode to be specified in :open_args" do
+    string = IO.read(@fname, nil, 0, open_args: [{encoding: Encoding::US_ASCII}])
+    string.encoding.should == Encoding::US_ASCII
+  end
+
+  it "doesn't require mode to be specified in :open_args even if flags option passed" do
+    string = IO.read(@fname, nil, 0, open_args: [{encoding: Encoding::US_ASCII, flags: File::CREAT}])
+    string.encoding.should == Encoding::US_ASCII
+  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/readlines_spec.rb

@@ -127,6 +127,12 @@
       end
     end
   end
+
+  describe "when passed arbitrary keyword argument" do
+    it "tolerates it" do
+      @io.readlines(chomp: true, foo: :bar).should == IOSpecs.lines_without_newline_characters
+    end
+  end
 end
 
 describe "IO#readlines" do

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

@@ -21,6 +21,16 @@
     IO.send(@method, @filename, "abcde").should == 5
   end
 
+  ruby_version_is "3.0" do
+    it "accepts options as a keyword argument" do
+      IO.send(@method, @filename, "hi", 0, flags: File::CREAT).should == 2
+
+      -> {
+        IO.send(@method, @filename, "hi", 0, {flags: File::CREAT})
+      }.should raise_error(ArgumentError, "wrong number of arguments (given 4, expected 2..3)")
+    end
+  end
+
   it "creates a file if missing" do
     fn = @filename + "xxx"
     begin
@@ -67,6 +77,11 @@
     File.read(@filename).should == "\0\0foo"
   end
 
+  it "accepts a :flags option without :mode one" do
+    IO.send(@method, @filename, "hello, world!", flags: File::CREAT)
+    File.read(@filename).should == "hello, world!"
+  end
+
   it "raises an error if readonly mode is specified" do
     -> { IO.send(@method, @filename, "abcde", mode: "r") }.should raise_error(IOError)
   end

spec/ruby/core/io/write_spec.rb

@@ -81,6 +81,20 @@
     File.binread(@filename).should == "h\u0000\u0000\u0000i\u0000\u0000\u0000"
   end
 
+  it "uses the encoding from the given option for non-ascii encoding even if in binary mode" do
+    File.open(@filename, "w", encoding: Encoding::UTF_32LE, binmode: true) do |file|
+      file.should.binmode?
+      file.write("hi").should == 8
+    end
+    File.binread(@filename).should == "h\u0000\u0000\u0000i\u0000\u0000\u0000"
+
+    File.open(@filename, "wb", encoding: Encoding::UTF_32LE) do |file|
+      file.should.binmode?
+      file.write("hi").should == 8
+    end
+    File.binread(@filename).should == "h\u0000\u0000\u0000i\u0000\u0000\u0000"
+  end
+
   it "uses the encoding from the given option for non-ascii encoding when multiple arguments passes" do
     File.open(@filename, "w", encoding: Encoding::UTF_32LE) do |file|
       file.write("h", "i").should == 8
@@ -131,10 +145,38 @@
     File.read(@filename).should == "\0\0hi"
   end
 
+  it "requires mode to be specified in :open_args" do
+    -> {
+      IO.write(@filename, 'hi', open_args: [{encoding: Encoding::UTF_32LE, binmode: true}])
+    }.should raise_error(IOError, "not opened for writing")
+
+    IO.write(@filename, 'hi', open_args: ["w", {encoding: Encoding::UTF_32LE, binmode: true}]).should == 8
+    IO.write(@filename, 'hi', open_args: [{encoding: Encoding::UTF_32LE, binmode: true, mode: "w"}]).should == 8
+  end
+
+  it "requires mode to be specified in :open_args even if flags option passed" do
+    -> {
+      IO.write(@filename, 'hi', open_args: [{encoding: Encoding::UTF_32LE, binmode: true, flags: File::CREAT}])
+    }.should raise_error(IOError, "not opened for writing")
+
+    IO.write(@filename, 'hi', open_args: ["w", {encoding: Encoding::UTF_32LE, binmode: true, flags: File::CREAT}]).should == 8
+    IO.write(@filename, 'hi', open_args: [{encoding: Encoding::UTF_32LE, binmode: true, flags: File::CREAT, mode: "w"}]).should == 8
+  end
+
   it "uses the given encoding and returns the number of bytes written" do
     IO.write(@filename, 'hi', mode: "w", encoding: Encoding::UTF_32LE).should == 8
   end
 
+  it "raises ArgumentError if encoding is specified in mode parameter and is given as :encoding option" do
+    -> {
+      IO.write(@filename, 'hi', mode: "w:UTF-16LE:UTF-16BE", encoding: Encoding::UTF_32LE)
+    }.should raise_error(ArgumentError, "encoding specified twice")
+
+    -> {
+      IO.write(@filename, 'hi', mode: "w:UTF-16BE", encoding: Encoding::UTF_32LE)
+    }.should raise_error(ArgumentError, "encoding specified twice")
+  end
+
   it "writes the file with the permissions in the :perm parameter" do
     rm_r @filename
     IO.write(@filename, 'write :perm spec', mode: "w", perm: 0o755).should == 16

spec/tags/core/io/read_tags.txt

@@ -1,5 +1,2 @@
 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
+fails(hangs):IO#read raises IOError when stream is closed by another thread

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

@@ -329,20 +329,14 @@ def self.binread(file, length=nil, offset=0)
     end
   end
 
-  def self.binwrite(file, string, *args)
-    offset, opts = args
-    opts ||= {}
-    if Primitive.object_kind_of?(offset, Hash)
-      offset, opts = nil, offset
-    end
+  def self.binwrite(file, string, offset=nil, **options)
+    default_mode = File::CREAT | File::RDWR | File::BINARY
+    default_mode |= File::TRUNC unless offset
+
+    mode, _binary, external, _internal, _autoclose, perm = IO.normalize_options(nil, nil, options, default_mode)
 
-    mode, _binary, external, _internal, _autoclose, perm = IO.normalize_options(nil, nil, opts)
-    unless mode
-      mode = File::CREAT | File::RDWR | File::BINARY
-      mode |= File::TRUNC unless offset
-    end
     File.open(file, mode, encoding: (external || 'ASCII-8BIT'), perm: perm) do |f|
-      f.seek(offset || 0)
+      f.seek(offset) if offset
       f.write(string)
     end
   end
@@ -501,11 +495,10 @@ def self.read_encode(io, str)
 
   def self.write(file, string, offset=nil, **options)
     if Primitive.nil?(options[:open_args])
-      mode, _binary, external, _internal, _autoclose, perm = IO.normalize_options(nil, nil, options)
-      unless mode
-        mode = File::CREAT | File::WRONLY
-        mode |= File::TRUNC unless offset
-      end
+      default_mode = File::CREAT | File::WRONLY
+      default_mode |= File::TRUNC unless offset
+
+      mode, _binary, external, _internal, _autoclose, perm = IO.normalize_options(nil, nil, options, default_mode)
 
       open_args = [mode]
       open_kw = { encoding: (external || 'ASCII-8BIT'), perm: perm }
@@ -527,7 +520,6 @@ def self.for_fd(fd, mode=nil, **options)
   def self.read(name, length=nil, offset=0, **options)
     offset = 0 if Primitive.nil? offset
     name = Truffle::Type.coerce_to_path name
-    mode = options.delete(:mode) || 'r'
 
     offset = Primitive.rb_to_int(offset || 0)
     raise Errno::EINVAL, 'offset must not be negative' if offset < 0
@@ -537,12 +529,20 @@ def self.read(name, length=nil, offset=0, **options)
       raise ArgumentError, 'length must not be negative' if length < 0
     end
 
+    if Primitive.nil?(options[:open_args])
+      file_new_args = [name]
+      file_new_options = options
+    else
+      file_new_args = [name] + options[:open_args]
+      file_new_options = Primitive.object_kind_of?(file_new_args.last, Hash) ? file_new_args.pop : {}
+    end
+
     # Detect pipe mode
     if name[0] == ?|
       io = IO.popen(name[1..-1], 'r')
       return nil unless io # child process
     else
-      io = File.new(name, mode, **options)
+      io = File.new(*file_new_args, **file_new_options)
     end
 
     str = nil
@@ -565,7 +565,7 @@ def self.try_convert(obj)
     Truffle::Type.rb_check_convert_type obj, IO, :to_io
   end
 
-  def self.normalize_options(mode, perm, options)
+  def self.normalize_options(mode, perm, options, default_mode=nil)
     autoclose = true
 
     if mode
@@ -584,6 +584,7 @@ def self.normalize_options(mode, perm, options)
       end
 
       mode ||= optmode
+      mode ||= default_mode
 
       if flags = options[:flags]
         flags = Truffle::Type.rb_convert_type(flags, Integer, :to_int)
@@ -611,6 +612,8 @@ def self.normalize_options(mode, perm, options)
       autoclose = Primitive.as_boolean(options[:autoclose]) if options.key?(:autoclose)
     end
 
+    mode ||= default_mode
+
     if Primitive.object_kind_of?(mode, String)
       mode, external, internal = mode.split(':', 3)
       raise ArgumentError, 'invalid access mode' unless mode
@@ -1704,6 +1707,8 @@ def read(length=nil, buffer=nil)
       return nil
     end
 
+    raise ArgumentError, 'length must not be negative' if length < 0
+
     str = +''
     needed = length
     while needed > 0 and not @ibuffer.exhausted?
@@ -1817,12 +1822,12 @@ def readline(sep_or_limit=$/, limit=nil, chomp: false)
   #
   #  f = File.new("testfile")
   #  f.readlines[0]   #=> "This is line one\n"
-  def readlines(sep_or_limit=$/, limit=nil, chomp: false)
-    ret = []
-
+  def readlines(sep_or_limit=$/, limit=nil, **options)
+    chomp = options.fetch(:chomp, false)
     each_reader = create_each_reader(sep_or_limit, limit, chomp, true)
-    each_reader&.each { |line| ret << line }
 
+    ret = []
+    each_reader&.each { |line| ret << line }
     ret
   end
 
@@ -2319,7 +2324,7 @@ def write(*objects)
 
       ensure_open_and_writable
 
-      if !binmode? && external_encoding && external_encoding != string.encoding && external_encoding != Encoding::BINARY
+      if external_encoding && external_encoding != string.encoding && external_encoding != Encoding::BINARY
         unless string.ascii_only? && external_encoding.ascii_compatible?
           string = string.encode(external_encoding)
         end