New rule: comparison to boolean

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

src/ameba/rule.cr

@@ -1,5 +1,6 @@
 module Ameba
   RULES = [
+    Rules::ComparisonToBoolean,
     Rules::LineLength,
     Rules::TrailingBlankLines,
     Rules::TrailingWhitespace,

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