Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve IO#read, IO#readpartial, IO#write and IO.copy_stream #521

Merged
merged 1 commit into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/builtin.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ interface _Reader
def read: (?int length, ?string outbuf) -> String?
end

interface _ReaderPartial
def readpartial: (int maxlen, ?string outbuf) -> String
end

interface _Writer
# Writes the +data+ string. Returns the number of bytes written
def write: (*_ToS data) -> Integer
Expand Down
152 changes: 147 additions & 5 deletions core/io.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,63 @@ class IO < Object

def puts: (*untyped arg0) -> NilClass

def read: (?Integer length, ?String outbuf) -> String?
# Reads *length* bytes from the I/O stream.
#
# *length* must be a non-negative integer or `nil`.
#
# If *length* is a positive integer, `read` tries to read *length* bytes without
# any conversion (binary mode). It returns `nil` if an EOF is encountered before
# anything can be read. Fewer than *length* bytes are returned if an EOF is
# encountered during the read. In the case of an integer *length*, the resulting
# string is always in ASCII-8BIT encoding.
#
# If *length* is omitted or is `nil`, it reads until EOF and the encoding
# conversion is applied, if applicable. A string is returned even if EOF is
# encountered before any data is read.
#
# If *length* is zero, it returns an empty string (`""`).
#
# If the optional *outbuf* argument is present, it must reference a String,
# which will receive the data. The *outbuf* will contain only the received data
# after the method call even if it is not empty at the beginning.
#
# When this method is called at end of file, it returns `nil` or `""`, depending
# on *length*: `read`, `read(nil)`, and `read(0)` return `""`,
# `read(*positive_integer*)` returns `nil`.
#
# f = File.new("testfile")
# f.read(16) #=> "This is line one"
#
# # read whole file
# open("file") do |f|
# data = f.read # This returns a string even if the file is empty.
# # ...
# end
#
# # iterate over fixed length records
# open("fixed-record-file") do |f|
# while record = f.read(256)
# # ...
# end
# end
#
# # iterate over variable length records,
# # each record is prefixed by its 32-bit length
# open("variable-record-file") do |f|
# while len = f.read(4)
# len = len.unpack("N")[0] # 32-bit length
# record = f.read(len) # This returns a string even if len is 0.
# end
# end
#
# Note that this method behaves like the fread() function in C. This means it
# retries to invoke read(2) system calls to read data with the specified length
# (or until EOF). This behavior is preserved even if *ios* is in non-blocking
# mode. (This method is non-blocking flag insensitive as other methods.) If you
# need the behavior like a single read(2) system call, consider #readpartial,
# #read_nonblock, and #sysread.
#
def read: (?Integer? length, ?String outbuf) -> String?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] Make the first argument type nilable because the API doc says:

length must be a non-negative integer or nil.

-def read: (?Integer length, ?String outbuf) -> String?
+def read: (?Integer? length, ?String outbuf) -> String?


def read_nonblock: (Integer len) -> String
| (Integer len, ?String buf) -> String
Expand All @@ -428,8 +484,63 @@ class IO < Object

def readlines: (?String sep, ?Integer limit) -> ::Array[String]

