Use named parameters for `File::new`.

This is a breaking change, but now is the time to do this as we've
already done the same for `Entry::new`.
This commit is contained in:
Robert Haines 2021-06-08 21:24:52 +01:00
parent e1e1cab39c
commit f033ae760d
14 changed files with 45 additions and 63 deletions

View File

@ -104,12 +104,11 @@ Style/NumericPredicate:
- 'lib/zip/ioextras.rb' - 'lib/zip/ioextras.rb'
- 'lib/zip/ioextras/abstract_input_stream.rb' - 'lib/zip/ioextras/abstract_input_stream.rb'
# Offense count: 6 # Offense count: 1
# Configuration parameters: AllowedMethods. # Configuration parameters: AllowedMethods.
# AllowedMethods: respond_to_missing? # AllowedMethods: respond_to_missing?
Style/OptionalBooleanParameter: Style/OptionalBooleanParameter:
Exclude: Exclude:
- 'lib/zip/file.rb'
- 'lib/zip/file_split.rb' - 'lib/zip/file_split.rb'
# Offense count: 17 # Offense count: 17

View File

@ -50,7 +50,7 @@ input_filenames = ['image.jpg', 'description.txt', 'stats.csv']
zipfile_name = "/Users/me/Desktop/archive.zip" zipfile_name = "/Users/me/Desktop/archive.zip"
Zip::File.open(zipfile_name, Zip::File::CREATE) do |zipfile| Zip::File.open(zipfile_name, create: true) do |zipfile|
input_filenames.each do |filename| input_filenames.each do |filename|
# Two arguments: # Two arguments:
# - The name of the file as it will appear in the archive # - The name of the file as it will appear in the archive
@ -89,7 +89,7 @@ class ZipFileGenerator
def write def write
entries = Dir.entries(@input_dir) - %w[. ..] entries = Dir.entries(@input_dir) - %w[. ..]
::Zip::File.open(@output_file, ::Zip::File::CREATE) do |zipfile| ::Zip::File.open(@output_file, create: true) do |zipfile|
write_entries entries, '', zipfile write_entries entries, '', zipfile
end end
end end
@ -318,7 +318,7 @@ Where X is an integer between 0 and 9, inclusive. If this option is set to 0 (`Z
This can also be set for each archive as an option to `Zip::File`: This can also be set for each archive as an option to `Zip::File`:
```ruby ```ruby
Zip::File.open('foo.zip', Zip::File::CREATE, {compression_level: 9}) do |zip| Zip::File.open('foo.zip', create:true, compression_level: 9) do |zip|
zip.add ... zip.add ...
end end
``` ```

View File

@ -25,7 +25,7 @@ module Zip
# #
# require 'zip' # require 'zip'
# #
# Zip::File.open("my.zip", Zip::File::CREATE) { # Zip::File.open("my.zip", create: true) {
# |zipfile| # |zipfile|
# zipfile.get_output_stream("first.txt") { |f| f.puts "Hello from ZipFile" } # zipfile.get_output_stream("first.txt") { |f| f.puts "Hello from ZipFile" }
# zipfile.mkdir("a_dir") # zipfile.mkdir("a_dir")
@ -37,7 +37,7 @@ module Zip
# #
# require 'zip' # require 'zip'
# #
# Zip::File.open("my.zip", Zip::File::CREATE) { # Zip::File.open("my.zip", create: true) {
# |zipfile| # |zipfile|
# puts zipfile.read("first.txt") # puts zipfile.read("first.txt")
# zipfile.remove("first.txt") # zipfile.remove("first.txt")
@ -49,7 +49,6 @@ module Zip
class File < CentralDirectory class File < CentralDirectory
extend FileSplit extend FileSplit
CREATE = true
IO_METHODS = [:tell, :seek, :read, :eof, :close].freeze IO_METHODS = [:tell, :seek, :read, :eof, :close].freeze
attr_reader :name attr_reader :name
@ -66,9 +65,9 @@ module Zip
# Returns the zip files comment, if it has one # Returns the zip files comment, if it has one
attr_accessor :comment attr_accessor :comment
# Opens a zip archive. Pass true as the second parameter to create # Opens a zip archive. Pass create: true to create
# a new archive if it doesn't exist already. # a new archive if it doesn't exist already.
def initialize(path_or_io, create = false, buffer = false, options = {}) def initialize(path_or_io, create: false, buffer: false, **options)
super() super()
options = DEFAULT_RESTORE_OPTIONS options = DEFAULT_RESTORE_OPTIONS
.merge(compression_level: ::Zip.default_compression) .merge(compression_level: ::Zip.default_compression)
@ -115,8 +114,8 @@ module Zip
# Similar to ::new. If a block is passed the Zip::File object is passed # Similar to ::new. If a block is passed the Zip::File object is passed
# to the block and is automatically closed afterwards, just as with # to the block and is automatically closed afterwards, just as with
# ruby's builtin File::open method. # ruby's builtin File::open method.
def open(file_name, create = false, options = {}) def open(file_name, create: false, **options)
zf = ::Zip::File.new(file_name, create, false, options) zf = ::Zip::File.new(file_name, create: create, **options)
return zf unless block_given? return zf unless block_given?
begin begin
@ -129,7 +128,7 @@ module Zip
# Same as #open. But outputs data to a buffer instead of a file # Same as #open. But outputs data to a buffer instead of a file
def add_buffer def add_buffer
io = ::StringIO.new io = ::StringIO.new
zf = ::Zip::File.new(io, true, true) zf = ::Zip::File.new(io, create: true, buffer: true)
yield zf yield zf
zf.write_buffer(io) zf.write_buffer(io)
end end
@ -138,7 +137,7 @@ module Zip
# stream, and outputs data to a buffer. # stream, and outputs data to a buffer.
# (This can be used to extract data from a # (This can be used to extract data from a
# downloaded zip archive without first saving it to disk.) # downloaded zip archive without first saving it to disk.)
def open_buffer(io, options = {}) def open_buffer(io, **options)
unless IO_METHODS.map { |method| io.respond_to?(method) }.all? || io.kind_of?(String) unless IO_METHODS.map { |method| io.respond_to?(method) }.all? || io.kind_of?(String)
raise 'Zip::File.open_buffer expects a String or IO-like argument' \ raise 'Zip::File.open_buffer expects a String or IO-like argument' \
"(responds to #{IO_METHODS.join(', ')}). Found: #{io.class}" "(responds to #{IO_METHODS.join(', ')}). Found: #{io.class}"
@ -149,7 +148,7 @@ module Zip
# https://github.com/rubyzip/rubyzip/issues/119 # https://github.com/rubyzip/rubyzip/issues/119
io.binmode if io.respond_to?(:binmode) io.binmode if io.respond_to?(:binmode)
zf = ::Zip::File.new(io, true, true, options) zf = ::Zip::File.new(io, create: true, buffer: true, **options)
return zf unless block_given? return zf unless block_given?
yield zf yield zf

View File

@ -21,7 +21,7 @@ module Zip
# #
# require 'zip/filesystem' # require 'zip/filesystem'
# #
# Zip::File.open("my.zip", Zip::File::CREATE) { # Zip::File.open("my.zip", create: true) {
# |zipfile| # |zipfile|
# zipfile.file.open("first.txt", "w") { |f| f.puts "Hello world" } # zipfile.file.open("first.txt", "w") { |f| f.puts "Hello world" }
# zipfile.dir.mkdir("mydir") # zipfile.dir.mkdir("mydir")

View File

@ -9,7 +9,7 @@ EXAMPLE_ZIP = 'filesystem.zip'
File.delete(EXAMPLE_ZIP) if File.exist?(EXAMPLE_ZIP) File.delete(EXAMPLE_ZIP) if File.exist?(EXAMPLE_ZIP)
Zip::File.open(EXAMPLE_ZIP, Zip::File::CREATE) do |zf| Zip::File.open(EXAMPLE_ZIP, create: true) do |zf|
zf.file.open('file1.txt', 'w') { |os| os.write 'first file1.txt' } zf.file.open('file1.txt', 'w') { |os| os.write 'first file1.txt' }
zf.dir.mkdir('dir1') zf.dir.mkdir('dir1')
zf.dir.chdir('dir1') zf.dir.chdir('dir1')

View File

@ -23,7 +23,7 @@ class ZipFileGenerator
def write def write
entries = Dir.entries(@input_dir) - %w[. ..] entries = Dir.entries(@input_dir) - %w[. ..]
::Zip::File.open(@output_file, ::Zip::File::CREATE) do |zipfile| ::Zip::File.open(@output_file, create: true) do |zipfile|
write_entries entries, '', zipfile write_entries entries, '', zipfile
end end
end end

View File

@ -19,7 +19,7 @@ class ZipCaseSensitivityTest < MiniTest::Test
::Zip.case_insensitive_match = false ::Zip.case_insensitive_match = false
SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) } SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) }
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE) zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
SRC_FILES.each { |fn, en| zf.add(en, fn) } SRC_FILES.each { |fn, en| zf.add(en, fn) }
zf.close zf.close
@ -38,7 +38,7 @@ class ZipCaseSensitivityTest < MiniTest::Test
::Zip.case_insensitive_match = true ::Zip.case_insensitive_match = true
SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) } SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) }
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE) zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
assert_raises Zip::EntryExistsError do assert_raises Zip::EntryExistsError do
SRC_FILES.each { |fn, en| zf.add(en, fn) } SRC_FILES.each { |fn, en| zf.add(en, fn) }
@ -50,7 +50,7 @@ class ZipCaseSensitivityTest < MiniTest::Test
::Zip.case_insensitive_match = false ::Zip.case_insensitive_match = false
SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) } SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) }
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE) zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
SRC_FILES.each { |fn, en| zf.add(en, fn) } SRC_FILES.each { |fn, en| zf.add(en, fn) }
zf.close zf.close

View File

@ -167,7 +167,7 @@ class ZipEntryTest < MiniTest::Test
z.write_zip64_support = false z.write_zip64_support = false
end end
zipfile = Zip::File.open(tmp_zip, Zip::File::CREATE) zipfile = Zip::File.open(tmp_zip, create: true)
mimetype_entry = Zip::Entry.new( mimetype_entry = Zip::Entry.new(
zipfile, # @zipfile zipfile, # @zipfile

View File

@ -99,7 +99,7 @@ class ZipFileExtractTest < MiniTest::Test
true_size = 500_000 true_size = 500_000
fake_size = 1 fake_size = 1
::Zip::File.open(real_zip, ::Zip::File::CREATE) do |zf| ::Zip::File.open(real_zip, create: true) do |zf|
zf.get_output_stream(file_name) do |os| zf.get_output_stream(file_name) do |os|
os.write 'a' * true_size os.write 'a' * true_size
end end

View File

@ -30,13 +30,13 @@ class FileOptionsTest < MiniTest::Test
::FileUtils.cp(TXTPATH, TXTPATH_755) ::FileUtils.cp(TXTPATH, TXTPATH_755)
::File.chmod(0o755, TXTPATH_755) ::File.chmod(0o755, TXTPATH_755)
::Zip::File.open(ZIPPATH, true) do |zip| ::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH) zip.add(ENTRY_1, TXTPATH)
zip.add(ENTRY_2, TXTPATH_600) zip.add(ENTRY_2, TXTPATH_600)
zip.add(ENTRY_3, TXTPATH_755) zip.add(ENTRY_3, TXTPATH_755)
end end
::Zip::File.open(ZIPPATH, false, restore_permissions: true) do |zip| ::Zip::File.open(ZIPPATH, restore_permissions: true) do |zip|
zip.extract(ENTRY_1, EXTPATH_1) zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2) zip.extract(ENTRY_2, EXTPATH_2)
zip.extract(ENTRY_3, EXTPATH_3) zip.extract(ENTRY_3, EXTPATH_3)
@ -54,13 +54,13 @@ class FileOptionsTest < MiniTest::Test
::FileUtils.cp(TXTPATH, TXTPATH_755) ::FileUtils.cp(TXTPATH, TXTPATH_755)
::File.chmod(0o755, TXTPATH_755) ::File.chmod(0o755, TXTPATH_755)
::Zip::File.open(ZIPPATH, true) do |zip| ::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH) zip.add(ENTRY_1, TXTPATH)
zip.add(ENTRY_2, TXTPATH_600) zip.add(ENTRY_2, TXTPATH_600)
zip.add(ENTRY_3, TXTPATH_755) zip.add(ENTRY_3, TXTPATH_755)
end end
::Zip::File.open(ZIPPATH, false, restore_permissions: false) do |zip| ::Zip::File.open(ZIPPATH, restore_permissions: false) do |zip|
zip.extract(ENTRY_1, EXTPATH_1) zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2) zip.extract(ENTRY_2, EXTPATH_2)
zip.extract(ENTRY_3, EXTPATH_3) zip.extract(ENTRY_3, EXTPATH_3)
@ -79,7 +79,7 @@ class FileOptionsTest < MiniTest::Test
::FileUtils.cp(TXTPATH, TXTPATH_755) ::FileUtils.cp(TXTPATH, TXTPATH_755)
::File.chmod(0o755, TXTPATH_755) ::File.chmod(0o755, TXTPATH_755)
::Zip::File.open(ZIPPATH, true) do |zip| ::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH) zip.add(ENTRY_1, TXTPATH)
zip.add(ENTRY_2, TXTPATH_600) zip.add(ENTRY_2, TXTPATH_600)
zip.add(ENTRY_3, TXTPATH_755) zip.add(ENTRY_3, TXTPATH_755)
@ -97,12 +97,12 @@ class FileOptionsTest < MiniTest::Test
end end
def test_restore_times_true def test_restore_times_true
::Zip::File.open(ZIPPATH, true) do |zip| ::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH) zip.add(ENTRY_1, TXTPATH)
zip.add_stored(ENTRY_2, TXTPATH) zip.add_stored(ENTRY_2, TXTPATH)
end end
::Zip::File.open(ZIPPATH, false, restore_times: true) do |zip| ::Zip::File.open(ZIPPATH, restore_times: true) do |zip|
zip.extract(ENTRY_1, EXTPATH_1) zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2) zip.extract(ENTRY_2, EXTPATH_2)
end end
@ -112,12 +112,12 @@ class FileOptionsTest < MiniTest::Test
end end
def test_restore_times_false def test_restore_times_false
::Zip::File.open(ZIPPATH, true) do |zip| ::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH) zip.add(ENTRY_1, TXTPATH)
zip.add_stored(ENTRY_2, TXTPATH) zip.add_stored(ENTRY_2, TXTPATH)
end end
::Zip::File.open(ZIPPATH, false, restore_times: false) do |zip| ::Zip::File.open(ZIPPATH, restore_times: false) do |zip|
zip.extract(ENTRY_1, EXTPATH_1) zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2) zip.extract(ENTRY_2, EXTPATH_2)
end end
@ -127,7 +127,7 @@ class FileOptionsTest < MiniTest::Test
end end
def test_restore_times_true_as_default def test_restore_times_true_as_default
::Zip::File.open(ZIPPATH, true) do |zip| ::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH) zip.add(ENTRY_1, TXTPATH)
zip.add_stored(ENTRY_2, TXTPATH) zip.add_stored(ENTRY_2, TXTPATH)
end end

View File

@ -48,7 +48,7 @@ class FilePermissionsTest < MiniTest::Test
end end
def create_files def create_files
::Zip::File.open(ZIPNAME, ::Zip::File::CREATE) do |zip| ::Zip::File.open(ZIPNAME, create: true) do |zip|
zip.comment = 'test' zip.comment = 'test'
end end

View File

@ -32,21 +32,7 @@ class ZipFileTest < MiniTest::Test
def test_create_from_scratch def test_create_from_scratch
comment = 'a short comment' comment = 'a short comment'
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE) zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
zf.get_output_stream('myFile') { |os| os.write 'myFile contains just this' }
zf.mkdir('dir1')
zf.comment = comment
zf.close
zf_read = ::Zip::File.new(EMPTY_FILENAME)
assert_equal(comment, zf_read.comment)
assert_equal(2, zf_read.entries.length)
end
def test_create_from_scratch_with_old_create_parameter
comment = 'a short comment'
zf = ::Zip::File.new(EMPTY_FILENAME, 1)
zf.get_output_stream('myFile') { |os| os.write 'myFile contains just this' } zf.get_output_stream('myFile') { |os| os.write 'myFile contains just this' }
zf.mkdir('dir1') zf.mkdir('dir1')
zf.comment = comment zf.comment = comment
@ -184,7 +170,7 @@ class ZipFileTest < MiniTest::Test
end end
def test_cleans_up_tempfiles_after_close def test_cleans_up_tempfiles_after_close
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE) zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
zf.get_output_stream('myFile') do |os| zf.get_output_stream('myFile') do |os|
@tempfile_path = os.path @tempfile_path = os.path
os.write 'myFile contains just this' os.write 'myFile contains just this'
@ -208,9 +194,7 @@ class ZipFileTest < MiniTest::Test
sizes = [] sizes = []
files.each do |name, comp| files.each do |name, comp|
zf = ::Zip::File.new( zf = ::Zip::File.new(name, create: true, compression_level: comp)
name, ::Zip::File::CREATE, false, { compression_level: comp }
)
zf.add(entry_name, src_file) zf.add(entry_name, src_file)
zf.close zf.close
@ -243,7 +227,7 @@ class ZipFileTest < MiniTest::Test
files.each do |name, comp| files.each do |name, comp|
::Zip.default_compression = comp ::Zip.default_compression = comp
zf = ::Zip::File.new(name, ::Zip::File::CREATE) zf = ::Zip::File.new(name, create: true)
zf.add(entry_name, src_file) zf.add(entry_name, src_file)
zf.close zf.close
@ -268,7 +252,7 @@ class ZipFileTest < MiniTest::Test
src_file = 'test/data/file2.txt' src_file = 'test/data/file2.txt'
entry_name = 'newEntryName.rb' entry_name = 'newEntryName.rb'
assert(::File.exist?(src_file)) assert(::File.exist?(src_file))
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE) zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
zf.add_stored(entry_name, src_file) zf.add_stored(entry_name, src_file)
zf.close zf.close
@ -294,7 +278,7 @@ class ZipFileTest < MiniTest::Test
::File.chmod(0o664, src_zip) ::File.chmod(0o664, src_zip)
assert_equal(0o100664, ::File.stat(src_zip).mode) assert_equal(0o100664, ::File.stat(src_zip).mode)
zf = ::Zip::File.new(src_zip, ::Zip::File::CREATE) zf = ::Zip::File.new(src_zip, create: true)
zf.add('newEntryName.rb', 'test/data/file2.txt') zf.add('newEntryName.rb', 'test/data/file2.txt')
zf.close zf.close
@ -385,7 +369,7 @@ class ZipFileTest < MiniTest::Test
::File.unlink(zf_name) if ::File.exist?(zf_name) ::File.unlink(zf_name) if ::File.exist?(zf_name)
arr = [] arr = []
arr_renamed = [] arr_renamed = []
::Zip::File.open(zf_name, ::Zip::File::CREATE) do |zf| ::Zip::File.open(zf_name, create: true) do |zf|
zf.mkdir('test') zf.mkdir('test')
arr << 'test/' arr << 'test/'
arr_renamed << 'Ztest/' arr_renamed << 'Ztest/'
@ -398,7 +382,7 @@ class ZipFileTest < MiniTest::Test
zf = ::Zip::File.open(zf_name) zf = ::Zip::File.open(zf_name)
assert_equal(zf.entries.map(&:name), arr) assert_equal(zf.entries.map(&:name), arr)
zf.close zf.close
Zip::File.open(zf_name, 'wb') do |z| Zip::File.open(zf_name) do |z|
z.each do |f| z.each do |f|
z.rename(f, "Z#{f.name}") z.rename(f, "Z#{f.name}")
end end
@ -522,7 +506,7 @@ class ZipFileTest < MiniTest::Test
def test_double_commit(filename = 'test/data/generated/double_commit_test.zip') def test_double_commit(filename = 'test/data/generated/double_commit_test.zip')
::FileUtils.touch('test/data/generated/test_double_commit1.txt') ::FileUtils.touch('test/data/generated/test_double_commit1.txt')
::FileUtils.touch('test/data/generated/test_double_commit2.txt') ::FileUtils.touch('test/data/generated/test_double_commit2.txt')
zf = ::Zip::File.open(filename, ::Zip::File::CREATE) zf = ::Zip::File.open(filename, create: true)
zf.add('test1.txt', 'test/data/generated/test_double_commit1.txt') zf.add('test1.txt', 'test/data/generated/test_double_commit1.txt')
zf.commit zf.commit
zf.add('test2.txt', 'test/data/generated/test_double_commit2.txt') zf.add('test2.txt', 'test/data/generated/test_double_commit2.txt')
@ -695,7 +679,7 @@ class ZipFileTest < MiniTest::Test
def test_streaming def test_streaming
fname = ::File.join(__dir__, '..', 'README.md') fname = ::File.join(__dir__, '..', 'README.md')
zname = 'test/data/generated/README.zip' zname = 'test/data/generated/README.zip'
Zip::File.open(zname, Zip::File::CREATE) do |zipfile| Zip::File.open(zname, create: true) do |zipfile|
zipfile.get_output_stream(File.basename(fname)) do |f| zipfile.get_output_stream(File.basename(fname)) do |f|
f.puts File.read(fname) f.puts File.read(fname)
end end

View File

@ -53,7 +53,7 @@ class ZipUnicodeFileNamesAndComments < MiniTest::Test
def test_unicode_comment def test_unicode_comment
str = '渠道升级' str = '渠道升级'
::Zip::File.open(FILENAME, Zip::File::CREATE) do |z| ::Zip::File.open(FILENAME, create: true) do |z|
z.comment = str z.comment = str
end end

View File

@ -21,7 +21,7 @@ class Zip64FullTest < MiniTest::Test
last_text = 'this tests files starting after 4GB in the archive' last_text = 'this tests files starting after 4GB in the archive'
comment_text = 'this is a file comment in a zip64 archive' comment_text = 'this is a file comment in a zip64 archive'
::Zip::File.open(HUGE_ZIP, ::Zip::File::CREATE) do |zf| ::Zip::File.open(HUGE_ZIP, create: true) do |zf|
zf.comment = comment_text zf.comment = comment_text
zf.get_output_stream('first_file.txt') do |io| zf.get_output_stream('first_file.txt') do |io|