Align remote file reads with IO length semantics

lib/net/sftp/operations/file.rb

@@ -63,6 +63,15 @@ def eof?
     #
     # This will advance the file pointer (#pos).
     def read(n=nil)
+      unless n.nil? || n.is_a?(Integer)
+        converted = n.to_int if n.respond_to?(:to_int)
+        raise TypeError, "no implicit conversion of #{n.class} into Integer" unless converted.is_a?(Integer)
+
+        n = converted
+      end
+      raise ArgumentError, "negative length #{n} given" if n && n < 0
+      return "" if n == 0
+
       loop do
         break if n && @buffer.length >= n
         break unless fill
@@ -74,6 +83,8 @@ def read(n=nil)
         result, @buffer = @buffer, ""
       end
 
+      return nil if n && result.empty? && @real_eof
+
       @pos += result.length
       return result
     end
@@ -181,9 +192,11 @@ def stat
       # Fills the buffer. Returns +true+ if it succeeded, and +false+ if
       # EOF was encountered before any data was read.
       def fill
+        return false if @real_eof
+
         data = sftp.read!(handle, @real_pos, 8192)
 
-        if data.nil?
+        if data.nil? || data.empty?
           @real_eof = true
           return false
         else

test/test_file.rb

@@ -58,6 +58,41 @@ def test_read_with_argument_should_read_and_return_n_bytes_and_set_pos
     assert_equal 5, @file.pos
   end
 
+  def test_read_with_positive_length_at_eof_should_return_nil
+    @sftp.expects(:read!).once.returns(nil)
+    assert_nil @file.read(1)
+    assert_nil @file.read(1)
+    assert_equal 0, @file.pos
+  end
+
+  def test_read_with_zero_length_should_not_read_remotely
+    @sftp.expects(:read!).never
+    assert_equal "", @file.read(0)
+  end
+
+  def test_read_with_negative_length_should_raise_argument_error
+    @sftp.expects(:read!).never
+    assert_raises(ArgumentError) { @file.read(-1) }
+  end
+
+  def test_read_should_coerce_length_with_to_int
+    length = Object.new
+    length.define_singleton_method(:to_int) { 5 }
+    @sftp.expects(:read!).returns("hello world")
+    assert_equal "hello", @file.read(length)
+  end
+
+  def test_read_should_reject_non_integer_lengths
+    @sftp.expects(:read!).never
+    assert_raises(TypeError) { @file.read("5") }
+  end
+
+  def test_read_should_treat_empty_data_response_as_eof
+    @sftp.expects(:read!).once.returns("")
+    assert_nil @file.read(1)
+    assert @file.eof?
+  end
+
   def test_read_after_pos_assignment_should_read_from_specified_position
     @sftp.expects(:read!).with("handle", 5, 8192).returns("hello world")
     @file.pos = 5