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

Add ThreadSafety::DirChdir cop #36

Merged
merged 2 commits into from
Sep 7, 2024
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 config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ ThreadSafety/NewThread:
Avoid starting new threads.
Let a framework like Sidekiq handle the threads.
Enabled: true

ThreadSafety/DirChdir:
Description: Avoid using `Dir.chdir` due to its process-wide effect.
Enabled: true
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
* xref:cops_threadsafety.adoc#threadsafetyinstancevariableinclassmethod[ThreadSafety/InstanceVariableInClassMethod]
* xref:cops_threadsafety.adoc#threadsafetymutableclassinstancevariable[ThreadSafety/MutableClassInstanceVariable]
* xref:cops_threadsafety.adoc#threadsafetynewthread[ThreadSafety/NewThread]
* xref:cops_threadsafety.adoc#threadsafetydirchdir[ThreadSafety/DirChdir]
22 changes: 22 additions & 0 deletions docs/modules/ROOT/pages/cops_threadsafety.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@ class User
end
----

== ThreadSafety/DirChdir

|===
| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed

| Enabled
| Yes
| No
| -
| -
|===

Avoid using `Dir.chdir` due to its process-wide effect.

=== Examples

[source,ruby]
----
# bad
Dir.chdir("/var/run")
----

== ThreadSafety/InstanceVariableInClassMethod

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop-thread_safety.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@
require 'rubocop/cop/thread_safety/class_and_module_attributes'
require 'rubocop/cop/thread_safety/mutable_class_instance_variable'
require 'rubocop/cop/thread_safety/new_thread'
require 'rubocop/cop/thread_safety/dir_chdir'
26 changes: 26 additions & 0 deletions lib/rubocop/cop/thread_safety/dir_chdir.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true

module RuboCop
module Cop
module ThreadSafety
# Avoid using `Dir.chdir` due to its process-wide effect.
#
# @example
# # bad
# Dir.chdir("/var/run")
class DirChdir < Base
MSG = 'Avoid using `Dir.chdir` due to its process-wide effect.'
RESTRICT_ON_SEND = %i[chdir].freeze

# @!method dir_chdir?(node)
def_node_matcher :dir_chdir?, <<~MATCHER
(send (const {nil? cbase} :Dir) :chdir ...)
MATCHER

def on_send(node)
dir_chdir?(node) { add_offense(node) }
end
end
end
end
end
40 changes: 40 additions & 0 deletions spec/rubocop/cop/thread_safety/dir_chdir_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::ThreadSafety::DirChdir, :config do
let(:msg) { 'Avoid using `Dir.chdir` due to its process-wide effect.' }

context 'with `Dir.chdir` method' do
it 'registers an offense' do
expect_offense(<<~RUBY)
Dir.chdir("/var/run")
^^^^^^^^^^^^^^^^^^^^^ #{msg}
RUBY
end

it 'registers an offense when called without arguments' do
expect_offense(<<~RUBY)
Dir.chdir
^^^^^^^^^ #{msg}
RUBY
end

it 'registers an offense with top-level constant' do
expect_offense(<<~RUBY)
::Dir.chdir("/var/run")
^^^^^^^^^^^^^^^^^^^^^^^ #{msg}
RUBY
end
end

context 'with another `Dir` class method' do
it 'does not register an offense' do
expect_no_offenses 'Dir.pwd'
end
end

context 'when received is not `Dir`' do
it 'does not register an offense' do
expect_no_offenses 'chdir("/tmp")'
end
end
end