Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc refactors #300

Merged
merged 5 commits into from
Nov 14, 2022
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
2 changes: 1 addition & 1 deletion spec/ameba/ast/scope_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ module Ameba::AST
it "returns true if node is nested to Crystal::Macro" do
nodes = as_nodes %(
macro included
{{@type.each do |type| a = type end}}
{{ @type.each do |type| a = type end }}
end
)
outer_scope = Scope.new nodes.macro_nodes.first
Expand Down
8 changes: 6 additions & 2 deletions spec/ameba/config_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ module Ameba
---
Globs: 100
CONFIG
expect_raises(Exception, "incorrect 'Globs' section in a config file") { Config.new(yml) }
expect_raises(Exception, "Incorrect 'Globs' section in a config file") do
Config.new(yml)
end
end

it "initializes excluded as string" do
Expand Down Expand Up @@ -68,7 +70,9 @@ module Ameba
---
Excluded: true
CONFIG
expect_raises(Exception, "incorrect 'Excluded' section in a config file") { Config.new(yml) }
expect_raises(Exception, "Incorrect 'Excluded' section in a config file") do
Config.new(yml)
end
end
end

Expand Down
6 changes: 4 additions & 2 deletions spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,13 @@ module Ameba::Rule::Lint
end

def bar
{{@type.instance_vars.map do |ivar|
{{
@type.instance_vars.map do |ivar|
ivar.annotations(Name).each do |ann|
puts ann.args
end
end}}
end
}}
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/ameba/rule/lint/unused_argument_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,11 @@ module Ameba::Rule::Lint
s = Source.new %(
record X do
macro foo(a, b)
{{a}} + {{b}}
{{ a }} + {{ b }}
end

macro bar(a, b, c)
{{a}} + {{b}} + {{c}}
{{ a }} + {{ b }} + {{ c }}
end
end
)
Expand Down
2 changes: 1 addition & 1 deletion spec/ameba/rule/lint/useless_assign_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ module Ameba::Rule::Lint
foo = 22

{% for x in %w(foo) %}
add({{x.id}})
add({{ x.id }})
{% end %}
)
subject.catch(s).should be_valid
Expand Down
12 changes: 6 additions & 6 deletions spec/spec_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ module Ameba
def test(source, node : Crystal::ClassDef)
return unless location = node.location

end_location = location.adjust(column_number: {{"class".size - 1}})
end_location = location.adjust(column_number: {{ "class".size - 1 }})

issue_for(location, end_location, message: "class to module") do |corrector|
corrector.replace(location, end_location, "module")
Expand All @@ -208,7 +208,7 @@ module Ameba
def test(source, node : Crystal::ModuleDef)
return unless location = node.location

end_location = location.adjust(column_number: {{"module".size - 1}})
end_location = location.adjust(column_number: {{ "module".size - 1 }})

issue_for(location, end_location, message: "module to class") do |corrector|
corrector.replace(location, end_location, "class")
Expand Down Expand Up @@ -265,12 +265,12 @@ module Ameba
end

{% for node in NODES %}
{{getter_name = node.stringify.split("::").last.underscore + "_nodes"}}
{{ getter_name = node.stringify.split("::").last.underscore + "_nodes" }}

getter {{getter_name.id}} = [] of {{node}}
getter {{ getter_name.id }} = [] of {{ node }}

def visit(node : {{node}})
{{getter_name.id}} << node
def visit(node : {{ node }})
{{ getter_name.id }} << node
true
end
{% end %}
Expand Down
4 changes: 3 additions & 1 deletion src/ameba/ast/flow_expression.cr
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ module Ameba::AST
control_flow_found ||= !loop?(exp) && flow_expression?(exp, in_loop?)
end
when Crystal::BinaryOp
unreachable_nodes << current_node.right if flow_expression?(current_node.left, in_loop?)
if flow_expression?(current_node.left, in_loop?)
unreachable_nodes << current_node.right
end
end

unreachable_nodes
Expand Down
1 change: 1 addition & 0 deletions src/ameba/ast/scope.cr
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ module Ameba::AST
# ```
def spawn_block?
return false unless node.is_a?(Crystal::Block)

