From f5fa9bf1fa52a534652180e50034739cba059e89 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 17:19:44 +0300 Subject: [PATCH 01/19] Provide basic implementation --- lib/rubocop-thread_safety.rb | 1 + .../rack_middleware_instance_variable.rb | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb diff --git a/lib/rubocop-thread_safety.rb b/lib/rubocop-thread_safety.rb index 78866a6..f29d7ff 100644 --- a/lib/rubocop-thread_safety.rb +++ b/lib/rubocop-thread_safety.rb @@ -13,3 +13,4 @@ require 'rubocop/cop/thread_safety/mutable_class_instance_variable' require 'rubocop/cop/thread_safety/new_thread' require 'rubocop/cop/thread_safety/dir_chdir' +require 'rubocop/cop/thread_safety/rack_middleware_instance_variable' diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb new file mode 100644 index 0000000..3ae4bff --- /dev/null +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module ThreadSafety + # Avoid instance variables in Rack middleware. + class RackMiddlewareInstanceVariable < Base + MSG = 'Avoid instance variables in rack middleware.' + + # @!method rack_middleware_application_variable(node) + def_node_search :rack_middleware_application_variable, <<~MATCHER + (class + (const nil _) + nil + ) + MATCHER + + # @!method rack_middleware_like_class?(node) + def_node_matcher :rack_middleware_like_class?, <<~MATCHER + (class (const nil? _) nil? (begin <(def :initialize (args (arg _)) ...) (def :call ...)>)) + MATCHER + + # @!method constructor_method(node) + def_node_search :constructor_method, <<~MATCHER + (def :initialize (args (arg $_)) `(ivasgn $_ (lvar $_))) + MATCHER + + def on_class(node) + return unless rack_middleware_like_class?(node) + return unless (application_variable = extract_application_variable_from_class_node(node)) + + node.each_node(:def) do |method_definition_node| + method_definition_node.each_node(*%i[ivar ivasgn]) do |ivar_node| + assignable, = ivar_node.to_a + next if assignable == application_variable + + add_offense ivar_node + end + end + end + + private + + def extract_application_variable_from_class_node(class_node) + require 'pry' + + class_node + .each_node(:def) + .find { |node| node.method?(:initialize) && node.arguments.size == 1 } + .then { |node| constructor_method(node) } + .then { |variables| variables.first[1] if variables.first } + end + end + end + end +end From 98f9decc224219b592e5016f562e5ec7e6a6b9c0 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 17:19:56 +0300 Subject: [PATCH 02/19] Add tests for basic implementation --- .../rack_middleware_instance_variable_spec.rb | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb new file mode 100644 index 0000000..0e22f7d --- /dev/null +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::ThreadSafety::RackMiddlewareInstanceVariable, :config do + let(:msg) { 'Avoid instance variables in rack middleware.' } + + context 'with unrelated source' do + it { expect_no_offenses '' } + + specify do + expect_no_offenses(<<~RUBY) + class SomeClass + def initialize(user) + @user = user + end + end + RUBY + end + + specify do + expect_no_offenses(<<~RUBY) + class SomeClass + def initialize(user, context) + @user = user + @context = context + end + end + RUBY + end + + specify do + expect_no_offenses(<<~RUBY) + class SomeClass + def initialize(user, context) + @user = user + @context = context + end + + def call + [@user, @context] + end + end + RUBY + end + + specify do + expect_no_offenses(<<~RUBY) + class SomeClass + def initialize(user, context) + @user = user + @context = context + end + + def call(env) + [@user, @context] + end + end + RUBY + end + + specify do + expect_no_offenses(<<~RUBY) + class SomeClass + def initialize(app) + @user = User.new + end + + def call(env) + TOPLEVEL_BINDING + end + end + RUBY + end + end + + it 'registers an offense' do # rubocop:disable RSpec/ExampleLength + expect_offense(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + @foo = 1 + ^^^^^^^^ #{msg} + end + + def call(env) + @app.call(env) + end + end + RUBY + end + + it 'registers an offense with mismatched local and instance variables' do # rubocop:disable RSpec/ExampleLength + expect_offense(<<~RUBY) + class TestMiddleware + def initialize(app) + @foo = fsa + ^^^^^^^^^^ #{msg} + @a = app + end + + def call(env) + @a.call(env) + end + end + RUBY + end + + it 'registers an offense for nested middleware' do # rubocop:disable RSpec/ExampleLength + expect_offense(<<~RUBY) + module MyMiddlewares + class TestMiddleware + def initialize(app) + @app = app + @foo = 1 + ^^^^^^^^ #{msg} + end + + def call(env) + @app.call(env) + end + end + end + RUBY + end + + it 'registers an offense for multiple middlewares' do # rubocop:disable RSpec/ExampleLength + expect_offense(<<~RUBY) + module MyMiddlewares + class TestMiddleware + def initialize(app) + @app = app + @foo = 1 + ^^^^^^^^ #{msg} + end + + def call(env) + @app.call(env) + end + end + + class TestMiddleware2 + def initialize(app) + @app = app + @foo = 1 + ^^^^^^^^ #{msg} + end + + def call(env) + @app.call(env) + end + end + end + RUBY + end + + it 'registers an offense with `call` before constructor definition' do # rubocop:disable RSpec/ExampleLength + expect_offense(<<~RUBY) + class TestMiddleware + def call(env) + @app.call(env) + end + + def initialize(app) + @app = app + @foo = 1 + ^^^^^^^^ #{msg} + end + end + RUBY + end +end From 7181a39aad7524dc2708c6543be4b2c7294842b5 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 17:20:46 +0300 Subject: [PATCH 03/19] Remove development artifact --- .../cop/thread_safety/rack_middleware_instance_variable.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index 3ae4bff..0cc2e9e 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -42,8 +42,6 @@ def on_class(node) private def extract_application_variable_from_class_node(class_node) - require 'pry' - class_node .each_node(:def) .find { |node| node.method?(:initialize) && node.arguments.size == 1 } From 3636f77c335f8c12529883d2f9939f8fed0057f8 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 19:46:18 +0300 Subject: [PATCH 04/19] Make cop more precise --- .rubocop.yml | 2 + .../rack_middleware_instance_variable.rb | 12 +-- .../rack_middleware_instance_variable_spec.rb | 79 +++++++++++++++++-- 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index fd05c0f..5118607 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -81,6 +81,8 @@ Style/SymbolArray: RSpec/ExampleLength: Max: 11 + Exclude: + - spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb RSpec/ContextWording: Exclude: diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index 0cc2e9e..f4f7101 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -7,17 +7,9 @@ module ThreadSafety class RackMiddlewareInstanceVariable < Base MSG = 'Avoid instance variables in rack middleware.' - # @!method rack_middleware_application_variable(node) - def_node_search :rack_middleware_application_variable, <<~MATCHER - (class - (const nil _) - nil - ) - MATCHER - # @!method rack_middleware_like_class?(node) def_node_matcher :rack_middleware_like_class?, <<~MATCHER - (class (const nil? _) nil? (begin <(def :initialize (args (arg _)) ...) (def :call ...)>)) + (class (const nil? _) nil? (begin <(def :initialize (args (arg _)) ...) (def :call (args (arg _)) ...) ...>)) MATCHER # @!method constructor_method(node) @@ -30,7 +22,7 @@ def on_class(node) return unless (application_variable = extract_application_variable_from_class_node(node)) node.each_node(:def) do |method_definition_node| - method_definition_node.each_node(*%i[ivar ivasgn]) do |ivar_node| + method_definition_node.each_node(:ivasgn, :ivar) do |ivar_node| assignable, = ivar_node.to_a next if assignable == application_variable diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index 0e22f7d..e4d3af9 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -65,14 +65,43 @@ def initialize(app) end def call(env) - TOPLEVEL_BINDING + @x = TOPLEVEL_BINDING + end + end + RUBY + end + + specify do + expect_no_offenses(<<~RUBY) + class SomeClass + def initialize(app) + @app = app + @user = User.new + end + + def call(env, user) + @x = TOPLEVEL_BINDING end end RUBY end end - it 'registers an offense' do # rubocop:disable RSpec/ExampleLength + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + end + end + RUBY + end + + it 'registers an offense' do expect_offense(<<~RUBY) class TestMiddleware def initialize(app) @@ -88,7 +117,7 @@ def call(env) RUBY end - it 'registers an offense with mismatched local and instance variables' do # rubocop:disable RSpec/ExampleLength + it 'registers an offense with mismatched local and instance variables' do expect_offense(<<~RUBY) class TestMiddleware def initialize(app) @@ -104,7 +133,7 @@ def call(env) RUBY end - it 'registers an offense for nested middleware' do # rubocop:disable RSpec/ExampleLength + it 'registers an offense for nested middleware' do expect_offense(<<~RUBY) module MyMiddlewares class TestMiddleware @@ -122,7 +151,7 @@ def call(env) RUBY end - it 'registers an offense for multiple middlewares' do # rubocop:disable RSpec/ExampleLength + it 'registers an offense for multiple middlewares' do expect_offense(<<~RUBY) module MyMiddlewares class TestMiddleware @@ -152,7 +181,45 @@ def call(env) RUBY end - it 'registers an offense with `call` before constructor definition' do # rubocop:disable RSpec/ExampleLength + it 'registers an offense for extra methods' do + expect_offense(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + @a = 1 + ^^^^^^ #{msg} + end + + def foo + @a = 1 + ^^^^^^ #{msg} + end + end + RUBY + + expect_offense(<<~RUBY) + class TestMiddleware + def foo + @a = 1 + ^^^^^^ #{msg} + end + + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + end + end + RUBY + end + + it 'registers an offense with `call` before constructor definition' do expect_offense(<<~RUBY) class TestMiddleware def call(env) From 7f734a3347085f84939a956457ee8ff449ecb5f0 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 19:55:15 +0300 Subject: [PATCH 05/19] Handle `instance_variable_get` and `instance_variable_set` calls --- .../rack_middleware_instance_variable.rb | 6 +++ .../rack_middleware_instance_variable_spec.rb | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index f4f7101..7391727 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -7,6 +7,8 @@ module ThreadSafety class RackMiddlewareInstanceVariable < Base MSG = 'Avoid instance variables in rack middleware.' + RESTRICT_ON_SEND = %i[instance_variable_get instance_variable_set].freeze + # @!method rack_middleware_like_class?(node) def_node_matcher :rack_middleware_like_class?, <<~MATCHER (class (const nil? _) nil? (begin <(def :initialize (args (arg _)) ...) (def :call (args (arg _)) ...) ...>)) @@ -31,6 +33,10 @@ def on_class(node) end end + def on_send(node) + add_offense node + end + private def extract_application_variable_from_class_node(class_node) diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index e4d3af9..5258dc8 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -217,6 +217,28 @@ def call(env) end end RUBY + + expect_offense(<<~RUBY) + class TestMiddleware + def foo + @a = 1 + ^^^^^^ #{msg} + end + + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + end + + def bar + @b = 1 + ^^^^^^ #{msg} + end + end + RUBY end it 'registers an offense with `call` before constructor definition' do @@ -234,4 +256,24 @@ def initialize(app) end RUBY end + + context 'with `instance_variable_set` and `instance_variable_get` methods' do + it 'registers an offense' do + expect_offense(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + instance_variable_set(:counter, 1) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in rack middleware. + end + + def call(env) + @app.call(env) + instance_variable_get(:@counter) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in rack middleware. + end + end + RUBY + end + end end From 5f1f267676c4a0084658a08c9acdef0fcfac7949 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 20:44:18 +0300 Subject: [PATCH 06/19] Specify cop config defaults --- .vscode/settings.json | 3 +++ config/default.yml | 9 +++++++++ 2 files changed, 12 insertions(+) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..7134e38 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "simplecov-vscode.enabled": false +} diff --git a/config/default.yml b/config/default.yml index 4f844e5..ad8fb28 100644 --- a/config/default.yml +++ b/config/default.yml @@ -35,3 +35,12 @@ ThreadSafety/NewThread: ThreadSafety/DirChdir: Description: Avoid using `Dir.chdir` due to its process-wide effect. Enabled: true + +ThreadSafety/RackMiddlewareInstanceVariable: + Description: Avoid instance variables in rack middleware. + Enabled: true + Include: + - 'app/middleware/**/*.rb' + - 'lib/middleware/**/*.rb' + - 'app/middlewares/**/*.rb' + - 'lib/middlewares/**/*.rb' From 98be947a32a1c23afb18f37fe79597f69f165be6 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 21:03:57 +0300 Subject: [PATCH 07/19] Apply fixes --- config/default.yml | 2 +- .../rack_middleware_instance_variable.rb | 43 ++++++++++++++++++- .../rack_middleware_instance_variable_spec.rb | 42 ++++++++++++++++-- 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/config/default.yml b/config/default.yml index ad8fb28..046e130 100644 --- a/config/default.yml +++ b/config/default.yml @@ -37,7 +37,7 @@ ThreadSafety/DirChdir: Enabled: true ThreadSafety/RackMiddlewareInstanceVariable: - Description: Avoid instance variables in rack middleware. + Description: Avoid instance variables in Rack middleware. Enabled: true Include: - 'app/middleware/**/*.rb' diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index 7391727..396233f 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -3,9 +3,48 @@ module RuboCop module Cop module ThreadSafety - # Avoid instance variables in Rack middleware. + # Avoid instance variables in rack middleware. + # + # @example + # # bad + # class CounterMiddleware + # def initialize(app) + # @app = app + # @counter = 0 + # end + # + # def call(env) + # app.call(env) + # ensure + # @counter += 1 + # end + # end + # + # # good + # class CounterMiddleware + # def initialize(app) + # @app = app + # @counter = Concurrent::AtomicReference.new(0) + # end + # + # def call(env) + # app.call(env) + # ensure + # @counter.update { |ref| ref + 1 } + # end + # end + # + # class IdentityMiddleware + # def initialize(app) + # @app = app + # end + # + # def call(env) + # app.call(env) + # end + # end class RackMiddlewareInstanceVariable < Base - MSG = 'Avoid instance variables in rack middleware.' + MSG = 'Avoid instance variables in Rack middleware.' RESTRICT_ON_SEND = %i[instance_variable_get instance_variable_set].freeze diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index 5258dc8..37b9f6e 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::ThreadSafety::RackMiddlewareInstanceVariable, :config do - let(:msg) { 'Avoid instance variables in rack middleware.' } + let(:msg) { 'Avoid instance variables in Rack middleware.' } context 'with unrelated source' do it { expect_no_offenses '' } @@ -112,6 +112,42 @@ def initialize(app) def call(env) @app.call(env) + p @foo + ^^^^ #{msg} + end + end + RUBY + + expect_offense(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + @counter = 0 + ^^^^^^^^^^^^ #{msg} + end + + def call(env) + @app.call(env) + ensure + @counter += 1 + ^^^^^^^^ #{msg} + end + end + RUBY + end + + it 'does not register an offense with thread-safe wrappers', skip: :FIXME do + expect_no_offenses(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + @counter = Concurrent::AtomicReference.new(0) + end + + def call(env) + @app.call(env) + ensure + @counter.update { |ref| ref + 1 } end end RUBY @@ -264,13 +300,13 @@ class TestMiddleware def initialize(app) @app = app instance_variable_set(:counter, 1) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in rack middleware. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end def call(env) @app.call(env) instance_variable_get(:@counter) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid instance variables in rack middleware. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} end end RUBY From e074d9060d7870564dfd6b0059eb9517364eebf5 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 21:04:38 +0300 Subject: [PATCH 08/19] Generate cop docs --- .vscode/settings.json | 3 - .../modules/ROOT/pages/cops_threadsafety.adoc | 67 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 7134e38..0000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "simplecov-vscode.enabled": false -} diff --git a/docs/modules/ROOT/pages/cops_threadsafety.adoc b/docs/modules/ROOT/pages/cops_threadsafety.adoc index 676057c..1c03497 100644 --- a/docs/modules/ROOT/pages/cops_threadsafety.adoc +++ b/docs/modules/ROOT/pages/cops_threadsafety.adoc @@ -267,3 +267,70 @@ Let a framework like Sidekiq handle the threads. # bad Thread.new { do_work } ---- + +== ThreadSafety/RackMiddlewareInstanceVariable + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Enabled +| Yes +| No +| - +| - +|=== + +Avoid instance variables in rack middleware. + +=== Examples + +[source,ruby] +---- +# bad +class CounterMiddleware + def initialize(app) + @app = app + @counter = 0 + end + + def call(env) + app.call(env) + ensure + @counter += 1 + end +end + +# good +class CounterMiddleware + def initialize(app) + @app = app + @counter = Concurrent::AtomicReference.new(0) + end + + def call(env) + app.call(env) + ensure + @counter.update { |ref| ref + 1 } + end +end + +class IdentityMiddleware + def initialize(app) + @app = app + end + + def call(env) + app.call(env) + end +end +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| Include +| `+app/middleware/**/*.rb+`, `+lib/middleware/**/*.rb+`, `+app/middlewares/**/*.rb+`, `+lib/middlewares/**/*.rb+` +| Array +|=== From e8005b1e2a900d4e12b11bf8a983aaced4f53738 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Sun, 22 Sep 2024 21:07:03 +0300 Subject: [PATCH 09/19] Fix cop docs --- docs/modules/ROOT/pages/cops.adoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 3bc2787..ada6362 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -5,3 +5,4 @@ * xref:cops_threadsafety.adoc#threadsafetymutableclassinstancevariable[ThreadSafety/MutableClassInstanceVariable] * xref:cops_threadsafety.adoc#threadsafetynewthread[ThreadSafety/NewThread] * xref:cops_threadsafety.adoc#threadsafetydirchdir[ThreadSafety/DirChdir] +* xref:cops_threadsafety.adoc#threadsafetydirchdir[ThreadSafety/RackMiddlewareInstanceVariable] From ccefe831f8f4594d826b70be8f1face34860f284 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Tue, 24 Sep 2024 16:44:07 +0300 Subject: [PATCH 10/19] Handle ivars with thread-safe semantics --- lib/rubocop-thread_safety.rb | 2 + .../mixin/operation_with_threadsafe_result.rb | 44 +++++++++++++++++++ .../mutable_class_instance_variable.rb | 2 + .../rack_middleware_instance_variable.rb | 18 +++++++- .../rack_middleware_instance_variable_spec.rb | 8 +++- 5 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 lib/rubocop/cop/mixin/operation_with_threadsafe_result.rb diff --git a/lib/rubocop-thread_safety.rb b/lib/rubocop-thread_safety.rb index f29d7ff..ccd0e72 100644 --- a/lib/rubocop-thread_safety.rb +++ b/lib/rubocop-thread_safety.rb @@ -8,6 +8,8 @@ RuboCop::ThreadSafety::Inject.defaults! +require 'rubocop/cop/mixin/operation_with_threadsafe_result' + require 'rubocop/cop/thread_safety/instance_variable_in_class_method' require 'rubocop/cop/thread_safety/class_and_module_attributes' require 'rubocop/cop/thread_safety/mutable_class_instance_variable' diff --git a/lib/rubocop/cop/mixin/operation_with_threadsafe_result.rb b/lib/rubocop/cop/mixin/operation_with_threadsafe_result.rb new file mode 100644 index 0000000..fd8ce18 --- /dev/null +++ b/lib/rubocop/cop/mixin/operation_with_threadsafe_result.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Common functionality for checking if a well-known operation + # produces an object with thread-safe semantics. + module OperationWithThreadsafeResult + extend NodePattern::Macros + + # @!method operation_produces_threadsafe_object?(node) + def_node_matcher :operation_produces_threadsafe_object?, <<~PATTERN + { + (send (const {nil? cbase} :Queue) :new ...) + (send + (const (const {nil? cbase} :ThreadSafe) {:Hash :Array}) + :new ...) + (block + (send + (const (const {nil? cbase} :ThreadSafe) {:Hash :Array}) + :new ...) + ...) + (send (const (const {nil? cbase} :Concurrent) _) :new ...) + (block + (send (const (const {nil? cbase} :Concurrent) _) :new ...) + ...) + (send (const (const (const {nil? cbase} :Concurrent) _) _) :new ...) + (block + (send + (const (const (const {nil? cbase} :Concurrent) _) _) + :new ...) + ...) + (send + (const (const (const (const {nil? cbase} :Concurrent) _) _) _) + :new ...) + (block + (send + (const (const (const (const {nil? cbase} :Concurrent) _) _) _) + :new ...) + ...) + } + PATTERN + end + end +end diff --git a/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb b/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb index d82f1d5..a2f9d98 100644 --- a/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb @@ -74,8 +74,10 @@ module ThreadSafety # end class MutableClassInstanceVariable < Base extend AutoCorrector + include FrozenStringLiteral include ConfigurableEnforcedStyle + include OperationWithThreadsafeResult MSG = 'Freeze mutable objects assigned to class instance variables.' FROZEN_STRING_LITERAL_TYPES_RUBY27 = %i[str dstr].freeze diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index 396233f..1d13fa1 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -44,6 +44,8 @@ module ThreadSafety # end # end class RackMiddlewareInstanceVariable < Base + include OperationWithThreadsafeResult + MSG = 'Avoid instance variables in Rack middleware.' RESTRICT_ON_SEND = %i[instance_variable_get instance_variable_set].freeze @@ -62,10 +64,12 @@ def on_class(node) return unless rack_middleware_like_class?(node) return unless (application_variable = extract_application_variable_from_class_node(node)) + safe_variables = extract_safe_variables_from_class_node(node) + node.each_node(:def) do |method_definition_node| method_definition_node.each_node(:ivasgn, :ivar) do |ivar_node| assignable, = ivar_node.to_a - next if assignable == application_variable + next if assignable == application_variable || safe_variables.include?(assignable) add_offense ivar_node end @@ -85,6 +89,18 @@ def extract_application_variable_from_class_node(class_node) .then { |node| constructor_method(node) } .then { |variables| variables.first[1] if variables.first } end + + def extract_safe_variables_from_class_node(class_node) + class_node + .each_node(:def) + .find { |node| node.method?(:initialize) && node.arguments.size == 1 } + .then do |node| + node + .each_node(:ivasgn) + .select { |ivasgn_node| operation_produces_threadsafe_object?(ivasgn_node.to_a[1]) } + .map { _1.to_a[0] } + end + end end end end diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index 37b9f6e..f40df36 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -136,17 +136,21 @@ def call(env) RUBY end - it 'does not register an offense with thread-safe wrappers', skip: :FIXME do - expect_no_offenses(<<~RUBY) + it 'registers an offense with thread-safe wrappers' do + expect_offense(<<~RUBY) class TestMiddleware def initialize(app) @app = app @counter = Concurrent::AtomicReference.new(0) + @unsafe_counter = 0 + ^^^^^^^^^^^^^^^^^^^ Avoid instance variables in Rack middleware. end def call(env) @app.call(env) ensure + @unsafe_counter += 1 + ^^^^^^^^^^^^^^^ Avoid instance variables in Rack middleware. @counter.update { |ref| ref + 1 } end end From 68b6491383ceb6dc32f72125fd76701a81eabf0e Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Tue, 24 Sep 2024 16:45:38 +0300 Subject: [PATCH 11/19] Drop obsolete `operation_produces_threadsafe_object?` definition --- .../mutable_class_instance_variable.rb | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb b/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb index a2f9d98..808b1a3 100644 --- a/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/mutable_class_instance_variable.rb @@ -243,39 +243,6 @@ def correct_splat_expansion(corrector, expr, splat_value) } PATTERN - # @!method operation_produces_threadsafe_object?(node) - def_node_matcher :operation_produces_threadsafe_object?, <<~PATTERN - { - (send (const {nil? cbase} :Queue) :new ...) - (send - (const (const {nil? cbase} :ThreadSafe) {:Hash :Array}) - :new ...) - (block - (send - (const (const {nil? cbase} :ThreadSafe) {:Hash :Array}) - :new ...) - ...) - (send (const (const {nil? cbase} :Concurrent) _) :new ...) - (block - (send (const (const {nil? cbase} :Concurrent) _) :new ...) - ...) - (send (const (const (const {nil? cbase} :Concurrent) _) _) :new ...) - (block - (send - (const (const (const {nil? cbase} :Concurrent) _) _) - :new ...) - ...) - (send - (const (const (const (const {nil? cbase} :Concurrent) _) _) _) - :new ...) - (block - (send - (const (const (const (const {nil? cbase} :Concurrent) _) _) _) - :new ...) - ...) - } - PATTERN - # @!method range_enclosed_in_parentheses?(node) def_node_matcher :range_enclosed_in_parentheses?, <<~PATTERN (begin ({irange erange} _ _)) From fc96729f87f3d078b6b3df15031413e75a33aa24 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Tue, 24 Sep 2024 19:22:13 +0300 Subject: [PATCH 12/19] Add `CHANGELOG.md` entry --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0651690..20ed862 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,9 @@ ## master (unreleased) -* [#52](https://github.com/rubocop/rubocop-thread_safety/pull/52): Drop support for RuboCop older than 1.48. ([@viralpraxis](https://github.com/viralpraxis)) +<<<<<<< HEAD +* [#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)) * [#48](https://github.com/rubocop/rubocop-thread_safety/pull/48): Do not report instance variables in `ActionDispatch` callbacks in singleton methods. ([@viralpraxis](https://github.com/viralpraxis)) * [#43](https://github.com/rubocop/rubocop-thread_safety/pull/43): Make detection of ActiveSupport's `class_attribute` configurable. ([@viralpraxis](https://github.com/viralpraxis)) * [#42](https://github.com/rubocop/rubocop-thread_safety/pull/42): Fix some `InstanceVariableInClassMethod` cop false positive offenses. ([@viralpraxis](https://github.com/viralpraxis)) From abe9970df3ade34d73b5ea46258d9fcb485f2cfe Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Tue, 24 Sep 2024 19:29:05 +0300 Subject: [PATCH 13/19] Add `AllowedIdentifiers` configuration option --- config/default.yml | 1 + .../modules/ROOT/pages/cops_threadsafety.adoc | 4 ++++ .../rack_middleware_instance_variable.rb | 7 ++++-- .../rack_middleware_instance_variable_spec.rb | 23 +++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/config/default.yml b/config/default.yml index 046e130..bb4af3c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -44,3 +44,4 @@ ThreadSafety/RackMiddlewareInstanceVariable: - 'lib/middleware/**/*.rb' - 'app/middlewares/**/*.rb' - 'lib/middlewares/**/*.rb' + AllowedIdentifiers: [] diff --git a/docs/modules/ROOT/pages/cops_threadsafety.adoc b/docs/modules/ROOT/pages/cops_threadsafety.adoc index 1c03497..7f354cd 100644 --- a/docs/modules/ROOT/pages/cops_threadsafety.adoc +++ b/docs/modules/ROOT/pages/cops_threadsafety.adoc @@ -333,4 +333,8 @@ end | Include | `+app/middleware/**/*.rb+`, `+lib/middleware/**/*.rb+`, `+app/middlewares/**/*.rb+`, `+lib/middlewares/**/*.rb+` | Array + +| AllowedIdentifiers +| `[]` +| Array |=== diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index 1d13fa1..621d08e 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -44,6 +44,7 @@ module ThreadSafety # end # end class RackMiddlewareInstanceVariable < Base + include AllowedIdentifiers include OperationWithThreadsafeResult MSG = 'Avoid instance variables in Rack middleware.' @@ -68,8 +69,10 @@ def on_class(node) node.each_node(:def) do |method_definition_node| method_definition_node.each_node(:ivasgn, :ivar) do |ivar_node| - assignable, = ivar_node.to_a - next if assignable == application_variable || safe_variables.include?(assignable) + variable, = ivar_node.to_a + if variable == application_variable || safe_variables.include?(variable) || allowed_identifier?(variable) + next + end add_offense ivar_node end diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index f40df36..c1ede4a 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -316,4 +316,27 @@ def call(env) RUBY end end + + context 'with non-empty `AllowedIdentifiers` config' do + let(:cop_config) do + { 'AllowedIdentifiers' => ['options'] } + end + + it 'registers an offense' do + expect_offense(<<~RUBY) + class TestMiddleware + def call(env) + @app.call(env) + end + + def initialize(app) + @app = app + @some_var = 2 + ^^^^^^^^^^^^^ #{msg} + @options = 1 + end + end + RUBY + end + end end From d413896b66d00bcbbc3986e7bc2056f270c726e9 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Wed, 25 Sep 2024 10:38:18 +0300 Subject: [PATCH 14/19] Allow middleware with provided options --- .../rack_middleware_instance_variable.rb | 42 +++++++++---------- .../rack_middleware_instance_variable_spec.rb | 37 +++++++++------- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index 621d08e..c86e827 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -53,22 +53,23 @@ class RackMiddlewareInstanceVariable < Base # @!method rack_middleware_like_class?(node) def_node_matcher :rack_middleware_like_class?, <<~MATCHER - (class (const nil? _) nil? (begin <(def :initialize (args (arg _)) ...) (def :call (args (arg _)) ...) ...>)) + (class (const nil? _) nil? (begin <(def :initialize (args (arg _)+) ...) (def :call (args (arg _)) ...) ...>)) MATCHER - # @!method constructor_method(node) - def_node_search :constructor_method, <<~MATCHER - (def :initialize (args (arg $_)) `(ivasgn $_ (lvar $_))) + # @!method app_variable(node) + def_node_search :app_variable, <<~MATCHER + (def :initialize (args (arg $_) ...) `(ivasgn $_ (lvar $_))) MATCHER def on_class(node) return unless rack_middleware_like_class?(node) - return unless (application_variable = extract_application_variable_from_class_node(node)) + return unless (constructor_method = find_constructor_method(node)) + return unless (application_variable = extract_application_variable_from_contructor_method(constructor_method)) - safe_variables = extract_safe_variables_from_class_node(node) + safe_variables = extract_safe_variables_from_constructor_method(constructor_method) - node.each_node(:def) do |method_definition_node| - method_definition_node.each_node(:ivasgn, :ivar) do |ivar_node| + node.each_node(:def) do |def_node| + def_node.each_node(:ivasgn, :ivar) do |ivar_node| variable, = ivar_node.to_a if variable == application_variable || safe_variables.include?(variable) || allowed_identifier?(variable) next @@ -85,24 +86,23 @@ def on_send(node) private - def extract_application_variable_from_class_node(class_node) + def find_constructor_method(class_node) class_node .each_node(:def) - .find { |node| node.method?(:initialize) && node.arguments.size == 1 } - .then { |node| constructor_method(node) } + .find { |node| node.method?(:initialize) && node.arguments.size >= 1 } + end + + def extract_application_variable_from_contructor_method(constructor_method) + constructor_method + .then { |node| app_variable(node) } .then { |variables| variables.first[1] if variables.first } end - def extract_safe_variables_from_class_node(class_node) - class_node - .each_node(:def) - .find { |node| node.method?(:initialize) && node.arguments.size == 1 } - .then do |node| - node - .each_node(:ivasgn) - .select { |ivasgn_node| operation_produces_threadsafe_object?(ivasgn_node.to_a[1]) } - .map { _1.to_a[0] } - end + def extract_safe_variables_from_constructor_method(constructor_method) + constructor_method + .each_node(:ivasgn) + .select { |ivasgn_node| operation_produces_threadsafe_object?(ivasgn_node.to_a[1]) } + .map { _1.to_a[0] } end end end diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index c1ede4a..564afd5 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -42,21 +42,6 @@ def call RUBY end - specify do - expect_no_offenses(<<~RUBY) - class SomeClass - def initialize(user, context) - @user = user - @context = context - end - - def call(env) - [@user, @context] - end - end - RUBY - end - specify do expect_no_offenses(<<~RUBY) class SomeClass @@ -339,4 +324,26 @@ def initialize(app) RUBY end end + + context 'with middleware with provided options' do + it 'registers an offense' do + expect_offense(<<~RUBY) + class TestMiddleware + def initialize(app, options) + @app = app + @options = options + ^^^^^^^^^^^^^^^^^^ #{msg} + end + + def call(env) + if options[:noop] + [200, {}, []] + else + @app.call(env) + end + end + end + RUBY + end + end end From d9286c0ceff0295b7702e3ab821c3b7ad6fd0589 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Wed, 25 Sep 2024 10:41:39 +0300 Subject: [PATCH 15/19] Add cop reasoning --- docs/modules/ROOT/pages/cops_threadsafety.adoc | 4 ++++ .../cop/thread_safety/rack_middleware_instance_variable.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/docs/modules/ROOT/pages/cops_threadsafety.adoc b/docs/modules/ROOT/pages/cops_threadsafety.adoc index 7f354cd..3ed2cee 100644 --- a/docs/modules/ROOT/pages/cops_threadsafety.adoc +++ b/docs/modules/ROOT/pages/cops_threadsafety.adoc @@ -282,6 +282,10 @@ Thread.new { do_work } Avoid instance variables in rack middleware. +Middlewares are initialized once, meaning any instance variables are shared between executor threads. +To avoid potential race conditions, it's recommended to design middlewares to be stateless +or to implement proper synchronization mechanisms. + === Examples [source,ruby] diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index c86e827..dd9f800 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -5,6 +5,10 @@ module Cop module ThreadSafety # Avoid instance variables in rack middleware. # + # Middlewares are initialized once, meaning any instance variables are shared between executor threads. + # To avoid potential race conditions, it's recommended to design middlewares to be stateless + # or to implement proper synchronization mechanisms. + # # @example # # bad # class CounterMiddleware From 0ef8d430ad0e77ef261e7f178829696424badeb8 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Wed, 25 Sep 2024 10:59:32 +0300 Subject: [PATCH 16/19] Handle `on_send` more precisely --- .../rack_middleware_instance_variable.rb | 8 ++- .../rack_middleware_instance_variable_spec.rb | 63 ++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb index dd9f800..c864b43 100644 --- a/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb +++ b/lib/rubocop/cop/thread_safety/rack_middleware_instance_variable.rb @@ -67,7 +67,8 @@ class RackMiddlewareInstanceVariable < Base def on_class(node) return unless rack_middleware_like_class?(node) - return unless (constructor_method = find_constructor_method(node)) + + constructor_method = find_constructor_method(node) return unless (application_variable = extract_application_variable_from_contructor_method(constructor_method)) safe_variables = extract_safe_variables_from_constructor_method(constructor_method) @@ -85,6 +86,11 @@ def on_class(node) end def on_send(node) + argument = node.first_argument + + return unless argument&.sym_type? || argument&.str_type? + return if allowed_identifier?(argument.value) + add_offense node end diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index 564afd5..89ad531 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -27,6 +27,16 @@ def initialize(user, context) RUBY end + specify do + expect_no_offenses(<<~RUBY) + class SomeClass + def call(env) + @env = env + end + end + RUBY + end + specify do expect_no_offenses(<<~RUBY) class SomeClass @@ -294,11 +304,60 @@ def initialize(app) def call(env) @app.call(env) - instance_variable_get(:@counter) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + instance_variable_get("@counter") + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + instance_variable_set + end + + def call(env) + @app.call(env) + instance_variable_get end end RUBY + + expect_no_offenses(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + instance_variable_set(1) + end + + def call(env) + @app.call(env) + instance_variable_get({}) + end + end + RUBY + end + + context 'with non-empty `AllowedIdentifiers` config' do + let(:cop_config) do + { 'AllowedIdentifiers' => ['options'] } + end + + it 'registers no offenses' do + expect_no_offenses(<<~RUBY) + class TestMiddleware + def initialize(app) + @app = app + instance_variable_set(:@options, {}) + end + + def call(env) + @app.call(env) + end + end + RUBY + end end end From 0bbc0eae3d3268ece46ac395ddf94241f543fcfc Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Wed, 25 Sep 2024 11:03:15 +0300 Subject: [PATCH 17/19] Apply `rubocop` fix --- .../cop/thread_safety/rack_middleware_instance_variable_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb index 89ad531..8bd6296 100644 --- a/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb +++ b/spec/rubocop/cop/thread_safety/rack_middleware_instance_variable_spec.rb @@ -309,7 +309,9 @@ def call(env) end end RUBY + end + it 'registers no offenses' do expect_no_offenses(<<~RUBY) class TestMiddleware def initialize(app) From eda52598027bbe5effea4471b27dfc555306ebc4 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Wed, 25 Sep 2024 11:11:06 +0300 Subject: [PATCH 18/19] Fix broken rebase on `CHANGELOG.md` --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20ed862..f1a6f4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ ## master (unreleased) -<<<<<<< HEAD * [#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)) * [#48](https://github.com/rubocop/rubocop-thread_safety/pull/48): Do not report instance variables in `ActionDispatch` callbacks in singleton methods. ([@viralpraxis](https://github.com/viralpraxis)) From 6a73e2088a9aa76ff67670537da62e49d5a0e010 Mon Sep 17 00:00:00 2001 From: viralpraxis Date: Thu, 26 Sep 2024 11:52:56 +0300 Subject: [PATCH 19/19] Fix rdoc typo --- docs/modules/ROOT/pages/cops.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index ada6362..f868ec7 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -5,4 +5,4 @@ * xref:cops_threadsafety.adoc#threadsafetymutableclassinstancevariable[ThreadSafety/MutableClassInstanceVariable] * xref:cops_threadsafety.adoc#threadsafetynewthread[ThreadSafety/NewThread] * xref:cops_threadsafety.adoc#threadsafetydirchdir[ThreadSafety/DirChdir] -* xref:cops_threadsafety.adoc#threadsafetydirchdir[ThreadSafety/RackMiddlewareInstanceVariable] +* xref:cops_threadsafety.adoc#threadsafetyrackmiddlewareinstancevariable[ThreadSafety/RackMiddlewareInstanceVariable]