Skip to content

Commit

Permalink
Avoid exponential recursion while finding variable references in scop…
Browse files Browse the repository at this point in the history
…es (#203)

* Avoid exponential recursion while finding variable references in scopes

* Adjust source example in test
  • Loading branch information
veelenga authored Jan 31, 2021
1 parent d28f9f7 commit 51b0a07
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 5 deletions.
62 changes: 62 additions & 0 deletions spec/ameba/ast/scope_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,68 @@ module Ameba::AST
end
end

describe "#references?" do
it "returns true if current scope references variable" do
nodes = as_nodes %(
def method
a = 2
block do
3.times { |i| a = a + i }
end
end
)
scope = Scope.new nodes.def_nodes.first
var_node = nodes.var_nodes.first
scope.add_variable var_node
scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)

variable = Variable.new(var_node, scope)
variable.reference nodes.var_nodes.first, scope.inner_scopes.first

scope.references?(variable).should be_true
end

it "returns false if inner scopes are not checked" do
nodes = as_nodes %(
def method
a = 2
block do
3.times { |i| a = a + i }
end
end
)
scope = Scope.new nodes.def_nodes.first
var_node = nodes.var_nodes.first
scope.add_variable var_node
scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)

variable = Variable.new(var_node, scope)
variable.reference nodes.var_nodes.first, scope.inner_scopes.first

scope.references?(variable, check_inner_scopes: false).should be_false
end

it "returns false if current scope does not reference variable" do
nodes = as_nodes %(
def method
a = 2
block do
b = 3
3.times { |i| b = b + i }
end
end
)
scope = Scope.new nodes.def_nodes.first
var_node = nodes.var_nodes.first
scope.add_variable var_node
scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)

variable = Variable.new(var_node, scope)

scope.inner_scopes.first.references?(variable).should be_false
end
end

describe "#add_variable" do
it "adds a new variable to the scope" do
scope = Scope.new as_node("")
Expand Down
42 changes: 42 additions & 0 deletions spec/ameba/rule/lint/useless_assign_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,48 @@ module Ameba::Rule::Lint
end
end

it "does not report if variable is referenced and there is a deep level scope" do
s = Source.new %(
response = JSON.build do |json|
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
json.object do
anything
end
end
end
end
end
end
end
end
end
end
end
end
end
end
end
end
response = JSON.parse(response)
response
)
subject.catch(s).should be_valid
end

context "uninitialized" do
it "reports if uninitialized assignment is not referenced at a top level" do
s = Source.new %(
Expand Down
6 changes: 3 additions & 3 deletions src/ameba/ast/scope.cr
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ module Ameba::AST

# Returns true if current scope (or any of inner scopes) references variable,
# false if not.
def references?(variable : Variable)
def references?(variable : Variable, check_inner_scopes = true)
variable.references.any? do |reference|
reference.scope == self ||
inner_scopes.any?(&.references? variable)
return true if reference.scope == self
check_inner_scopes && inner_scopes.any?(&.references?(variable))
end || variable.used_in_macro?
end

Expand Down
2 changes: 1 addition & 1 deletion src/ameba/ast/variabling/variable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ module Ameba::AST
# ```
def captured_by_block?(scope = @scope)
scope.inner_scopes.each do |inner_scope|
return true if inner_scope.block? && inner_scope.references?(self)
return true if inner_scope.block? && inner_scope.references?(self, check_inner_scopes: false)
return true if captured_by_block?(inner_scope)
end

Expand Down
2 changes: 1 addition & 1 deletion src/ameba/rule/lint/useless_assign.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module Ameba::Rule::Lint

def test(source, node, scope : AST::Scope)
scope.variables.each do |var|
next if var.captured_by_block? || var.used_in_macro? || var.ignored?
next if var.ignored? || var.used_in_macro? || var.captured_by_block?

var.assignments.each do |assign|
next if assign.referenced? || assign.transformed?
Expand Down

0 comments on commit 51b0a07

Please sign in to comment.