Skip to content

Commit

Permalink
Use custom class for passing around implicit return information
Browse files Browse the repository at this point in the history
  • Loading branch information
nobodywasishere committed Feb 12, 2025
1 parent 12cb00a commit 809ac1b
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 66 deletions.
12 changes: 12 additions & 0 deletions src/ameba/ast/implicit_return_scope.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module Ameba::AST
class ImplicitReturnScope
# When greater than zero, indicates the current node's return value is used
property stack : Int32 = 0

property? in_macro : Bool = false

def node_is_used? : Bool
@stack.positive?
end
end
end
98 changes: 49 additions & 49 deletions src/ameba/ast/visitors/implicit_return_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ module Ameba::AST
# parent to its children, such as from an `if` statements parent to it's body,
# as the body is not used by the `if` itself, but by its parent scope.
class ImplicitReturnVisitor < BaseVisitor
# When greater than zero, indicates the current node's return value is used
@stack : Int32 = 0
@in_macro : Bool = false
# This keeps track of whether the current node is used, and
# whether said node is also inside a macro expression
@scope : ImplicitReturnScope = ImplicitReturnScope.new

# The stack is swapped out here as `Crystal::Expressions` are isolated from
# their parents scope. Only the last line in an expressions node can be
# captured by their parent node.
def visit(node : Crystal::Expressions) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

last_idx = node.expressions.size - 1

Expand All @@ -38,7 +38,7 @@ module Ameba::AST
end

def visit(node : Crystal::BinaryOp) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

if node.right.is_a?(Crystal::Call) ||
node.right.is_a?(Crystal::Expressions) ||
Expand All @@ -54,7 +54,7 @@ module Ameba::AST
end

def visit(node : Crystal::Call) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack do
node.obj.try &.accept(self)
Expand All @@ -68,39 +68,39 @@ module Ameba::AST
end

def visit(node : Crystal::Arg) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.default_value.try &.accept(self) }

false
end

def visit(node : Crystal::EnumDef) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

node.members.each &.accept(self)

false
end

def visit(node : Crystal::Assign | Crystal::OpAssign) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.value.accept(self) }

false
end

def visit(node : Crystal::MultiAssign) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.values.each &.accept(self) }

false
end

def visit(node : Crystal::If | Crystal::Unless) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.cond.accept(self) }
node.then.accept(self)
Expand All @@ -110,7 +110,7 @@ module Ameba::AST
end

def visit(node : Crystal::While | Crystal::Until) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.cond.accept(self) }
node.body.accept(self)
Expand All @@ -119,7 +119,7 @@ module Ameba::AST
end

def visit(node : Crystal::Def) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack do
node.args.each &.accept(self)
Expand All @@ -143,7 +143,7 @@ module Ameba::AST
end

def visit(node : Crystal::Macro) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack do
node.args.each &.accept(self)
Expand All @@ -159,15 +159,15 @@ module Ameba::AST
end

def visit(node : Crystal::ClassDef | Crystal::ModuleDef) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

node.body.accept(self)

false
end

def visit(node : Crystal::FunDef) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack do
node.args.each &.accept(self)
Expand All @@ -178,15 +178,15 @@ module Ameba::AST
end

def visit(node : Crystal::Cast | Crystal::NilableCast) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.obj.accept(self) }

false
end

def visit(node : Crystal::Annotation) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack do
node.args.each &.accept(self)
Expand All @@ -197,23 +197,23 @@ module Ameba::AST
end

def visit(node : Crystal::TypeDeclaration) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.value.try &.accept(self) }

false
end

def visit(node : Crystal::ArrayLiteral | Crystal::TupleLiteral) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.elements.each &.accept(self) }

false
end

def visit(node : Crystal::StringInterpolation) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

node.expressions.each do |exp|
incr_stack { exp.accept(self) }
Expand All @@ -223,15 +223,15 @@ module Ameba::AST
end

def visit(node : Crystal::HashLiteral | Crystal::NamedTupleLiteral) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.entries.each &.value.accept(self) }