call = node.as(Crystal::Block).call
call.try(&.name) == "spawn"
end
Expand Down
11 changes: 5 additions & 6 deletions src/ameba/ast/util.cr
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ module Ameba::AST::Util
# to determine and cut a piece of source of the node.
def node_source(node, code_lines)
loc, end_loc = node.location, node.end_location

return unless loc && end_loc

source_between(loc, end_loc, code_lines)
Expand All @@ -46,7 +45,7 @@ module Ameba::AST::Util
first_line, last_line = node_lines[0]?, node_lines[-1]?

return if first_line.nil? || last_line.nil?
return if first_line.size < column # compiler reports incorrection location
return if first_line.size < column # compiler reports incorrect location

node_lines[0] = first_line.sub(0...column, "")

Expand Down Expand Up @@ -162,11 +161,11 @@ module Ameba::AST::Util
# Returns the exp code of a control expression.
# Wraps implicit tuple literal with curly brackets (e.g. multi-return).
def control_exp_code(node : Crystal::ControlExpression, code_lines)
return unless (exp = node.exp)
return unless (exp_code = node_source(exp, code_lines))
return unless exp = node.exp
return unless exp_code = node_source(exp, code_lines)
return exp_code unless exp.is_a?(Crystal::TupleLiteral) && exp_code[0] != '{'
return unless (exp_start = exp.elements.first.location)
return unless (exp_end = exp.end_location)
return unless exp_start = exp.elements.first.location
return unless exp_end = exp.end_location

"{#{source_between(exp_start, exp_end, code_lines)}}"
end
Expand Down
6 changes: 3 additions & 3 deletions src/ameba/ast/variabling/argument.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ module Ameba::AST
# Name of the argument.
def name
case current_node = node
when Crystal::Var then current_node.name
when Crystal::Arg then current_node.name
when Crystal::Var, Crystal::Arg
current_node.name
else
raise ArgumentError.new "invalid node"
raise ArgumentError.new "Invalid node"
end
end
end
Expand Down
1 change: 1 addition & 0 deletions src/ameba/ast/variabling/assignment.cr
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ module Ameba::AST
return false unless (assign = node).is_a?(Crystal::Assign)
return false unless (value = assign.value).is_a?(Crystal::Call)
return false unless (obj = value.obj).is_a?(Crystal::Var)

obj.name.starts_with? "__arg"
end
end
Expand Down
33 changes: 18 additions & 15 deletions src/ameba/ast/variabling/variable.cr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
module Ameba::AST
# Represents the existence of the local variable.
# Holds the var node and variable assigments.
# Holds the var node and variable assignments.
class Variable
# List of the assigments of this variable.
# List of the assignments of this variable.
getter assignments = [] of Assignment

# List of the references of this variable.
Expand Down Expand Up @@ -30,7 +30,7 @@ module Ameba::AST
def initialize(@node, @scope)
end

# Returns true if it is a special variable, i.e `$?`.
# Returns `true` if it is a special variable, i.e `$?`.
def special?
@node.special_var?
end
Expand All @@ -50,7 +50,7 @@ module Ameba::AST
update_assign_reference!
end

# Returns true if variable has any reference.
# Returns `true` if variable has any reference.
#
# ```
# variable = Variable.new(node, scope)
Expand Down Expand Up @@ -85,15 +85,15 @@ module Ameba::AST
consumed_branches = Set(Branch).new

assignments.reverse_each do |assignment|
next if consumed_branches.includes?(assignment.branch)
next if assignment.branch.in?(consumed_branches)
assignment.referenced = true

break unless branch = assignment.branch
consumed_branches << branch
end
end

# Returns true if the current var is referenced in
# Returns `true` if the current var is referenced in
# in the block. For example this variable is captured
# by block:
#
Expand All @@ -110,26 +110,29 @@ 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, check_inner_scopes: false)
return true if inner_scope.block? &&
inner_scope.references?(self, check_inner_scopes: false)
return true if captured_by_block?(inner_scope)
end

