Skip to content

Commit

Permalink
Merge pull request #12330 from koic/change_make_style_auto_resource_c…
Browse files Browse the repository at this point in the history
…leanup_aware_of_tempfile_open

[Fix #12328] Make `Style/AutoResourceCleanup` aware of `Tempfile.open`
  • Loading branch information
koic committed Nov 5, 2023
2 parents 6230707 + 4d9efdc commit d3de2ae
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12328](https://github.com/rubocop/rubocop/issues/12328): Make `Style/AutoResourceCleanup` aware of `Tempfile.open`. ([@koic][])
35 changes: 21 additions & 14 deletions lib/rubocop/cop/style/auto_resource_cleanup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,38 @@ module Style
# File.open('file') do |f|
# # ...
# end
#
# # bad
# f = Tempfile.open('temp')
#
# # good
# Tempfile.open('temp') do |f|
# # ...
# end
class AutoResourceCleanup < Base
MSG = 'Use the block version of `%<class>s.%<method>s`.'

TARGET_METHODS = { File: :open }.freeze
MSG = 'Use the block version of `%<current>s`.'
RESTRICT_ON_SEND = %i[open].freeze

RESTRICT_ON_SEND = TARGET_METHODS.values.freeze
# @!method file_open_method?(node)
def_node_matcher :file_open_method?, <<~PATTERN
(send (const {nil? cbase} {:File :Tempfile}) :open ...)
PATTERN

def on_send(node)
TARGET_METHODS.each do |target_class, target_method|
next unless node.method?(target_method)
return if !file_open_method?(node) || cleanup?(node)

target_receiver = s(:const, nil, target_class)
next if node.receiver != target_receiver
current = node.receiver.source_range.begin.join(node.selector.end).source

next if cleanup?(node)

add_offense(node, message: format(MSG, class: target_class, method: target_method))
end
add_offense(node, message: format(MSG, current: current))
end

private

def cleanup?(node)
parent = node.parent
node.block_argument? || (parent && (parent.block_type? || !parent.lvasgn_type?))
return true if node.block_argument?
return false unless (parent = node.parent)

parent.block_type? || !parent.lvasgn_type?
end
end
end
Expand Down
21 changes: 21 additions & 0 deletions spec/rubocop/cop/style/auto_resource_cleanup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,27 @@
RUBY
end

it 'registers an offense for Tempfile.open without block' do
expect_offense(<<~RUBY)
Tempfile.open("filename")
^^^^^^^^^^^^^^^^^^^^^^^^^ Use the block version of `Tempfile.open`.
RUBY
end

it 'registers an offense for ::File.open without block' do
expect_offense(<<~RUBY)
::File.open("filename")
^^^^^^^^^^^^^^^^^^^^^^^ Use the block version of `::File.open`.
RUBY
end

it 'registers an offense for ::Tempfile.open without block' do
expect_offense(<<~RUBY)
::Tempfile.open("filename")
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the block version of `::Tempfile.open`.
RUBY
end

it 'does not register an offense for File.open with block' do
expect_no_offenses('File.open("file") { |f| something }')
end
Expand Down

0 comments on commit d3de2ae

Please sign in to comment.