Enable Layout/EmptyLinesAroundAccessModifier cop

We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).

Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).

That cop and enforced style will reduce the our code review cost.
This commit is contained in:
Ryuta Kamizono 2019-06-12 21:30:49 +09:00
parent 0ad238f478
commit c81af6ae72
434 changed files with 8 additions and 514 deletions

@ -61,6 +61,10 @@ Layout/EndAlignment:
Layout/EmptyLineAfterMagicComment:
Enabled: true
Layout/EmptyLinesAroundAccessModifier:
Enabled: true
EnforcedStyle: only_before
Layout/EmptyLinesAroundBlockBody:
Enabled: true

@ -66,7 +66,6 @@ def invoke(receiver, method, *args, connection:, &block)
end
private
def logger
ActionCable.server.logger
end

@ -591,7 +591,6 @@ def deliver_mail(mail) #:nodoc:
end
private
def set_payload_for_mail(payload, mail)
payload[:mail] = mail.encoded
payload[:mailer] = name
@ -873,7 +872,6 @@ def mail(headers = {}, &block)
end
private
# Used by #mail to set the content type of the message.
#
# It will use the given +user_content_type+, or multipart if the mail

@ -55,7 +55,6 @@ def unregister_preview_interceptor(interceptor)
end
private
def interceptor_class_for(interceptor)
case interceptor
when String, Symbol

@ -22,7 +22,6 @@ module ClearTestDeliveries
end
private
def clear_test_deliveries
if ActionMailer::Base.delivery_method == :test
ActionMailer::Base.deliveries.clear
@ -76,7 +75,6 @@ def determine_default_mailer(name)
end
private
def initialize_test_deliveries
set_delivery_method :test
@old_perform_deliveries = ActionMailer::Base.perform_deliveries

@ -152,7 +152,6 @@ def assert_no_enqueued_emails(&block)
end
private
def delivery_job_filter(job)
job_class = job.is_a?(Hash) ? job.fetch(:job) : job.class

@ -939,7 +939,6 @@ def a_callback
end
private
# Execute the block setting the given values and restoring old values after
# the block is executed.
def swap(klass, new_values)

@ -189,7 +189,6 @@ def test_fragment_cache_instrumentation
end
private
def template_digest(name, format)
ActionView::Digestor.digest(name: name, format: format, finder: @mailer.lookup_context)
end

@ -67,7 +67,6 @@ def test_send_mail
end
private
def with_translation(locale, data)
I18n.backend.store_translations(locale, data)
yield

@ -69,7 +69,6 @@ class LegacyDeliveryJob < ActionMailer::DeliveryJob
end
private
def with_delivery_job(job)
old_params_delivery_job = ParamsMailer.delivery_job
old_regular_delivery_job = DelayedMailer.delivery_job

@ -68,7 +68,6 @@ def use_cache
end
private
def mail_with_defaults(&block)
mail(to: "test@localhost", from: "tester@example.com",
subject: "using helpers", &block)

@ -18,7 +18,6 @@ def computed_value
end
private
def give_a_greeting
"Thanks for signing up this afternoon"
end

@ -81,7 +81,6 @@ class DummyDeliveryJob < ActionMailer::MailDeliveryJob
end
private
def with_delivery_job(job)
old_delivery_job = ParamsMailer.delivery_job
ParamsMailer.delivery_job = job

@ -176,7 +176,6 @@ def self.supports_path?
end
private
# Returns true if the name can be considered an action because
# it has a method defined in the controller.
#

@ -22,7 +22,6 @@ def #{sym}(*args, &block)
end
private
def method_missing(symbol, &block)
unless mime_constant = Mime[symbol]
raise NoMethodError, "To respond to a custom format, register it as a MIME type first: " \

