Skip to content

Commit

Permalink
Require RuboCop 0.90 or higher due to use RESTRICT_ON_SEND
Browse files Browse the repository at this point in the history
Follow rubocop/rubocop#8365

This PR uses `RESTRICT_ON_SEND` to restrict callbacks `on_send`
to specific method names only.
`RESTRICT_ON_SEND` has been introduced since RuboCop 0.90.
  • Loading branch information
koic committed Nov 11, 2020
1 parent 88df394 commit 1e7370c
Show file tree
Hide file tree
Showing 63 changed files with 119 additions and 62 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -13,6 +13,10 @@
* [#371](https://github.com/rubocop-hq/rubocop-rails/pull/371): Fix an infinite loop error for `Rails/ActiveRecordCallbacksOrder` when callbacks have inline comments. ([@fatkodima][])
* [#364](https://github.com/rubocop-hq/rubocop-rails/pull/364): Fix a problem that `Rails/UniqueValidationWithoutIndex` doesn't work in classes defined with compact style. ([@sinsoku][])

### Changes

* [#383](https://github.com/rubocop-hq/rubocop-rails/pull/383): Require RuboCop 0.90 or higher. ([@koic][])

## 2.8.1 (2020-09-16)

### Bug fixes
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/mixin/index_method.rb
Expand Up @@ -4,6 +4,8 @@ module RuboCop
module Cop
# Common functionality for Rails/IndexBy and Rails/IndexWith
module IndexMethod # rubocop:disable Metrics/ModuleLength
RESTRICT_ON_SEND = %i[each_with_object to_h map collect []].freeze

def on_block(node)
on_bad_each_with_object(node) do |*match|
handle_possible_offense(node, match, 'each_with_object')
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/rails/action_filter.rb
Expand Up @@ -66,6 +66,8 @@ class ActionFilter < Cop
skip_action_callback
].freeze

RESTRICT_ON_SEND = FILTER_METHODS + ACTION_METHODS

def on_block(node)
check_method_node(node.send_node)
end
Expand Down
17 changes: 9 additions & 8 deletions lib/rubocop/cop/rails/active_record_aliases.rb
Expand Up @@ -20,16 +20,17 @@ class ActiveRecordAliases < Cop
update_attributes!: :update!
}.freeze

RESTRICT_ON_SEND = ALIASES.keys.freeze

def on_send(node)
ALIASES.each do |bad, good|
next unless node.method?(bad)
method_name = node.method_name

add_offense(node,
message: format(MSG, prefer: good, current: bad),
location: :selector,
severity: :warning)
break
end
add_offense(
node,
message: format(MSG, prefer: ALIASES[method_name], current: method_name),
location: :selector,
severity: :warning
)
end

alias on_csend on_send
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/active_support_aliases.rb
Expand Up @@ -21,6 +21,7 @@ module Rails
#
class ActiveSupportAliases < Cop
MSG = 'Use `%<prefer>s` instead of `%<current>s`.'
RESTRICT_ON_SEND = %i[starts_with? ends_with? append prepend].freeze

ALIASES = {
starts_with?: {
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/assert_not.rb
Expand Up @@ -15,6 +15,7 @@ module Rails
#
class AssertNot < RuboCop::Cop::Cop
MSG = 'Prefer `assert_not` over `assert !`.'
RESTRICT_ON_SEND = %i[assert].freeze

def_node_matcher :offensive?, '(send nil? :assert (send ... :!) ...)'

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/attribute_default_block_value.rb
Expand Up @@ -61,6 +61,7 @@ module Rails
# end
class AttributeDefaultBlockValue < Cop
MSG = 'Pass method in a block to `:default` option.'
RESTRICT_ON_SEND = %i[attribute].freeze
TYPE_OFFENDERS = %i[send array hash].freeze

def_node_matcher :default_attribute, <<~PATTERN
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/belongs_to.rb
Expand Up @@ -64,6 +64,7 @@ class BelongsTo < Cop
'option is deprecated and you want to use `optional: false`. ' \
'In most configurations, this is the default and you can omit ' \
'this option altogether'
RESTRICT_ON_SEND = %i[belongs_to].freeze

def_node_matcher :match_belongs_to_with_options, <<~PATTERN
(send _ :belongs_to _
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/blank.rb
Expand Up @@ -58,6 +58,7 @@ class Blank < Cop
MSG_NOT_PRESENT = 'Use `%<prefer>s` instead of `%<current>s`.'
MSG_UNLESS_PRESENT = 'Use `if %<prefer>s` instead of ' \
'`%<current>s`.'
RESTRICT_ON_SEND = %i[!].freeze

# `(send nil $_)` is not actually a valid match for an offense. Nodes
# that have a single method call on the left hand side
Expand Down
3 changes: 1 addition & 2 deletions lib/rubocop/cop/rails/content_tag.rb
Expand Up @@ -25,10 +25,9 @@ class ContentTag < Cop
minimum_target_rails_version 5.1

MSG = 'Use `tag` instead of `content_tag`.'
RESTRICT_ON_SEND = %i[content_tag].freeze

def on_send(node)
return unless node.method?(:content_tag)

first_argument = node.first_argument
return unless first_argument

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/create_table_with_timestamps.rb
Expand Up @@ -42,6 +42,7 @@ module Rails
# end
class CreateTableWithTimestamps < Cop
MSG = 'Add timestamps when creating a new table.'
RESTRICT_ON_SEND = %i[create_table].freeze

def_node_matcher :create_table_with_block?, <<~PATTERN
(block
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/rails/date.rb
Expand Up @@ -52,6 +52,8 @@ class Date < Cop
MSG_SEND = 'Do not use `%<method>s` on Date objects, because they ' \
'know nothing about the time zone in use.'

RESTRICT_ON_SEND = %i[to_time to_time_in_current_zone].freeze

BAD_DAYS = %i[today current yesterday tomorrow].freeze

DEPRECATED_METHODS = [
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/default_scope.rb
Expand Up @@ -24,6 +24,7 @@ module Rails
#
class DefaultScope < Cop
MSG = 'Avoid use of `default_scope`. It is better to use explicitly named scopes.'
RESTRICT_ON_SEND = %i[default_scope].freeze

def_node_matcher :method_call?, <<~PATTERN
(send nil? :default_scope ...)
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/delegate_allow_blank.rb
Expand Up @@ -15,6 +15,7 @@ module Rails
# delegate :foo, to: :bar, allow_nil: true
class DelegateAllowBlank < Cop
MSG = '`allow_blank` is not a valid option, use `allow_nil`.'
RESTRICT_ON_SEND = %i[delegate].freeze

def_node_matcher :allow_blank_option, <<~PATTERN
(send nil? :delegate _ (hash <$(pair (sym :allow_blank) true) ...>))
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/enum_hash.rb
Expand Up @@ -20,6 +20,7 @@ module Rails
class EnumHash < Cop
MSG = 'Enum defined as an array found in `%<enum>s` enum declaration. '\
'Use hash syntax instead.'
RESTRICT_ON_SEND = %i[enum].freeze

def_node_matcher :enum?, <<~PATTERN
(send nil? :enum (hash $...))
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/enum_uniqueness.rb
Expand Up @@ -22,6 +22,7 @@ class EnumUniqueness < Cop

MSG = 'Duplicate value `%<value>s` found in `%<enum>s` ' \
'enum declaration.'
RESTRICT_ON_SEND = %i[enum].freeze

def_node_matcher :enum?, <<~PATTERN
(send nil? :enum (hash $...))
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/rails/environment_comparison.rb
Expand Up @@ -21,6 +21,8 @@ class EnvironmentComparison < Cop
SYM_MSG = 'Do not compare `Rails.env` with a symbol, it will always ' \
'evaluate to `false`.'

RESTRICT_ON_SEND = %i[== !=].freeze

def_node_matcher :comparing_str_env_with_rails_env_on_lhs?, <<~PATTERN
(send
(send (const {nil? cbase} :Rails) :env)
Expand Down
10 changes: 2 additions & 8 deletions lib/rubocop/cop/rails/exit.rb
Expand Up @@ -27,7 +27,7 @@ class Exit < Cop
include ConfigurableEnforcedStyle

MSG = 'Do not use `exit` in Rails applications.'
TARGET_METHODS = %i[exit exit!].freeze
RESTRICT_ON_SEND = %i[exit exit!].freeze
EXPLICIT_RECEIVERS = %i[Kernel Process].freeze

def on_send(node)
Expand All @@ -37,13 +37,7 @@ def on_send(node)
private

def offending_node?(node)
right_method_name?(node.method_name) &&
right_argument_count?(node.arguments) &&
right_receiver?(node.receiver)
end

def right_method_name?(method_name)
TARGET_METHODS.include?(method_name)
right_argument_count?(node.arguments) && right_receiver?(node.receiver)
end

# More than 1 argument likely means it is a different
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/file_path.rb
Expand Up @@ -33,6 +33,7 @@ class FilePath < Cop
'instead.'
MSG_ARGUMENTS = 'Please use `Rails.root.join(\'path\', \'to\')` ' \
'instead.'
RESTRICT_ON_SEND = %i[join].freeze

def_node_matcher :file_join_nodes?, <<~PATTERN
(send (const nil? :File) :join ...)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/find_by.rb
Expand Up @@ -17,7 +17,7 @@ class FindBy < Cop
include RangeHelp

MSG = 'Use `find_by` instead of `where.%<method>s`.'
TARGET_SELECTORS = %i[first take].freeze
RESTRICT_ON_SEND = %i[first take].freeze

def_node_matcher :where_first?, <<~PATTERN
(send ({send csend} _ :where ...) {:first :take})
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/find_by_id.rb
Expand Up @@ -20,6 +20,7 @@ class FindById < Cop
include RangeHelp

MSG = 'Use `%<good_method>s` instead of `%<bad_method>s`.'
RESTRICT_ON_SEND = %i[take! find_by_id! find_by!].freeze

def_node_matcher :where_take?, <<~PATTERN
(send
Expand Down
5 changes: 2 additions & 3 deletions lib/rubocop/cop/rails/find_each.rb
Expand Up @@ -14,6 +14,7 @@ module Rails
# User.all.find_each
class FindEach < Cop
MSG = 'Use `find_each` instead of `each`.'
RESTRICT_ON_SEND = %i[each].freeze

SCOPE_METHODS = %i[
all eager_load includes joins left_joins left_outer_joins not preload
Expand All @@ -22,9 +23,7 @@ class FindEach < Cop
IGNORED_METHODS = %i[order limit select].freeze

def on_send(node)
return unless node.receiver&.send_type? &&
node.method?(:each)

return unless node.receiver&.send_type?
return unless SCOPE_METHODS.include?(node.receiver.method_name)
return if method_chain(node).any? { |m| ignored_by_find_each?(m) }

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/has_and_belongs_to_many.rb
Expand Up @@ -13,6 +13,7 @@ module Rails
# # has_many :ingredients, through: :recipe_ingredients
class HasAndBelongsToMany < Cop
MSG = 'Prefer `has_many :through` to `has_and_belongs_to_many`.'
RESTRICT_ON_SEND = %i[has_and_belongs_to_many].freeze

def on_send(node)
return unless node.command?(:has_and_belongs_to_many)
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb
Expand Up @@ -22,6 +22,7 @@ module Rails
# end
class HasManyOrHasOneDependent < Cop
MSG = 'Specify a `:dependent` option.'
RESTRICT_ON_SEND = %i[has_many has_one].freeze

def_node_search :active_resource_class?, <<~PATTERN
(const (const nil? :ActiveResource) :Base)
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/rails/http_positional_arguments.rb
Expand Up @@ -25,12 +25,12 @@ class HttpPositionalArguments < Cop
KEYWORD_ARGS = %i[
method params session body flash xhr as headers env to
].freeze
HTTP_METHODS = %i[get post put patch delete head].freeze
RESTRICT_ON_SEND = %i[get post put patch delete head].freeze

minimum_target_rails_version 5.0

def_node_matcher :http_request?, <<~PATTERN
(send nil? {#{HTTP_METHODS.map(&:inspect).join(' ')}} !nil? $_ ...)
(send nil? {#{RESTRICT_ON_SEND.map(&:inspect).join(' ')}} !nil? $_ ...)
PATTERN

def_node_matcher :kwsplat_hash?, <<~PATTERN
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/rails/http_status.rb
Expand Up @@ -34,6 +34,8 @@ module Rails
class HttpStatus < Cop
include ConfigurableEnforcedStyle

RESTRICT_ON_SEND = %i[render redirect_to].freeze

def_node_matcher :http_status, <<~PATTERN
{
(send nil? {:render :redirect_to} _ $hash)
Expand Down
12 changes: 7 additions & 5 deletions lib/rubocop/cop/rails/ignored_skip_action_filter_option.rb
Expand Up @@ -42,13 +42,15 @@ class IgnoredSkipActionFilterOption < Cop
`%<ignore>s` option will be ignored when `%<prefer>s` and `%<ignore>s` are used together.
MSG

FILTERS = %w[
:skip_after_action
:skip_around_action
:skip_before_action
:skip_action_callback
RESTRICT_ON_SEND = %i[
skip_after_action
skip_around_action
skip_before_action
skip_action_callback
].freeze

FILTERS = RESTRICT_ON_SEND.map { |method_name| ":#{method_name}" }

def_node_matcher :filter_options, <<~PATTERN
(send
nil?
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/rails/inquiry.rb
Expand Up @@ -24,9 +24,10 @@ module Rails
#
class Inquiry < Cop
MSG = "Prefer Ruby's comparison operators over Active Support's `inquiry`."
RESTRICT_ON_SEND = %i[inquiry].freeze

def on_send(node)
return unless node.method?(:inquiry) && node.arguments.empty?
return unless node.arguments.empty?
return unless (receiver = node.receiver)
return if !receiver.str_type? && !receiver.array_type?

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/inverse_of.rb
Expand Up @@ -132,6 +132,7 @@ class InverseOf < Cop
SPECIFY_MSG = 'Specify an `:inverse_of` option.'
NIL_MSG = 'You specified `inverse_of: nil`, you probably meant to ' \
'use `inverse_of: false`.'
RESTRICT_ON_SEND = %i[has_many has_one belongs_to].freeze

def_node_matcher :association_recv_arguments, <<~PATTERN
(send $_ {:has_many :has_one :belongs_to} _ $...)
Expand Down
30 changes: 16 additions & 14 deletions lib/rubocop/cop/rails/lexically_scoped_action_filter.rb
Expand Up @@ -85,22 +85,24 @@ module Rails
class LexicallyScopedActionFilter < Cop
MSG = '%<action>s not explicitly defined on the %<type>s.'

FILTERS = %w[
:after_action
:append_after_action
:append_around_action
:append_before_action
:around_action
:before_action
:prepend_after_action
:prepend_around_action
:prepend_before_action
:skip_after_action
:skip_around_action
:skip_before_action
:skip_action_callback
RESTRICT_ON_SEND = %i[
after_action
append_after_action
append_around_action
append_before_action
around_action
before_action
prepend_after_action
prepend_around_action
prepend_before_action
skip_after_action
skip_around_action
skip_before_action
skip_action_callback
].freeze

FILTERS = RESTRICT_ON_SEND.map { |method_name| ":#{method_name}" }

def_node_matcher :only_or_except_filter_methods, <<~PATTERN
(send
nil?
Expand Down
5 changes: 1 addition & 4 deletions lib/rubocop/cop/rails/link_to_blank.rb
Expand Up @@ -22,6 +22,7 @@ module Rails
# link_to 'Click here', url, target: '_blank', rel: 'noreferrer'
class LinkToBlank < Cop
MSG = 'Specify a `:rel` option containing noopener.'
RESTRICT_ON_SEND = %i[link_to].freeze

def_node_matcher :blank_target?, <<~PATTERN
(pair {(sym :target) (str "target")} {(str "_blank") (sym :_blank)})
Expand All @@ -35,18 +36,14 @@ class LinkToBlank < Cop
(pair {(sym :rel) (str "rel")} (str _))
PATTERN

# rubocop:disable Metrics/CyclomaticComplexity
def on_send(node)
return unless node.method?(:link_to)

option_nodes = node.each_child_node(:hash)

option_nodes.map(&:children).each do |options|
blank = options.find { |o| blank_target?(o) }
add_offense(blank) if blank && options.none? { |o| includes_noopener?(o) }
end
end
# rubocop:enable Metrics/CyclomaticComplexity

def autocorrect(node)
lambda do |corrector|
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/match_route.rb
Expand Up @@ -22,6 +22,7 @@ module Rails
#
class MatchRoute < Cop
MSG = 'Use `%<http_method>s` instead of `match` to define a route.'
RESTRICT_ON_SEND = %i[match].freeze
HTTP_METHODS = %i[get post put patch delete].freeze

def_node_matcher :match_method_call?, <<~PATTERN
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/negate_include.rb
Expand Up @@ -17,6 +17,7 @@ module Rails
#
class NegateInclude < Cop
MSG = 'Use `.exclude?` and remove the negation part.'
RESTRICT_ON_SEND = %i[!].freeze

def_node_matcher :negate_include_call?, <<~PATTERN
(send (send $_ :include? $_) :!)
Expand Down

0 comments on commit 1e7370c

Please sign in to comment.