def readpartial: (Integer maxlen) -> String
| (Integer maxlen, ?String outbuf) -> String
# Reads at most *maxlen* bytes from the I/O stream. It blocks only if *ios* has
# no data immediately available. It doesn't block if some data available.
#
# If the optional *outbuf* argument is present, it must reference a String,
# which will receive the data. The *outbuf* will contain only the received data
# after the method call even if it is not empty at the beginning.
#
# It raises EOFError on end of file.
#
# readpartial is designed for streams such as pipe, socket, tty, etc. It blocks
# only when no data immediately available. This means that it blocks only when
# following all conditions hold.
# * the byte buffer in the IO object is empty.
# * the content of the stream is empty.
# * the stream is not reached to EOF.
#
#
# When readpartial blocks, it waits data or EOF on the stream. If some data is
# reached, readpartial returns with the data. If EOF is reached, readpartial
# raises EOFError.
#
# When readpartial doesn't blocks, it returns or raises immediately. If the byte
# buffer is not empty, it returns the data in the buffer. Otherwise if the
# stream has some content, it returns the data in the stream. Otherwise if the
# stream is reached to EOF, it raises EOFError.
#
# r, w = IO.pipe # buffer pipe content
# w << "abc" # "" "abc".
# r.readpartial(4096) #=> "abc" "" ""
# r.readpartial(4096) # blocks because buffer and pipe is empty.
#
# r, w = IO.pipe # buffer pipe content
# w << "abc" # "" "abc"
# w.close # "" "abc" EOF
# r.readpartial(4096) #=> "abc" "" EOF
# r.readpartial(4096) # raises EOFError
#
# r, w = IO.pipe # buffer pipe content
# w << "abc\ndef\n" # "" "abc\ndef\n"
# r.gets #=> "abc\n" "def\n" ""
# w << "ghi\n" # "def\n" "ghi\n"
# r.readpartial(4096) #=> "def\n" "" "ghi\n"
# r.readpartial(4096) #=> "ghi\n" "" ""
#
# Note that readpartial behaves similar to sysread. The differences are:
# * If the byte buffer is not empty, read from the byte buffer instead of
# "sysread for buffered IO (IOError)".
# * It doesn't cause Errno::EWOULDBLOCK and Errno::EINTR. When readpartial
# meets EWOULDBLOCK and EINTR by read system call, readpartial retry the
# system call.
#
#
# The latter means that readpartial is nonblocking-flag insensitive. It blocks
# on the situation IO#sysread causes Errno::EWOULDBLOCK as if the fd is blocking
# mode.
#
def readpartial: (Integer maxlen, ?String outbuf) -> String
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] Simplify overloading.

-def readpartial: (Integer maxlen) -> String
-               | (Integer maxlen, ?String outbuf) -> String
+def readpartial: (Integer maxlen, ?String outbuf) -> String


def reopen: (IO other_IO_or_path) -> IO
| (String other_IO_or_path, ?String mode_str) -> IO
Expand Down Expand Up @@ -508,7 +619,19 @@ class IO < Object

def ungetc: (String arg0) -> NilClass

def write: (*_ToS arg0) -> Integer
# Writes the given strings to *ios*. The stream must be opened for writing.
# Arguments that are not a string will be converted to a string using `to_s`.
# Returns the number of bytes written in total.
#
# count = $stdout.write("This is", " a test\n")
# puts "That was #{count} bytes of data"
#
# *produces:*
#
# This is a test
# That was 15 bytes of data
#
def write: (*_ToS string) -> Integer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] Rename the argument (arg0 -> string) for readability according to the API doc.


# Opens the file, optionally seeks to the given *offset*, then returns *length*
# bytes (defaulting to the rest of the file). #binread ensures the file is
Expand All @@ -525,7 +648,26 @@ class IO < Object
#
def self.binwrite: (String name, _ToS string, ?Integer offset, ?mode: String mode) -> Integer

def self.copy_stream: (_Reader src, _Writer dst, ?Integer copy_length, ?Integer src_offset) -> Integer
# IO.copy_stream copies *src* to *dst*. *src* and *dst* is either a filename or
# an IO-like object. IO-like object for *src* should have #readpartial or #read
# method. IO-like object for *dst* should have #write method. (Specialized
# mechanisms, such as sendfile system call, may be used on appropriate
# situation.)
#
# This method returns the number of bytes copied.
#
# If optional arguments are not given, the start position of the copy is the
# beginning of the filename or the current file offset of the IO. The end
# position of the copy is the end of file.
#
# If *copy_length* is given, No more than *copy_length* bytes are copied.
#
# If *src_offset* is given, it specifies the start position of the copy.
#
# When *src_offset* is specified and *src* is an IO, IO.copy_stream doesn't move
# the current file offset.
#
def self.copy_stream: ((String | _Reader | _ReaderPartial) src, (String | _Writer) dst, ?Integer copy_length, ?Integer src_offset) -> Integer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] Make the argument types more accurate according to the API doc:

