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

Enable Style/ExplicitBlockArgument cop #43170

Merged
merged 1 commit into from Sep 5, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
Expand Up @@ -143,6 +143,9 @@ Style/DefWithParentheses:
Style/MethodDefParentheses:
Enabled: true

Style/ExplicitBlockArgument:
Enabled: true

Style/FrozenStringLiteralComment:
Enabled: true
EnforcedStyle: always
Expand Down
Expand Up @@ -18,10 +18,10 @@ def add_tags(*tags)
@tags = @tags.uniq
end

def tag(logger)
def tag(logger, &block)
if logger.respond_to?(:tagged)
current_tags = tags - logger.formatter.current_tags
logger.tagged(*current_tags) { yield }
logger.tagged(*current_tags, &block)
else
yield
end
Expand Down
6 changes: 2 additions & 4 deletions actioncable/lib/action_cable/server/worker.rb
Expand Up @@ -36,12 +36,10 @@ def stopping?
@executor.shuttingdown?
end

def work(connection)
def work(connection, &block)
self.connection = connection

run_callbacks :work do
yield
end
run_callbacks :work, &block
ensure
self.connection = nil
end
Expand Down
Expand Up @@ -12,8 +12,8 @@ module ActiveRecordConnectionManagement
end
end

def with_database_connections
connection.logger.tag(ActiveRecord::Base.logger) { yield }
def with_database_connections(&block)
connection.logger.tag(ActiveRecord::Base.logger, &block)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions actionpack/lib/abstract_controller/caching/fragments.rb
Expand Up @@ -142,8 +142,8 @@ def expire_fragment(key, options = nil)
end
end

def instrument_fragment_cache(name, key) # :nodoc:
ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", instrument_payload(key)) { yield }
def instrument_fragment_cache(name, key, &block) # :nodoc:
ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", instrument_payload(key), &block)
end
end
end
Expand Down
Expand Up @@ -940,7 +940,7 @@ def convert_value_to_parameters(value)
def each_element(object, &block)
case object
when Array
object.grep(Parameters).filter_map { |el| yield el }
object.grep(Parameters).filter_map(&block)
when Parameters
if object.nested_attributes?
object.each_nested_attribute(&block)
Expand Down
8 changes: 4 additions & 4 deletions actionpack/lib/action_dispatch/http/mime_type.rb
Expand Up @@ -13,8 +13,8 @@ def initialize
@symbols = []
end

def each
@mimes.each { |x| yield x }
def each(&block)
@mimes.each(&block)
end

def <<(type)
Expand Down Expand Up @@ -42,9 +42,9 @@ def [](type)
Type.lookup_by_extension(type)
end

def fetch(type)
def fetch(type, &block)
return type if type.is_a?(Type)
EXTENSION_LOOKUP.fetch(type.to_s) { |k| yield k }
EXTENSION_LOOKUP.fetch(type.to_s, &block)
end
end

Expand Down
4 changes: 2 additions & 2 deletions actionpack/lib/action_dispatch/middleware/stack.rb
Expand Up @@ -83,8 +83,8 @@ def initialize(*args)
yield(self) if block_given?
end

def each
@middlewares.each { |x| yield x }
def each(&block)
@middlewares.each(&block)
end

def size
Expand Down
36 changes: 16 additions & 20 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Expand Up @@ -922,7 +922,7 @@ def controller(controller)
# namespace :admin, as: "sekret" do
# resources :posts
# end
def namespace(path, options = {})
def namespace(path, options = {}, &block)
path = path.to_s

defaults = {
Expand All @@ -933,7 +933,7 @@ def namespace(path, options = {})
}

path_scope(options.delete(:path) { path }) do
scope(defaults.merge!(options)) { yield }
scope(defaults.merge!(options), &block)
end
end

Expand Down Expand Up @@ -992,8 +992,8 @@ def namespace(path, options = {})
# constraints(Iphone) do
# resources :iphones
# end
def constraints(constraints = {})
scope(constraints: constraints) { yield }
def constraints(constraints = {}, &block)
scope(constraints: constraints, &block)
end

