From 3df665fd236832fc336ff3ccfaee99a557422176 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 3 May 2023 04:32:30 -0400 Subject: [PATCH] Fix Lint/ConstantDefinitionInBlock cop (#24763) --- .rubocop_todo.yml | 13 ---- spec/controllers/api/base_controller_spec.rb | 14 ++-- .../application_controller_spec.rb | 12 +-- .../concerns/accountable_concern_spec.rb | 14 ++-- .../concerns/signature_verification_spec.rb | 16 ++-- spec/lib/activitypub/adapter_spec.rb | 76 ++++++++++--------- .../shared_connection_pool_spec.rb | 16 ++-- .../shared_timed_stack_spec.rb | 28 +++---- spec/models/concerns/remotable_spec.rb | 49 ++++++------ 9 files changed, 120 insertions(+), 118 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2b2a110fad..0b076c05dc 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -106,19 +106,6 @@ Lint/AmbiguousOperatorPrecedence: Exclude: - 'config/initializers/rack_attack.rb' -# Configuration parameters: AllowedMethods. -# AllowedMethods: enums -Lint/ConstantDefinitionInBlock: - Exclude: - - 'spec/controllers/api/base_controller_spec.rb' - - 'spec/controllers/application_controller_spec.rb' - - 'spec/controllers/concerns/accountable_concern_spec.rb' - - 'spec/controllers/concerns/signature_verification_spec.rb' - - 'spec/lib/activitypub/adapter_spec.rb' - - 'spec/lib/connection_pool/shared_connection_pool_spec.rb' - - 'spec/lib/connection_pool/shared_timed_stack_spec.rb' - - 'spec/models/concerns/remotable_spec.rb' - # Configuration parameters: AllowComments, AllowEmptyLambdas. Lint/EmptyBlock: Exclude: diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb index dd90aead23..9f3ba81474 100644 --- a/spec/controllers/api/base_controller_spec.rb +++ b/spec/controllers/api/base_controller_spec.rb @@ -74,7 +74,11 @@ describe Api::BaseController do end describe 'error handling' do - ERRORS_WITH_CODES = { + before do + routes.draw { get 'error' => 'api/base#error' } + end + + { ActiveRecord::RecordInvalid => 422, Mastodon::ValidationError => 422, ActiveRecord::RecordNotFound => 404, @@ -82,13 +86,7 @@ describe Api::BaseController do HTTP::Error => 503, OpenSSL::SSL::SSLError => 503, Mastodon::NotPermittedError => 403, - } - - before do - routes.draw { get 'error' => 'api/base#error' } - end - - ERRORS_WITH_CODES.each do |error, code| + }.each do |error, code| it "Handles error class of #{error}" do expect(FakeService).to receive(:new).and_raise(error) diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index c4710f3495..46f7664d87 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -223,14 +223,16 @@ describe ApplicationController, type: :controller do end describe 'cache_collection' do - class C < ApplicationController - public :cache_collection + subject do + Class.new(ApplicationController) do + public :cache_collection + end end shared_examples 'receives :with_includes' do |fabricator, klass| it 'uses raw if it is not an ActiveRecord::Relation' do record = Fabricate(fabricator) - expect(C.new.cache_collection([record], klass)).to eq [record] + expect(subject.new.cache_collection([record], klass)).to eq [record] end end @@ -241,13 +243,13 @@ describe ApplicationController, type: :controller do record = Fabricate(fabricator) relation = klass.none allow(relation).to receive(:cache_ids).and_return([record]) - expect(C.new.cache_collection(relation, klass)).to eq [record] + expect(subject.new.cache_collection(relation, klass)).to eq [record] end end it 'returns raw unless class responds to :with_includes' do raw = Object.new - expect(C.new.cache_collection(raw, Object)).to eq raw + expect(subject.new.cache_collection(raw, Object)).to eq raw end context 'Status' do diff --git a/spec/controllers/concerns/accountable_concern_spec.rb b/spec/controllers/concerns/accountable_concern_spec.rb index 5c5180bc24..3c10082c34 100644 --- a/spec/controllers/concerns/accountable_concern_spec.rb +++ b/spec/controllers/concerns/accountable_concern_spec.rb @@ -3,18 +3,20 @@ require 'rails_helper' RSpec.describe AccountableConcern do - class Hoge - include AccountableConcern - attr_reader :current_account + let(:hoge_class) do + Class.new do + include AccountableConcern + attr_reader :current_account - def initialize(current_account) - @current_account = current_account + def initialize(current_account) + @current_account = current_account + end end end let(:user) { Fabricate(:account) } let(:target) { Fabricate(:account) } - let(:hoge) { Hoge.new(user) } + let(:hoge) { hoge_class.new(user) } describe '#log_action' do it 'creates Admin::ActionLog' do diff --git a/spec/controllers/concerns/signature_verification_spec.rb b/spec/controllers/concerns/signature_verification_spec.rb index 13655f3133..e5b5ed6173 100644 --- a/spec/controllers/concerns/signature_verification_spec.rb +++ b/spec/controllers/concerns/signature_verification_spec.rb @@ -3,14 +3,16 @@ require 'rails_helper' describe ApplicationController, type: :controller do - class WrappedActor - attr_reader :wrapped_account + let(:wrapped_actor_class) do + Class.new do + attr_reader :wrapped_account - def initialize(wrapped_account) - @wrapped_account = wrapped_account + def initialize(wrapped_account) + @wrapped_account = wrapped_account + end + + delegate :uri, :keypair, to: :wrapped_account end - - delegate :uri, :keypair, to: :wrapped_account end controller do @@ -93,7 +95,7 @@ describe ApplicationController, type: :controller do end context 'with a valid actor that is not an Account' do - let(:actor) { WrappedActor.new(author) } + let(:actor) { wrapped_actor_class.new(author) } before do get :success diff --git a/spec/lib/activitypub/adapter_spec.rb b/spec/lib/activitypub/adapter_spec.rb index b981ea9c68..f9f8b8dce0 100644 --- a/spec/lib/activitypub/adapter_spec.rb +++ b/spec/lib/activitypub/adapter_spec.rb @@ -3,43 +3,51 @@ require 'rails_helper' RSpec.describe ActivityPub::Adapter do - class TestObject < ActiveModelSerializers::Model - attributes :foo - end - - class TestWithBasicContextSerializer < ActivityPub::Serializer - attributes :foo - end - - class TestWithNamedContextSerializer < ActivityPub::Serializer - context :security - attributes :foo - end - - class TestWithNestedNamedContextSerializer < ActivityPub::Serializer - attributes :foo - - has_one :virtual_object, key: :baz, serializer: TestWithNamedContextSerializer - - def virtual_object - object + before do + test_object_class = Class.new(ActiveModelSerializers::Model) do + attributes :foo end - end + stub_const('TestObject', test_object_class) - class TestWithContextExtensionSerializer < ActivityPub::Serializer - context_extensions :sensitive - attributes :foo - end - - class TestWithNestedContextExtensionSerializer < ActivityPub::Serializer - context_extensions :manually_approves_followers - attributes :foo - - has_one :virtual_object, key: :baz, serializer: TestWithContextExtensionSerializer - - def virtual_object - object + test_with_basic_context_serializer = Class.new(ActivityPub::Serializer) do + attributes :foo end + stub_const('TestWithBasicContextSerializer', test_with_basic_context_serializer) + + test_with_named_context_serializer = Class.new(ActivityPub::Serializer) do + context :security + attributes :foo + end + stub_const('TestWithNamedContextSerializer', test_with_named_context_serializer) + + test_with_nested_named_context_serializer = Class.new(ActivityPub::Serializer) do + attributes :foo + + has_one :virtual_object, key: :baz, serializer: TestWithNamedContextSerializer + + def virtual_object + object + end + end + stub_const('TestWithNestedNamedContextSerializer', test_with_nested_named_context_serializer) + + test_with_context_extension_serializer = Class.new(ActivityPub::Serializer) do + context_extensions :sensitive + attributes :foo + end + stub_const('TestWithContextExtensionSerializer', test_with_context_extension_serializer) + + test_with_nested_context_extension_serializer = Class.new(ActivityPub::Serializer) do + context_extensions :manually_approves_followers + attributes :foo + + has_one :virtual_object, key: :baz, serializer: TestWithContextExtensionSerializer + + def virtual_object + object + end + end + stub_const('TestWithNestedContextExtensionSerializer', test_with_nested_context_extension_serializer) end describe '#serializable_hash' do diff --git a/spec/lib/connection_pool/shared_connection_pool_spec.rb b/spec/lib/connection_pool/shared_connection_pool_spec.rb index 1144645580..a2fe75f742 100644 --- a/spec/lib/connection_pool/shared_connection_pool_spec.rb +++ b/spec/lib/connection_pool/shared_connection_pool_spec.rb @@ -3,22 +3,24 @@ require 'rails_helper' describe ConnectionPool::SharedConnectionPool do - class MiniConnection - attr_reader :site + subject { described_class.new(size: 5, timeout: 5) { |site| mini_connection_class.new(site) } } - def initialize(site) - @site = site + let(:mini_connection_class) do + Class.new do + attr_reader :site + + def initialize(site) + @site = site + end end end - subject { described_class.new(size: 5, timeout: 5) { |site| MiniConnection.new(site) } } - describe '#with' do it 'runs a block with a connection' do block_run = false subject.with('foo') do |connection| - expect(connection).to be_a MiniConnection + expect(connection).to be_a mini_connection_class block_run = true end diff --git a/spec/lib/connection_pool/shared_timed_stack_spec.rb b/spec/lib/connection_pool/shared_timed_stack_spec.rb index f680c59667..04d550eec5 100644 --- a/spec/lib/connection_pool/shared_timed_stack_spec.rb +++ b/spec/lib/connection_pool/shared_timed_stack_spec.rb @@ -3,30 +3,32 @@ require 'rails_helper' describe ConnectionPool::SharedTimedStack do - class MiniConnection - attr_reader :site + subject { described_class.new(5) { |site| mini_connection_class.new(site) } } - def initialize(site) - @site = site + let(:mini_connection_class) do + Class.new do + attr_reader :site + + def initialize(site) + @site = site + end end end - subject { described_class.new(5) { |site| MiniConnection.new(site) } } - describe '#push' do it 'keeps the connection in the stack' do - subject.push(MiniConnection.new('foo')) + subject.push(mini_connection_class.new('foo')) expect(subject.size).to eq 1 end end describe '#pop' do it 'returns a connection' do - expect(subject.pop('foo')).to be_a MiniConnection + expect(subject.pop('foo')).to be_a mini_connection_class end it 'returns the same connection that was pushed in' do - connection = MiniConnection.new('foo') + connection = mini_connection_class.new('foo') subject.push(connection) expect(subject.pop('foo')).to be connection end @@ -36,8 +38,8 @@ describe ConnectionPool::SharedTimedStack do end it 'repurposes a connection for a different site when maximum amount is reached' do - 5.times { subject.push(MiniConnection.new('foo')) } - expect(subject.pop('bar')).to be_a MiniConnection + 5.times { subject.push(mini_connection_class.new('foo')) } + expect(subject.pop('bar')).to be_a mini_connection_class end end @@ -47,14 +49,14 @@ describe ConnectionPool::SharedTimedStack do end it 'returns false when there are connections on the stack' do - subject.push(MiniConnection.new('foo')) + subject.push(mini_connection_class.new('foo')) expect(subject.empty?).to be false end end describe '#size' do it 'returns the number of connections on the stack' do - 2.times { subject.push(MiniConnection.new('foo')) } + 2.times { subject.push(mini_connection_class.new('foo')) } expect(subject.size).to eq 2 end end diff --git a/spec/models/concerns/remotable_spec.rb b/spec/models/concerns/remotable_spec.rb index 9645204276..9fcf48619d 100644 --- a/spec/models/concerns/remotable_spec.rb +++ b/spec/models/concerns/remotable_spec.rb @@ -3,48 +3,47 @@ require 'rails_helper' RSpec.describe Remotable do - class Foo - def initialize - @attrs = {} - end + let(:foo_class) do + Class.new do + def initialize + @attrs = {} + end - def [](arg) - @attrs[arg] - end + def [](arg) + @attrs[arg] + end - def []=(arg1, arg2) - @attrs[arg1] = arg2 - end + def []=(arg1, arg2) + @attrs[arg1] = arg2 + end - def hoge=(arg); end + def hoge=(arg); end - def hoge_file_name; end + def hoge_file_name; end - def hoge_file_name=(arg); end + def hoge_file_name=(arg); end - def has_attribute?(arg); end + def has_attribute?(arg); end - def self.attachment_definitions - { hoge: nil } - end - end - - before do - class Foo - include Remotable - - remotable_attachment :hoge, 1.kilobyte + def self.attachment_definitions + { hoge: nil } + end end end let(:attribute_name) { "#{hoge}_remote_url".to_sym } let(:code) { 200 } let(:file) { 'filename="foo.txt"' } - let(:foo) { Foo.new } + let(:foo) { foo_class.new } let(:headers) { { 'content-disposition' => file } } let(:hoge) { :hoge } let(:url) { 'https://google.com' } + before do + foo_class.include described_class + foo_class.remotable_attachment :hoge, 1.kilobyte + end + it 'defines a method #hoge_remote_url=' do expect(foo).to respond_to(:hoge_remote_url=) end