Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/change_fix_334_make_body_consistent_for.md
Original file line number Diff line number Diff line change
@@ -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][])
11 changes: 6 additions & 5 deletions lib/rubocop/ast/node/ensure_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ module AST
# node when the builder constructs the AST, making its methods available
# to all `ensure` nodes within RuboCop.
class EnsureNode < Node
# 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
branch
return rescue_node.body if rescue_node

node_parts[0]
end

# Returns an the ensure branch in the exception handling statement.
Expand All @@ -25,7 +26,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.
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/ast/node/keyword_begin_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions spec/rubocop/ast/ensure_node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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' }

Expand Down
39 changes: 29 additions & 10 deletions spec/rubocop/ast/rescue_node_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand Down
Loading