# Allows you to set default parameters for a route, such as this:
Expand Down Expand Up @@ -1493,15 +1493,13 @@ def resources(*resources, &block)
# with GET, and route to the search action of +PhotosController+. It will also
# create the <tt>search_photos_url</tt> and <tt>search_photos_path</tt>
# route helpers.
def collection
def collection(&block)
unless resource_scope?
raise ArgumentError, "can't use collection outside resource(s) scope"
end

with_scope_level(:collection) do
path_scope(parent_resource.collection_scope) do
yield
end
path_scope(parent_resource.collection_scope, &block)
end
end

Expand All @@ -1516,35 +1514,33 @@ def collection
# This will recognize <tt>/photos/1/preview</tt> with GET, and route to the
# preview action of +PhotosController+. It will also create the
# <tt>preview_photo_url</tt> and <tt>preview_photo_path</tt> helpers.
def member
def member(&block)
unless resource_scope?
raise ArgumentError, "can't use member outside resource(s) scope"
end

with_scope_level(:member) do
if shallow?
shallow_scope {
path_scope(parent_resource.member_scope) { yield }
path_scope(parent_resource.member_scope, &block)
}
else
path_scope(parent_resource.member_scope) { yield }
path_scope(parent_resource.member_scope, &block)
end
end
end

def new
def new(&block)
unless resource_scope?
raise ArgumentError, "can't use new outside resource(s) scope"
end

with_scope_level(:new) do
path_scope(parent_resource.new_scope(action_path(:new))) do
yield
end
path_scope(parent_resource.new_scope(action_path(:new)), &block)
end
end

def nested
def nested(&block)
unless resource_scope?
raise ArgumentError, "can't use nested outside resource(s) scope"
end
Expand All @@ -1553,12 +1549,12 @@ def nested
if shallow? && shallow_nesting_depth >= 1
shallow_scope do
path_scope(parent_resource.nested_scope) do
scope(nested_options) { yield }
scope(nested_options, &block)
end
end
else
path_scope(parent_resource.nested_scope) do
scope(nested_options) { yield }
scope(nested_options, &block)
end
end
end
Expand Down Expand Up @@ -1744,10 +1740,10 @@ def with_scope_level(kind) # :doc:
@scope = @scope.parent
end

def resource_scope(resource)
def resource_scope(resource, &block)
@scope = @scope.new(scope_level_resource: resource)

controller(resource.resource_scope) { yield }
controller(resource.resource_scope, &block)
ensure
@scope = @scope.parent
end
Expand Down
4 changes: 2 additions & 2 deletions actionpack/lib/action_dispatch/routing/route_set.rb
Expand Up @@ -131,8 +131,8 @@ def key?(name)
alias [] get
alias clear clear!

def each
routes.each { |name, route| yield name, route }
def each(&block)
routes.each(&block)
self
end

Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/controller/live_stream_test.rb
Expand Up @@ -314,8 +314,8 @@ def capture_log_output
def setup
super

def @controller.new_controller_thread
Thread.new { yield }
def @controller.new_controller_thread(&block)
Thread.new(&block)
end
end

Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/controller/render_test.rb
Expand Up @@ -987,8 +987,8 @@ class LiveHeadRenderTest < ActionController::TestCase
def setup
super

def @controller.new_controller_thread
Thread.new { yield }
def @controller.new_controller_thread(&block)
Thread.new(&block)
end

def @controller.response_body=(body)
Expand Down
22 changes: 8 additions & 14 deletions actionpack/test/controller/request_forgery_protection_test.rb
Expand Up @@ -634,14 +634,12 @@ def assert_not_blocked(&block)
assert_response :success
end

def assert_cross_origin_blocked
assert_raises(ActionController::InvalidCrossOriginRequest) do
yield
end
def assert_cross_origin_blocked(&block)
assert_raises(ActionController::InvalidCrossOriginRequest, &block)
end

def assert_cross_origin_not_blocked
assert_not_blocked { yield }
def assert_cross_origin_not_blocked(&block)
assert_not_blocked(&block)
end

def forgery_protection_origin_check
Expand Down Expand Up @@ -701,10 +699,8 @@ def setup