@ -30,7 +30,6 @@ module Caching
end
private
def instrument_payload(key)
{
controller: controller_name,

@ -35,7 +35,6 @@ def build(action, app = nil, &block)
end
private
INCLUDE = ->(list, action) { list.include? action }
EXCLUDE = ->(list, action) { !list.include? action }
NULL = ->(list, action) { true }

@ -36,7 +36,6 @@ def content_security_policy_report_only(report_only = true, **options)
end
private
def content_security_policy?
request.content_security_policy
end

@ -69,7 +69,6 @@ def redirect_to(*args)
end
private
# A hook invoked every time a before callback is halted.
def halted_callback_hook(filter)
ActiveSupport::Notifications.instrument("halted_callback.action_controller", filter: filter)

@ -107,7 +107,6 @@ def write(object, options = {})
end
private
def perform_write(json, options)
current_options = @options.merge(options).stringify_keys
@ -205,7 +204,6 @@ def call_on_error
end
private
def each_chunk(&block)
loop do
str = nil
@ -220,7 +218,6 @@ def each_chunk(&block)
class Response < ActionDispatch::Response #:nodoc: all
private
def before_committed
super
jar = request.cookie_jar
@ -286,7 +283,6 @@ def response_body=(body)
end
private
# Spawn a new thread to serve up the controller in. This is to get
# around the fact that Rack isn't based around IOs and we need to use
# a thread to stream data from the response bodies. Nobody should call

@ -246,7 +246,6 @@ def process_action(*args)
end
private
# Returns the wrapper key which will be used to store wrapped parameters.
def _wrapper_key
_wrapper_options.name

@ -53,7 +53,6 @@ def render_to_body(options = {})
end
private
def _process_variant(options)
if defined?(request) && !request.nil? && request.variant.present?
options[:variant] = request.variant

@ -151,7 +151,6 @@ def skip_forgery_protection(options = {})
end
private
def protection_method_class(name)
ActionController::RequestForgeryProtection::ProtectionMethods.const_get(name.to_s.classify)
rescue NameError
@ -175,7 +174,6 @@ def handle_unverified_request
end
private
class NullSessionHash < Rack::Session::Abstract::SessionHash #:nodoc:
def initialize(req)
super(nil, req)

@ -196,7 +196,6 @@ module Streaming
extend ActiveSupport::Concern
private
# Set proper cache control and transfer encoding when streaming
def _process_options(options)
super

@ -158,7 +158,6 @@ def content_type
end.new
private
def params_parsers
super.merge @custom_param_parsers
end
@ -208,7 +207,6 @@ def fetch(key, *args, &block)
end
private
def load!
@id
end
@ -594,7 +592,6 @@ def build_response(klass)
end
private
def scrub_env!(env)
env.delete_if { |k, v| k =~ /^(action_dispatch|rack)\.request/ }
env.delete_if { |k, v| k =~ /^action_dispatch\.rescue/ }

@ -123,7 +123,6 @@ def strong_etag?
end
private
DATE = "Date"
LAST_MODIFIED = "Last-Modified"
SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public private must-revalidate])

@ -30,7 +30,6 @@ def call(env)
end
private
def html_response?(headers)
if content_type = headers[CONTENT_TYPE]
content_type =~ /html/
@ -91,7 +90,6 @@ def content_security_policy_nonce
end
private
def generate_content_security_policy_nonce
content_security_policy_nonce_generator.call(self)
end

