From 619740f38bde3ee8d228ff4b6c6bb3ba201213c3 Mon Sep 17 00:00:00 2001 From: Maxim Krizhanovski Date: Tue, 30 Apr 2019 00:35:23 +0300 Subject: [PATCH] Refactor cops to use matching on children --- .../redundant_location_argument.rb | 21 ++------ lib/rubocop/cop/rails/belongs_to.rb | 54 +++++++------------ lib/rubocop/cop/rails/delegate_allow_blank.rb | 24 ++------- lib/rubocop/cop/rails/http_status.rb | 11 +--- .../cop/rails/reflection_class_name.rb | 17 +++--- 5 files changed, 37 insertions(+), 90 deletions(-) diff --git a/lib/rubocop/cop/internal_affairs/redundant_location_argument.rb b/lib/rubocop/cop/internal_affairs/redundant_location_argument.rb index 5b8b6376f03..75e90f6552f 100644 --- a/lib/rubocop/cop/internal_affairs/redundant_location_argument.rb +++ b/lib/rubocop/cop/internal_affairs/redundant_location_argument.rb @@ -21,12 +21,10 @@ class RedundantLocationArgument < Cop MSG = 'Redundant location argument to `#add_offense`.'.freeze - def_node_matcher :add_offense_kwargs, <<-PATTERN - (send nil? :add_offense _ $hash) - PATTERN - - def_node_matcher :redundant_location_argument?, <<-PATTERN - (pair (sym :location) (sym :expression)) + def_node_matcher :redundant_location_argument, <<-PATTERN + (send nil? :add_offense _ + (hash <$(pair (sym :location) (sym :expression)) ...>) + ) PATTERN def on_send(node) @@ -39,17 +37,6 @@ def autocorrect(node) ->(corrector) { corrector.remove(range) } end - private - - def redundant_location_argument(node) - add_offense_kwargs(node) do |kwargs| - result = - kwargs.pairs.find { |arg| redundant_location_argument?(arg) } - - yield result if result - end - end - def offending_range(node) with_space = range_with_surrounding_space(range: node.loc.expression) diff --git a/lib/rubocop/cop/rails/belongs_to.rb b/lib/rubocop/cop/rails/belongs_to.rb index db335683646..907fe9ad82d 100644 --- a/lib/rubocop/cop/rails/belongs_to.rb +++ b/lib/rubocop/cop/rails/belongs_to.rb @@ -66,54 +66,36 @@ class BelongsTo < Cop 'this option altogether'.freeze def_node_matcher :match_belongs_to_with_options, <<-PATTERN - (send $_ :belongs_to _ (hash $...)) - PATTERN - - def_node_matcher :match_required_false?, <<-PATTERN - (pair (sym :required) false) - PATTERN - - def_node_matcher :match_required_true?, <<-PATTERN - (pair (sym :required) true) + (send _ :belongs_to _ + (hash <$(pair (sym :required) ${true false}) ...>) + ) PATTERN def on_send(node) - opt = extract_required_option(node) - return unless opt - return unless match_required_true?(opt) || match_required_false?(opt) - - message = - if match_required_true?(opt) - SUPERFLOUS_REQUIRE_TRUE_MSG - elsif match_required_false?(opt) - SUPERFLOUS_REQUIRE_FALSE_MSG - end + match_belongs_to_with_options(node) do |_option_node, option_value| + message = + if option_value.true_type? + SUPERFLOUS_REQUIRE_TRUE_MSG + elsif option_value.false_type? + SUPERFLOUS_REQUIRE_FALSE_MSG + end - add_offense(node, message: message, location: :selector) + add_offense(node, message: message, location: :selector) + end end def autocorrect(node) - opt = extract_required_option(node) - return unless opt + option_node, option_value = match_belongs_to_with_options(node) + return unless option_node lambda do |corrector| - if match_required_true?(opt) - corrector.replace(opt.loc.expression, 'optional: false') - elsif match_required_false?(opt) - corrector.replace(opt.loc.expression, 'optional: true') + if option_value.true_type? + corrector.replace(option_node.loc.expression, 'optional: false') + elsif option_value.false_type? + corrector.replace(option_node.loc.expression, 'optional: true') end end end - - def extract_required_option(node) - _, opts = match_belongs_to_with_options(node) - return unless opts - - opts.find do |opt| - match_required_true?(opt) || - match_required_false?(opt) - end - end end end end diff --git a/lib/rubocop/cop/rails/delegate_allow_blank.rb b/lib/rubocop/cop/rails/delegate_allow_blank.rb index 80e90f72d97..a1b14767744 100644 --- a/lib/rubocop/cop/rails/delegate_allow_blank.rb +++ b/lib/rubocop/cop/rails/delegate_allow_blank.rb @@ -16,20 +16,14 @@ module Rails class DelegateAllowBlank < Cop MSG = '`allow_blank` is not a valid option, use `allow_nil`.'.freeze - def_node_matcher :delegate_options, <<-PATTERN - (send nil? :delegate _ $hash) - PATTERN - - def_node_matcher :allow_blank_option?, <<-PATTERN - (pair (sym :allow_blank) true) + def_node_matcher :allow_blank_option, <<-PATTERN + (send nil? :delegate _ (hash <$(pair (sym :allow_blank) true) ...>)) PATTERN def on_send(node) - offending_node = allow_blank_option(node) - - return unless offending_node - - add_offense(offending_node) + allow_blank_option(node) do |offending_node| + add_offense(offending_node) + end end def autocorrect(pair_node) @@ -37,14 +31,6 @@ def autocorrect(pair_node) corrector.replace(pair_node.key.source_range, 'allow_nil') end end - - private - - def allow_blank_option(node) - delegate_options(node) do |hash| - hash.pairs.find { |opt| allow_blank_option?(opt) } - end - end end end end diff --git a/lib/rubocop/cop/rails/http_status.rb b/lib/rubocop/cop/rails/http_status.rb index af3e7bff50e..89c6998847e 100644 --- a/lib/rubocop/cop/rails/http_status.rb +++ b/lib/rubocop/cop/rails/http_status.rb @@ -48,8 +48,8 @@ class HttpStatus < Cop } PATTERN - def_node_matcher :status_pair?, <<-PATTERN - (pair (sym :status) ${int sym}) + def_node_matcher :status_code, <<-PATTERN + (hash <(pair (sym :status) ${int sym}) ...>) PATTERN def on_send(node) @@ -77,13 +77,6 @@ def autocorrect(node) private - def status_code(node) - node.each_pair.each do |pair| - status_pair?(pair) { |code| return code } - end - false - end - def checker_class case style when :symbolic diff --git a/lib/rubocop/cop/rails/reflection_class_name.rb b/lib/rubocop/cop/rails/reflection_class_name.rb index 1af134c949a..29d646d7181 100644 --- a/lib/rubocop/cop/rails/reflection_class_name.rb +++ b/lib/rubocop/cop/rails/reflection_class_name.rb @@ -16,21 +16,20 @@ module Rails class ReflectionClassName < Cop MSG = 'Use a string value for `class_name`.'.freeze - def_node_matcher :association_with_options?, <<-PATTERN - (send nil? {:has_many :has_one :belongs_to} _ (hash $...)) + def_node_matcher :association_with_reflection, <<-PATTERN + (send nil? {:has_many :has_one :belongs_to} _ + (hash <$#reflection_class_name ...>) + ) PATTERN - def_node_search :reflection_class_name, <<-PATTERN + def_node_matcher :reflection_class_name, <<-PATTERN (pair (sym :class_name) [!dstr !str !sym]) PATTERN def on_send(node) - return unless association_with_options?(node) - - reflection_class_name = reflection_class_name(node).first - return unless reflection_class_name - - add_offense(node, location: reflection_class_name.loc.expression) + association_with_reflection(node) do |reflection_class_name| + add_offense(node, location: reflection_class_name.loc.expression) + end end end end