Fixes on top of Jirapong's fix for Kernel.open

Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/kernel/open_tags.txt

@@ -1,2 +1,4 @@
 fails:Kernel#open opens an io when path starts with a pipe
 fails:Kernel#open opens an io when called with a block
+fails:Kernel#open sets the file as writable if perm is nil
+fails:Kernel#open allows nil for perm

Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/core/kernel/open_spec.rb

@@ -8,28 +8,43 @@
 
   before :each do
     @file = "test.txt"
+    @newfile = "test.new"
+    @fh = nil
     File.open(@file, "w"){ |f| f.puts "This is a test" }
   end
 
   after :each do
-    File.delete(@file) 
+    @fh.close if @fh and not @fh.closed?
+    File.delete(@file)
+    File.delete(@newfile) if File.exist? @newfile
   end
 
   it "opens a file when given a valid filename" do
-    @output = open("test.txt")
-    @output.class.should == File
-    @output.close
+    @fh = open(@file)
+    @fh.class.should == File
   end
 
   it "opens a file when called with a block" do
-    @output = open("test.txt", "r") { |f| f.gets }
-    @output.should == "This is a test\n"
+    open(@file, "r") { |f| f.gets }.should == "This is a test\n"
   end
 
-  it "opens a file to write with permission" do
-	open("test.txt", "w", 0644){ |f| f.puts "This is a test2" }
-	File.readable?(@file).should == true
-    File.writable?(@file).should == true
+  it "sets a default permission of writable" do
+    File.writable?(@file).should be_true  
+  end
+
+  it "sets permissions of newly created file" do
+	open(@newfile, "w", 0444){ }
+    File.writable?(@newfile).should be_false
+  end
+
+  it "sets the file as writable if perm is nil" do
+	open(@newfile, "w", nil){ }
+    File.writable?(@newfile).should be_true
+  end
+
+  it "ignores perm for existing file" do
+    open(@file, "r", 0444) { }
+    File.writable?(@file).should be_true  
   end
 
   platform_is_not :windows do
@@ -40,8 +55,7 @@
     end
 
     it "opens an io when called with a block" do
-      @output = open("|date") { |f| f.gets }
-      @output.should_not == ''
+      open("|date") { |f| f.gets }.should_not == ''
     end
 
   end
@@ -54,21 +68,49 @@
     end
 
     it "opens an io when called with a block" do
-      @output = open("|date /t") { |f| f.gets }
-      @output.should_not == ''
+      open("|date /t") { |f| f.gets }.should_not == ''
     end
 
   end
 
-
+  it "returns block return value" do
+    open(@file) { :end_of_block }.should == :end_of_block
+  end
+
   it "raises an ArgumentError if not passed one argument" do
     lambda { open }.should raise_error(ArgumentError)
   end
 
-  it "raises a TypeError if not passed a String type" do
-    lambda { open(nil)       }.should raise_error(TypeError)
-    lambda { open(7)         }.should raise_error(TypeError)
-    lambda { open(mock('x')) }.should raise_error(TypeError)
+  it "raises a TypeError if passed nil for path" do
+    lambda { open(nil) }.should raise_error(TypeError)
+  end
+
+  it "accepts String-like objects for path" do
+    file = mock('filename')
+    file.should_receive(:to_str).and_return(@file)
+    open(file) { :in_block }.should == :in_block
+  end
+
+  it "allows nil for mode" do
+    open(@file, nil) { |f| lambda { f << "some output" }.should raise_error(IOError) }
+  end
+
+  it "allows String-like objects for mode" do
+    mode = mock('mode')
+    mode.should_receive(:to_str).and_return("r")
+    open(@file, mode) { :in_block }.should == :in_block
+  end
+
+  it "allows nil for perm" do
+    open(@newfile, "w", nil) { }
+    File.writable?(@file).should be_true
+  end
+
+  it "allows Integer-like objects for perm" do
+    perm = mock('perm')
+    perm.should_receive(:to_int).and_return(0444)
+    open(@newfile, "w", perm) { }
+    File.writable?(@newfile).should be_false
   end
 end
 

Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/FileOps.cs

