From 94e1d4567aec6d5101bcf11b11f107ec1da66e8d Mon Sep 17 00:00:00 2001
From: Vitalii Elenhaupt <velenhaupt@gmail.com>
Date: Sun, 5 May 2019 18:04:25 +0300
Subject: [PATCH] Properly report performance rules in macros

closes #102
---
 spec/ameba/rule/performance/any_after_filter_spec.cr |  9 +++++++++
 .../rule/performance/first_last_after_filter_spec.cr |  9 +++++++++
 .../ameba/rule/performance/size_after_filter_spec.cr |  9 +++++++++
 src/ameba/ast/visitors/node_visitor.cr               | 12 ++++++++++++
 .../rule/performance/first_last_after_filter.cr      |  7 ++++++-
 src/ameba/rule/performance/size_after_filter.cr      |  7 ++++++-
 6 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/spec/ameba/rule/performance/any_after_filter_spec.cr b/spec/ameba/rule/performance/any_after_filter_spec.cr
index ad084869..d78722c2 100644
--- a/spec/ameba/rule/performance/any_after_filter_spec.cr
+++ b/spec/ameba/rule/performance/any_after_filter_spec.cr
@@ -48,6 +48,15 @@ module Ameba::Rule::Performance
       end
     end
 
+    context "macro" do
+      it "reports in macro scope" do
+        source = Source.new %(
+          {{ [1, 2, 3].reject { |e| e > 2  }.any? }}
+        )
+        subject.catch(source).should_not be_valid
+      end
+    end
+
     it "reports rule, pos and message" do
       s = Source.new %(
         [1, 2, 3].reject { |e| e > 2 }.any?
diff --git a/spec/ameba/rule/performance/first_last_after_filter_spec.cr b/spec/ameba/rule/performance/first_last_after_filter_spec.cr
index 60bee676..6c80cd4b 100644
--- a/spec/ameba/rule/performance/first_last_after_filter_spec.cr
+++ b/spec/ameba/rule/performance/first_last_after_filter_spec.cr
@@ -107,6 +107,15 @@ module Ameba::Rule::Performance
       issue.message.should eq "Use `reverse_each.find {...}` instead of `select {...}.last`"
     end
 
+    context "macro" do
+      it "doesn't report in macro scope" do
+        source = Source.new %(
+          {{[1, 2, 3].select { |e| e > 2  }.last }}
+        )
+        subject.catch(source).should be_valid
+      end
+    end
+
     it "reports a correct message for last?" do
       s = Source.new %(
         [1, 2, 3].select { |e| e > 2 }.last?
diff --git a/spec/ameba/rule/performance/size_after_filter_spec.cr b/spec/ameba/rule/performance/size_after_filter_spec.cr
index 31278d53..bfd67336 100644
--- a/spec/ameba/rule/performance/size_after_filter_spec.cr
+++ b/spec/ameba/rule/performance/size_after_filter_spec.cr
@@ -49,6 +49,15 @@ module Ameba::Rule::Performance
       end
     end
 
+    context "macro" do
+      it "doesn't report in macro scope" do
+        source = Source.new %(
+          {{[1, 2, 3].select { |v| v > 1 }.size}}
+        )
+        subject.catch(source).should be_valid
+      end
+    end
+
     it "reports rule, pos and message" do
       s = Source.new %(
         lines.split("\n").reject(&.empty?).size
diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr
index 872cb6f9..e114da74 100644
--- a/src/ameba/ast/visitors/node_visitor.cr
+++ b/src/ameba/ast/visitors/node_visitor.cr
@@ -34,6 +34,13 @@ module Ameba::AST
   # ```
   #
   class NodeVisitor < BaseVisitor
+    @skip : Array(Crystal::ASTNode.class)?
+
+    def initialize(@rule, @source, skip = nil)
+      @skip = skip.try &.map { |el| el.as(Crystal::ASTNode.class) }
+      super @rule, @source
+    end
+
     {% for name in NODES %}
       # A visit callback for `Crystal::{{name}}` node.
       # Returns true meaning that child nodes will be traversed as well.
@@ -42,5 +49,10 @@ module Ameba::AST
         true
       end
     {% end %}
+
+    def visit(node)
+      return true unless (skip = @skip)
+      !skip.includes?(node.class)
+    end
   end
 end
diff --git a/src/ameba/rule/performance/first_last_after_filter.cr b/src/ameba/rule/performance/first_last_after_filter.cr
index 2cfb9ec5..8e0db658 100644
--- a/src/ameba/rule/performance/first_last_after_filter.cr
+++ b/src/ameba/rule/performance/first_last_after_filter.cr
@@ -35,7 +35,12 @@ module Ameba::Rule::Performance
     end
 
     def test(source)
-      AST::NodeVisitor.new self, source
+      AST::NodeVisitor.new self, source, skip: [
+        Crystal::Macro,
+        Crystal::MacroExpression,
+        Crystal::MacroIf,
+        Crystal::MacroFor,
+      ]
     end
 
     def test(source, node : Crystal::Call)
diff --git a/src/ameba/rule/performance/size_after_filter.cr b/src/ameba/rule/performance/size_after_filter.cr
index 8127284b..1fe26359 100644
--- a/src/ameba/rule/performance/size_after_filter.cr
+++ b/src/ameba/rule/performance/size_after_filter.cr
@@ -41,7 +41,12 @@ module Ameba::Rule::Performance
     end
 
     def test(source)
-      AST::NodeVisitor.new self, source
+      AST::NodeVisitor.new self, source, skip: [
+        Crystal::Macro,
+        Crystal::MacroExpression,
+        Crystal::MacroIf,
+        Crystal::MacroFor,
+      ]
     end
 
     def test(source, node : Crystal::Call)