Merge pull request #1346 from herwinw/io_binwrite

Fix keyword arguments in IO.binwrite
natalie-lang · Oct 14, 2023 · 9d3f8ca · 9d3f8ca
2 parents 71902be + 00b89c8
commit 9d3f8ca
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 8 deletions.
diff --git a/include/natalie/io_object.hpp b/include/natalie/io_object.hpp
@@ -52,7 +52,7 @@ class IoObject : public Object {
     Value append(Env *, Value);
     Value autoclose(Env *, Value);
     static Value binread(Env *, Value, Value = nullptr, Value = nullptr);
-    static Value binwrite(Env *, Value, Value, Value = nullptr);
+    static Value binwrite(Env *, Args);
     Value binmode(Env *);
     Value close(Env *);
     Value dup(Env *) const;

diff --git a/lib/natalie/compiler/binding_gen.rb b/lib/natalie/compiler/binding_gen.rb
@@ -846,7 +846,7 @@ def generate_name
 gen.binding('Integer', 'zero?', 'IntegerObject', 'is_zero', argc: 0, pass_env: false, pass_block: false, return_type: :bool, optimized: true)
 
 gen.static_binding('IO', 'binread', 'IoObject', 'binread', argc: 1..3, pass_env: true, pass_block: false, return_type: :Object)
-gen.static_binding('IO', 'binwrite', 'IoObject', 'binwrite', argc: 2..3, pass_env: true, pass_block: false, return_type: :Object)
+gen.static_binding('IO', 'binwrite', 'IoObject', 'binwrite', argc: :any, pass_env: true, pass_block: false, return_type: :Object)
 gen.static_binding('IO', 'pipe', 'IoObject', 'pipe', argc: 0..2, pass_env: true, pass_block: true, pass_klass: true, return_type: :Object)
 gen.static_binding('IO', 'read', 'IoObject', 'read_file', argc: :any, pass_env: true, pass_block: false, return_type: :Object)
 gen.static_binding('IO', 'select', 'IoObject', 'select', argc: 1..4, pass_env: true, pass_block: false, return_type: :Object)

diff --git a/spec/core/io/shared/binwrite.rb b/spec/core/io/shared/binwrite.rb
@@ -22,7 +22,15 @@
   end
 
   it "accepts options as a keyword argument" do
-    NATFIXME 'Keyword arguments', exception: ArgumentError, message: 'wrong number of arguments' do
+    if @method == :write
+      NATFIXME 'Keyword arguments', exception: ArgumentError, message: 'wrong number of arguments (given 4, expected 2)' 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
+    else
       IO.send(@method, @filename, "hi", 0, flags: File::CREAT).should == 2
 
       -> {
@@ -94,7 +102,14 @@
   end
 
   it "accepts a :mode option" do
-    NATFIXME 'Keyword arguments (exact exception differs between callers)' do
+    if @method == :write
+      NATFIXME 'Keyword arguments (exact exception differs between callers)' do
+        IO.send(@method, @filename, "hello, world!", mode: 'a')
+        File.read(@filename).should == "012345678901234567890123456789hello, world!"
+        IO.send(@method, @filename, "foo", 2, mode: 'w')
+        File.read(@filename).should == "\0\0foo"
+      end
+    else
       IO.send(@method, @filename, "hello, world!", mode: 'a')
       File.read(@filename).should == "012345678901234567890123456789hello, world!"
       IO.send(@method, @filename, "foo", 2, mode: 'w')
@@ -103,14 +118,23 @@
   end
 
   it "accepts a :flags option without :mode one" do
-    NATFIXME 'Keyword arguments (exact exception differs between callers)' do
+    if @method == :write
+      NATFIXME 'Keyword arguments', exception: ArgumentError, message: 'wrong number of arguments (given 3, expected 2)' do
+        IO.send(@method, @filename, "hello, world!", flags: File::CREAT)
+        File.read(@filename).should == "hello, world!"
+      end
+    else
       IO.send(@method, @filename, "hello, world!", flags: File::CREAT)
       File.read(@filename).should == "hello, world!"
     end
   end
 
   it "raises an error if readonly mode is specified" do
-    NATFIXME 'Keyword arguments', exception: SpecFailedException do
+    if @method == :write
+      NATFIXME 'Keyword arguments', exception: SpecFailedException do
+        -> { IO.send(@method, @filename, "abcde", mode: "r") }.should raise_error(IOError)
+      end
+    else
       -> { IO.send(@method, @filename, "abcde", mode: "r") }.should raise_error(IOError)
     end
   end

diff --git a/src/io_object.cpp b/src/io_object.cpp
@@ -116,12 +116,24 @@ Value IoObject::binread(Env *env, Value filename, Value length, Value offset) {
     return data;
 }
 
-Value IoObject::binwrite(Env *env, Value filename, Value string, Value offset) {
+Value IoObject::binwrite(Env *env, Args args) {
+    auto kwargs = args.pop_keyword_hash();
+    args.ensure_argc_between(env, 2, 3);
+    auto filename = args.at(0);
+    auto string = args.at(1);
+    auto offset = args.at(2, nullptr);
     ClassObject *File = GlobalEnv::the()->Object()->const_fetch("File"_s)->as_class();
     auto mode = O_WRONLY | O_CREAT | O_CLOEXEC;
     if (!offset || offset->is_nil())
         mode |= O_TRUNC;
-    auto kwargs = new HashObject { env, { "mode"_s, IntegerObject::create(mode), "binmode"_s, TrueObject::the() } };
+    if (kwargs) {
+        kwargs = kwargs->dup(env)->as_hash();
+    } else {
+        kwargs = new HashObject {};
+    }
+    if (!kwargs->has_key(env, "mode"_s))
+        kwargs->put(env, "mode"_s, IntegerObject::create(mode));
+    kwargs->put(env, "binmode"_s, TrueObject::the());
     FileObject *file = _new(env, File, Args({ filename, kwargs }, true), nullptr)->as_file();
     if (offset && !offset->is_nil())
         file->set_pos(env, offset);