false
end

# Returns true if current variable potentially referenced in a macro,
# false if not.
# 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 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)
return true if (outer_scope = scope.outer_scope) &&
used_in_macro?(outer_scope)

false
end

# Returns true if the variable is a target (on the left) of the assignment,
# false otherwise.
# Returns `true` if the variable is a target (on the left) of the assignment,
# `false` otherwise.
def target_of?(assign)
case assign
when Crystal::Assign then eql?(assign.target)
Expand All @@ -141,20 +144,20 @@ module Ameba::AST
end
end

# Returns true if the name starts with '_', false if not.
# Returns `true` if the name starts with '_', `false` if not.
def ignored?
name.starts_with? '_'
end

# Returns true if the `node` represents exactly
# Returns `true` if the `node` represents exactly
# the same Crystal node as `@node`.
def eql?(node)
node.is_a?(Crystal::Var) &&
node.name == @node.name &&
node.location == @node.location
end

# Returns true if the variable is declared before the `node`.
# Returns `true` if the variable is declared before the `node`.
def declared_before?(node)
var_location, node_location = location, node.location

Expand Down
2 changes: 2 additions & 0 deletions src/ameba/ast/visitors/counting_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module Ameba::AST
# AST Visitor that counts occurrences of certain keywords
class CountingVisitor < Crystal::Visitor
DEFAULT_COMPLEXITY = 1

getter macro_condition = false

# Creates a new counting visitor
Expand Down Expand Up @@ -44,6 +45,7 @@ module Ameba::AST
def visit(node : Crystal::MacroIf | Crystal::MacroFor)
@macro_condition = true
@complexity = DEFAULT_COMPLEXITY

false
end
end
Expand Down
4 changes: 2 additions & 2 deletions src/ameba/ast/visitors/node_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ module Ameba::AST
end

{% for name in NODES %}
# A visit callback for `Crystal::{{name}}` node.
# A visit callback for `Crystal::{{ name }}` node.
#
# Returns `true` if the child nodes should be traversed as well,
# `false` otherwise.
def visit(node : Crystal::{{name}})
def visit(node : Crystal::{{ name }})
return false if skip?(node)

@rule.test @source, node
Expand Down
20 changes: 12 additions & 8 deletions src/ameba/ast/visitors/scope_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ module Ameba::AST
end

private def on_assign_end(target, node)
target.is_a?(Crystal::Var) && @current_scope.assign_variable(target.name, node)
target.is_a?(Crystal::Var) &&
@current_scope.assign_variable(target.name, node)
end

# :nodoc:
Expand All @@ -58,7 +59,7 @@ module Ameba::AST

{% for name in NODES %}
# :nodoc:
def visit(node : Crystal::{{name}})
def visit(node : Crystal::{{ name }})
on_scope_enter(node)
end
{% end %}
Expand Down Expand Up @@ -96,13 +97,15 @@ module Ameba::AST

# :nodoc:
def visit(node : Crystal::TypeDeclaration)
return if @current_scope.type_definition? || !(var = node.var).is_a?(Crystal::Var)
@current_scope.add_variable var
return if @current_scope.type_definition?
return if !(var = node.var).is_a?(Crystal::Var)

@current_scope.add_variable(var)
end

# :nodoc:
def visit(node : Crystal::Arg)
@current_scope.add_argument node
@current_scope.add_argument(node)
end

# :nodoc:
Expand All @@ -114,11 +117,12 @@ module Ameba::AST
def visit(node : Crystal::Var)
variable = @current_scope.find_variable node.name

if @current_scope.arg?(node) # node is an argument
case
when @current_scope.arg?(node) # node is an argument
@current_scope.add_argument(node)
elsif variable.nil? && @current_assign # node is a variable
when variable.nil? && @current_assign # node is a variable
@current_scope.add_variable(node)
elsif variable # node is a reference
when variable # node is a reference
reference = variable.reference node, @current_scope
if @current_assign.is_a?(Crystal::OpAssign) || !reference.target_of?(@current_assign)
variable.reference_assignments!
Expand Down
Loading