Merge pull request #510 from nobodywasishere/nobody/logical-without-p…

…aren

Add `Lint/RequireParentheses`

spec/ameba/rule/lint/require_parentheses_spec.cr

@@ -0,0 +1,52 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  describe RequireParentheses do
+    subject = RequireParentheses.new
+
+    it "passes if logical operator in call args has parentheses" do
+      expect_no_issues subject, <<-CRYSTAL
+        if foo.includes?("bar") || foo.includes?("batz")
+          puts "this code is bug-free"
+        end
+
+        if foo.includes?("bar" || foo.includes? "batz")
+          puts "this code is bug-free"
+        end
+
+        form.add("query", "val_1" || "val_2")
+        form.add "query", "val_1" || "val_2"
+        form.add "query", ("val_1" || "val_2")
+        CRYSTAL
+    end
+
+    it "passes if logical operator in assignment call" do
+      expect_no_issues subject, <<-CRYSTAL
+        hello.there = "world" || method.call
+        hello.there ||= "world" || method.call
+        CRYSTAL
+    end
+
+    it "passes if logical operator in square bracket call" do
+      expect_no_issues subject, <<-CRYSTAL
+        hello["world" || :thing]
+        hello["world" || :thing]?
+        this.is[1 || method.call]
+        CRYSTAL
+    end
+
+    it "fails if logical operator in call args doesn't have parentheses" do
+      expect_issue subject, <<-CRYSTAL
+        if foo.includes? "bar" || foo.includes? "batz"
+         # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use parentheses in the method call to avoid confusion about precedence
+          puts "this code is not bug-free"
+        end
+
+        if foo.in? "bar", "baz" || foo.ends_with? "qux"
+         # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use parentheses in the method call to avoid confusion about precedence
+          puts "this code is not bug-free"
+        end
+        CRYSTAL
+    end
+  end
+end

src/ameba/rule/lint/require_parentheses.cr

@@ -0,0 +1,51 @@
+module Ameba::Rule::Lint
+  # A rule that disallows method calls with at least one argument, where no
+  # parentheses are used around the argument list, and a logical operator
+  # (`&&` or `||`) is used within the argument list.
+  #
+  # For example, this is considered invalid:
+  #
+  # ```
+  # if foo.includes? "bar" || foo.includes? "batz"
+  # end
+  # ```
+  #
+  # And need to be written as:
+  #
+  # ```
+  # if foo.includes?("bar") || foo.includes?("batz")
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Lint/RequireParentheses:
+  #   Enabled: true
+  # ```
+  class RequireParentheses < Base
+    properties do
+      since_version "1.7.0"
+      description "Disallows method calls with no parentheses and a logical operator in the argument list"
+    end
+
+    MSG = "Use parentheses in the method call to avoid confusion about precedence"
+
+    ALLOWED_CALL_NAMES = %w{[]? []}
+
+    def test(source, node : Crystal::Call)
+      return if node.args.empty? ||
+                node.has_parentheses? ||
+                node.name.ends_with?('=') ||
+                node.name.in?(ALLOWED_CALL_NAMES)
+
+      node.args.each do |arg|
+        if arg.is_a?(Crystal::BinaryOp)
+          if (right = arg.right).is_a?(Crystal::Call)
+            issue_for node, MSG unless right.args.empty?
+          end
+        end
+      end
+    end
+  end
+end