Merge pull request #301 from crystal-ameba/Sija/style-redundant-paren…

…theses-rule

Add `Style/RedundantParentheses` rule

spec/ameba/rule/style/redundant_parentheses_spec.cr

@@ -0,0 +1,101 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Style
+  subject = RedundantParentheses.new
+
+  describe RedundantParentheses do
+    {% for keyword in %w(if unless while until) %}
+      context "{{ keyword.id }}" do
+        it "reports if redundant parentheses are found" do
+          source = expect_issue subject, <<-CRYSTAL, keyword: {{ keyword }}
+            %{keyword}   (foo > 10)
+            _{keyword} # ^^^^^^^^^^ error: Redundant parentheses
+              foo
+            end
+            CRYSTAL
+
+          expect_correction source, <<-CRYSTAL
+            {{ keyword.id }}   foo > 10
+              foo
+            end
+            CRYSTAL
+        end
+      end
+    {% end %}
+
+    context "case" do
+      it "reports if redundant parentheses are found" do
+        source = expect_issue subject, <<-CRYSTAL
+          case (foo = @foo)
+             # ^^^^^^^^^^^^ error: Redundant parentheses
+          when String then "string"
+          when Symbol then "symbol"
+          end
+          CRYSTAL
+
+        expect_correction source, <<-CRYSTAL
+          case foo = @foo
+          when String then "string"
+          when Symbol then "symbol"
+          end
+          CRYSTAL
+      end
+    end
+
+    context "properties" do
+      context "#exclude_ternary=" do
+        it "skips ternary control expressions by default" do
+          expect_no_issues subject, <<-CRYSTAL
+            (foo > bar) ? true : false
+            CRYSTAL
+        end
+
+        it "allows to configure assignments" do
+          rule = Rule::Style::RedundantParentheses.new
+          rule.exclude_ternary = false
+
+          expect_issue rule, <<-CRYSTAL
+            (foo.empty?) ? true : false
+            # ^^^^^^^^^^ error: Redundant parentheses
+            CRYSTAL
+
+          expect_no_issues subject, <<-CRYSTAL
+            (foo && bar) ? true : false
+            (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
+
+      context "#exclude_assignments=" do
+        it "reports assignments by default" do
+          expect_issue subject, <<-CRYSTAL
+            if (foo = @foo)
+             # ^^^^^^^^^^^^ error: Redundant parentheses
+              foo
+            end
+            CRYSTAL
+        end
+
+        it "allows to configure assignments" do
+          rule = Rule::Style::RedundantParentheses.new
+          rule.exclude_assignments = true
+
+          expect_no_issues rule, <<-CRYSTAL
+            if (foo = @foo)
+              foo
+            end
+            CRYSTAL
+        end
+      end
+    end
+  end
+end

src/ameba/rule/style/redundant_parentheses.cr

@@ -0,0 +1,70 @@
+module Ameba::Rule::Style
+  # A rule that disallows redundant parentheses around control expressions.
+  #
+  # For example, this is considered invalid:
+  #
+  # ```
+  # if (foo == 42)
+  #   do_something
+  # end
+  # ```
+  #
+  # And should be replaced by the following:
+  #
+  # ```
+  # if foo == 42
+  #   do_something
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Style/RedundantParentheses:
+  #   Enabled: true
+  #   ExcludeTernary: false
+  #   ExcludeAssignments: false
+  # ```
+  class RedundantParentheses < Base
+    properties do
+      description "Disallows redundant parentheses around control expressions"
+
+      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 is_ternary && exclude_ternary
+
+      return unless (cond = node.cond).is_a?(Crystal::Expressions)
+      return unless cond.keyword.paren?
+
+      return unless exp = cond.single_expression?
+      return unless strip_parentheses?(exp, is_ternary)
+
+      issue_for cond, MSG do |corrector|
+        corrector.remove_trailing(cond, 1)
+        corrector.remove_leading(cond, 1)
+      end
+    end
+  end
+end

src/ameba/spec/annotated_source.cr

@@ -73,7 +73,7 @@ class Ameba::Spec::AnnotatedSource
   private def message_to_regex(expected_annotation)
     String.build do |io|
       offset = 0
-      while (index = expected_annotation.index(ABBREV, offset))
+      while index = expected_annotation.index(ABBREV, offset)
         io << Regex.escape(expected_annotation[offset...index])
         io << ".*?"
         offset = index + ABBREV.size