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

Support Ruby 2.7's pattern match for Layout/ElseAlignment cop #7786

Merged
merged 1 commit into from Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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