Add Performance/MinMaxAfterMap rule

This commit is contained in:
Sijawusz Pur Rahnama 2023-07-10 04:26:45 +02:00
parent 5cff76071a
commit ddff8d226b
2 changed files with 112 additions and 0 deletions

View file

@ -0,0 +1,45 @@
require "../../../spec_helper"
module Ameba::Rule::Performance
subject = MinMaxAfterMap.new
describe MinMaxAfterMap do
it "passes if there is no potential performance improvements" do
expect_no_issues subject, <<-CRYSTAL
%w[Alice Bob].map { |name| name.size }.min(2)
%w[Alice Bob].map { |name| name.size }.max(2)
CRYSTAL
end
it "reports if there is a `min/max/minmax` call followed by `map`" do
source = expect_issue subject, <<-CRYSTAL
%w[Alice Bob].map { |name| name.size }.min
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `min_of {...}` instead of `map {...}.min`.
%w[Alice Bob].map(&.size).max.zero?
# ^^^^^^^^^^^^^^^ error: Use `max_of {...}` instead of `map {...}.max`.
%w[Alice Bob].map(&.size).minmax?
# ^^^^^^^^^^^^^^^^^^^ error: Use `minmax_of? {...}` instead of `map {...}.minmax?`.
CRYSTAL
expect_correction source, <<-CRYSTAL
%w[Alice Bob].min_of { |name| name.size }
%w[Alice Bob].max_of(&.size).zero?
%w[Alice Bob].minmax_of?(&.size)
CRYSTAL
end
it "does not report if source is a spec" do
expect_no_issues subject, path: "source_spec.cr", code: <<-CRYSTAL
%w[Alice Bob].map(&.size).min
CRYSTAL
end
context "macro" do
it "doesn't report in macro scope" do
expect_no_issues subject, <<-CRYSTAL
{{ %w[Alice Bob].map(&.size).min }}
CRYSTAL
end
end
end
end

View file

@ -0,0 +1,67 @@
require "./base"
module Ameba::Rule::Performance
# This rule is used to identify usage of `min/max/minmax` calls that follow `map`.
#
# For example, this is considered invalid:
#
# ```
# %w[Alice Bob].map(&.size).min
# %w[Alice Bob].map(&.size).max
# %w[Alice Bob].map(&.size).minmax
# ```
#
# And it should be written as this:
#
# ```
# %w[Alice Bob].min_of(&.size)
# %w[Alice Bob].max_of(&.size)
# %w[Alice Bob].minmax_of(&.size)
# ```
#
# YAML configuration example:
#
# ```
# Performance/MinMaxAfterMap:
# Enabled: true
# ```
class MinMaxAfterMap < Base
properties do
description "Identifies usage of `min/max/minmax` calls that follow `map`"
end
MSG = "Use `%s {...}` instead of `map {...}.%s`."
CALL_NAMES = %w[min min? max max? minmax minmax?]
def test(source)
AST::NodeVisitor.new self, source, skip: :macro
end
def test(source, node : Crystal::Call)
return unless node.name.in?(CALL_NAMES) && node.block.nil? && node.args.empty?
return unless (obj = node.obj) && obj.is_a?(Crystal::Call)
return unless obj.name == "map" && obj.block && obj.args.empty?
return unless name_location = obj.name_location
return unless end_location = node.name_end_location
of_name = node.name.sub(/(.+?)(\?)?$/, "\\1_of\\2")
message = MSG % {of_name, node.name}
issue_for name_location, end_location, message do |corrector|
next unless node_name_location = node.name_location
# FIXME: switching the order of the below calls breaks the corrector
corrector.replace(
name_location,
name_location.adjust(column_number: {{ "map".size - 1 }}),
of_name
)
corrector.remove(
node_name_location.adjust(column_number: -1),
end_location
)
end
end
end
end