New rule: SharedVarInSpawn

closes #132

spec/ameba/ast/scope_spec.cr

@@ -33,6 +33,20 @@ module Ameba::AST
     end
   end
 
+  describe "#references" do
+    it "can return an empty list of references" do
+      scope = Scope.new as_node("")
+      scope.references.should be_empty
+    end
+
+    it "allows to add variable references" do
+      scope = Scope.new as_node("")
+      nodes = as_nodes "a = 2"
+      scope.references << Reference.new(nodes.var_nodes.first, scope)
+      scope.references.size.should eq 1
+    end
+  end
+
   describe "#add_variable" do
     it "adds a new variable to the scope" do
       scope = Scope.new as_node("")
@@ -85,6 +99,21 @@ module Ameba::AST
     end
   end
 
+  describe "#spawn_block?" do
+    it "returns true if a node is a spawn block" do
+      nodes = as_nodes %(
+        spawn {}
+      )
+      scope = Scope.new nodes.block_nodes.first
+      scope.spawn_block?.should be_true
+    end
+
+    it "returns false otherwise" do
+      scope = Scope.new as_node "a = 1"
+      scope.spawn_block?.should be_false
+    end
+  end
+
   describe "#macro?" do
     it "returns true if Crystal::Macro" do
       nodes = as_nodes %(

spec/ameba/ast/variabling/assignment_spec.cr

@@ -8,14 +8,14 @@ module Ameba::AST
 
     describe "#initialize" do
       it "creates a new assignment with node and var" do
-        assignment = Assignment.new(node, variable)
+        assignment = Assignment.new(node, variable, scope)
         assignment.node.should_not be_nil
       end
     end
 
     describe "#reference=" do
       it "creates a new reference" do
-        assignment = Assignment.new(node, variable)
+        assignment = Assignment.new(node, variable, scope)
         assignment.referenced = true
         assignment.referenced?.should be_true
       end
@@ -23,18 +23,18 @@ module Ameba::AST
 
     describe "delegation" do
       it "delegates locations" do
-        assignment = Assignment.new(node, variable)
+        assignment = Assignment.new(node, variable, scope)
         assignment.location.should eq node.location
         assignment.end_location.should eq node.end_location
       end
 
       it "delegates to_s" do
-        assignment = Assignment.new(node, variable)
+        assignment = Assignment.new(node, variable, scope)
         assignment.to_s.should eq node.to_s
       end
 
       it "delegates scope" do
-        assignment = Assignment.new(node, variable)
+        assignment = Assignment.new(node, variable, scope)
         assignment.scope.should eq variable.scope
       end
     end
@@ -52,7 +52,7 @@ module Ameba::AST
 
         scope = Scope.new nodes.def_nodes.first
         variable = Variable.new(nodes.var_nodes.first, scope)
-        assignment = Assignment.new(nodes.assign_nodes.first, variable)
+        assignment = Assignment.new(nodes.assign_nodes.first, variable, scope)
         assignment.branch.should_not be_nil
         assignment.branch.not_nil!.node.class.should eq Crystal::Expressions
       end
@@ -69,7 +69,7 @@ module Ameba::AST
         )
         scope = Scope.new nodes.def_nodes.first
         variable = Variable.new(nodes.var_nodes.first, scope)
-        assignment = Assignment.new(nodes.assign_nodes.first, variable)
+        assignment = Assignment.new(nodes.assign_nodes.first, variable, scope)
         assignment.branch.should_not be_nil
         assignment.branch.not_nil!.node.class.should eq Crystal::Assign
       end
@@ -83,7 +83,7 @@ module Ameba::AST
 
         scope = Scope.new nodes.def_nodes.first
         variable = Variable.new(nodes.var_nodes.first, scope)
-        assignment = Assignment.new(nodes.assign_nodes.first, variable)
+        assignment = Assignment.new(nodes.assign_nodes.first, variable, scope)
         assignment.branch.should be_nil
       end
     end
@@ -98,7 +98,7 @@ module Ameba::AST
 
         scope = Scope.new nodes.def_nodes.first
         variable = Variable.new(nodes.var_nodes.first, scope)
-        assignment = Assignment.new(nodes.assign_nodes.first, variable)
+        assignment = Assignment.new(nodes.assign_nodes.first, variable, scope)
         assignment.transformed?.should be_false
       end
 
@@ -110,7 +110,7 @@ module Ameba::AST
 
         scope = Scope.new nodes.block_nodes.first
         variable = Variable.new(nodes.var_nodes.first, scope)
-        assignment = Assignment.new(nodes.assign_nodes.first, variable)
+        assignment = Assignment.new(nodes.assign_nodes.first, variable, scope)
         assignment.transformed?.should be_true
       end
     end

spec/ameba/ast/variabling/variable_spec.cr

@@ -47,14 +47,14 @@ module Ameba::AST
 
       it "assigns the variable (creates a new assignment)" do
         variable = Variable.new(var_node, scope)
-        variable.assign(assign_node)
+        variable.assign(assign_node, scope)
         variable.assignments.any?.should be_true
       end
 
       it "can create multiple assignments" do
         variable = Variable.new(var_node, scope)
-        variable.assign(assign_node)
-        variable.assign(assign_node)
+        variable.assign(assign_node, scope)
+        variable.assign(assign_node, scope)
         variable.assignments.size.should eq 2
       end
     end
@@ -62,10 +62,19 @@ module Ameba::AST
     describe "#reference" do
       it "references the existed assignment" do
         variable = Variable.new(var_node, scope)
-        variable.assign(as_node "foo=1")
+        variable.assign(as_node("foo=1"), scope)
         variable.reference(var_node, scope)
         variable.references.any?.should be_true
       end
