From 84540d2a22397692f82d064563ecab5dabff990e Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Sun, 13 Nov 2022 01:25:24 +0100
Subject: [PATCH] Change to alternative approach skipping all macro contexts

---
 .../rule/lint/literals_comparison_spec.cr     |  4 +--
 src/ameba/rule/lint/literals_comparison.cr    | 27 +++++++++++--------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/spec/ameba/rule/lint/literals_comparison_spec.cr b/spec/ameba/rule/lint/literals_comparison_spec.cr
index 920a133b..595197b8 100644
--- a/spec/ameba/rule/lint/literals_comparison_spec.cr
+++ b/spec/ameba/rule/lint/literals_comparison_spec.cr
@@ -41,7 +41,7 @@ module Ameba::Rule::Lint
         CRYSTAL
     end
 
-    pending "reports if there is a static path comparison evaluating to false" do
+    it "reports if there is a static path comparison evaluating to false" do
       expect_issue subject, <<-CRYSTAL
         String == Nil
         # ^^^^^^^^^^^ error: Comparison always evaluates to false
@@ -49,7 +49,7 @@ module Ameba::Rule::Lint
     end
 
     context "macro" do
-      it "reports in macro scope" do
+      pending "reports in macro scope" do
         expect_issue subject, <<-CRYSTAL
           {{ "foo" == "foo" }}
            # ^^^^^^^^^^^^^^ error: Comparison always evaluates to true
diff --git a/src/ameba/rule/lint/literals_comparison.cr b/src/ameba/rule/lint/literals_comparison.cr
index 7bdbfe60..59ecc0a8 100644
--- a/src/ameba/rule/lint/literals_comparison.cr
+++ b/src/ameba/rule/lint/literals_comparison.cr
@@ -52,6 +52,22 @@ module Ameba::Rule::Lint
     LITERAL_TYPES =
       PRIMITIVE_LITERAL_TYPES + DYNAMIC_LITERAL_TYPES
 
+    # Edge-case: `{{ T == Nil }}`
+    #
+    # Current implementation just skips all macro contexts,
+    # regardless of the free variable being present.
+    #
+    # Ideally we should only check whether either of the sides
+    # is a free var
+    def test(source)
+      AST::NodeVisitor.new self, source, skip: [
+        Crystal::Macro,
+        Crystal::MacroExpression,
+        Crystal::MacroIf,
+        Crystal::MacroFor,
+      ]
+    end
+
     def test(source, node : Crystal::Call)
       return unless node.name.in?(OP_NAMES)
       return unless (obj = node.obj) && (arg = node.args.first?)
@@ -59,17 +75,6 @@ module Ameba::Rule::Lint
       return unless obj.class.in?(LITERAL_TYPES) &&
                     arg.class.in?(LITERAL_TYPES)
 
-      # Edge-case: `{{ T == Nil }}`
-      #
-      # Current implementation works for any single name path comparisons:
-      # `String == Nil`, regardless of the free variable being present.
-      #
-      # Ideally we should only check whether either of the sides
-      # is a free var
-      if obj.is_a?(Crystal::Path) && arg.is_a?(Crystal::Path)
-        return if obj.single_name? && arg.single_name? && (obj != arg)
-      end
-
       is_dynamic = obj.class.in?(DYNAMIC_LITERAL_TYPES) ||
                    arg.class.in?(DYNAMIC_LITERAL_TYPES)