false
end

def visit(node : Crystal::Case) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.cond.try &.accept(self) }
node.whens.each &.accept(self)
Expand All @@ -241,7 +241,7 @@ module Ameba::AST
end

def visit(node : Crystal::Select) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

node.whens.each &.accept(self)
node.else.try &.accept(self)
Expand All @@ -250,7 +250,7 @@ module Ameba::AST
end

def visit(node : Crystal::When) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.conds.each &.accept(self) }
node.body.accept(self)
Expand All @@ -259,15 +259,15 @@ module Ameba::AST
end

def visit(node : Crystal::Rescue) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

node.body.accept(self)

false
end

def visit(node : Crystal::ExceptionHandler) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

if node.else
# Last line of body isn't implicitly returned if there's an else
Expand All @@ -286,23 +286,23 @@ module Ameba::AST
end

def visit(node : Crystal::Block) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

node.body.accept(self)

false
end

def visit(node : Crystal::ControlExpression) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.exp.try &.accept(self) }

false
end

def visit(node : Crystal::RangeLiteral) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack do
node.from.accept(self)
Expand All @@ -313,7 +313,7 @@ module Ameba::AST
end

def visit(node : Crystal::RegexLiteral) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

# Regex literals either contain string literals or string interpolations,
# both of which are "captured" by the parent regex literal
Expand All @@ -326,21 +326,21 @@ module Ameba::AST
node : Crystal::BoolLiteral | Crystal::CharLiteral | Crystal::NumberLiteral |
Crystal::StringLiteral | Crystal::SymbolLiteral | Crystal::ProcLiteral,
) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

true
end

def visit(node : Crystal::Yield) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

incr_stack { node.exps.each &.accept(self) }

false
end

def visit(node : Crystal::MacroExpression) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

in_macro do
if node.output?
Expand All @@ -354,7 +354,7 @@ module Ameba::AST
end

def visit(node : Crystal::MacroIf) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

in_macro do
swap_stack do
Expand All @@ -368,7 +368,7 @@ module Ameba::AST
end

def visit(node : Crystal::MacroFor) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

in_macro do
swap_stack { node.body.accept(self) }
Expand All @@ -382,25 +382,25 @@ module Ameba::AST
end

def visit(node : Crystal::Generic | Crystal::Path | Crystal::Union) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

false
end

def visit(node : Crystal::UninitializedVar) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

false
end

def visit(node : Crystal::LibDef) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

false
end

def visit(node : Crystal::Include | Crystal::Extend) : Bool
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

false
end
Expand All @@ -414,30 +414,30 @@ module Ameba::AST
end

def visit(node)
@rule.test(@source, node, @stack.positive?, @in_macro)
@rule.test(@source, node, @scope)

true
end

# Indicates that any nodes visited within the block are captured / used.
private def incr_stack(&) : Nil
@stack += 1
@scope.stack += 1
yield
@stack -= 1
@scope.stack -= 1
end

private def swap_stack(& : Int32 -> Nil) : Nil
old_stack = @stack
@stack = 0
old_stack = @scope.stack
@scope.stack = 0
yield old_stack
@stack = old_stack
@scope.stack = old_stack
end

private def in_macro(&) : Nil
old_value = @in_macro
@in_macro = true
old_value = @scope.in_macro?
@scope.in_macro = true
yield
@in_macro = old_value
@scope.in_macro = old_value
end
end
end
4 changes: 2 additions & 2 deletions src/ameba/rule/lint/unused_class_variable_access.cr
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ module Ameba::Rule::Lint
AST::ImplicitReturnVisitor.new(self, source)
end

def test(source, node : Crystal::ClassVar, node_is_used : Bool, in_macro : Bool)
return if node_is_used
def test(source, node : Crystal::ClassVar, scope : AST::ImplicitReturnScope)
return if scope.node_is_used?

issue_for node, MSG
end
Expand Down
Loading

0 comments on commit 809ac1b

Please sign in to comment.