From 4c662128b01f819b13e690e4b8cce9ca2fd85e04 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 29 Sep 2024 13:55:44 +0300 Subject: [PATCH 1/4] Rename `ThreadSafety::InstanceVariableInClassMethod` to `ThreadSafety::ClassInstanceVariable` --- lib/rubocop-thread_safety.rb | 2 +- ...s_method.rb => class_instance_variable.rb} | 6 +-- ...pec.rb => class_instance_variable_spec.rb} | 42 ++++++++++--------- 3 files changed, 26 insertions(+), 24 deletions(-) rename lib/rubocop/cop/thread_safety/{instance_variable_in_class_method.rb => class_instance_variable.rb} (97%) rename spec/rubocop/cop/thread_safety/{instance_variable_in_class_method_spec.rb => class_instance_variable_spec.rb} (88%) diff --git a/lib/rubocop-thread_safety.rb b/lib/rubocop-thread_safety.rb index ccd0e72..c246c4e 100644 --- a/lib/rubocop-thread_safety.rb +++ b/lib/rubocop-thread_safety.rb @@ -10,7 +10,7 @@ require 'rubocop/cop/mixin/operation_with_threadsafe_result' -require 'rubocop/cop/thread_safety/instance_variable_in_class_method' +require 'rubocop/cop/thread_safety/class_instance_variable' 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' diff --git a/lib/rubocop/cop/thread_safety/instance_variable_in_class_method.rb b/lib/rubocop/cop/thread_safety/class_instance_variable.rb similarity index 97% rename from lib/rubocop/cop/thread_safety/instance_variable_in_class_method.rb rename to lib/rubocop/cop/thread_safety/class_instance_variable.rb index 10de928..4128d2b 100644 --- a/lib/rubocop/cop/thread_safety/instance_variable_in_class_method.rb +++ b/lib/rubocop/cop/thread_safety/class_instance_variable.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module ThreadSafety - # Avoid instance variables in class methods. + # Avoid class instance variables. # # @example # # bad @@ -61,8 +61,8 @@ module ThreadSafety # # module_function :test # end - class InstanceVariableInClassMethod < Base - MSG = 'Avoid instance variables in class methods.' + class ClassInstanceVariable < Base + MSG = 'Avoid class instance variables.' RESTRICT_ON_SEND = %i[ instance_variable_set instance_variable_get diff --git a/spec/rubocop/cop/thread_safety/instance_variable_in_class_method_spec.rb b/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb similarity index 88% rename from spec/rubocop/cop/thread_safety/instance_variable_in_class_method_spec.rb rename to spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb index 5eb322b..e53b185 100644 --- a/spec/rubocop/cop/thread_safety/instance_variable_in_class_method_spec.rb +++ b/spec/rubocop/cop/thread_safety/class_instance_variable_spec.rb @@ -1,12 +1,14 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::ThreadSafety::InstanceVariableInClassMethod, :config do +RSpec.describe RuboCop::Cop::ThreadSafety::ClassInstanceVariable, :config do + let(:msg) { 'Avoid class instance variables.' } + it 'registers an offense for assigning to an ivar in a class method' do expect_offense(<<~RUBY) class Test def self.some_method(params) @params = params - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end end RUBY @@ -112,7 +114,7 @@ class Test Class.new do def self.area @area ||= some_computation - ^^^^^ Avoid instance variables in class methods. + ^^^^^ #{msg} end end end @@ -125,7 +127,7 @@ def self.area class Test def self.some_method do_work(@params) - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end end RUBY @@ -136,7 +138,7 @@ def self.some_method module ClassMethods def some_method(params) @params = params - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end end RUBY @@ -148,7 +150,7 @@ module Test class_methods do def some_method(params) @params = params - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end end end @@ -161,7 +163,7 @@ class Test class << self def some_method(params) @params = params - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end end end @@ -173,7 +175,7 @@ def some_method(params) class Test define_singleton_method(:some_method) do |params| @params = params - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end end RUBY @@ -184,7 +186,7 @@ class Test class Test def self.some_method do_work(instance_variable_get(:@params)) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end RUBY @@ -196,7 +198,7 @@ class Test class << self def some_method(name, params) instance_variable_set(:"@\#{name}", params) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end end @@ -208,7 +210,7 @@ def some_method(name, params) module ClassMethods def some_method(params) instance_variable_set(:@params, params) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end RUBY @@ -220,7 +222,7 @@ module Test class_methods do def some_method(params) instance_variable_set(:@params, params) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end end @@ -232,7 +234,7 @@ def some_method(params) class Test define_singleton_method(:some_method) do |params| instance_variable_set(:@params, params) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end RUBY @@ -245,7 +247,7 @@ module Test def some_method(params) instance_variable_set(:@params, params) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end RUBY @@ -256,7 +258,7 @@ def some_method(params) module Test def some_method(params) instance_variable_set(:@params, params) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end module_function :some_method @@ -271,7 +273,7 @@ module Test def some_method(params) @params = params - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end end RUBY @@ -282,7 +284,7 @@ def some_method(params) module Test def some_method(params) @params = params - ^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^ #{msg} end module_function :some_method @@ -295,7 +297,7 @@ def some_method(params) def separate_with(separator) Example.class_eval do @separator = separator - ^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^ #{msg} end end RUBY @@ -304,7 +306,7 @@ def separate_with(separator) def separate_with(separator) ::Example.class_eval do @separator = separator - ^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^ #{msg} end end RUBY @@ -315,7 +317,7 @@ def separate_with(separator) def separate_with(separator) Example.class_exec do @separator = separator - ^^^^^^^^^^ Avoid instance variables in class methods. + ^^^^^^^^^^ #{msg} end end RUBY From e340d16dd1a5d0a27d3c02e58101519983cb1b85 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 29 Sep 2024 13:55:55 +0300 Subject: [PATCH 2/4] Regenerate documentation --- docs/modules/ROOT/pages/cops.adoc | 2 +- .../modules/ROOT/pages/cops_threadsafety.adoc | 54 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index f868ec7..dcb236e 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -1,7 +1,7 @@ === Department xref:cops_threadsafety.adoc[ThreadSafety] * xref:cops_threadsafety.adoc#threadsafetyclassandmoduleattributes[ThreadSafety/ClassAndModuleAttributes] -* xref:cops_threadsafety.adoc#threadsafetyinstancevariableinclassmethod[ThreadSafety/InstanceVariableInClassMethod] +* xref:cops_threadsafety.adoc#threadsafetyclassinstancevariable[ThreadSafety/ClassInstanceVariable] * xref:cops_threadsafety.adoc#threadsafetymutableclassinstancevariable[ThreadSafety/MutableClassInstanceVariable] * xref:cops_threadsafety.adoc#threadsafetynewthread[ThreadSafety/NewThread] * xref:cops_threadsafety.adoc#threadsafetydirchdir[ThreadSafety/DirChdir] diff --git a/docs/modules/ROOT/pages/cops_threadsafety.adoc b/docs/modules/ROOT/pages/cops_threadsafety.adoc index 3ed2cee..f8151e8 100644 --- a/docs/modules/ROOT/pages/cops_threadsafety.adoc +++ b/docs/modules/ROOT/pages/cops_threadsafety.adoc @@ -42,32 +42,7 @@ end | Boolean |=== -== 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") - -# bad -FileUtils.chdir("/var/run") ----- - -== ThreadSafety/InstanceVariableInClassMethod +== ThreadSafety/ClassInstanceVariable |=== | Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed @@ -79,7 +54,7 @@ FileUtils.chdir("/var/run") | - |=== -Avoid instance variables in class methods. +Avoid class instance variables. === Examples @@ -142,6 +117,31 @@ module Example 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") + +# bad +FileUtils.chdir("/var/run") +---- + == ThreadSafety/MutableClassInstanceVariable |=== From ea413bd75b5e7f80f290d746555a14609c3ad201 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 29 Sep 2024 13:58:32 +0300 Subject: [PATCH 3/4] Add `CHANGELOG.md` entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8ece4c..6d98be4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## master (unreleased) +* [#59](https://github.com/rubocop/rubocop-thread_safety/pull/59): Rename `ThreadSafety::InstanceVariableInClassMethod` cop to `ThreadSafety::ClassInstanceVariable` to better reflect its purpose. ([@viralpraxis](https://github.com/viralpraxis)) * [#55](https://github.com/rubocop/rubocop-thread_safety/pull/55): Enhance `ThreadSafety::InstanceVariableInClassMethod` cop to detect offenses within `class_eval/exec` blocks. ([@viralpraxis](https://github.com/viralpraxis)) * [#54](https://github.com/rubocop/rubocop-thread_safety/pull/54): Drop support for RuboCop older than 1.48. ([@viralpraxis](https://github.com/viralpraxis)) * [#52](https://github.com/rubocop/rubocop-thread_safety/pull/52): Add new `RackMiddlewareInstanceVariable` cop to detect instance variables in Rack middleware. ([@viralpraxis](https://github.com/viralpraxis)) From 77c65acb05b7ed47acbced1e34e50e1758d5fe2b Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 29 Sep 2024 23:09:41 +0300 Subject: [PATCH 4/4] Add config obsoletion --- config/obsoletion.yml | 2 ++ lib/rubocop/thread_safety.rb | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 config/obsoletion.yml diff --git a/config/obsoletion.yml b/config/obsoletion.yml new file mode 100644 index 0000000..000f830 --- /dev/null +++ b/config/obsoletion.yml @@ -0,0 +1,2 @@ +renamed: + ThreadSafety/InstanceVariableInClassMethod: ThreadSafety/ClassInstanceVariable diff --git a/lib/rubocop/thread_safety.rb b/lib/rubocop/thread_safety.rb index a393186..a024fb7 100644 --- a/lib/rubocop/thread_safety.rb +++ b/lib/rubocop/thread_safety.rb @@ -8,5 +8,7 @@ module ThreadSafety CONFIG = YAML.safe_load(CONFIG_DEFAULT.read).freeze private_constant(:CONFIG_DEFAULT, :PROJECT_ROOT) + + ::RuboCop::ConfigObsoletion.files << PROJECT_ROOT.join('config', 'obsoletion.yml') end end