Skip to content

Commit

Permalink
Optimize code to avoid intermediate arrays allocation 🚀
Browse files Browse the repository at this point in the history
  • Loading branch information
Sija committed Oct 29, 2024
1 parent 2e62b5d commit cc47787
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 41 deletions.
3 changes: 2 additions & 1 deletion src/ameba/formatter/disabled_formatter.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ module Ameba::Formatter
output << "Disabled rules using inline directives:\n\n"

sources.each do |source|
source.issues.select(&.disabled?).each do |issue|
source.issues.each do |issue|
next unless issue.disabled?
next unless loc = issue.location

output << "#{source.path}:#{loc.line_number}".colorize(:cyan)
Expand Down
10 changes: 7 additions & 3 deletions src/ameba/rule/lint/literal_in_interpolation.cr
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ module Ameba::Rule::Lint
MSG = "Literal value found in interpolation"

def test(source, node : Crystal::StringInterpolation)
node.expressions
.select { |exp| !exp.is_a?(Crystal::StringLiteral) && literal?(exp) }
.each { |exp| issue_for exp, MSG }
each_literal_node(node) { |exp| issue_for exp, MSG }
end

private def each_literal_node(node, &)
node.expressions.each do |exp|
yield exp if !exp.is_a?(Crystal::StringLiteral) && literal?(exp)
end
end
end
end
16 changes: 8 additions & 8 deletions src/ameba/rule/lint/redundant_string_coercion.cr
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,18 @@ module Ameba::Rule::Lint
MSG = "Redundant use of `Object#to_s` in interpolation"

def test(source, node : Crystal::StringInterpolation)
string_coercion_nodes(node).each do |expr|
each_string_coercion_node(node) do |expr|
issue_for name_location(expr), expr.end_location, MSG
end
end

private def string_coercion_nodes(node)
node.expressions.select do |exp|
exp.is_a?(Crystal::Call) &&
exp.name == "to_s" &&
exp.args.size.zero? &&
exp.named_args.nil? &&
exp.obj
private def each_string_coercion_node(node, &)
node.expressions.each do |exp|
yield exp if exp.is_a?(Crystal::Call) &&
exp.name == "to_s" &&
exp.args.size.zero? &&
exp.named_args.nil? &&
exp.obj
end
end
end
Expand Down
8 changes: 7 additions & 1 deletion src/ameba/rule/lint/shadowing_outer_local_var.cr
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ module Ameba::Rule::Lint
private def find_shadowing(source, scope)
return unless outer_scope = scope.outer_scope

scope.arguments.reject(&.ignored?).each do |arg|
each_argument_node(scope) do |arg|
# TODO: handle unpacked variables from `Block#unpacks`
next unless name = arg.name.presence

Expand All @@ -66,5 +66,11 @@ module Ameba::Rule::Lint
issue_for arg.node, MSG % name
end
end

private def each_argument_node(scope, &)
scope.arguments.each do |arg|
yield arg unless arg.ignored?
end
end
end
end
10 changes: 5 additions & 5 deletions src/ameba/rule/lint/shared_var_in_fiber.cr
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ module Ameba::Rule::Lint
private def mutated_in_loop?(variable)
declared_in = variable.assignments.first?.try &.branch

variable.assignments
.reject(&.scope.spawn_block?)
.any? do |assign|
assign.branch.try(&.in_loop?) && assign.branch != declared_in
end
variable.assignments.any? do |assign|
!assign.scope.spawn_block? &&
assign.branch.try(&.in_loop?) &&
assign.branch != declared_in
end
end
end
end
21 changes: 12 additions & 9 deletions src/ameba/rule/naming/accessor_method_name.cr
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,7 @@ module Ameba::Rule::Naming
MSG = "Favour method name '%s' over '%s'"

def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
defs =
case body = node.body
when Crystal::Def
[body]
when Crystal::Expressions
body.expressions.select(Crystal::Def)
end

defs.try &.each do |def_node|
each_def_node(node) do |def_node|
# skip defs with explicit receiver, as they'll be handled
# by the `test(source, node : Crystal::Def)` overload
check_issue(source, def_node) unless def_node.receiver
Expand All @@ -64,6 +56,17 @@ module Ameba::Rule::Naming
check_issue(source, node) if node.receiver
end

private def each_def_node(node, &)
case body = node.body
when Crystal::Def
yield body
when Crystal::Expressions
body.expressions.each do |exp|
yield exp if exp.is_a?(Crystal::Def)
end
end
end

private def check_issue(source, node : Crystal::Def)
case node.name
when /^get_([a-z]\w*)$/
Expand Down
23 changes: 13 additions & 10 deletions src/ameba/rule/naming/query_bool_methods.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,9 @@ module Ameba::Rule::Naming
CALL_NAMES = %w[getter class_getter property class_property]

def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
calls =
case body = node.body
when Crystal::Call
[body] if body.name.in?(CALL_NAMES)
when Crystal::Expressions
body.expressions
.select(Crystal::Call)
.select!(&.name.in?(CALL_NAMES))
end
each_call_node(node) do |exp|
next unless exp.name.in?(CALL_NAMES)

calls.try &.each do |exp|
exp.args.each do |arg|
name_node, is_bool =
case arg
Expand All @@ -67,5 +59,16 @@ module Ameba::Rule::Naming
end
end
end

private def each_call_node(node, &)
case body = node.body
when Crystal::Call
yield body
when Crystal::Expressions
body.expressions.each do |exp|
yield exp if exp.is_a?(Crystal::Call)
end
end
end
end
end
6 changes: 2 additions & 4 deletions src/ameba/runner.cr
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,8 @@ module Ameba
# runner.success? # => true or false
# ```
def success?
@sources.all? do |source|
source.issues
.reject(&.disabled?)
.none?(&.rule.severity.<=(@severity))
@sources.all? &.issues.none? do |issue|
issue.enabled? && issue.rule.severity <= @severity
end
end

Expand Down

0 comments on commit cc47787

Please sign in to comment.