Merge pull request #10600 from nobuyo/add-new-internal-affairs-method…

…-name-end-with-cop

Add new `InternalAffairs/MethodNameEndWith` cop

lib/rubocop/cop/gemspec/duplicated_assignment.rb

@@ -44,7 +44,7 @@ class DuplicatedAssignment < Base
         # @!method assignment_method_declarations(node)
         def_node_search :assignment_method_declarations, <<~PATTERN
           (send
-            (lvar #match_block_variable_name?) #assignment_method? ...)
+            (lvar #match_block_variable_name?) _ ...)
         PATTERN
 
         def on_new_investigation
@@ -65,12 +65,9 @@ def match_block_variable_name?(receiver_name)
           end
         end
 
-        def assignment_method?(method_name)
-          method_name.to_s.end_with?('=')
-        end
-
         def duplicated_assignment_method_nodes
           assignment_method_declarations(processed_source.ast)
+            .select(&:assignment_method?)
             .group_by(&:method_name)
             .values
             .select { |nodes| nodes.size > 1 }

lib/rubocop/cop/internal_affairs.rb

@@ -4,6 +4,7 @@
 require_relative 'internal_affairs/example_description'
 require_relative 'internal_affairs/inherit_deprecated_cop_class'
 require_relative 'internal_affairs/location_line_equality_comparison'
+require_relative 'internal_affairs/method_name_end_with'
 require_relative 'internal_affairs/method_name_equal'
 require_relative 'internal_affairs/node_destructuring'
 require_relative 'internal_affairs/node_matcher_directive'

lib/rubocop/cop/internal_affairs/method_name_end_with.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module InternalAffairs
+      # This cop checks potentially usage of method identifier predicates
+      # defined in rubocop-ast instead of `method_name.end_with?`.
+      #
+      # @example
+      #   # bad
+      #   node.method_name.to_s.end_with?('=')
+      #
+      #   # good
+      #   node.assignment_method?
+      #
+      #   # bad
+      #   node.method_name.to_s.end_with?('?')
+      #
+      #   # good
+      #   node.predicate_method?
+      #
+      #   # bad
+      #   node.method_name.to_s.end_with?('!')
+      #
+      #   # good
+      #   node.bang_method?
+      #
+      class MethodNameEndWith < Base
+        include RangeHelp
+        extend AutoCorrector
+
+        MSG = 'Use `%<method_name>s` instead of `%<method_suffix>s`.'
+        SUGGEST_METHOD_FOR_SUFFIX = {
+          '=' => 'assignment_method?',
+          '!' => 'bang_method?',
+          '?' => 'predicate_method?'
+        }.freeze
+
+        # @!method method_name_end_with?(node)
+        def_node_matcher :method_name_end_with?, <<~PATTERN
+          {
+            (call
+              (call
+                $(... :method_name) :to_s) :end_with?
+              $(str {"=" "?" "!"}))
+            (call
+              $(... :method_name) :end_with?
+            $(str {"=" "?" "!"}))
+          }
+        PATTERN
+
+        def on_send(node)
+          method_name_end_with?(node) do |method_name_node, end_with_arg|
+            range = range(method_name_node, node)
+            message = format(
+              MSG,
+              method_name: SUGGEST_METHOD_FOR_SUFFIX[end_with_arg.value],
+              method_suffix: range.source
+            )
+
+            add_offense(node, message: message)
+          end
+        end
+        alias on_csend on_send
+
+        private
+
+        def range(method_name_node, node)
+          range = if method_name_node.call_type?
+                    method_name_node.loc.selector
+                  else
+                    method_name_node.source_range
+                  end
+
+          range_between(range.begin_pos, node.source_range.end_pos)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/lint/return_in_void_context.rb

@@ -40,31 +40,19 @@ def on_return(return_node)
           context_node = non_void_context(return_node)
 
           return unless context_node&.def_type?
+          return unless context_node&.void_context?
 
-          method_name = method_name(context_node)
-
-          return unless method_name && void_context_method?(method_name)
-
-          add_offense(return_node.loc.keyword, message: format(message, method: method_name))
+          add_offense(
+            return_node.loc.keyword,
+            message: format(message, method: context_node.method_name)
+          )
         end
 
         private
 
         def non_void_context(return_node)
           return_node.each_ancestor(:block, :def, :defs).first
         end
-
-        def method_name(context_node)
-          context_node.children.first
-        end
-
-        def void_context_method?(method_name)
-          method_name == :initialize || setter_method?(method_name)
-        end
-
-        def setter_method?(method_name)
-          method_name.to_s.end_with?('=') && !AST::Node::COMPARISON_OPERATORS.include?(method_name)
-        end
       end
     end
   end

lib/rubocop/cop/naming/predicate_name.rb

@@ -70,7 +70,7 @@ def allowed_method_name?(method_name, prefix)
           !(method_name.start_with?(prefix) && # cheap check to avoid allocating Regexp
               method_name.match?(/^#{prefix}[^0-9]/)) ||
             method_name == expected_name(method_name, prefix) ||
-            method_name.end_with?('=') ||
+            method_name.end_with?('=') || # rubocop:todo InternalAffairs/MethodNameEndWith
             allowed_method?(method_name)
         end
 
@@ -80,7 +80,7 @@ def expected_name(method_name, prefix)
                      else
                        method_name.dup
                      end
-          new_name << '?' unless method_name.end_with?('?')
+          new_name << '?' unless method_name.end_with?('?') # rubocop:todo InternalAffairs/MethodNameEndWith
           new_name
         end
 

lib/rubocop/cop/style/collection_compact.rb

@@ -70,7 +70,7 @@ class CollectionCompact < Base
         def on_send(node)
           return unless (range = offense_range(node))
 
-          good = good_method_name(node.method_name)
+          good = good_method_name(node)
           message = format(MSG, good: good, bad: range.source)
 
           add_offense(range, message: message) { |corrector| corrector.replace(range, good) }
@@ -94,8 +94,8 @@ def offense_range(node)
           end
         end
 
-        def good_method_name(method_name)
-          if method_name.to_s.end_with?('!')
+        def good_method_name(node)
+          if node.bang_method?
             'compact!'
           else
             'compact'

lib/rubocop/cop/style/redundant_self_assignment.rb

@@ -67,8 +67,7 @@ def on_lvasgn(node)
         alias on_gvasgn on_lvasgn
 
         def on_send(node)
-          # TODO: Remove `Symbol#to_s` after supporting only Ruby >= 2.7.
-          return unless node.method_name.to_s.end_with?('=')
+          return unless node.assignment_method?
           return unless redundant_assignment?(node)
 
           message = format(MSG, method_name: node.first_argument.method_name)

lib/rubocop/cop/style/trivial_accessors.rb

@@ -167,8 +167,8 @@ def allowed_method_names
           allowed_methods.map(&:to_sym) + [:initialize]
         end
 
-        def dsl_writer?(method_name)
-          !method_name.to_s.end_with?('=')
+        def dsl_writer?(node)
+          !node.assignment_method?
         end
 
         def trivial_reader?(node)
@@ -180,8 +180,7 @@ def looks_like_trivial_reader?(node)
         end
 
         def trivial_writer?(node)
-          looks_like_trivial_writer?(node) &&
-            !allowed_method_name?(node) && !allowed_writer?(node.method_name)
+          looks_like_trivial_writer?(node) && !allowed_method_name?(node) && !allowed_writer?(node)
         end
 
         # @!method looks_like_trivial_writer?(node)
@@ -195,8 +194,8 @@ def allowed_method_name?(node)
             (exact_name_match? && !names_match?(node))
         end
 
-        def allowed_writer?(method_name)
-          allow_dsl_writers? && dsl_writer?(method_name)
+        def allowed_writer?(node)
+          allow_dsl_writers? && dsl_writer?(node)
         end
 
         def allowed_reader?(node)
@@ -210,7 +209,7 @@ def names_match?(node)
         end
 
         def trivial_accessor_kind(node)
-          if trivial_writer?(node) && !dsl_writer?(node.method_name)
+          if trivial_writer?(node) && !dsl_writer?(node)
             'writer'
           elsif trivial_reader?(node)
             'reader'

spec/rubocop/cop/internal_affairs/method_name_end_with_spec.rb

@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::InternalAffairs::MethodNameEndWith, :config do
+  it 'registers an offense if there is potentially usage of `assignment_method?`' do
+    expect_offense(<<~RUBY)
+      node.method_name.to_s.end_with?('=')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `assignment_method?` instead of `method_name.to_s.end_with?('=')`.
+    RUBY
+  end
+
+  it 'registers an offense if `method_name` is a variable and there is potentially usage of `assignment_method?`' do
+    expect_offense(<<~RUBY)
+      def assignment_method?(method_name)
+        method_name.to_s.end_with?('=')
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `assignment_method?` instead of `method_name.to_s.end_with?('=')`.
+      end
+    RUBY
+  end
+
+  it 'registers offense if there is potentially usage of `predicate_method?`' do
+    expect_offense(<<~RUBY)
+      node.method_name.to_s.end_with?('?')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `predicate_method?` instead of `method_name.to_s.end_with?('?')`.
+    RUBY
+  end
+
+  it 'registers offense if there is potentially usage of `bang_method?`' do
+    expect_offense(<<~RUBY)
+      node.method_name.to_s.end_with?('!')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `bang_method?` instead of `method_name.to_s.end_with?('!')`.
+    RUBY
+  end
+
+  it 'registers offense if there is potentially usage of `bang_method?` with safe navigation operator' do
+    expect_offense(<<~RUBY)
+      node.method_name&.to_s&.end_with?('!')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `bang_method?` instead of `method_name&.to_s&.end_with?('!')`.
+    RUBY
+  end
+
+  it 'does not register offense if argument for end_with? is some other string' do
+    expect_no_offenses(<<~RUBY)
+      node.method_name.to_s.end_with?('_foo')
+    RUBY
+  end
+
+  context 'Ruby >= 2.7', :ruby27 do
+    it 'registers an offense if method_name is symbol' do
+      expect_offense(<<~RUBY)
+        node.method_name.end_with?('=')
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `assignment_method?` instead of `method_name.end_with?('=')`.
+      RUBY
+    end
+
+    it 'registers an offense if method_name is symbol with safe navigation operator' do
+      expect_offense(<<~RUBY)
+        node&.method_name&.end_with?('=')
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `assignment_method?` instead of `method_name&.end_with?('=')`.
+      RUBY
+    end
+
+    it 'registers offense if argument for Symbol#end_with? is \'?\'' do
+      expect_offense(<<~RUBY)
+        node.method_name.end_with?('?')
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `predicate_method?` instead of `method_name.end_with?('?')`.
+      RUBY
+    end
+
+    it 'registers offense if argument for Symbol#end_with? is \'?\' with safe navigation operator' do
+      expect_offense(<<~RUBY)
+        node.method_name&.end_with?('?')
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `predicate_method?` instead of `method_name&.end_with?('?')`.
+      RUBY
+    end
+
+    it 'registers offense if argument for Symbol#end_with? is \'!\'' do
+      expect_offense(<<~RUBY)
+        node.method_name.end_with?('!')
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `bang_method?` instead of `method_name.end_with?('!')`.
+      RUBY
+    end
+
+    it 'registers offense if argument for Symbol#end_with? is \'!\' with safe navigation operator' do
+      expect_offense(<<~RUBY)
+        node.method_name&.end_with?('!')
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `bang_method?` instead of `method_name&.end_with?('!')`.
+      RUBY
+    end
+
+    it 'does not register offense if argument for Symbol#end_with? is some other string' do
+      expect_no_offenses(<<~RUBY)
+        node.method_name.end_with?('_foo')
+      RUBY
+    end
+  end
+end