Do not report shadowing outer local var is var is declared below the

shadowed var

closes #144

spec/ameba/ast/variabling/variable_spec.cr

@@ -160,5 +160,42 @@ module Ameba::AST
         variable.eql?(variable.node).should be_true
       end
     end
+
+    describe "#declared_before?" do
+      it "is falsey if variable doesn't have location" do
+        var1 = Crystal::Var.new("foo")
+        var2 = Crystal::Var.new("bar").at(Crystal::Location.new(nil, 1, 2))
+        Variable.new(var1, scope).declared_before?(var2).should be_falsey
+      end
+
+      it "is falsey if node doesn't have location" do
+        var1 = Crystal::Var.new("foo").at(Crystal::Location.new(nil, 1, 2))
+        var2 = Crystal::Var.new("bar")
+        Variable.new(var1, scope).declared_before?(var2).should be_falsey
+      end
+
+      it "is true if var's line_number below the node" do
+        var1 = Crystal::Var.new("foo").at(Crystal::Location.new(nil, 1, 2))
+        var2 = Crystal::Var.new("bar").at(Crystal::Location.new(nil, 2, 2))
+        Variable.new(var1, scope).declared_before?(var2).should be_true
+      end
+
+      it "is true if var's column_number is after the node" do
+        var1 = Crystal::Var.new("foo").at(Crystal::Location.new(nil, 1, 2))
+        var2 = Crystal::Var.new("bar").at(Crystal::Location.new(nil, 1, 3))
+        Variable.new(var1, scope).declared_before?(var2).should be_true
+      end
+
+      it "is false if var's location is before the node" do
+        var1 = Crystal::Var.new("foo").at(Crystal::Location.new(nil, 2, 2))
+        var2 = Crystal::Var.new("bar").at(Crystal::Location.new(nil, 1, 3))
+        Variable.new(var1, scope).declared_before?(var2).should be_false
+      end
+
+      it "is false is the node is the same var" do
+        var = Crystal::Var.new("foo").at(Crystal::Location.new(nil, 2, 2))
+        Variable.new(var, scope).declared_before?(var).should be_false
+      end
+    end
   end
 end

spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr

@@ -34,6 +34,14 @@ module Ameba::Rule::Lint
       subject.catch(source).should_not be_valid
     end
 
+    it "does not report outer vars declared below shadowed block" do
+      source = Source.new %(
+        methods = klass.methods.select { |m| m.annotation(MyAnn) }
+        m = methods.last
+      )
+      subject.catch(source).should be_valid
+    end
+
     it "reports if there is a shadowing in a proc" do
       source = Source.new %(
         def some_method
@@ -176,7 +184,7 @@ module Ameba::Rule::Lint
     end
 
     context "macro" do
-      it "does not report shadowed vars A" do
+      it "does not report shadowed vars in outer scope" do
         source = Source.new %(
           macro included
             def foo
@@ -192,6 +200,19 @@ module Ameba::Rule::Lint
         )
         subject.catch(source).should be_valid
       end
+
+      it "does not report shadowed vars in macro withing the same scope" do
+        source = Source.new %(
+          {% methods = klass.methods.select { |m| m.annotation(MyAnn) } %}
+
+          {% for m, m_idx in methods %}
+            {% if d = m.annotation(MyAnn) %}
+              {% d %}
+            {% end %}
+          {% end %}
+        )
+        subject.catch(source).should be_valid
+      end
     end
   end
 end

src/ameba/ast/scope.cr

@@ -110,7 +110,7 @@ module Ameba::AST
       call.try(&.name) == "spawn"
     end
 
-    # Returns true if currency scope represents a macro.
+    # Returns true if current scope represents a macro.
     def macro?
       node.is_a?(Crystal::Macro)
     end

src/ameba/ast/variabling/variable.cr

@@ -151,6 +151,17 @@ module Ameba::AST
         node.location == @node.location
     end
 
+    # Returns true if the variable is delcared before the `node`.
+    def declared_before?(node)
+      var_location, node_location = location, node.location
+
+      return if var_location.nil? || node_location.nil?
+
+      (var_location.line_number < node_location.line_number) ||
+        (var_location.line_number == node_location.line_number &&
+          var_location.column_number < node_location.column_number)
+    end
+
     private class MacroLiteralFinder < Crystal::Visitor
       @macro_literals = [] of Crystal::MacroLiteral
 

src/ameba/rule/lint/shadowing_local_outer_var.cr

@@ -52,14 +52,17 @@ module Ameba::Rule::Lint
     end
 
     private def find_shadowing(source, scope)
-      scope.arguments.each do |arg|
-        outer_scope = scope.outer_scope
+      outer_scope = scope.outer_scope
 
-        next if arg.ignored? || outer_scope.nil?
+      return if outer_scope.nil? || outer_scope.macro?
 
-        if !outer_scope.macro? && outer_scope.find_variable(arg.name) && !outer_scope.assigns_ivar?(arg.name)
-          issue_for arg.node, MSG % arg.name
-        end
+      scope.arguments.reject(&.ignored?).each do |arg|
+        variable = outer_scope.find_variable(arg.name)
+
+        next if variable.nil? || !variable.declared_before?(arg)
+        next if outer_scope.assigns_ivar?(arg.name)
+
+        issue_for arg.node, MSG % arg.name
       end
     end
   end