Skip to content

Make Lint/SharedVarInFiber rule account for loop { ... } #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions spec/ameba/ast/branch_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,34 @@ module Ameba::AST
end
end

context "Crystal::Call" do
context "loop" do
it "constructs a branch in block" do
branch = branch_of_assign_in_def <<-CRYSTAL
def method(a)
loop do
b = (a = 1)
end
end
CRYSTAL
branch.to_s.should eq "b = (a = 1)"
end
end

context "other" do
it "skips constructing a branch in block" do
branch = branch_of_assign_in_def <<-CRYSTAL
def method(a)
1.upto(10) do
b = (a = 1)
end
end
CRYSTAL
branch.should be_nil
end
end
end

describe "#initialize" do
it "creates new branch" do
nodes = as_nodes <<-CRYSTAL
Expand Down Expand Up @@ -358,6 +386,30 @@ module Ameba::AST
branch = Branch.new nodes.assign_nodes.first, branchable
branch.in_loop?.should be_false
end

context "Crystal::Call" do
it "returns true if branch is in a loop" do
nodes = as_nodes <<-CRYSTAL
loop do
a = 1
end
CRYSTAL
branchable = Branchable.new nodes.call_nodes.first
branch = Branch.new nodes.assign_nodes.first, branchable
branch.in_loop?.should be_true
end

it "returns false if branch is not in a loop" do
nodes = as_nodes <<-CRYSTAL
1.upto(10) do
a = 1
end
CRYSTAL
branchable = Branchable.new nodes.call_nodes.first
branch = Branch.new nodes.assign_nodes.first, branchable
branch.in_loop?.should be_false
end
end
end
end
end
20 changes: 19 additions & 1 deletion spec/ameba/rule/lint/shared_var_in_fiber_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module Ameba::Rule::Lint
CRYSTAL
end

it "reports if there is a shared var in spawn" do
it "reports if there is a shared var in spawn (while)" do
source = expect_issue subject, <<-CRYSTAL
i = 0
while i < 10
Expand All @@ -56,6 +56,24 @@ module Ameba::Rule::Lint
expect_no_corrections source
end

it "reports if there is a shared var in spawn (loop)" do
source = expect_issue subject, <<-CRYSTAL
i = 0
loop do
break if i >= 10
spawn do
puts(i)
# ^ error: Shared variable `i` is used in fiber
end
i += 1
end

Fiber.yield
CRYSTAL

expect_no_corrections source
end

it "reports reassigned reference to shared var in spawn" do
source = expect_issue subject, <<-CRYSTAL
channel = Channel(String).new
Expand Down
1 change: 1 addition & 0 deletions spec/spec_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ module Ameba
Crystal::MacroLiteral,
Crystal::Expressions,
Crystal::ControlExpression,
Crystal::Call,
}

def initialize(node)
Expand Down
18 changes: 17 additions & 1 deletion src/ameba/ast/branch.cr
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require "./util"

module Ameba::AST
# Represents the branch in Crystal code.
# Branch is a part of a branchable statement.
Expand Down Expand Up @@ -67,6 +69,8 @@ module Ameba::AST

# :nodoc:
private class BranchVisitor < Crystal::Visitor
include Util

@current_branch : Crystal::ASTNode?

property branchable : Branchable?
Expand All @@ -79,7 +83,7 @@ module Ameba::AST
on_branchable_start(node, branches)
end

private def on_branchable_start(node, branches : Array | Tuple)
private def on_branchable_start(node, branches : Enumerable)
@branchable = Branchable.new(node, @branchable)

branches.each do |branch_node|
Expand Down Expand Up @@ -172,6 +176,18 @@ module Ameba::AST
def end_visit(node : Crystal::MacroFor)
on_branchable_end node
end

def visit(node : Crystal::Call)
if loop?(node) && (block = node.block)
on_branchable_start node, block.body
end
end

def end_visit(node : Crystal::Call)
if loop?(node) && node.block
on_branchable_end node
end
end
end
end
end