diff --git a/config/default.yml b/config/default.yml index 961956e..c139de0 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4,4 +4,8 @@ Overhaul/MutableReformDefaults: Enabled: true Include: app/concepts/**/*.rb - VersionAdded: '<>' + VersionAdded: '0.0.1' +Overhaul/AssignmentInsteadOfComparison: + Description: Detect enumerable comparison blocks returning an assignment. + Enabled: true + VersionAdded: '0.0.1' diff --git a/lib/rubocop/cop/overhaul/assignment_instead_of_comparison.rb b/lib/rubocop/cop/overhaul/assignment_instead_of_comparison.rb new file mode 100644 index 0000000..efd12bf --- /dev/null +++ b/lib/rubocop/cop/overhaul/assignment_instead_of_comparison.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Overhaul + # Checks whether the return value of an Enumerator block method (fetch, + # minmax, group, search and sort methods) isn't an assignment. + # + # @example + # # bad + # my_array.select { |x| x = 4 } + # + # # good + # my_array.select { |x| x == 4 } + # + class AssignmentInsteadOfComparison < RuboCop::Cop::Cop + MSG = "Do not return an assignment from the block of an Enumerable's search method." + + RESTRICT_ENUM_METHODS = [ + :take_while, :drop_while, # fetch + :min, :max, :min_by, :max_by, :minmax_by, # minmax + :group_by, :partition, :slice_after, :slice_before, :slice_when, :chunk, :chunk_while, # group + :find, :detect, :find_all, :filter, :select, :find_index, :reject, :uniq_by, # search + :sort, :sort_by # sort + ].freeze + + # @!method search_method_calls?(node) + def_node_matcher :search_method_calls, <<~PATTERN + (block + (call _ /#{RESTRICT_ENUM_METHODS.join("|")}/) + (args ...) + $... + ) + PATTERN + + def on_block(node) + search_method_calls(node) do |value| + value = value.first + block_return_operation = value.begin_type? ? value.children.last : value + next unless block_return_operation.lvasgn_type? + + add_offense(block_return_operation) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/overhaul_cops.rb b/lib/rubocop/cop/overhaul_cops.rb index 82113e4..9097088 100644 --- a/lib/rubocop/cop/overhaul_cops.rb +++ b/lib/rubocop/cop/overhaul_cops.rb @@ -1,3 +1,4 @@ # frozen_string_literal: true require_relative "overhaul/mutable_reform_defaults" +require_relative "overhaul/assignment_instead_of_comparison" diff --git a/spec/rubocop/cop/overhaul/assignment_instead_of_comparison_spec.rb b/spec/rubocop/cop/overhaul/assignment_instead_of_comparison_spec.rb new file mode 100644 index 0000000..d0e5668 --- /dev/null +++ b/spec/rubocop/cop/overhaul/assignment_instead_of_comparison_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Overhaul::AssignmentInsteadOfComparison, :config do + shared_examples "block returning comparison" do |block_statements| + it "allows #{block_statements}" do + source = "my_arr.#{method} { #{block_statements} }" + expect_no_offenses(source) + end + end + + shared_examples "block returning assignment" do |block_statements| + it "registers an offense for #{block_statements}" do + *prefix, o = block_statements.split("; ") + prefix = prefix.join("; ") + prefix += "; " if prefix.length.positive? + + expect_offense(<<~RUBY, o: o, block_statements: block_statements, prefix: prefix, method: method) + my_arr.%{method} { %{block_statements} } + _{method} _{prefix}^{o} Do not return an assignment from the block of an Enumerable's search method. + RUBY + end + end + + described_class::RESTRICT_ENUM_METHODS.each do |method| + context "when method is #{method}" do + let(:method) { method } + + it_behaves_like "block returning assignment", "x = 3" + it_behaves_like "block returning assignment", "var = 10; x = 3" + + it_behaves_like "block returning comparison", "x == 3" + it_behaves_like "block returning comparison", "x = 3; a" + end + end +end