Merge pull request #386 from crystal-ameba/fix-issue-385

This commit is contained in:
Sijawusz Pur Rahnama 2023-06-30 20:58:56 +02:00 committed by GitHub
commit 21051acfff
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 127 additions and 0 deletions

View file

@ -0,0 +1,57 @@
require "../../../spec_helper"
module Ameba::Rule::Performance
subject = ExcessiveAllocations.new
describe ExcessiveAllocations do
it "passes if there is no potential performance improvements" do
expect_no_issues subject, <<-CRYSTAL
"Alice".chars.each(arg) { |c| puts c }
"Alice".chars(arg).each { |c| puts c }
"Alice\nBob".lines.each(arg) { |l| puts l }
"Alice\nBob".lines(arg).each { |l| puts l }
CRYSTAL
end
it "reports if there is a collection method followed by each" do
source = expect_issue subject, <<-CRYSTAL
"Alice".chars.each { |c| puts c }
# ^^^^^^^^^^ error: Use `each_char {...}` instead of `chars.each {...}` to avoid excessive allocation
"Alice\nBob".lines.each { |l| puts l }
# ^^^^^^^^^^ error: Use `each_line {...}` instead of `lines.each {...}` to avoid excessive allocation
CRYSTAL
expect_correction source, <<-CRYSTAL
"Alice".each_char { |c| puts c }
"Alice\nBob".each_line { |l| puts l }
CRYSTAL
end
it "does not report if source is a spec" do
expect_no_issues subject, <<-CRYSTAL, "source_spec.cr"
"Alice".chars.each { |c| puts c }
CRYSTAL
end
context "properties" do
it "#call_names" do
rule = ExcessiveAllocations.new
rule.call_names = {
"children" => "each_child",
}
expect_no_issues rule, <<-CRYSTAL
"Alice".chars.each { |c| puts c }
CRYSTAL
end
end
context "macro" do
it "doesn't report in macro scope" do
expect_no_issues subject, <<-CRYSTAL
{{ "Alice".chars.each { |c| puts c } }}
CRYSTAL
end
end
end
end

View file

@ -0,0 +1,70 @@
require "./base"
module Ameba::Rule::Performance
# This rule is used to identify excessive collection allocations,
# that can be avoided by using `each_<member>` instead of `<collection>.each`.
#
# For example, this is considered inefficient:
#
# ```
# "Alice".chars.each { |c| puts c }
# "Alice\nBob".lines.each { |l| puts l }
# ```
#
# And can be written as this:
#
# ```
# "Alice".each_char { |c| puts c }
# "Alice\nBob".each_line { |l| puts l }
# ```
#
# YAML configuration example:
#
# ```
# Performance/ExcessiveAllocations:
# Enabled: true
# CallNames:
# codepoints: each_codepoint
# graphemes: each_grapheme
# chars: each_char
# lines: each_line
# ```
class ExcessiveAllocations < Base
include AST::Util
properties do
description "Identifies usage of excessive collection allocations"
call_names({
"codepoints" => "each_codepoint",
"graphemes" => "each_grapheme",
"chars" => "each_char",
"lines" => "each_line",
# "keys" => "each_key",
# "values" => "each_value",
# "children" => "each_child",
})
end
MSG = "Use `%s {...}` instead of `%s.each {...}` to avoid excessive allocation"
def test(source)
AST::NodeVisitor.new self, source, skip: :macro
end
def test(source, node : Crystal::Call)
return unless node.name == "each" && node.args.empty?
return unless (obj = node.obj).is_a?(Crystal::Call)
return unless obj.args.empty? && obj.block.nil?
return unless method = call_names[obj.name]?
return unless name_location = obj.name_location
return unless end_location = name_end_location(node)
msg = MSG % {method, obj.name}
issue_for name_location, end_location, msg do |corrector|
corrector.replace(name_location, end_location, method)
end
end
end
end