Skip to content

Commit

Permalink
Merge pull request #7786 from koic/add_case_match_node
Browse files Browse the repository at this point in the history
Support Ruby 2.7's pattern match for `Layout/ElseAlignment` cop
  • Loading branch information
koic committed Mar 19, 2020
2 parents 8de00a9 + d62e023 commit 8633992
Show file tree
Hide file tree
Showing 8 changed files with 266 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@
* [#7654](https://github.com/rubocop-hq/rubocop/issues/7654): Support `with_fixed_indentation` option for `Layout/ArrayAlignment` cop. ([@nikitasakov][])
* [#7783](https://github.com/rubocop-hq/rubocop/pull/7783): Support Ruby 2.7's numbered parameter for `Style/RedundantSort`. ([@koic][])
* [#7795](https://github.com/rubocop-hq/rubocop/issues/7795): Make `Layout/EmptyLineAfterGuardClause` aware of case where `and` or `or` is used before keyword that break control (e.g. `and return`). ([@koic][])
* [#7786](https://github.com/rubocop-hq/rubocop/pull/7786): Support Ruby 2.7's pattern match for `Layout/ElseAlignment` cop. ([@koic][])

### Bug fixes

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -37,6 +37,7 @@
require_relative 'rubocop/ast/node/array_node'
require_relative 'rubocop/ast/node/block_node'
require_relative 'rubocop/ast/node/break_node'
require_relative 'rubocop/ast/node/case_match_node'
require_relative 'rubocop/ast/node/case_node'
require_relative 'rubocop/ast/node/class_node'
require_relative 'rubocop/ast/node/def_node'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/ast/builder.rb
Expand Up @@ -21,6 +21,7 @@ class Builder < Parser::Builders::Default
array: ArrayNode,
block: BlockNode,
break: BreakNode,
case_match: CaseMatchNode,
case: CaseNode,
class: ClassNode,
def: DefNode,
Expand Down
56 changes: 56 additions & 0 deletions lib/rubocop/ast/node/case_match_node.rb
@@ -0,0 +1,56 @@
# frozen_string_literal: true

module RuboCop
module AST
# A node extension for `case_match` nodes. This will be used in place of
# a plain node when the builder constructs the AST, making its methods
# available to all `case_match` nodes within RuboCop.
class CaseMatchNode < Node
include ConditionalNode

# Returns the keyword of the `case` statement as a string.
#
# @return [String] the keyword of the `case` statement
def keyword
'case'
end

# Calls the given block for each `in_pattern` node in the `in` statement.
# If no block is given, an `Enumerator` is returned.
#
# @return [self] if a block is given
# @return [Enumerator] if no block is given
def each_in_pattern
return in_pattern_branches.to_enum(__method__) unless block_given?

in_pattern_branches.each do |condition|
yield condition
end

self
end

# Returns an array of all the when branches in the `case` statement.
#
# @return [Array<Node>] an array of `in_pattern` nodes
def in_pattern_branches
node_parts[1...-1]
end

# Returns the else branch of the `case` statement, if any.
#
# @return [Node] the else branch node of the `case` statement
# @return [nil] if the case statement does not have an else branch.
def else_branch
node_parts[-1]
end

# Checks whether this case statement has an `else` branch.
#
# @return [Boolean] whether the `case` statement has an `else` branch
def else?
!loc.else.nil?
end
end
end
end
20 changes: 11 additions & 9 deletions lib/rubocop/ast/traversal.rb
Expand Up @@ -21,7 +21,7 @@ def walk(node)
arg restarg blockarg shadowarg
kwrestarg zsuper lambda redo retry
forward_args forwarded_args
match_var match_nil_pattern].freeze
match_var match_nil_pattern empty_else].freeze
ONE_CHILD_NODE = %i[splat kwsplat block_pass not break next
preexe postexe match_current_line defined?
arg_expr pin match_rest if_guard unless_guard
Expand All @@ -31,7 +31,7 @@ def walk(node)
undef alias args super yield or and
while_post until_post iflipflop eflipflop
match_with_lvasgn begin kwbegin return
in_match case_match in_pattern match_alt
in_match match_alt
match_as array_pattern array_pattern_with_tail
hash_pattern const_pattern].freeze
SECOND_CHILD_ONLY = %i[lvasgn ivasgn cvasgn gvasgn optarg kwarg
Expand Down Expand Up @@ -178,13 +178,15 @@ def on_case(node)
nil
end

alias on_rescue on_case
alias on_resbody on_case
alias on_ensure on_case
alias on_for on_case
alias on_when on_case
alias on_irange on_case
alias on_erange on_case
alias on_rescue on_case
alias on_resbody on_case
alias on_ensure on_case
alias on_for on_case
alias on_when on_case
alias on_case_match on_case
alias on_in_pattern on_case
alias on_irange on_case
alias on_erange on_case

def on_numblock(node)
children = node.children
Expand Down
8 changes: 8 additions & 0 deletions lib/rubocop/cop/layout/else_alignment.rb
Expand Up @@ -59,6 +59,14 @@ def on_case(node)
check_alignment(node.when_branches.last.loc.keyword, node.loc.else)
end

def on_case_match(node)
return unless node.else?

check_alignment(
node.in_pattern_branches.last.loc.keyword, node.loc.else
)
end

def autocorrect(node)
AlignmentCorrector.correct(processed_source, node, column_delta)
end
Expand Down
130 changes: 130 additions & 0 deletions spec/rubocop/ast/case_match_node_spec.rb
@@ -0,0 +1,130 @@
# frozen_string_literal: true

RSpec.describe RuboCop::AST::CaseMatchNode do
let(:case_match_node) { parse_source(source).ast }

context 'when using Ruby 2.7 or newer', :ruby27 do
describe '.new' do
let(:source) do
<<~RUBY
case expr
in pattern
end
RUBY
end

it { expect(case_match_node.is_a?(described_class)).to be(true) }
end

describe '#keyword' do
let(:source) do
<<~RUBY
case expr
in pattern
end
RUBY
end

it { expect(case_match_node.keyword).to eq('case') }
end

describe '#in_pattern_branches' do
let(:source) do
<<~RUBY
case expr
in pattern
in pattern
in pattern
end
RUBY
end

it { expect(case_match_node.in_pattern_branches.size).to eq(3) }
it {
expect(case_match_node.in_pattern_branches).to all(be_in_pattern_type)
}
end

describe '#each_in_pattern' do
let(:source) do
<<~RUBY
case expr
in pattern
in pattern
in pattern
end
RUBY
end

context 'when not passed a block' do
it {
expect(case_match_node.each_in_pattern.is_a?(Enumerator)).to be(true)
}
end

context 'when passed a block' do
it 'yields all the conditions' do
expect { |b| case_match_node.each_in_pattern(&b) }
.to yield_successive_args(*case_match_node.in_pattern_branches)
end
end
end

describe '#else?' do
context 'without an else statement' do
let(:source) do
<<~RUBY
case expr
in pattern
end
RUBY
end

it { expect(case_match_node.else?).to be(false) }
end

context 'with an else statement' do
let(:source) do
<<~RUBY
case expr
in pattern
else
end
RUBY
end

it { expect(case_match_node.else?).to be(true) }
end
end

describe '#else_branch' do
describe '#else?' do
context 'without an else statement' do
let(:source) do
<<~RUBY
case expr
in pattern
end
RUBY
end

it { expect(case_match_node.else_branch.nil?).to be(true) }
end

context 'with an else statement' do
let(:source) do
<<~RUBY
case expr
in pattern
else
:foo
end
RUBY
end

it { expect(case_match_node.else_branch.sym_type?).to be(true) }
end
end
end
end
end
58 changes: 58 additions & 0 deletions spec/rubocop/cop/layout/else_alignment_spec.rb
Expand Up @@ -359,6 +359,64 @@
RUBY
end

context '>= Ruby 2.7', :ruby27 do
context 'with case match' do
it 'registers an offense for misaligned else' do
expect_offense(<<~RUBY)
case 0
in 0
foo
in -1..1
bar
in Integer
baz
else
^^^^ Align `else` with `in`.
qux
end
RUBY
end

it 'accepts correctly aligned case/when/else' do
expect_no_offenses(<<~RUBY)
case 0
in 0
foo
in -1..1
bar
in Integer
baz
else
qux
end
RUBY
end

it 'accepts correctly aligned empty else' do
expect_no_offenses(<<~RUBY)
case 0
in 0
foo
in -1..1
bar
in Integer
baz
else
end
RUBY
end

it 'accepts case match without else' do
expect_no_offenses(<<~'RUBY')
case 0
in a
p a
end
RUBY
end
end
end

it 'accepts else aligned with when but not with case' do
# "Indent when as deep as case" is the job of another cop, and this is
# one of the possible styles supported by configuration.
Expand Down

0 comments on commit 8633992

Please sign in to comment.