@ -56,7 +56,6 @@ def filtered_path
end
private
def parameter_filter # :doc:
parameter_filter_for fetch_header("action_dispatch.parameter_filter") {
return NULL_PARAM_FILTER

@ -14,7 +14,6 @@ def filtered_location # :nodoc:
end
private
def location_filters
if request
request.get_header("action_dispatch.redirect_filter") || []

@ -116,7 +116,6 @@ def merge!(headers_or_env)
def env; @req.env.dup; end
private
# Converts an HTTP header name to an environment variable name if it is
# not contained within the headers hash.
def env_name(key)

@ -154,7 +154,6 @@ def negotiate_mime(order)
end
private
BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/
def valid_accept_header # :doc:

@ -290,11 +290,9 @@ def html?
def all?; false; end
protected
attr_reader :string, :synonyms
private
def to_ary; end
def to_a; end

@ -85,7 +85,6 @@ def path_parameters
end
private
def set_binary_encoding(params, controller, action)
return params unless controller && controller.valid_encoding?

@ -143,7 +143,6 @@ def closed?
end
private
def each_chunk(&block)
@buf.each(&block)
end
@ -409,7 +408,6 @@ def cookies
end
private
ContentTypeHeader = Struct.new :mime_type, :charset
NullContentTypeHeader = ContentTypeHeader.new nil, nil

@ -78,7 +78,6 @@ def path_for(options)
end
private
def add_params(path, params)
params = { params: params } unless params.is_a?(Hash)
params.reject! { |_, v| v.to_param.nil? }

@ -62,7 +62,6 @@ def clear
end
private
def extract_parameterized_parts(route, options, recall, parameterize = nil)
parameterized_parts = recall.merge(options)

@ -128,7 +128,6 @@ def followpos(node)
end
private
def followpos_table
@followpos ||= build_followpos
end

@ -141,7 +141,6 @@ def transitions
end
private
def states_hash_for(sym)
case sym
when String

@ -94,7 +94,6 @@ def transitions
end
private
def inverted
return @inverted if @inverted

@ -174,7 +174,6 @@ def to_regexp
end
private
def regexp_visitor
@anchored ? AnchoredRegexp : UnanchoredRegexp
end

@ -81,7 +81,6 @@ def visualizer
end
private
def partitioned_routes
routes.partition { |r|
r.path.anchored && r.ast.grep(Nodes::Symbol).all? { |n| n.default_regexp? }

@ -71,7 +71,6 @@ def add_route(name, mapping)
end
private
def clear_cache!
@ast = nil
@simulator = nil

@ -33,7 +33,6 @@ def next_token
end
private
# takes advantage of String @- deduping capabilities in Ruby 2.5 upwards
# see: https://bugs.ruby-lang.org/issues/13077
def dedup_scan(regex)

@ -59,7 +59,6 @@ def accept(node)
end
private
def visit(node)
send(DISPATCH_CACHE[node.type], node)
end
@ -168,7 +167,6 @@ def visit(node, block)
class String < FunctionalVisitor # :nodoc:
private
def binary(node, seed)
visit(node.right, visit(node.left, seed))
end
@ -214,7 +212,6 @@ def accept(node, seed = [[], []])
end
private
def binary(node, seed)
seed.last.concat node.children.map { |c|
"#{node.object_id} -> #{c.object_id};"

@ -252,7 +252,6 @@ def signed_or_encrypted
end
private
def upgrade_legacy_hmac_aes_cbc_cookies?
request.secret_key_base.present? &&
request.encrypted_signed_cookie_salt.present? &&
@ -428,7 +427,6 @@ def write(headers)
mattr_accessor :always_write_cookie, default: false
private
def escape(string)
::Rack::Utils.escape(string)
end

@ -42,7 +42,6 @@ def call(env)
end
private
def invoke_interceptors(request, exception)
backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner")
wrapper = ExceptionWrapper.new(backtrace_cleaner, exception)

@ -130,7 +130,6 @@ def source_to_show_id
end
private
def backtrace
Array(@exception.backtrace)
end

@ -30,7 +30,6 @@ def allows?(host)
end
private
def sanitize_hosts(hosts)
Array(hosts).map do |host|
case host
@ -87,7 +86,6 @@ def call(env)
end
private
def authorized?(request)
origin_host = request.get_header("HTTP_HOST").to_s.sub(/:\d+\z/, "")
forwarded_host = request.x_forwarded_host.to_s.split(/,\s?/).last.to_s.sub(/:\d+\z/, "")

@ -32,7 +32,6 @@ def call(env)
end
private
def render(status, content_type, body)
format = "to_#{content_type.to_sym}" if content_type
if format && body.respond_to?(format)

@ -156,7 +156,6 @@ def to_s
end
private
def ips_from(header) # :doc:
return [] unless header
# Split the comma-separated list into an array of strings.

@ -30,7 +30,6 @@ def generate_sid
end
private
def initialize_sid # :doc:
@default_options.delete(:sidbits)
@default_options.delete(:secure_random)
@ -83,7 +82,6 @@ class AbstractStore < Rack::Session::Abstract::Persisted
include SessionObject
private
def set_cookie(request, session_id, cookie)
request.cookie_jar[key] = cookie
end

@ -67,7 +67,6 @@ def load_session(req)
end
private
def extract_session_id(req)
stale_session_check! do
unpacked_cookie_data(req)["session_id"]

@ -40,7 +40,6 @@ def call(env)
end
private
def render_exception(request, exception)
backtrace_cleaner = request.get_header "action_dispatch.backtrace_cleaner"
wrapper = ExceptionWrapper.new(backtrace_cleaner, exception)

@ -134,7 +134,6 @@ def build(app = nil, &block)
end
private
def assert_index(index, where)
i = index.is_a?(Integer) ? index : middlewares.index { |m| m.klass == index }
raise "No such middleware to insert #{where}: #{index.inspect}" unless i

@ -216,7 +216,6 @@ def each(&block)
end
private
def load_for_read!
load! if !loaded? && exists?
end

@ -177,7 +177,6 @@ def header(routes)
end
private
def draw_section(routes)
header_lengths = ["Prefix", "Verb", "URI Pattern"].map(&:length)
name_width, verb_width, path_width = widths(routes).zip(header_lengths).map(&:max)
@ -210,7 +209,6 @@ def section(routes)
end
private
def draw_expanded_section(routes)
routes.map.each_with_index do |r, i|
<<~MESSAGE.chomp

@ -1673,7 +1673,6 @@ def root(path, options = {})
end
private
def parent_resource
@scope[:scope_level_resource]
end

@ -156,7 +156,6 @@ def #{action}_polymorphic_path(record_or_hash, options = {})
end
private
def polymorphic_url_for_action(action, record_or_hash, options)
polymorphic_url(record_or_hash, options.merge(action: action))
end
@ -323,7 +322,6 @@ def handle_list(list)
end
private
def polymorphic_mapping(target, record)
if record.respond_to?(:to_model)
target._routes.polymorphic_mappings[record.to_model.model_name.name]

@ -40,7 +40,6 @@ def serve(req)
end
private
def controller(req)
req.controller_class
rescue NameError => e
@ -59,7 +58,6 @@ def initialize(controller_class)
end
private
def controller(_); @controller_class; end
end
@ -215,7 +213,6 @@ def call(t, args, inner_options)
end
private
def optimized_helper(args)
params = parameterize_args(args) do
raise_generation_error(args)

@ -215,13 +215,11 @@ def route_for(name, *args)
end
protected
def optimize_routes_generation?
_routes.optimize_routes_generation? && default_url_options.empty?
end
private
def _with_routes(routes) # :doc:
old_routes, @_routes = @_routes, routes
yield

@ -35,7 +35,6 @@ def code_and_name
end
private
def code_from_name(name)
GENERIC_RESPONSE_CODES[name] || Rack::Utils::SYMBOL_TO_STATUS_CODE[name]
end

@ -335,7 +335,6 @@ def initialize(block, strict = false)
end
private
def make_request(env)
Request.new super, url_helpers, @block, strict
end

@ -18,7 +18,6 @@ def two
end
private
def handle_last_modified_and_etags
fresh_when(last_modified: Time.now.utc.beginning_of_day, etag: [ :foo, 123 ])
end

@ -132,7 +132,6 @@ def test_change_for_builder
end
private
def with_default_charset(charset)
old_default_charset = ActionDispatch::Response.default_charset
ActionDispatch::Response.default_charset = charset

@ -310,7 +310,6 @@ class ConditionalParentOfConditionalSkippingController < ConditionalFilterContro
after_action :conditional_in_parent_after, only: [:show, :another_action]
private
def conditional_in_parent_before
@ran_filter ||= []
@ran_filter << "conditional_in_parent_before"
@ -508,7 +507,6 @@ def index
end
private
def filter_one
@filters ||= []
@filters << "filter_one"
@ -532,7 +530,6 @@ class ImplicitActionsController < ActionController::Base
before_action :find_except, except: :edit
private
def find_only
@only = "Only"
end

@ -361,7 +361,6 @@ def test_flash_usable_in_metal_without_helper
end
private
# Overwrite get to send SessionSecret in env hash
def get(path, *args)
args[0] ||= {}

@ -32,7 +32,6 @@ def search
end
private
def authenticate
authenticate_or_request_with_http_basic do |username, password|
username == "lifo" && password == "world"
@ -172,7 +171,6 @@ def test_encode_credentials_has_no_newline
end
private
def encode_credentials(username, password)
"Basic #{::Base64.encode64("#{username}:#{password}")}"
end

@ -20,7 +20,6 @@ def display
end
private
def authenticate
authenticate_or_request_with_http_digest("SuperSecret") do |username|
# Returns the password
@ -254,7 +253,6 @@ def authenticate_with_request
end
private
def encode_credentials(options)
options.reverse_merge!(nc: "00000001", cnonce: "0a4f113b", password_is_ha1: false)
password = options.delete(:password)

@ -21,7 +21,6 @@ def show
end
private
def authenticate
authenticate_or_request_with_http_token do |token, _|
token == "lifo"
@ -190,7 +189,6 @@ def authenticate_long_credentials
end
private
def sample_request(token, options = { nonce: "def" })
authorization = options.inject([%{Token token="#{token}"}]) do |arr, (k, v)|
arr << "#{k}=\"#{v}\""

@ -43,7 +43,6 @@ def index
end
private
def with_iphone
request.format = "iphone" if request.env["HTTP_ACCEPT"] == "text/iphone"
yield

@ -67,7 +67,6 @@ def with_error
end
private
def show_detailed_exceptions?
request.local?
end

@ -37,7 +37,6 @@ def overridden
end
private
def secretz
render plain: "FAIL WHALE!"
end

@ -52,7 +52,6 @@ def teardown
end
private
def assert_logged(message)
old_logger = ActionController::Base.logger
log = StringIO.new

@ -24,7 +24,6 @@ def test_parse_error_logged_once
end
private
def capture_log_output
output = StringIO.new
request.set_header "action_dispatch.logger", ActiveSupport::Logger.new(output)

@ -411,7 +411,6 @@ def test_uses_model_attribute_names_with_irregular_inflection
end
private
def with_dup
original = ActiveSupport::Inflector::Inflections.instance_variable_get(:@__instance__)[:en]
ActiveSupport::Inflector::Inflections.instance_variable_set(:@__instance__, en: original.dup)

@ -265,7 +265,6 @@ def head_default_content_type
end
private
def set_variable_for_layout
@variable_for_layout = nil
end

@ -112,7 +112,6 @@ def index
end
private
def add_called_callback(name)
@called_callbacks ||= []
@called_callbacks << name

@ -304,7 +304,6 @@ def test_block_rescue_handler_with_argument_as_string
end
private
def capture_log_output
output = StringIO.new
request.set_header "action_dispatch.logger", ActiveSupport::Logger.new(output)
@ -351,7 +350,6 @@ def show_errors(exception)
end
private
def with_test_routing
with_routing do |set|
set.draw do

@ -51,7 +51,6 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
class ShowExceptionsOverriddenController < ShowExceptionsController
private
def show_detailed_exceptions?
params["detailed"] == "1"
end

@ -165,7 +165,6 @@ def boom
end
private
def generate_url(opts)
url_for(opts.merge(action: "test_uri"))
end

@ -38,7 +38,6 @@ def test_before_and_after_callbacks
end
private
def dispatch(&block)
ActionDispatch::Callbacks.new(block || DummyApp.new).call(
"rack.input" => StringIO.new("")

@ -307,7 +307,6 @@ def test_redirect_works_with_dynamic_sources
end
private
def assert_policy(expected, report_only: false)
if report_only
expected_header = "Content-Security-Policy-Report-Only"
@ -470,7 +469,6 @@ def test_generates_no_content_security_policy
end
private
def assert_policy(expected, report_only: false)
assert_response :success

@ -29,7 +29,6 @@ class RequestIdTest < ActiveSupport::TestCase
end
private
def stub_request(env = {})
ActionDispatch::RequestId.new(lambda { |environment| [ 200, environment, [] ] }).call(env)
ActionDispatch::Request.new(env)
@ -58,7 +57,6 @@ def index
end
private
def with_test_route_set
with_routing do |set|
set.draw do

@ -3810,7 +3810,6 @@ def test_nested_routes_under_format_resource
end
private
def draw(&block)
self.class.stub_controllers do |routes|
routes.default_url_options = { host: "www.example.com" }
@ -4953,7 +4952,6 @@ def test_paths_with_partial_dynamic_segments_are_recognised
end
private
def assert_params(params)
assert_equal(params, request.path_parameters)
end
@ -5184,7 +5182,6 @@ def test_class_constraints_dont_leak_between_routes
end
private
def recognize_path(*args)
Routes.recognize_path(*args)
end

@ -379,7 +379,6 @@ def test_session_store_with_all_domains
end
private
# Overwrite get to send SessionSecret in env hash
def get(path, *args)
args[0] ||= {}

@ -232,7 +232,6 @@ def test_serves_static_file_with_asterisk
end
private
def assert_gzip(file_name, response)
expected = File.read("#{FIXTURE_LOAD_PATH}/#{public_path}" + file_name)
actual = ActiveSupport::Gzip.decompress(response.body)

@ -66,7 +66,6 @@ def setup
end
private
def assert_tokens(expected_tokens, scanner, pattern)
actual_tokens = []
while token = scanner.next_token

@ -503,7 +503,6 @@ def test_eager_load_without_routes
end
private
def get(*args)
ActiveSupport::Deprecation.silence do
mapper.get(*args)

@ -41,7 +41,6 @@ def clear_cache
end
private
def dirs_to_watch
fs_paths = all_view_paths.grep(FileSystemResolver)
fs_paths.map(&:path).sort.uniq

@ -68,7 +68,6 @@ def append!(key, value)
end
private
def inside_fiber?
Fiber.current.object_id != @root
end

@ -38,7 +38,6 @@ def error_message
end
private
def object_has_errors?
object.respond_to?(:errors) && object.errors.respond_to?(:[]) && error_message.present?
end

@ -227,7 +227,6 @@ def digest_path_from_template(template) # :nodoc:
end
private
def fragment_name_with_digest(name, virtual_path, digest_path)
virtual_path ||= @virtual_path

@ -688,7 +688,6 @@ def time_tag(date_or_time, *args, &block)
end
private
def normalize_distance_of_time_argument_to_time(value)
if value.is_a?(Numeric)
Time.at(value)

@ -403,7 +403,6 @@ def number_to_human(number, options = {})
end
private
def delegate_number_helper_method(method, number, options)
return unless number
options = escape_unsafe_options(options.symbolize_keys)

@ -34,7 +34,6 @@ def render
end
private
def value
if @allow_method_names_outside_object
object.public_send @method_name if object && object.respond_to?(@method_name)

@ -39,7 +39,6 @@ def render
end
private
def checked?(value)
case value
when TrueClass, FalseClass

@ -22,7 +22,6 @@ def render(&block)
end
private
def render_component(builder)
builder.check_box + builder.label
end

@ -37,7 +37,6 @@ def initialize(object_name, method_name, template_object, collection, value_meth
end
private
def instantiate_builder(builder_class, item, value, text, html_options)
builder_class.new(@template_object, @object_name, @method_name, item,
sanitize_attribute_name(value), text, value, html_options)

@ -21,7 +21,6 @@ def render(&block)
end
private
def render_component(builder)
builder.radio_button + builder.label
end

@ -12,7 +12,6 @@ def render
end
private
def validate_color_string(string)
regex = /#[0-9a-fA-F]{6}/
if regex.match?(string)

Some files were not shown because too many files have changed in this diff Show More