[#Fix 334] Make body consistent for RescueNode, EnsureNode and KeywordBeginNode

changelog/change_fix_334_make_body_consistent_for.md

@@ -0,0 +1 @@
+* [#335](https://github.com/rubocop/rubocop-ast/issues/334): **(Breaking change)** [#Fix 334] Make `body` consistent for `RescueNode`, `EnsureNode` and `KeywordBeginNode`. ([@dvandersluis][])

lib/rubocop/ast/node/ensure_node.rb

@@ -9,22 +9,13 @@ class EnsureNode < Node
       DEPRECATION_WARNING_LOCATION_CACHE = [] # rubocop:disable Style/MutableConstant
       private_constant :DEPRECATION_WARNING_LOCATION_CACHE
 
-      # Returns the body of the `ensure` clause.
+      # Returns the body of the `ensure` node.
       #
-      # @return [Node, nil] The body of the `ensure`.
-      # @deprecated Use `EnsureNode#branch`
+      # @return [Node, nil] The body of the `ensure` node.
       def body
-        first_caller = caller(1..1).first
+        return rescue_node.body if rescue_node
 
-        unless DEPRECATION_WARNING_LOCATION_CACHE.include?(first_caller)
-          warn '`EnsureNode#body` is deprecated and will be changed in the next major version of ' \
-               'rubocop-ast. Use `EnsureNode#branch` instead to get the body of the `ensure` branch.'
-          warn "Called from:\n#{caller.join("\n")}\n\n"
-
-          DEPRECATION_WARNING_LOCATION_CACHE << first_caller
-        end
-
-        branch
+        node_parts[0]
       end
 
       # Returns an the ensure branch in the exception handling statement.
@@ -38,7 +29,7 @@ def branch
       #
       # @return [Node, nil] The `rescue` node.
       def rescue_node
-        node_parts[0] if node_parts[0].rescue_type?
+        node_parts[0] if node_parts[0]&.rescue_type?
       end
 
       # Checks whether this node body is a void context.

lib/rubocop/ast/node/keyword_begin_node.rb

@@ -16,7 +16,7 @@ def body
         if rescue_node
           rescue_node.body
         elsif ensure_node
-          ensure_node.node_parts[0]
+          ensure_node.body
         elsif node_parts.one?
           node_parts[0]
         else

spec/rubocop/ast/ensure_node_spec.rb

@@ -11,6 +11,54 @@
     it { expect(ensure_node).to be_a(described_class) }
   end
 
+  describe '#body' do
+    subject(:body) { ensure_node.body }
+
+    context 'when there is no body' do
+      let(:source) { 'begin; ensure; ensurebody; end' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'when the body is a single line' do
+      let(:source) { 'begin; >>beginbody<<; ensure; ensurebody; end' }
+
+      it { is_expected.to eq(node) }
+    end
+
+    context 'when the body is multiple lines' do
+      let(:source) { 'begin; >>foo<<; bar; ensure; ensurebody; end' }
+
+      it 'returns a begin node' do
+        expect(body).to be_begin_type
+        expect(body.children).to include(node)
+      end
+    end
+
+    context 'with `rescue`' do
+      context 'when there is no body' do
+        let(:source) { 'begin; rescue; rescuebody; ensure; ensurebody; end' }
+
+        it { is_expected.to be_nil }
+      end
+
+      context 'when the body is a single line' do
+        let(:source) { 'begin; >>beginbody<<; rescue; rescuebody; ensure; ensurebody; end' }
+
+        it { is_expected.to eq(node) }
+      end
+
+      context 'when the body is multiple lines' do
+        let(:source) { 'begin; >>foo<<; bar; rescue; rescuebody; ensure; ensurebody; end' }
+
+        it 'returns a begin node' do
+          expect(body).to be_begin_type
+          expect(body.children).to include(node)
+        end
+      end
+    end
+  end
+
   describe '#branch' do
     let(:source) { 'begin; beginbody; ensure; >>ensurebody<<; end' }
 

spec/rubocop/ast/rescue_node_spec.rb

@@ -1,8 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe RuboCop::AST::RescueNode do
-  subject(:ast) { parse_source(source).ast }
+  subject(:ast) { parsed_source.ast }
 
+  let(:parsed_source) { parse_source(source) }
+  let(:node) { parsed_source.node }
   let(:rescue_node) { ast.children.first }
 
   describe '.new' do
@@ -16,26 +18,43 @@
   end
 
   describe '#body' do
-    let(:source) { <<~RUBY }
-      begin
-        foo
-      rescue => e
-      end
-    RUBY
+    subject(:body) { rescue_node.body }
 
-    it { expect(rescue_node.body).to be_send_type }
+    context 'when the body is empty' do
+      let(:source) { <<~RUBY }
+        begin
+        rescue => e
+        end
+      RUBY
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'when the body is a single line' do
+      let(:source) { <<~RUBY }
+        begin
+          >>foo<<
+        rescue => e
+        end
+      RUBY
+
+      it { is_expected.to eq(node) }
+    end
 
     context 'with multiple lines in body' do
       let(:source) { <<~RUBY }
         begin
-          foo
+          >>foo<<
           bar
         rescue => e
           baz
         end
       RUBY
 
-      it { expect(rescue_node.body).to be_begin_type }
+      it 'returns a begin node' do
+        expect(body).to be_begin_type
+        expect(body.children).to include(node)
+      end
     end
   end