From 6d9f1c67ed30920c49db9c699ac0625523ebcac9 Mon Sep 17 00:00:00 2001
From: Vitalii Elenhaupt <velenhaupt@gmail.com>
Date: Tue, 31 Oct 2017 23:15:24 +0200
Subject: [PATCH] New rule: comparison to boolean

---
 .../ameba/rules/comparison_to_boolean_spec.cr | 113 ++++++++++++++++++
 src/ameba/rule.cr                             |   1 +
 src/ameba/rules/comparison_to_boolean.cr      |  26 ++++
 3 files changed, 140 insertions(+)
 create mode 100644 spec/ameba/rules/comparison_to_boolean_spec.cr
 create mode 100644 src/ameba/rules/comparison_to_boolean.cr

diff --git a/spec/ameba/rules/comparison_to_boolean_spec.cr b/spec/ameba/rules/comparison_to_boolean_spec.cr
new file mode 100644
index 00000000..0e068930
--- /dev/null
+++ b/spec/ameba/rules/comparison_to_boolean_spec.cr
@@ -0,0 +1,113 @@
+require "../../spec_helper"
+
+module Ameba::Rules
+  subject = ComparisonToBoolean.new
+
+  describe ComparisonToBoolean do
+    it "passes if there is no comparison to boolean" do
+      source = Source.new %(
+        a = true
+
+        if a
+          :ok
+        end
+
+        if true
+          :ok
+        end
+
+        unless s.empty?
+          :ok
+        end
+
+        :ok if a
+
+        :ok if a != 1
+
+        :ok if a == "true"
+
+        case a
+        when true
+          :ok
+        when false
+          :not_ok
+        end
+      )
+      subject.catch(source).valid?.should be_true
+    end
+
+    context "boolean on the right" do
+      it "fails if there is == comparison to boolean" do
+        source = Source.new %(
+          if s.empty? == true
+            :ok
+          end
+        )
+        subject.catch(source).valid?.should be_false
+      end
+
+      it "fails if there is != comparison to boolean" do
+        source = Source.new %(
+          if a != false
+            :ok
+          end
+        )
+        subject.catch(source).valid?.should be_false
+      end
+
+      it "fails if there is case comparison to boolean" do
+        source = Source.new %(
+          a === true
+        )
+        subject.catch(source).valid?.should be_false
+      end
+
+      it "reports rule, pos and message" do
+        source = Source.new "a != true"
+        subject.catch(source)
+
+        error = source.errors.first
+        error.rule.should_not be_nil
+        error.pos.should eq 1
+        error.message.should eq "Comparison to a boolean is pointless"
+      end
+    end
+
+    context "boolean on the left" do
+      it "fails if there is == comparison to boolean" do
+        source = Source.new %(
+          if true == s.empty?
+            :ok
+          end
+        )
+        subject.catch(source).valid?.should be_false
+      end
+
+      it "fails if there is != comparison to boolean" do
+        source = Source.new %(
+          if false != a
+            :ok
+          end
+        )
+        subject.catch(source).valid?.should be_false
+      end
+
+      it "fails if there is case comparison to boolean" do
+        source = Source.new %(
+          true === a
+        )
+        subject.catch(source).valid?.should be_false
+      end
+
+      it "reports rule, pos and message" do
+        source = Source.new "true != a"
+        subject.catch(source)
+
+        error = source.errors.first
+        error.rule.should_not be_nil
+        error.pos.should eq 1
+        error.message.should eq "Comparison to a boolean is pointless"
+      end
+    end
+  end
+end
diff --git a/src/ameba/rule.cr b/src/ameba/rule.cr
index 132e4103..f4a0ec66 100644
--- a/src/ameba/rule.cr
+++ b/src/ameba/rule.cr
@@ -1,5 +1,6 @@
 module Ameba
   RULES = [
+    Rules::ComparisonToBoolean,
     Rules::LineLength,
     Rules::TrailingBlankLines,
     Rules::TrailingWhitespace,
diff --git a/src/ameba/rules/comparison_to_boolean.cr b/src/ameba/rules/comparison_to_boolean.cr
new file mode 100644
index 00000000..4bf6cedd
--- /dev/null
+++ b/src/ameba/rules/comparison_to_boolean.cr
@@ -0,0 +1,26 @@
+# A rule that disallows comparison to booleans.
+#
+# For example, these are considered invalid:
+#
+# ```crystal
+# foo == true
+# bar != false
+# false === baz
+# ```
+# This is because these expressions evaluate to `true` or `false`, so you
+# could get the same result by using either the variable directly, or negating
+# the variable.
+
+Ameba.rule ComparisonToBoolean do |source|
+  ComparisonToBooleanVisitor.new self, source
+end
+
+Ameba.visitor ComparisonToBoolean, Crystal::Call do |node|
+  if %w(== != ===).includes?(node.name) && (
+       node.args.first?.try &.is_a?(Crystal::BoolLiteral) ||
+       node.obj.is_a?(Crystal::BoolLiteral)
+     )
+    @source.error @rule, node.location.try &.line_number,
+      "Comparison to a boolean is pointless"
+  end
+end