From 935296b0410d87a6f3b98ce714595a06cdd904e0 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 15 Nov 2022 00:46:21 +0100 Subject: [PATCH] Refactor ternary handling in `Style/RedundantParentheses` rule --- .../rule/style/redundant_parentheses_spec.cr | 16 ++++++---- src/ameba/rule/style/redundant_parentheses.cr | 29 ++++++++++++------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/spec/ameba/rule/style/redundant_parentheses_spec.cr b/spec/ameba/rule/style/redundant_parentheses_spec.cr index 2968b975..c6890332 100644 --- a/spec/ameba/rule/style/redundant_parentheses_spec.cr +++ b/spec/ameba/rule/style/redundant_parentheses_spec.cr @@ -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 diff --git a/src/ameba/rule/style/redundant_parentheses.cr b/src/ameba/rule/style/redundant_parentheses.cr index 15c5bf20..b0c1ec15 100644 --- a/src/ameba/rule/style/redundant_parentheses.cr +++ b/src/ameba/rule/style/redundant_parentheses.cr @@ -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)