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.

Sign up for GitHub

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

Add new Style/FileTouch cop #13484

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Add new Style/FileTouch cop #13484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions changelog/new_add_style_touch_file_cop.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#13484](https://github.com/rubocop/rubocop/pull/13484): Add new `Style/FileTouch` cop. ([@lovro-bikic][])
6 changes: 6 additions & 0 deletions config/default.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3947,6 +3947,12 @@ Style/FileRead:
Enabled: pending
VersionAdded: '1.24'

Style/FileTouch:
Description: 'Favor `FileUtils.touch` for touching files.'
Enabled: pending
VersionAdded: '<<next>>'
SafeAutoCorrect: false

Style/FileWrite:
Description: 'Favor `File.(bin)write` convenience methods.'
StyleGuide: '#file-write'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@
require_relative 'rubocop/cop/style/fetch_env_var'
require_relative 'rubocop/cop/style/file_empty'
require_relative 'rubocop/cop/style/file_read'
require_relative 'rubocop/cop/style/file_touch'
require_relative 'rubocop/cop/style/file_write'
require_relative 'rubocop/cop/style/float_division'
require_relative 'rubocop/cop/style/for'
Expand Down
75 changes: 75 additions & 0 deletions lib/rubocop/cop/style/file_touch.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
Copy link
Contributor Author

@lovro-bikic lovro-bikic Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm wondering if this should be a Lint cop instead. In the wild, I've seen File.open used to make sure a file exists or to try update its timestamps (much like touch utility), the latter of which actually doesn't happen:

path = 'foo.tmp'
FileUtils.touch(path) # create a file

atime_1, ctime_1, mtime_1 = File.atime(path), File.ctime(path), File.mtime(path)
File.open(path, 'a') {}
atime_2, ctime_2, mtime_2 = File.atime(path), File.ctime(path), File.mtime(path)

atime_1 == atime_2 && ctime_1 == ctime_2 && mtime_1 == mtime_2
# => true

so this could be considered erroneous code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not on the fence about changing the department, but I think it'd be good to add this to the cop's rationale and examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've updated the docs, let me know if I made it clear enough.

# Checks for usage of `File.open` in append mode with empty block.
#
# Such a usage only creates a new file, but it doesn't update
# timestamps for an existing file, which might have been the intention.
#
# For example, for an existing file `foo.txt`:
#
# ruby -e "puts File.mtime('foo.txt')"
# # 2024-11-26 12:17:23 +0100
#
# ruby -e "File.open('foo.txt', 'a') {}"
#
# ruby -e "puts File.mtime('foo.txt')"
# # 2024-11-26 12:17:23 +0100 -> unchanged
#
# If the intention was to update timestamps, `FileUtils.touch('foo.txt')`
# should be used instead.
#
# @safety
lovro-bikic marked this conversation as resolved.
Show resolved Hide resolved
# Autocorrection is unsafe for this cop because unlike `File.open`,
# `FileUtils.touch` updates an existing file's timestamps.
#
# @example
# # bad
# File.open(filename, 'a') {}
# File.open(filename, 'a+') {}
#
# # good
# FileUtils.touch(filename)
#
class FileTouch < Base
extend AutoCorrector

MSG = 'Use `FileUtils.touch(%<argument>s)` instead of `File.open` in ' \
'append mode with empty block.'

RESTRICT_ON_SEND = %i[open].freeze

APPEND_FILE_MODES = %w[a a+ ab a+b at a+t].to_set.freeze
lovro-bikic marked this conversation as resolved.
Show resolved Hide resolved

# @!method file_open?(node)
def_node_matcher :file_open?, <<~PATTERN
(send
(const {nil? cbase} :File) :open
$(...)
(str %APPEND_FILE_MODES))
PATTERN

def on_send(node)
filename = file_open?(node)
parent = node.parent

return unless filename
return unless parent && empty_block?(parent)

message = format(MSG, argument: filename.source)
add_offense(parent, message: message) do |corrector|
corrector.replace(parent, "FileUtils.touch(#{filename.source})")
end
end

private

def empty_block?(node)
node.block_type? && !node.body
end
end
end
end
end
32 changes: 32 additions & 0 deletions spec/rubocop/cop/style/file_touch_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::FileTouch, :config do
it 'registers an offense when using `File.open` in append mode with empty block' do
expect_offense(<<~RUBY)
File.open(filename, 'a') {}
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `FileUtils.touch(filename)` instead of `File.open` in append mode with empty block.
RUBY

expect_correction(<<~RUBY)
FileUtils.touch(filename)
RUBY
end

it 'does not register an offense when using `File.open` in append mode without a block' do
expect_no_offenses(<<~RUBY)
File.open(filename, 'a')
RUBY
end

it 'does not register an offense when using `File.open` in write mode' do
expect_no_offenses(<<~RUBY)
File.open(filename, 'w') {}
RUBY
end

it 'does not register an offense when using `File.open` without an access mode' do
expect_no_offenses(<<~RUBY)
File.open(filename) {}
RUBY
end
end