+
+      it "adds a reference to the scope" do
+        scope = Scope.new as_node "foo = 1"
+        variable = Variable.new(var_node, scope)
+        variable.assign(as_node("foo=1"), scope)
+        variable.reference(var_node, scope)
+        scope.references.size.should eq 1
+        scope.references.first.node.to_s.should eq "foo"
+      end
     end
 
     describe "#captured_by_block?" do

spec/ameba/rule/lint/shared_var_in_fiber_spec.cr

@@ -0,0 +1,236 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  describe SharedVarInFiber do
+    subject = SharedVarInFiber.new
+
+    it "doesn't report if there is only local shared var in fiber" do
+      s = Source.new %(
+        spawn do
+          i = 1
+          puts i
+        end
+
+        Fiber.yield
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if there is only block shared var in fiber" do
+      s = Source.new %(
+        10.times do |i|
+          spawn do
+            puts i
+          end
+        end
+
+        Fiber.yield
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if there a spawn macro is used" do
+      s = Source.new %(
+        i = 0
+        while i < 10
+          spawn puts(i)
+          i += 1
+        end
+
+        Fiber.yield
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "reports if there is a shared var in spawn" do
+      s = Source.new %(
+        i = 0
+        while i < 10
+          spawn do
+            puts(i)
+          end
+          i += 1
+        end
+
+        Fiber.yield
+      )
+      subject.catch(s).should_not be_valid
+    end
+
+    it "reports reassigned reference to shared var in spawn" do
+      s = Source.new %(
+        channel = Channel(String).new
+        n = 0
+
+        while n < 10
+          n = n + 1
+          spawn do
+            m = n
+            channel.send m
+          end
+        end
+      )
+      subject.catch(s).should_not be_valid
+    end
+
+    it "doesn't report reassigned reference to shared var in block" do
+      s = Source.new %(
+        channel = Channel(String).new
+        n = 0
+
+        while n < 3
+          n = n + 1
+          m = n
+          spawn do
+            channel.send m
+          end
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "does not report block is called in a spawn" do
+      s = Source.new %(
+        def method(block)
+          spawn do
+            block.call(10)
+          end
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "reports multiple shared variables in spawn" do
+      s = Source.new %(
+        foo, bar, baz = 0, 0, 0
+        while foo < 10
+          baz += 1
+          spawn do
+            puts foo
+            puts foo + bar + baz
+          end
+          foo += 1
+        end
+      )
+      subject.catch(s).should_not be_valid
+      s.issues.size.should eq 3
+      s.issues[0].location.to_s.should eq ":5:10"
+      s.issues[0].end_location.to_s.should eq ":5:12"
+      s.issues[0].message.should eq "Shared variable `foo` is used in fiber"
+
+      s.issues[1].location.to_s.should eq ":6:10"
+      s.issues[1].end_location.to_s.should eq ":6:12"
+      s.issues[1].message.should eq "Shared variable `foo` is used in fiber"
+
+      s.issues[2].location.to_s.should eq ":6:22"
+      s.issues[2].end_location.to_s.should eq ":6:24"
+      s.issues[2].message.should eq "Shared variable `baz` is used in fiber"
+    end
+
+    it "doesn't report if variable is passed to the proc" do
+      s = Source.new %(
+        i = 0
+        while i < 10
+          proc = ->(x : Int32) do
+            spawn do
+            puts(x)
+            end
+          end
+          proc.call(i)
+          i += 1
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if a channel is declared in outer scope" do
+      s = Source.new %(
+        channel = Channel(Nil).new
+        spawn { channel.send(nil) }
+        channel.receive
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if there is a loop in spawn" do
+      s = Source.new %(
+        channel = Channel(String).new
+
+        spawn do
+          server = TCPServer.new("0.0.0.0", 8080)
+          socket = server.accept
+          while line = socket.gets
+            channel.send(line)
+          end
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if a var is mutated in spawn and referenced outside" do
+      s = Source.new %(
+        def method
+          foo = 1
+          spawn { foo = 2 }
+          foo
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if variable is changed without iterations" do
+      s = Source.new %(
+        def foo
+          i = 0
+          i += 1
+          spawn { i }
+        end
+      ), "source.cr"
+
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if variable is in a loop inside spawn" do
+      s = Source.new %(
+        i = 0
+        spawn do
+          while i < 10
+            i += 1
+          end
+        end
+      )
+
+      subject.catch(s).should be_valid
+    end
+
+    it "doesn't report if variable declared inside loop" do
+      s = Source.new %(
+        while true
+          i = 0
+          spawn { i += 1 }
+        end
+      )
+
+      subject.catch(s).should be_valid
+    end
+
+    it "reports rule, location and message" do
+      s = Source.new %(
+        i = 0
+        while true
+          i += 1
+          spawn { i }
+        end
+      ), "source.cr"
+
+      subject.catch(s).should_not be_valid
+      s.issues.size.should eq 1
+
+      issue = s.issues.first
+      issue.rule.should_not be_nil
+      issue.location.to_s.should eq "source.cr:4:11"
+      issue.end_location.to_s.should eq "source.cr:4:11"
+      issue.message.should eq "Shared variable `i` is used in fiber"
+    end
+  end
+end

spec/ameba/rule/lint/useless_assign_spec.cr

@@ -921,23 +921,6 @@ module Ameba::Rule::Lint
         subject.catch(s).should be_valid
       end
 
-      it "reports if assignment is referenced in macro def in a different scope" do
-        s = Source.new %(
-          class Foo
-            def foo
-              x = 1
-            end
-          end
-
-          class Bar
-            macro macro_call
-              puts x
-            end
-          end
-        )
-        subject.catch(s).should_not be_valid
-      end
-
       it "doesn't report if assignment is referenced in a macro below" do
         s = Source.new %(
           class Foo