From 57898fd797e7b938cce1e7d48b43912cc4a0eb0b Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Thu, 28 Dec 2023 14:15:12 +0100 Subject: [PATCH 1/2] =?UTF-8?q?Make=20`BranchVisitor`=20treat=20`loop=20{?= =?UTF-8?q?=20=E2=80=A6=20}`=20calls=20as=20branchable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/ameba/ast/branch_spec.cr | 52 +++++++++++++++++++++++++++++++++++ spec/spec_helper.cr | 1 + src/ameba/ast/branch.cr | 18 +++++++++++- 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/spec/ameba/ast/branch_spec.cr b/spec/ameba/ast/branch_spec.cr index 4da80472..3197eec4 100644 --- a/spec/ameba/ast/branch_spec.cr +++ b/spec/ameba/ast/branch_spec.cr @@ -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 @@ -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 diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 094e0192..04dfa74c 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -259,6 +259,7 @@ module Ameba Crystal::MacroLiteral, Crystal::Expressions, Crystal::ControlExpression, + Crystal::Call, } def initialize(node) diff --git a/src/ameba/ast/branch.cr b/src/ameba/ast/branch.cr index 73ee8dab..30929880 100644 --- a/src/ameba/ast/branch.cr +++ b/src/ameba/ast/branch.cr @@ -1,3 +1,5 @@ +require "./util" + module Ameba::AST # Represents the branch in Crystal code. # Branch is a part of a branchable statement. @@ -67,6 +69,8 @@ module Ameba::AST # :nodoc: private class BranchVisitor < Crystal::Visitor + include Util + @current_branch : Crystal::ASTNode? property branchable : Branchable? @@ -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| @@ -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 From 1feb5c279bbb65f76d03d32f6e4f78812809d002 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Thu, 28 Dec 2023 14:16:51 +0100 Subject: [PATCH 2/2] =?UTF-8?q?Add=20test=20spec=20covering=20the=20`loop?= =?UTF-8?q?=20{=20=E2=80=A6=20}`=20block=20to=20`Lint/SharedVarInFiber`=20?= =?UTF-8?q?rule=20specs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rule/lint/shared_var_in_fiber_spec.cr | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr b/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr index 3381c935..fa4b165b 100644 --- a/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr +++ b/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr @@ -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 @@ -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