IO.copy_stream copies src to dst. src and dst is either a filename or an IO-like object. IO-like object for src should have #readpartial or #read method. IO-like object for dst should have #write method.

-def self.copy_stream: (_Reader src, _Writer dst, ?Integer copy_length, ?Integer src_offset) -> Integer
+def self.copy_stream: ((String | _Reader | _ReaderPartial) src, (String | _Writer) dst, ?Integer copy_length, ?Integer src_offset) -> Integer

I've added a new interface _ReaderPartial (like _Reader) to core/builtin.rbs.
Please teach me if there is a better name than _ReaderPartial.


def self.popen: (*untyped args) -> untyped

Expand Down
90 changes: 90 additions & 0 deletions test/stdlib/IO_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,94 @@ def test_binwrite
IO, :binwrite, filename, content, 0, mode: "a"
end
end

def test_copy_stream
Dir.mktmpdir do |dir|
src_name = File.join(dir, "src_file").tap { |f| IO.write(f, "foo") }
dst_name = File.join(dir, "dst_file")

assert_send_type "(String, String) -> Integer",
IO, :copy_stream, src_name, dst_name
assert_send_type "(String, String, Integer) -> Integer",
IO, :copy_stream, src_name, dst_name, 1
assert_send_type "(String, String, Integer, Integer) -> Integer",
IO, :copy_stream, src_name, dst_name, 1, 0

File.open(dst_name, "w") do |dst_io|
assert_send_type "(String, IO) -> Integer",
IO, :copy_stream, src_name, dst_io
assert_send_type "(String, IO, Integer) -> Integer",
IO, :copy_stream, src_name, dst_io, 1
assert_send_type "(String, IO, Integer, Integer) -> Integer",
IO, :copy_stream, src_name, dst_io, 1, 0
end

File.open(src_name) do |src_io|
assert_send_type "(IO, String) -> Integer",
IO, :copy_stream, src_io, dst_name
assert_send_type "(IO, String, Integer) -> Integer",
IO, :copy_stream, src_io, dst_name, 1
assert_send_type "(IO, String, Integer, Integer) -> Integer",
IO, :copy_stream, src_io, dst_name, 1, 0
end

File.open(src_name) do |src_io|
File.open(dst_name, "w") do |dst_io|
assert_send_type "(IO, IO) -> Integer",
IO, :copy_stream, src_io, dst_io
assert_send_type "(IO, IO, Integer) -> Integer",
IO, :copy_stream, src_io, dst_io, 1
assert_send_type "(IO, IO, Integer, Integer) -> Integer",
IO, :copy_stream, src_io, dst_io, 1, 0
end
end
end
end
end

class IOInstanceTest < Minitest::Test
include TypeAssertions

testing "::IO"

def test_read
IO.open(IO.sysopen(__FILE__)) do |io|
assert_send_type "() -> String",
io, :read
assert_send_type "(Integer) -> String",
io, :read, 0
assert_send_type "(Integer) -> nil",
io, :read, 1
assert_send_type "(nil) -> String",
io, :read, nil
assert_send_type "(Integer, String) -> String",
io, :read, 0, "buffer"
assert_send_type "(Integer, String) -> nil",
io, :read, 1, "buffer"
assert_send_type "(nil, String) -> String",
io, :read, nil, "buffer"
end
end

def test_readpartial
IO.open(IO.sysopen(__FILE__)) do |io|
assert_send_type "(Integer) -> String",
io, :readpartial, 10
assert_send_type "(Integer, String) -> String",
io, :readpartial, 10, "buffer"
end
end

def test_write
Dir.mktmpdir do |dir|
File.open(File.join(dir, "some_file"), "w") do |io|
assert_send_type "() -> Integer",
io, :write
assert_send_type "(String) -> Integer",
io, :write, "foo"
assert_send_type "(String, Float) -> Integer",
io, :write, "foo", 1.5
end
end
end
end