Skip to content

Commit

Permalink
Do not report if variable is assigned and referenced in MacroFor/Macr…
Browse files Browse the repository at this point in the history
…oIf/MacroExpression

closes #194
  • Loading branch information
veelenga committed Jan 20, 2021
1 parent 4958fa2 commit 7aa7efd
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 10 deletions.
60 changes: 60 additions & 0 deletions spec/ameba/rule/lint/useless_assign_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,66 @@ module Ameba::Rule::Lint
)
subject.catch(s).should be_valid
end

it "doesn't report if assignment is referenced in a macro expression as string" do
s = Source.new %(
foo = 1
puts {{ "foo".id }}
)
subject.catch(s).should be_valid
end

it "doesn't report if assignement is referenced in for macro in exp" do
s = Source.new %(
foo = 22
{% for x in %w(foo) %}
add({{x.id}})
{% end %}
)
subject.catch(s).should be_valid
end

it "doesn't report if assignement is referenced in for macro in body" do
s = Source.new %(
foo = 22
{% for x in %w(bar) %}
puts {{ "foo".id }}
{% end %}
)
subject.catch(s).should be_valid
end

it "doesn't report if assignement is referenced in if macro in cond" do
s = Source.new %(
foo = 22
{% if "foo".id %}
{% end %}
)
subject.catch(s).should be_valid
end

it "doesn't report if assignement is referenced in if macro in then" do
s = Source.new %(
foo = 22
{% if true %}
puts {{ "foo".id }}
{% end %}
)
subject.catch(s).should be_valid
end

it "doesn't report if assignement is referenced in if macro in else" do
s = Source.new %(
foo = 22
{% if true %}
{% else %}
puts {{ "foo".id }}
{% end %}
)
subject.catch(s).should be_valid
end
end

context "uninitialized" do
Expand Down
33 changes: 23 additions & 10 deletions src/ameba/ast/variabling/variable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,13 @@ module Ameba::AST
false
end

# Returns true if current variable potentially referenced in a macro literal,
# Returns true if current variable potentially referenced in a macro,
# false if not.
def used_in_macro?(scope = @scope)
scope.inner_scopes.each do |inner_scope|
return true if MacroLiteralFinder.new(inner_scope.node).references? node
return true if MacroReferenceFinder.new(inner_scope.node, node.name).references
end
return true if MacroReferenceFinder.new(scope.node, node.name).references
return true if (outer_scope = scope.outer_scope) && used_in_macro?(outer_scope)
false
end
Expand Down Expand Up @@ -167,23 +168,35 @@ module Ameba::AST
var_location.column_number < node_location.column_number)
end

private class MacroLiteralFinder < Crystal::Visitor
@macro_literals = [] of Crystal::MacroLiteral
private class MacroReferenceFinder < Crystal::Visitor
property references = false

def initialize(node)
def initialize(node, @reference : String = reference)
node.accept self
end

def references?(node : Crystal::Var)
@macro_literals.any? { |literal| literal.value.includes? node.name }
end

def visit(node : Crystal::ASTNode)
true
end

def visit(node : Crystal::MacroLiteral)
@macro_literals << node
!(self.references ||= node.value.includes?(@reference))
end

def visit(node : Crystal::MacroExpression)
!(self.references ||= node.exp.to_s.includes?(@reference))
end

def visit(node : Crystal::MacroFor)
exp, body = node.exp, node.body
!(self.references ||= exp.to_s.includes?(@reference) ||
body.to_s.includes?(@reference))
end

def visit(node : Crystal::MacroIf)
!(self.references ||= node.cond.to_s.includes?(@reference) ||
node.then.to_s.includes?(@reference) ||
node.else.to_s.includes?(@reference))
end
end

Expand Down

0 comments on commit 7aa7efd

Please sign in to comment.