diff --git a/CHANGELOG.md b/CHANGELOG.md index 91ab799347..9784523eac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rubocop/cop/mixin/index_method.rb b/lib/rubocop/cop/mixin/index_method.rb index 3dea56dd85..53e99f96c6 100644 --- a/lib/rubocop/cop/mixin/index_method.rb +++ b/lib/rubocop/cop/mixin/index_method.rb @@ -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') diff --git a/lib/rubocop/cop/rails/action_filter.rb b/lib/rubocop/cop/rails/action_filter.rb index dea8b1ae7e..8b4771cb33 100644 --- a/lib/rubocop/cop/rails/action_filter.rb +++ b/lib/rubocop/cop/rails/action_filter.rb @@ -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 diff --git a/lib/rubocop/cop/rails/active_record_aliases.rb b/lib/rubocop/cop/rails/active_record_aliases.rb index 437dde77bd..55b7b86719 100644 --- a/lib/rubocop/cop/rails/active_record_aliases.rb +++ b/lib/rubocop/cop/rails/active_record_aliases.rb @@ -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 diff --git a/lib/rubocop/cop/rails/active_support_aliases.rb b/lib/rubocop/cop/rails/active_support_aliases.rb index 940effd5ef..7ce085cd2b 100644 --- a/lib/rubocop/cop/rails/active_support_aliases.rb +++ b/lib/rubocop/cop/rails/active_support_aliases.rb @@ -21,6 +21,7 @@ module Rails # class ActiveSupportAliases < Cop MSG = 'Use `%s` instead of `%s`.' + RESTRICT_ON_SEND = %i[starts_with? ends_with? append prepend].freeze ALIASES = { starts_with?: { diff --git a/lib/rubocop/cop/rails/assert_not.rb b/lib/rubocop/cop/rails/assert_not.rb index 730eec7ea2..b866e27485 100644 --- a/lib/rubocop/cop/rails/assert_not.rb +++ b/lib/rubocop/cop/rails/assert_not.rb @@ -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 ... :!) ...)' diff --git a/lib/rubocop/cop/rails/attribute_default_block_value.rb b/lib/rubocop/cop/rails/attribute_default_block_value.rb index 2011d88be1..e44c2050ef 100644 --- a/lib/rubocop/cop/rails/attribute_default_block_value.rb +++ b/lib/rubocop/cop/rails/attribute_default_block_value.rb @@ -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 diff --git a/lib/rubocop/cop/rails/belongs_to.rb b/lib/rubocop/cop/rails/belongs_to.rb index 9b0c3cd254..103d2f25f1 100644 --- a/lib/rubocop/cop/rails/belongs_to.rb +++ b/lib/rubocop/cop/rails/belongs_to.rb @@ -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 _ diff --git a/lib/rubocop/cop/rails/blank.rb b/lib/rubocop/cop/rails/blank.rb index 821af08457..055597d8f3 100644 --- a/lib/rubocop/cop/rails/blank.rb +++ b/lib/rubocop/cop/rails/blank.rb @@ -58,6 +58,7 @@ class Blank < Cop MSG_NOT_PRESENT = 'Use `%s` instead of `%s`.' MSG_UNLESS_PRESENT = 'Use `if %s` instead of ' \ '`%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 diff --git a/lib/rubocop/cop/rails/content_tag.rb b/lib/rubocop/cop/rails/content_tag.rb index b251c143d5..ebe191590c 100644 --- a/lib/rubocop/cop/rails/content_tag.rb +++ b/lib/rubocop/cop/rails/content_tag.rb @@ -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 diff --git a/lib/rubocop/cop/rails/create_table_with_timestamps.rb b/lib/rubocop/cop/rails/create_table_with_timestamps.rb index 73ee5a5759..4f47ec66fd 100644 --- a/lib/rubocop/cop/rails/create_table_with_timestamps.rb +++ b/lib/rubocop/cop/rails/create_table_with_timestamps.rb @@ -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 diff --git a/lib/rubocop/cop/rails/date.rb b/lib/rubocop/cop/rails/date.rb index 8ea8fd94b4..c5fa04377f 100644 --- a/lib/rubocop/cop/rails/date.rb +++ b/lib/rubocop/cop/rails/date.rb @@ -52,6 +52,8 @@ class Date < Cop MSG_SEND = 'Do not use `%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 = [ diff --git a/lib/rubocop/cop/rails/default_scope.rb b/lib/rubocop/cop/rails/default_scope.rb index 6a61b9604f..0f0e46b2b0 100644 --- a/lib/rubocop/cop/rails/default_scope.rb +++ b/lib/rubocop/cop/rails/default_scope.rb @@ -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 ...) diff --git a/lib/rubocop/cop/rails/delegate_allow_blank.rb b/lib/rubocop/cop/rails/delegate_allow_blank.rb index 78ddf361d8..dce4874aae 100644 --- a/lib/rubocop/cop/rails/delegate_allow_blank.rb +++ b/lib/rubocop/cop/rails/delegate_allow_blank.rb @@ -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) ...>)) diff --git a/lib/rubocop/cop/rails/enum_hash.rb b/lib/rubocop/cop/rails/enum_hash.rb index fa36006d5e..d8dc0f75dd 100644 --- a/lib/rubocop/cop/rails/enum_hash.rb +++ b/lib/rubocop/cop/rails/enum_hash.rb @@ -20,6 +20,7 @@ module Rails class EnumHash < Cop MSG = 'Enum defined as an array found in `%s` enum declaration. '\ 'Use hash syntax instead.' + RESTRICT_ON_SEND = %i[enum].freeze def_node_matcher :enum?, <<~PATTERN (send nil? :enum (hash $...)) diff --git a/lib/rubocop/cop/rails/enum_uniqueness.rb b/lib/rubocop/cop/rails/enum_uniqueness.rb index 7c5f13b300..72296cf759 100644 --- a/lib/rubocop/cop/rails/enum_uniqueness.rb +++ b/lib/rubocop/cop/rails/enum_uniqueness.rb @@ -22,6 +22,7 @@ class EnumUniqueness < Cop MSG = 'Duplicate value `%s` found in `%s` ' \ 'enum declaration.' + RESTRICT_ON_SEND = %i[enum].freeze def_node_matcher :enum?, <<~PATTERN (send nil? :enum (hash $...)) diff --git a/lib/rubocop/cop/rails/environment_comparison.rb b/lib/rubocop/cop/rails/environment_comparison.rb index 154b3df91e..7cf5281f60 100644 --- a/lib/rubocop/cop/rails/environment_comparison.rb +++ b/lib/rubocop/cop/rails/environment_comparison.rb @@ -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) diff --git a/lib/rubocop/cop/rails/exit.rb b/lib/rubocop/cop/rails/exit.rb index 1db315edc9..e1a4857c16 100644 --- a/lib/rubocop/cop/rails/exit.rb +++ b/lib/rubocop/cop/rails/exit.rb @@ -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) @@ -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 diff --git a/lib/rubocop/cop/rails/file_path.rb b/lib/rubocop/cop/rails/file_path.rb index d5b2cc20d1..ae84937a05 100644 --- a/lib/rubocop/cop/rails/file_path.rb +++ b/lib/rubocop/cop/rails/file_path.rb @@ -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 ...) diff --git a/lib/rubocop/cop/rails/find_by.rb b/lib/rubocop/cop/rails/find_by.rb index c894e40f34..51ffc8a10c 100644 --- a/lib/rubocop/cop/rails/find_by.rb +++ b/lib/rubocop/cop/rails/find_by.rb @@ -17,7 +17,7 @@ class FindBy < Cop include RangeHelp MSG = 'Use `find_by` instead of `where.%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}) diff --git a/lib/rubocop/cop/rails/find_by_id.rb b/lib/rubocop/cop/rails/find_by_id.rb index 7d7837ec78..f4c46ff075 100644 --- a/lib/rubocop/cop/rails/find_by_id.rb +++ b/lib/rubocop/cop/rails/find_by_id.rb @@ -20,6 +20,7 @@ class FindById < Cop include RangeHelp MSG = 'Use `%s` instead of `%s`.' + RESTRICT_ON_SEND = %i[take! find_by_id! find_by!].freeze def_node_matcher :where_take?, <<~PATTERN (send diff --git a/lib/rubocop/cop/rails/find_each.rb b/lib/rubocop/cop/rails/find_each.rb index 302fc33b6e..cc1ce0e27b 100644 --- a/lib/rubocop/cop/rails/find_each.rb +++ b/lib/rubocop/cop/rails/find_each.rb @@ -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 @@ -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) } diff --git a/lib/rubocop/cop/rails/has_and_belongs_to_many.rb b/lib/rubocop/cop/rails/has_and_belongs_to_many.rb index 4ef8c2631e..45a75ff745 100644 --- a/lib/rubocop/cop/rails/has_and_belongs_to_many.rb +++ b/lib/rubocop/cop/rails/has_and_belongs_to_many.rb @@ -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) diff --git a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb b/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb index 4956f746f2..ddcb1b8a06 100644 --- a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb +++ b/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb @@ -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) diff --git a/lib/rubocop/cop/rails/http_positional_arguments.rb b/lib/rubocop/cop/rails/http_positional_arguments.rb index 03ab9c4e26..b8925d006d 100644 --- a/lib/rubocop/cop/rails/http_positional_arguments.rb +++ b/lib/rubocop/cop/rails/http_positional_arguments.rb @@ -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 diff --git a/lib/rubocop/cop/rails/http_status.rb b/lib/rubocop/cop/rails/http_status.rb index e6dffc73d4..3888692b5d 100644 --- a/lib/rubocop/cop/rails/http_status.rb +++ b/lib/rubocop/cop/rails/http_status.rb @@ -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) diff --git a/lib/rubocop/cop/rails/ignored_skip_action_filter_option.rb b/lib/rubocop/cop/rails/ignored_skip_action_filter_option.rb index e82b137b1f..5683dbd4e9 100644 --- a/lib/rubocop/cop/rails/ignored_skip_action_filter_option.rb +++ b/lib/rubocop/cop/rails/ignored_skip_action_filter_option.rb @@ -42,13 +42,15 @@ class IgnoredSkipActionFilterOption < Cop `%s` option will be ignored when `%s` and `%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? diff --git a/lib/rubocop/cop/rails/inquiry.rb b/lib/rubocop/cop/rails/inquiry.rb index f5ea160187..b5174bfedc 100644 --- a/lib/rubocop/cop/rails/inquiry.rb +++ b/lib/rubocop/cop/rails/inquiry.rb @@ -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? diff --git a/lib/rubocop/cop/rails/inverse_of.rb b/lib/rubocop/cop/rails/inverse_of.rb index afd723bdbe..e9d118d761 100644 --- a/lib/rubocop/cop/rails/inverse_of.rb +++ b/lib/rubocop/cop/rails/inverse_of.rb @@ -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} _ $...) diff --git a/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb b/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb index 60957b1632..8426ff11cf 100644 --- a/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb +++ b/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb @@ -85,22 +85,24 @@ module Rails class LexicallyScopedActionFilter < Cop MSG = '%s not explicitly defined on the %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? diff --git a/lib/rubocop/cop/rails/link_to_blank.rb b/lib/rubocop/cop/rails/link_to_blank.rb index 79e976b27b..2ec78e44ec 100644 --- a/lib/rubocop/cop/rails/link_to_blank.rb +++ b/lib/rubocop/cop/rails/link_to_blank.rb @@ -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)}) @@ -35,10 +36,7 @@ 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| @@ -46,7 +44,6 @@ def on_send(node) add_offense(blank) if blank && options.none? { |o| includes_noopener?(o) } end end - # rubocop:enable Metrics/CyclomaticComplexity def autocorrect(node) lambda do |corrector| diff --git a/lib/rubocop/cop/rails/match_route.rb b/lib/rubocop/cop/rails/match_route.rb index d230b800b0..88bf23e7b3 100644 --- a/lib/rubocop/cop/rails/match_route.rb +++ b/lib/rubocop/cop/rails/match_route.rb @@ -22,6 +22,7 @@ module Rails # class MatchRoute < Cop MSG = 'Use `%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 diff --git a/lib/rubocop/cop/rails/negate_include.rb b/lib/rubocop/cop/rails/negate_include.rb index 04acdffc7f..468cf33754 100644 --- a/lib/rubocop/cop/rails/negate_include.rb +++ b/lib/rubocop/cop/rails/negate_include.rb @@ -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? $_) :!) diff --git a/lib/rubocop/cop/rails/not_null_column.rb b/lib/rubocop/cop/rails/not_null_column.rb index 119871769a..b17543ff8c 100644 --- a/lib/rubocop/cop/rails/not_null_column.rb +++ b/lib/rubocop/cop/rails/not_null_column.rb @@ -18,6 +18,7 @@ module Rails # add_reference :products, :category, null: false, default: 1 class NotNullColumn < Cop MSG = 'Do not add a NOT NULL column without a default value.' + RESTRICT_ON_SEND = %i[add_column add_reference].freeze def_node_matcher :add_not_null_column?, <<~PATTERN (send nil? :add_column _ _ _ (hash $...)) diff --git a/lib/rubocop/cop/rails/order_by_id.rb b/lib/rubocop/cop/rails/order_by_id.rb index 4cc13db266..62c1eae559 100644 --- a/lib/rubocop/cop/rails/order_by_id.rb +++ b/lib/rubocop/cop/rails/order_by_id.rb @@ -25,6 +25,7 @@ class OrderById < Base MSG = 'Do not use the `id` column for ordering. '\ 'Use a timestamp column to order chronologically.' + RESTRICT_ON_SEND = %i[order].freeze def_node_matcher :order_by_id?, <<~PATTERN (send _ :order @@ -37,8 +38,6 @@ class OrderById < Base PATTERN def on_send(node) - return unless node.method?(:order) - add_offense(offense_range(node)) if order_by_id?(node) end diff --git a/lib/rubocop/cop/rails/output.rb b/lib/rubocop/cop/rails/output.rb index 1c8c804030..72e83f2e67 100644 --- a/lib/rubocop/cop/rails/output.rb +++ b/lib/rubocop/cop/rails/output.rb @@ -16,6 +16,9 @@ module Rails class Output < Cop MSG = 'Do not write to stdout. ' \ "Use Rails's logger if you want to log." + RESTRICT_ON_SEND = %i[ + ap p pp pretty_print print puts binwrite syswrite write write_nonblock + ].freeze def_node_matcher :output?, <<~PATTERN (send nil? {:ap :p :pp :pretty_print :print :puts} ...) diff --git a/lib/rubocop/cop/rails/output_safety.rb b/lib/rubocop/cop/rails/output_safety.rb index 4c52a07a9a..e5ff9348d6 100644 --- a/lib/rubocop/cop/rails/output_safety.rb +++ b/lib/rubocop/cop/rails/output_safety.rb @@ -64,6 +64,7 @@ module Rails # # "<b>hi</b> <b>hi</b>" class OutputSafety < Cop MSG = 'Tagging a string as html safe may be a security risk.' + RESTRICT_ON_SEND = %i[html_safe raw safe_concat].freeze def on_send(node) return if non_interpolated_string?(node) diff --git a/lib/rubocop/cop/rails/pick.rb b/lib/rubocop/cop/rails/pick.rb index fd9cd92e48..a5aec08fe5 100644 --- a/lib/rubocop/cop/rails/pick.rb +++ b/lib/rubocop/cop/rails/pick.rb @@ -21,6 +21,7 @@ class Pick < Cop extend TargetRailsVersion MSG = 'Prefer `pick(%s)` over `pluck(%s).first`.' + RESTRICT_ON_SEND = %i[first].freeze minimum_target_rails_version 6.0 diff --git a/lib/rubocop/cop/rails/pluck_id.rb b/lib/rubocop/cop/rails/pluck_id.rb index cf94877f67..f06f1fcba7 100644 --- a/lib/rubocop/cop/rails/pluck_id.rb +++ b/lib/rubocop/cop/rails/pluck_id.rb @@ -27,6 +27,7 @@ class PluckId < Cop include ActiveRecordHelper MSG = 'Use `ids` instead of `%s`.' + RESTRICT_ON_SEND = %i[pluck].freeze def_node_matcher :pluck_id_call?, <<~PATTERN (send _ :pluck {(sym :id) (send nil? :primary_key)}) diff --git a/lib/rubocop/cop/rails/pluck_in_where.rb b/lib/rubocop/cop/rails/pluck_in_where.rb index bed118a8e7..fa752281e5 100644 --- a/lib/rubocop/cop/rails/pluck_in_where.rb +++ b/lib/rubocop/cop/rails/pluck_in_where.rb @@ -39,9 +39,10 @@ class PluckInWhere < Cop include ConfigurableEnforcedStyle MSG = 'Use `select` instead of `pluck` within `where` query method.' + RESTRICT_ON_SEND = %i[pluck].freeze def on_send(node) - return unless node.method?(:pluck) && in_where?(node) + return unless in_where?(node) return if style == :conservative && !root_receiver(node)&.const_type? add_offense(node, location: :selector) diff --git a/lib/rubocop/cop/rails/pluralization_grammar.rb b/lib/rubocop/cop/rails/pluralization_grammar.rb index 5bfbbd7464..47d9aa88a9 100644 --- a/lib/rubocop/cop/rails/pluralization_grammar.rb +++ b/lib/rubocop/cop/rails/pluralization_grammar.rb @@ -24,6 +24,8 @@ class PluralizationGrammar < Cop month: :months, year: :years }.freeze + RESTRICT_ON_SEND = SINGULAR_DURATION_METHODS.keys + SINGULAR_DURATION_METHODS.values + PLURAL_DURATION_METHODS = SINGULAR_DURATION_METHODS.invert.freeze MSG = 'Prefer `%s.%s`.' diff --git a/lib/rubocop/cop/rails/present.rb b/lib/rubocop/cop/rails/present.rb index 36a0d897b6..07737324ec 100644 --- a/lib/rubocop/cop/rails/present.rb +++ b/lib/rubocop/cop/rails/present.rb @@ -49,6 +49,7 @@ class Present < Cop '`%s`.' MSG_UNLESS_BLANK = 'Use `if %s` instead of ' \ '`%s`.' + RESTRICT_ON_SEND = %i[!].freeze def_node_matcher :exists_and_not_empty?, <<~PATTERN (and diff --git a/lib/rubocop/cop/rails/read_write_attribute.rb b/lib/rubocop/cop/rails/read_write_attribute.rb index d8908d0b26..91aee0655b 100644 --- a/lib/rubocop/cop/rails/read_write_attribute.rb +++ b/lib/rubocop/cop/rails/read_write_attribute.rb @@ -25,6 +25,7 @@ module Rails # self[:attr] = val class ReadWriteAttribute < Cop MSG = 'Prefer `%s` over `%s`.' + RESTRICT_ON_SEND = %i[read_attribute write_attribute].freeze def_node_matcher :read_write_attribute?, <<~PATTERN { diff --git a/lib/rubocop/cop/rails/redundant_allow_nil.rb b/lib/rubocop/cop/rails/redundant_allow_nil.rb index 6466274010..0abc878e00 100644 --- a/lib/rubocop/cop/rails/redundant_allow_nil.rb +++ b/lib/rubocop/cop/rails/redundant_allow_nil.rb @@ -35,9 +35,9 @@ class RedundantAllowNil < Cop MSG_ALLOW_NIL_FALSE = '`allow_nil: false` is redundant when `allow_blank` is true.' - def on_send(node) - return unless node.method?(:validates) + RESTRICT_ON_SEND = %i[validates].freeze + def on_send(node) allow_nil, allow_blank = find_allow_nil_and_allow_blank(node) return unless allow_nil && allow_blank diff --git a/lib/rubocop/cop/rails/redundant_foreign_key.rb b/lib/rubocop/cop/rails/redundant_foreign_key.rb index ebba1c9730..6df6a9e5d1 100644 --- a/lib/rubocop/cop/rails/redundant_foreign_key.rb +++ b/lib/rubocop/cop/rails/redundant_foreign_key.rb @@ -28,6 +28,7 @@ class RedundantForeignKey < Cop include RangeHelp MSG = 'Specifying the default value for `foreign_key` is redundant.' + RESTRICT_ON_SEND = %i[belongs_to has_one has_many has_and_belongs_to_many].freeze def_node_matcher :association_with_foreign_key, <<~PATTERN (send nil? ${:belongs_to :has_one :has_many :has_and_belongs_to_many} ({sym str} $_) diff --git a/lib/rubocop/cop/rails/reflection_class_name.rb b/lib/rubocop/cop/rails/reflection_class_name.rb index 336b95fc26..ed2bb88051 100644 --- a/lib/rubocop/cop/rails/reflection_class_name.rb +++ b/lib/rubocop/cop/rails/reflection_class_name.rb @@ -15,6 +15,7 @@ module Rails # has_many :accounts, class_name: 'Account' class ReflectionClassName < Cop MSG = 'Use a string value for `class_name`.' + RESTRICT_ON_SEND = %i[has_many has_one belongs_to].freeze def_node_matcher :association_with_reflection, <<~PATTERN (send nil? {:has_many :has_one :belongs_to} _ _ ? diff --git a/lib/rubocop/cop/rails/refute_methods.rb b/lib/rubocop/cop/rails/refute_methods.rb index 70bfe97a79..de2625bc27 100644 --- a/lib/rubocop/cop/rails/refute_methods.rb +++ b/lib/rubocop/cop/rails/refute_methods.rb @@ -53,6 +53,8 @@ class RefuteMethods < Cop REFUTE_METHODS = CORRECTIONS.keys.freeze ASSERT_NOT_METHODS = CORRECTIONS.values.freeze + RESTRICT_ON_SEND = REFUTE_METHODS + ASSERT_NOT_METHODS + def_node_matcher :offensive?, '(send nil? #bad_method? ...)' def on_send(node) diff --git a/lib/rubocop/cop/rails/render_inline.rb b/lib/rubocop/cop/rails/render_inline.rb index 5c8f0638be..de265bf10d 100644 --- a/lib/rubocop/cop/rails/render_inline.rb +++ b/lib/rubocop/cop/rails/render_inline.rb @@ -26,6 +26,7 @@ module Rails # class RenderInline < Cop MSG = 'Prefer using a template over inline rendering.' + RESTRICT_ON_SEND = %i[render].freeze def_node_matcher :render_with_inline_option?, <<~PATTERN (send nil? :render (hash <(pair {(sym :inline) (str "inline")} _) ...>)) diff --git a/lib/rubocop/cop/rails/render_plain_text.rb b/lib/rubocop/cop/rails/render_plain_text.rb index fb1bb9ef41..625e740af9 100644 --- a/lib/rubocop/cop/rails/render_plain_text.rb +++ b/lib/rubocop/cop/rails/render_plain_text.rb @@ -26,6 +26,7 @@ module Rails # class RenderPlainText < Cop MSG = 'Prefer `render plain:` over `render text:`.' + RESTRICT_ON_SEND = %i[render].freeze def_node_matcher :render_plain_text?, <<~PATTERN (send nil? :render $(hash <$(pair (sym :text) $_) ...>)) diff --git a/lib/rubocop/cop/rails/request_referer.rb b/lib/rubocop/cop/rails/request_referer.rb index f6413cce45..ece0daf4e2 100644 --- a/lib/rubocop/cop/rails/request_referer.rb +++ b/lib/rubocop/cop/rails/request_referer.rb @@ -24,6 +24,7 @@ class RequestReferer < Cop MSG = 'Use `request.%s` instead of ' \ '`request.%s`.' + RESTRICT_ON_SEND = %i[referer referrer].freeze def_node_matcher :referer?, <<~PATTERN (send (send nil? :request) {:referer :referrer}) diff --git a/lib/rubocop/cop/rails/safe_navigation.rb b/lib/rubocop/cop/rails/safe_navigation.rb index 45f8e211bb..8dfdb0b13d 100644 --- a/lib/rubocop/cop/rails/safe_navigation.rb +++ b/lib/rubocop/cop/rails/safe_navigation.rb @@ -40,6 +40,7 @@ class SafeNavigation < Cop include RangeHelp MSG = 'Use safe navigation (`&.`) instead of `%s`.' + RESTRICT_ON_SEND = %i[try try!].freeze def_node_matcher :try_call, <<~PATTERN (send !nil? ${:try :try!} $_ ...) diff --git a/lib/rubocop/cop/rails/save_bang.rb b/lib/rubocop/cop/rails/save_bang.rb index ee6066642f..5d1782fa29 100644 --- a/lib/rubocop/cop/rails/save_bang.rb +++ b/lib/rubocop/cop/rails/save_bang.rb @@ -113,8 +113,7 @@ class SaveBang < Cop first_or_create find_or_create_by].freeze MODIFY_PERSIST_METHODS = %i[save update update_attributes destroy].freeze - PERSIST_METHODS = (CREATE_PERSIST_METHODS + - MODIFY_PERSIST_METHODS).freeze + RESTRICT_ON_SEND = (CREATE_PERSIST_METHODS + MODIFY_PERSIST_METHODS).freeze def join_force?(force_class) force_class == VariableForce @@ -318,7 +317,7 @@ def return_value_assigned?(node) assignment&.lvasgn_type? end - def persist_method?(node, methods = PERSIST_METHODS) + def persist_method?(node, methods = RESTRICT_ON_SEND) methods.include?(node.method_name) && expected_signature?(node) && !allowed_receiver?(node) diff --git a/lib/rubocop/cop/rails/scope_args.rb b/lib/rubocop/cop/rails/scope_args.rb index 8155eecdc0..5b6b905a5a 100644 --- a/lib/rubocop/cop/rails/scope_args.rb +++ b/lib/rubocop/cop/rails/scope_args.rb @@ -15,6 +15,7 @@ module Rails # scope :something, -> { where(something: true) } class ScopeArgs < Cop MSG = 'Use `lambda`/`proc` instead of a plain method call.' + RESTRICT_ON_SEND = %i[scope].freeze def_node_matcher :scope?, '(send nil? :scope _ $send)' diff --git a/lib/rubocop/cop/rails/short_i18n.rb b/lib/rubocop/cop/rails/short_i18n.rb index ff22d850e2..c914097a9d 100644 --- a/lib/rubocop/cop/rails/short_i18n.rb +++ b/lib/rubocop/cop/rails/short_i18n.rb @@ -48,6 +48,8 @@ class ShortI18n < Cop localize: :l }.freeze + RESTRICT_ON_SEND = PREFERRED_METHODS.keys.freeze + def_node_matcher :long_i18n?, <<~PATTERN (send {nil? (const nil? :I18n)} ${:translate :localize} ...) PATTERN diff --git a/lib/rubocop/cop/rails/uniq_before_pluck.rb b/lib/rubocop/cop/rails/uniq_before_pluck.rb index c74d3f6d2d..1c7f2ddaa3 100644 --- a/lib/rubocop/cop/rails/uniq_before_pluck.rb +++ b/lib/rubocop/cop/rails/uniq_before_pluck.rb @@ -50,6 +50,7 @@ class UniqBeforePluck < RuboCop::Cop::Cop include RangeHelp MSG = 'Use `distinct` before `pluck`.' + RESTRICT_ON_SEND = %i[uniq distinct pluck].freeze NEWLINE = "\n" PATTERN = '[!^block (send (send %s :pluck ...) ' \ '${:uniq :distinct} ...)]' diff --git a/lib/rubocop/cop/rails/unique_validation_without_index.rb b/lib/rubocop/cop/rails/unique_validation_without_index.rb index 06cdd8d0f3..1c8350acc2 100644 --- a/lib/rubocop/cop/rails/unique_validation_without_index.rb +++ b/lib/rubocop/cop/rails/unique_validation_without_index.rb @@ -28,9 +28,9 @@ class UniqueValidationWithoutIndex < Cop include ActiveRecordHelper MSG = 'Uniqueness validation should be with a unique index.' + RESTRICT_ON_SEND = %i[validates].freeze def on_send(node) - return unless node.method?(:validates) return unless uniqueness_part(node) return if condition_part?(node) return unless schema diff --git a/lib/rubocop/cop/rails/validation.rb b/lib/rubocop/cop/rails/validation.rb index 0e4423fdec..e2a97af586 100644 --- a/lib/rubocop/cop/rails/validation.rb +++ b/lib/rubocop/cop/rails/validation.rb @@ -50,11 +50,11 @@ class Validation < Cop uniqueness ].freeze - DENYLIST = TYPES.map { |p| "validates_#{p}_of".to_sym }.freeze + RESTRICT_ON_SEND = TYPES.map { |p| "validates_#{p}_of".to_sym }.freeze ALLOWLIST = TYPES.map { |p| "validates :column, #{p}: value" }.freeze def on_send(node) - return unless !node.receiver && DENYLIST.include?(node.method_name) + return if node.receiver add_offense(node, location: :selector) end @@ -78,7 +78,7 @@ def message(node) end def preferred_method(method) - ALLOWLIST[DENYLIST.index(method.to_sym)] + ALLOWLIST[RESTRICT_ON_SEND.index(method.to_sym)] end def correct_validate_type(corrector, node) diff --git a/lib/rubocop/cop/rails/where_equals.rb b/lib/rubocop/cop/rails/where_equals.rb index b43069acaa..e2f1cc70c5 100644 --- a/lib/rubocop/cop/rails/where_equals.rb +++ b/lib/rubocop/cop/rails/where_equals.rb @@ -22,6 +22,7 @@ class WhereEquals < Cop include RangeHelp MSG = 'Use `%s` instead of manually constructing SQL.' + RESTRICT_ON_SEND = %i[where].freeze def_node_matcher :where_method_call?, <<~PATTERN { diff --git a/lib/rubocop/cop/rails/where_exists.rb b/lib/rubocop/cop/rails/where_exists.rb index 781f58385d..54170a3cb3 100644 --- a/lib/rubocop/cop/rails/where_exists.rb +++ b/lib/rubocop/cop/rails/where_exists.rb @@ -40,6 +40,7 @@ class WhereExists < Cop include ConfigurableEnforcedStyle MSG = 'Prefer `%s` over `%s`.' + RESTRICT_ON_SEND = %i[exists?].freeze def_node_matcher :where_exists_call?, <<~PATTERN (send (send _ :where $...) :exists?) diff --git a/lib/rubocop/cop/rails/where_not.rb b/lib/rubocop/cop/rails/where_not.rb index 4c406b6f9e..e97eec8117 100644 --- a/lib/rubocop/cop/rails/where_not.rb +++ b/lib/rubocop/cop/rails/where_not.rb @@ -25,6 +25,7 @@ class WhereNot < Cop include RangeHelp MSG = 'Use `%s` instead of manually constructing negated SQL in `where`.' + RESTRICT_ON_SEND = %i[where].freeze def_node_matcher :where_method_call?, <<~PATTERN { diff --git a/rubocop-rails.gemspec b/rubocop-rails.gemspec index d83af76259..b9c3b19c66 100644 --- a/rubocop-rails.gemspec +++ b/rubocop-rails.gemspec @@ -35,5 +35,5 @@ Gem::Specification.new do |s| # Rack::Utils::SYMBOL_TO_STATUS_CODE, which is used by HttpStatus cop, was # introduced in rack 1.1 s.add_runtime_dependency 'rack', '>= 1.1' - s.add_runtime_dependency 'rubocop', '>= 0.87.0' + s.add_runtime_dependency 'rubocop', '>= 0.90.0' end diff --git a/spec/rubocop/cop/rails/has_and_belongs_to_many_spec.rb b/spec/rubocop/cop/rails/has_and_belongs_to_many_spec.rb index 4dd8fd76d3..7aa75a33bc 100644 --- a/spec/rubocop/cop/rails/has_and_belongs_to_many_spec.rb +++ b/spec/rubocop/cop/rails/has_and_belongs_to_many_spec.rb @@ -9,4 +9,10 @@ ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `has_many :through` to `has_and_belongs_to_many`. RUBY end + + it 'does not register an offense for has_and_belongs_to_many with receiver' do + expect_no_offenses(<<~RUBY) + obj.has_and_belongs_to_many :groups + RUBY + end end diff --git a/spec/rubocop/cop/rails/validation_spec.rb b/spec/rubocop/cop/rails/validation_spec.rb index 40d0206e13..5795f62f73 100644 --- a/spec/rubocop/cop/rails/validation_spec.rb +++ b/spec/rubocop/cop/rails/validation_spec.rb @@ -7,7 +7,7 @@ expect_no_offenses('validates :name') end - described_class::DENYLIST.each_with_index do |validation, number| + described_class::RESTRICT_ON_SEND.each_with_index do |validation, number| it "registers an offense for #{validation}" do inspect_source("#{validation} :name") expect(cop.offenses.size).to eq(1)