From 8fce82fc5559726db652c9554f077f02b3b2e3e4 Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Sat, 14 Sep 2024 12:20:16 +0200 Subject: [PATCH] Fix Style/FileWrite offenses Also refactor specs to use `join` instead of `+` because it's shorter, clearer, and uses less memory, even if it is slightly less performant in terms of IPS Ref: #276 --- .rubocop_todo.yml | 8 -------- bin/cvheathen | 4 ++-- lib/document.rb | 2 +- lib/legacy_converter.rb | 2 +- spec/lib/document_spec.rb | 30 +++++++++++++++--------------- 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 03cfdb8..0b19db9 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -311,13 +311,6 @@ Style/FetchEnvVar: Exclude: - 'lib/heathen/processor_methods/libreoffice.rb' -# This cop supports safe autocorrection (--autocorrect). -Style/FileWrite: - Exclude: - - 'bin/cvheathen' - - 'lib/document.rb' - - 'lib/legacy_converter.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle. # SupportedStyles: format, sprintf, percent @@ -518,7 +511,6 @@ Style/StringConcatenation: - 'lib/heathen.rb' - 'lib/heathen/processor.rb' - 'lib/sidekiq_app.rb' - - 'spec/lib/document_spec.rb' - 'spec/lib/sidekiq_workers_spec.rb' - 'unicorn.rb' diff --git a/bin/cvheathen b/bin/cvheathen index 498fead..50023a0 100755 --- a/bin/cvheathen +++ b/bin/cvheathen @@ -39,7 +39,7 @@ files = [] if in_file.file? abort "Output is a directory, but expected a file" if out_file.directory? content = converter.convert action, File.read(in_file), language - File.open(out_file, 'wb') { |f| f.write content } + File.binwrite(out_file, content) elsif in_file.directory? abort "Invalid output directory" unless out_file.directory? Dir.glob(in_file + (recurse ? '**/*' : '*')).each do |file| @@ -49,7 +49,7 @@ elsif in_file.directory? content = converter.convert action, File.read(file), language new_file = Heathen::Filename.suggest_in_new_dir file, content.mime_type, in_file.to_s, out_file.to_s Pathname.new(new_file).parent.mkpath - File.open(new_file, 'wb') { |f| f.write content } + File.binwrite(new_file, content) rescue StandardError => e logger.error "Failed to convert #{file}: #{e.message}" end diff --git a/lib/document.rb b/lib/document.rb index 64283a2..58e5fd7 100644 --- a/lib/document.rb +++ b/lib/document.rb @@ -145,7 +145,7 @@ def add_file(version, filename, body, author = nil) body = StringIO.new(body) unless body.respond_to?(:read) # string -> IO File.open(directory + version + filename, "wb") { |f| IO.copy_stream(body, f) } - File.open(directory + version + AUTHOR_FILE, 'w') { |f| f.write author } + File.write(directory + version + AUTHOR_FILE, author) end # Sets the specified version as current. diff --git a/lib/legacy_converter.rb b/lib/legacy_converter.rb index 06f8f9d..d22b895 100644 --- a/lib/legacy_converter.rb +++ b/lib/legacy_converter.rb @@ -35,7 +35,7 @@ def convert_file(action, orig_content, language = 'en') # Stores the specified file in the legacy directory def store_file(filename, content) - File.open(@legacy_dir + filename, 'wb') { |f| f.write content } + File.binwrite(@legacy_dir + filename, content) end # Loads and returns a legacy converted file diff --git a/spec/lib/document_spec.rb b/spec/lib/document_spec.rb index 2ca389b..f61f449 100644 --- a/spec/lib/document_spec.rb +++ b/spec/lib/document_spec.rb @@ -71,9 +71,7 @@ describe '#directory' do it 'runs' do - dir = document.directory - expect(dir).not_to be_nil - expect(File.exist?(dir)).to be true + expect(document.directory).to exist end end @@ -133,7 +131,7 @@ it 'runs' do version = document.new_version expect(version).not_to be_nil - expect(File.exist?(document.directory + version)).to be true + expect(document.directory.join(version)).to exist new_doc = described_class.load storage_dir, doc_key expect(new_doc.versions.include?(version)).to be true end @@ -144,31 +142,31 @@ file = __FILE__ body = File.read(file) document.add_file 'v002', File.basename(file), body - expect(File.exist?(document.directory + 'v002' + File.basename(file))).to be true + expect(document.directory.join('v002', File.basename(file))).to exist + expect(document.directory.join('v002', described_class::AUTHOR_FILE).read.chomp).to eq '' end it 'runs with author' do file = __FILE__ body = File.read(file) document.add_file 'v002', File.basename(file), body, author - expect(File.exist?(document.directory + 'v002' + File.basename(file))).to be true - expect(File.exist?(document.directory + 'v002' + described_class::AUTHOR_FILE)).to be true - expect(File.read(document.directory + 'v002' + described_class::AUTHOR_FILE).chomp).to eq author + expect(document.directory.join('v002', File.basename(file))).to exist + expect(document.directory.join('v002', described_class::AUTHOR_FILE).read.chomp).to eq author end it 'runs with IO for body' do file = __FILE__ body = File.open(file) document.add_file 'v002', File.basename(file), body - expect(File.exist?(document.directory + 'v002' + File.basename(file))).to be true + expect(document.directory.join('v002', File.basename(file))).to exist end end describe '#set_current' do it 'runs' do document.set_current 'v001' - st1 = File.stat(document.directory + 'current') - st2 = File.stat(document.directory + 'v001') + st1 = document.directory.join('current').stat + st2 = document.directory.join('v001').stat expect(st1.ino).to eq st2.ino end @@ -188,7 +186,7 @@ describe '#delete_version' do it 'runs' do document.delete_version 'v001' - expect(File.exist?(document.directory + 'v001')).to be false + expect(document.directory.join('v001')).not_to exist end it 'refuses to delete "current"' do @@ -266,9 +264,11 @@ describe '#save_metadata' do it 'runs' do document.save_metadata - expect(File.exist?(document.directory + 'metadata.json')).to be true - # expect this to pass - JSON.parse File.read(document.directory + 'metadata.json') + expect(document.directory.join('metadata.json')).to exist + + expect do + JSON.parse document.directory.join('metadata.json').read + end.not_to raise_error end end end