diff --git a/spec/ameba/rule/performance/minmax_after_map_spec.cr b/spec/ameba/rule/performance/minmax_after_map_spec.cr new file mode 100644 index 000000000..e44629920 --- /dev/null +++ b/spec/ameba/rule/performance/minmax_after_map_spec.cr @@ -0,0 +1,45 @@ +require "../../../spec_helper" + +module Ameba::Rule::Performance + subject = MinMaxAfterMap.new + + describe MinMaxAfterMap do + it "passes if there are 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 diff --git a/src/ameba/rule/performance/minmax_after_map.cr b/src/ameba/rule/performance/minmax_after_map.cr new file mode 100644 index 000000000..1ed54158e --- /dev/null +++ b/src/ameba/rule/performance/minmax_after_map.cr @@ -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