diff --git a/spec/ameba/rule/performance/excessive_allocations_spec.cr b/spec/ameba/rule/performance/excessive_allocations_spec.cr new file mode 100644 index 00000000..010c5064 --- /dev/null +++ b/spec/ameba/rule/performance/excessive_allocations_spec.cr @@ -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 diff --git a/src/ameba/rule/performance/excessive_allocations.cr b/src/ameba/rule/performance/excessive_allocations.cr new file mode 100644 index 00000000..db8ad968 --- /dev/null +++ b/src/ameba/rule/performance/excessive_allocations.cr @@ -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_` instead of `.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