Merge pull request #293 from crystal-ameba/Sija/refactor-static-comparison-rule

spec/ameba/rule/lint/literals_comparison_spec.cr

@@ -1,9 +1,9 @@
 require "../../../spec_helper"
 
 module Ameba::Rule::Lint
-  subject = StaticComparison.new
+  subject = LiteralsComparison.new
 
-  describe StaticComparison do
+  describe LiteralsComparison do
     it "passes for valid cases" do
       expect_no_issues subject, <<-CRYSTAL
         "foo" == foo
@@ -13,6 +13,13 @@ module Ameba::Rule::Lint
         CRYSTAL
     end
 
+    it "reports if there is a regex comparison possibly evaluating to the same" do
+      expect_issue subject, <<-CRYSTAL
+        /foo/ === "foo"
+        # ^^^^^^^^^^^^^ error: Comparison most likely evaluates to the same
+        CRYSTAL
+    end
+
     it "reports if there is a static comparison evaluating to the same" do
       expect_issue subject, <<-CRYSTAL
         "foo" === "foo"

src/ameba/rule/lint/literals_comparison.cr

@@ -1,6 +1,9 @@
 module Ameba::Rule::Lint
-  # This rule is used to identify static comparisons -
+  # This rule is used to identify comparisons between two literals.
-  # the ones that will always have the same result.
+  #
+  # They usually have the same result - except for non-primitive
+  # types like containers, range or regex.
+  #
   #
   # For example, this will be always false:
   #
@@ -11,39 +14,53 @@ module Ameba::Rule::Lint
   # YAML configuration example:
   #
   # ```
-  # Lint/StaticComparison:
+  # Lint/LiteralsComparison:
   #   Enabled: true
   # ```
-  class StaticComparison < Base
+  class LiteralsComparison < Base
     include AST::Util
 
     properties do
-      description "Identifies static comparisons"
+      description "Identifies comparisons between literals"
     end
 
     OP_NAMES = %w(=== == !=)
-    MSG      = "Comparison always evaluates to %s"
 
-    PRIMITIVES = {
+    MSG        = "Comparison always evaluates to %s"
+    MSG_LIKELY = "Comparison most likely evaluates to %s"
+
+    PRIMITIVE_LITERAL_TYPES = {
       Crystal::NilLiteral,
       Crystal::BoolLiteral,
       Crystal::NumberLiteral,
       Crystal::CharLiteral,
       Crystal::StringLiteral,
       Crystal::SymbolLiteral,
+      Crystal::ProcLiteral,
+      Crystal::Path,
+    }
+
+    DYNAMIC_LITERAL_TYPES = {
       Crystal::RangeLiteral,
       Crystal::RegexLiteral,
       Crystal::TupleLiteral,
       Crystal::NamedTupleLiteral,
       Crystal::ArrayLiteral,
       Crystal::HashLiteral,
-      Crystal::ProcLiteral,
     }
 
+    LITERAL_TYPES =
+      PRIMITIVE_LITERAL_TYPES + DYNAMIC_LITERAL_TYPES
+
     def test(source, node : Crystal::Call)
       return unless node.name.in?(OP_NAMES)
       return unless (obj = node.obj) && (arg = node.args.first?)
-      return unless obj.class.in?(PRIMITIVES) && arg.class.in?(PRIMITIVES)
+
+      return unless obj.class.in?(LITERAL_TYPES) &&
+                    arg.class.in?(LITERAL_TYPES)
+
+      is_dynamic = obj.class.in?(DYNAMIC_LITERAL_TYPES) ||
+                   arg.class.in?(DYNAMIC_LITERAL_TYPES)
 
       what =
         case node.name
@@ -52,7 +69,7 @@ module Ameba::Rule::Lint
         when "!="  then (obj.to_s != arg.to_s).to_s
         end
 
-      issue_for node, MSG % what
+      issue_for node, (is_dynamic ? MSG_LIKELY : MSG) % what
     end
   end
 end