Skip to content

Commit

Permalink
Refactor ternary handling in Style/RedundantParentheses rule
Browse files Browse the repository at this point in the history
  • Loading branch information
Sija committed Nov 16, 2022
1 parent 3340613 commit 935296b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 15 deletions.
16 changes: 11 additions & 5 deletions spec/ameba/rule/style/redundant_parentheses_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,22 @@ module Ameba::Rule::Style
rule.exclude_ternary = false

expect_issue rule, <<-CRYSTAL
(foo > bar) ? true : false
# ^^^^^^^^^ error: Redundant parentheses
(foo.empty?) ? true : false
# ^^^^^^^^^^ error: Redundant parentheses
CRYSTAL

expect_no_issues subject, <<-CRYSTAL
(foo && bar) ? true : false
CRYSTAL

expect_no_issues subject, <<-CRYSTAL
(foo || bar) ? true : false
(foo = @foo) ? true : false
foo == 42 ? true : false
(foo = 42) ? true : false
(foo > 42) ? true : false
(foo >= 42) ? true : false
(3 >= foo >= 42) ? true : false
(3.in? 0..42) ? true : false
(yield 42) ? true : false
(foo rescue 42) ? true : false
CRYSTAL
end
end
Expand Down
29 changes: 19 additions & 10 deletions src/ameba/rule/style/redundant_parentheses.cr
Original file line number Diff line number Diff line change
Expand Up @@ -22,35 +22,44 @@ module Ameba::Rule::Style
# ```
# Style/RedundantParentheses:
# Enabled: true
# ExcludeTernary: true
# ExcludeTernary: false
# ExcludeAssignments: false
# ```
class RedundantParentheses < Base
properties do
description "Disallows redundant parentheses around control expressions"

exclude_ternary true
exclude_ternary false
exclude_assignments false
end

MSG = "Redundant parentheses"

protected def strip_parentheses?(node, in_ternary) : Bool
case node
when Crystal::BinaryOp, Crystal::ExceptionHandler
!in_ternary
when Crystal::Call
!in_ternary || node.has_parentheses? || node.args.empty?
when Crystal::Yield
!in_ternary || node.has_parentheses? || node.exps.empty?
when Crystal::Assign, Crystal::OpAssign, Crystal::MultiAssign
!in_ternary && !exclude_assignments
else
true
end
end

def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until)
is_ternary = node.is_a?(Crystal::If) && node.ternary?

return if exclude_ternary && is_ternary
return if is_ternary && exclude_ternary

return unless (cond = node.cond).is_a?(Crystal::Expressions)
return unless cond.keyword.paren?

return unless exp = cond.single_expression?

case exp
when Crystal::BinaryOp
return if is_ternary
when Crystal::Assign, Crystal::OpAssign
return if exclude_assignments
end
return unless strip_parentheses?(exp, is_ternary)

issue_for cond, MSG do |corrector|
corrector.remove_trailing(cond, 1)
Expand Down

0 comments on commit 935296b

Please sign in to comment.