class RequestForgeryProtectionControllerUsingExceptionTest < ActionController::TestCase
include RequestForgeryProtectionTests
def assert_blocked
assert_raises(ActionController::InvalidAuthenticityToken) do
yield
end
def assert_blocked(&block)
assert_raises(ActionController::InvalidAuthenticityToken, &block)
end

def test_raised_exception_message_explains_why_it_occurred
Expand Down Expand Up @@ -1105,10 +1101,8 @@ def test_should_allow_post_without_token_when_skipping
assert_not_blocked { post :index }
end

def assert_blocked
assert_raises(ActionController::InvalidAuthenticityToken) do
yield
end
def assert_blocked(&block)
assert_raises(ActionController::InvalidAuthenticityToken, &block)
end

def assert_not_blocked(&block)
Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/helpers/text_helper.rb
Expand Up @@ -133,7 +133,7 @@ def truncate(text, options = {}, &block)
#
# highlight('<a href="javascript:alert(\'no!\')">ruby</a> on rails', 'rails', sanitize: false)
# # => <a href="javascript:alert('no!')">ruby</a> on <mark>rails</mark>
def highlight(text, phrases, options = {})
def highlight(text, phrases, options = {}, &block)
text = sanitize(text) if options.fetch(:sanitize, true)

if text.blank? || phrases.blank?
Expand All @@ -144,7 +144,7 @@ def highlight(text, phrases, options = {})
end.join("|")

if block_given?
text.gsub(/(#{match})(?![^<]*?>)/i) { |found| yield found }
text.gsub(/(#{match})(?![^<]*?>)/i, &block)
else
highlighter = options.fetch(:highlighter, '<mark>\1</mark>')
text.gsub(/(#{match})(?![^<]*?>)/i, highlighter)
Expand Down
4 changes: 2 additions & 2 deletions actionview/test/lib/controller/fake_models.rb
Expand Up @@ -183,9 +183,9 @@ class ArelLike
def to_ary
true
end
def each
def each(&block)
a = Array.new(2) { |id| Comment.new(id + 1) }
a.each { |i| yield i }
a.each(&block)
end
end

Expand Down
4 changes: 2 additions & 2 deletions activejob/lib/active_job/logging.rb
Expand Up @@ -19,10 +19,10 @@ def perform_now
end

private
def tag_logger(*tags)
def tag_logger(*tags, &block)
if logger.respond_to?(:tagged)
tags.unshift "ActiveJob" unless logger_tagged_by_active_job?
logger.tagged(*tags) { yield }
logger.tagged(*tags, &block)
else
yield
end
Expand Down
6 changes: 2 additions & 4 deletions activejob/test/cases/logging_test.rb
Expand Up @@ -49,11 +49,9 @@ def set_logger(logger)
ActiveJob::Base.logger = logger
end

def subscribed
def subscribed(&block)
[].tap do |events|
ActiveSupport::Notifications.subscribed(-> (*args) { events << args }, /enqueue.*\.active_job/) do
yield
end
ActiveSupport::Notifications.subscribed(-> (*args) { events << args }, /enqueue.*\.active_job/, &block)
end
end

Expand Down
Expand Up @@ -115,9 +115,9 @@ def nullify_owner_attributes(record)
record[reflection.foreign_key] = nil
end

def transaction_if(value)
def transaction_if(value, &block)
if value
reflection.klass.transaction { yield }
reflection.klass.transaction(&block)
else
yield
end
Expand Down
Expand Up @@ -306,14 +306,14 @@ def truncate_tables(*table_names) # :nodoc:
#
# The mysql2 and postgresql adapters support setting the transaction
# isolation level.
def transaction(requires_new: nil, isolation: nil, joinable: true)
def transaction(requires_new: nil, isolation: nil, joinable: true, &block)
if !requires_new && current_transaction.joinable?
if isolation
raise ActiveRecord::TransactionIsolationError, "cannot set isolation when joining a transaction"
end
yield
else
transaction_manager.within_new_transaction(isolation: isolation, joinable: joinable) { yield }
transaction_manager.within_new_transaction(isolation: isolation, joinable: joinable, &block)
end
rescue ActiveRecord::Rollback
# rollbacks are silently swallowed
Expand Down