handle more cases for ruby/prefer-endless-method

Author: flyerhzm

lib/ruby/prefer-endless-method.rb

@@ -23,14 +23,17 @@ def one_plus_one = 1 + 1
 
   within_files Synvert::ALL_RUBY_FILES + Synvert::ALL_RAKE_FILES do
     find_node '.def_node[body!=nil][body.body.length=1]' do
+      break if node.name.to_s.end_with?('=')
+      break if !node.parameters.nil? && node.lparen.nil? && node.rparen.nil?
+
       first_body_node = node.body.body.first
-      if %i[if_node unless_node].include?(first_body_node.type) && first_body_node.end_keyword.nil?
-        break
-      end
-      break if first_body_node.type == :multi_write_node
+      break if %i[if_node unless_node].include?(first_body_node.type) && first_body_node.end_keyword.nil?
+      break if %i[multi_write_node instance_variable_or_write_node class_variable_or_write_node or_node and_node].include?(first_body_node.type)
+
       body_column = mutation_adapter.get_start_loc(first_body_node).column
       new_body = node.body.body.first.to_source.split("\n").map { |line| line.sub(/^ {#{body_column}}/, '') }.join("\n")
-      replace_with "def {{name}}{{lparen}}{{parameters}}{{rparen}} = #{new_body}"
+      receiver_and_name = node.receiver ? "#{node.receiver.to_source}.#{node.name}" : node.name.to_s
+      replace_with "def #{receiver_and_name}{{lparen}}{{parameters}}{{rparen}} = #{new_body}"
     end
   end
 end

spec/ruby/prefer-endless-method_spec.rb

@@ -115,4 +115,85 @@ def index
 
     include_examples 'convertable'
   end
+
+  context 'do not process for class_variable_or_write_node, instance_variable_or_write_node' do
+    let(:test_content) { <<~EOS }
+      def class_logger
+        @@logger ||= nil
+      end
+
+      def instance_logger
+        @logger ||= nil
+      end
+    EOS
+    let(:test_rewritten_content) { <<~EOS }
+      def class_logger
+        @@logger ||= nil
+      end
+
+      def instance_logger
+        @logger ||= nil
+      end
+    EOS
+
+    include_examples 'convertable'
+  end
+
+  context 'do not process for or_node' do
+    let(:test_content) { <<~EOS }
+      def new_rating=(new_rating)
+        self.rating = NEW_TO_OLD_MAPPER[new_rating] or raise "Unknown new rating"
+      end
+    EOS
+    let(:test_rewritten_content) { <<~EOS }
+      def new_rating=(new_rating)
+        self.rating = NEW_TO_OLD_MAPPER[new_rating] or raise "Unknown new rating"
+      end
+    EOS
+
+    include_examples 'convertable'
+  end
+
+  context 'do not process if name ends with =' do
+    let(:test_content) { <<~EOS }
+      def remove_item_ids=(item_ids)
+        Item.where(id: item_ids).destroy_all
+      end
+    EOS
+    let(:test_rewritten_content) { <<~EOS }
+      def remove_item_ids=(item_ids)
+        Item.where(id: item_ids).destroy_all
+      end
+    EOS
+
+    include_examples 'convertable'
+  end
+
+  context 'do not process if parameter without parentheses' do
+    let(:test_content) { <<~EOS }
+      def initialize item
+        @item = item
+      end
+    EOS
+    let(:test_rewritten_content) { <<~EOS }
+      def initialize item
+        @item = item
+      end
+    EOS
+
+    include_examples 'convertable'
+  end
+
+  context 'class method' do
+    let(:test_content) { <<~EOS }
+      def self.enqueue(item_id)
+        Resque.enqueue(self, item_id:)
+      end
+    EOS
+    let(:test_rewritten_content) { <<~EOS }
+      def self.enqueue(item_id) = Resque.enqueue(self, item_id:)
+    EOS
+
+    include_examples 'convertable'
+  end
 end