@@ -150,6 +150,9 @@ public static class Constants {
 
         #endregion
 
+        internal const int WriteModeMask = 0x80; // Oct 0200
+        internal const int ReadWriteMode = 0x1B6; // Oct 0666
+
         #region Public Singleton Methods
 
         [RubyMethod("open", RubyMethodAttributes.PublicSingleton)]
@@ -240,7 +243,7 @@ public static class Constants {
         internal static void Chmod(string path, int permission) {
 #if !SILVERLIGHT
             FileAttributes oldAttributes = File.GetAttributes(path);
-            if ((permission & 0x80) == 0) {
+            if ((permission & WriteModeMask) == 0) {
                 File.SetAttributes(path, oldAttributes | FileAttributes.ReadOnly);
             } else {
                 File.SetAttributes(path, oldAttributes & ~FileAttributes.ReadOnly);
@@ -267,7 +270,7 @@ public static class Constants {
             return RubyStatOps.CreateTime(RubyStatOps.Create(self.Context, path));
         }
 
-        private static bool FileExists(RubyContext/*!*/ context, string/*!*/ path) {
+        internal static bool FileExists(RubyContext/*!*/ context, string/*!*/ path) {
             return context.DomainManager.Platform.FileExists(path);
         }
 

Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/KernelOps.cs

@@ -466,49 +466,76 @@ public static class KernelOps {
 
         [RubyMethod("open", RubyMethodAttributes.PrivateInstance)]
         [RubyMethod("open", RubyMethodAttributes.PublicSingleton)]
-        public static RubyIO/*!*/ Open(RubyContext/*!*/ context, object self,
-            [DefaultProtocol, NotNull]MutableString/*!*/ path, [DefaultProtocol, Optional]MutableString mode, [DefaultProtocol, Optional]int permission) {
+        public static RubyIO/*!*/ Open(
+            RubyContext/*!*/ context, 
+            object self,
+            [DefaultProtocol, NotNull]MutableString/*!*/ path, 
+            [DefaultProtocol, Optional]MutableString mode, 
+            [DefaultProtocol, DefaultParameterValue(RubyFileOps.ReadWriteMode)]int permission) {
 
             string fileName = path.ConvertToString();
             if (fileName.Length > 0 && fileName[0] == '|') {
                 throw new NotImplementedError();
             }
+
+            bool existingFile = RubyFileOps.FileExists(context, path.ConvertToString());
+
             RubyIO file = new RubyFile(context, fileName, (mode != null) ? mode.ToString() : "r");
-            if (permission > 0) {
+
+            if (!existingFile) {
                 RubyFileOps.Chmod(fileName, permission);
             }
+
             return file;
         }
 
         [RubyMethod("open", RubyMethodAttributes.PrivateInstance)]
         [RubyMethod("open", RubyMethodAttributes.PublicSingleton)]
-        public static object Open(RubyContext/*!*/ context, [NotNull]BlockParam/*!*/ block, object self,
-            [DefaultProtocol, NotNull]MutableString/*!*/ path, [DefaultProtocol, Optional]MutableString mode, [DefaultProtocol, Optional]int permission) {
+        public static object Open(
+            RubyContext/*!*/ context, 
+            [NotNull]BlockParam/*!*/ block, 
+            object self,
+            [DefaultProtocol, NotNull]MutableString/*!*/ path, 
+            [DefaultProtocol, Optional]MutableString mode, 
+            [DefaultProtocol, DefaultParameterValue(RubyFileOps.ReadWriteMode)]int permission) {
 
             RubyIO file = Open(context, self, path, mode, permission);
             return OpenWithBlock(block, file);
         }
 
         [RubyMethod("open", RubyMethodAttributes.PrivateInstance)]
         [RubyMethod("open", RubyMethodAttributes.PublicSingleton)]
-        public static RubyIO/*!*/ Open(RubyContext/*!*/ context, object self,
-            [DefaultProtocol, NotNull]MutableString/*!*/ path, int mode, [DefaultProtocol, Optional]int permission) {
+        public static RubyIO/*!*/ Open(
+            RubyContext/*!*/ context, 
+            object self,
+            [DefaultProtocol, NotNull]MutableString/*!*/ path, 
+            int mode,
+            [DefaultProtocol, DefaultParameterValue(RubyFileOps.ReadWriteMode)]int permission) {
 
             string fileName = path.ConvertToString();
             if (fileName.Length > 0 && fileName[0] == '|') {
                 throw new NotImplementedError();
             }
+
+            bool existingFile = RubyFileOps.FileExists(context, path.ConvertToString());
+
             RubyIO file = new RubyFile(context, fileName, (RubyFileMode)mode);
-            if (permission > 0) {
+
+            if (!existingFile) {
                 RubyFileOps.Chmod(fileName, permission);
             }
             return file;
         }
 
         [RubyMethod("open", RubyMethodAttributes.PrivateInstance)]
         [RubyMethod("open", RubyMethodAttributes.PublicSingleton)]
-        public static object Open(RubyContext/*!*/ context, [NotNull]BlockParam/*!*/ block, object self,
-            [DefaultProtocol, NotNull]MutableString/*!*/ path, int mode, [DefaultProtocol, Optional]int permission) {
+        public static object Open(
+            RubyContext/*!*/ context, 
+            [NotNull]BlockParam/*!*/ block, 
+            object self,
+            [DefaultProtocol, NotNull]MutableString/*!*/ path, 
+            int mode,
+            [DefaultProtocol, DefaultParameterValue(RubyFileOps.ReadWriteMode)]int permission) {
 
             RubyIO file = Open(context, self, path, mode, permission);
             return OpenWithBlock(block, file);

Merlin/Main/Languages/Ruby/Ruby/Runtime/Calls/ProtocolConversionAction.cs

@@ -87,7 +87,7 @@ protected RubyConversionAction(RubyContext context)
                 return factory.Conversion<ConvertToStrAction>();
             }
 
-            // TODO: nullable int (see Array#fill, Sockets:ConvertToSocketFlag)
+            // TODO: nullable int (see Array#fill, Sockets:ConvertToSocketFlag, Kernel#open(perm=nil))
             if (parameterType == typeof(int)) {
                 return factory.Conversion<ConvertToFixnumAction>();
             }