Extend JoinAfterMap to check also calls to sum/product and rename it to MapInsteadOfBlock (#190)

This commit is contained in:
Sijawusz Pur Rahnama 2021-01-18 16:42:50 +01:00 committed by GitHub
parent e9ec91654c
commit d2fa75280f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 13 deletions

View file

@ -1,13 +1,14 @@
require "../../../spec_helper" require "../../../spec_helper"
module Ameba::Rule::Performance module Ameba::Rule::Performance
subject = JoinAfterMap.new subject = MapInsteadOfBlock.new
describe JoinAfterMap do describe MapInsteadOfBlock do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( source = Source.new %(
(1..3).join(&.to_s) (1..3).join(&.to_s)
(1..3).join('.', &.to_s) (1..3).sum(&.*(2))
(1..3).product(&.*(2))
) )
subject.catch(source).should be_valid subject.catch(source).should be_valid
end end
@ -52,7 +53,7 @@ module Ameba::Rule::Performance
issue.rule.should_not be_nil issue.rule.should_not be_nil
issue.location.to_s.should eq "source.cr:1:8" issue.location.to_s.should eq "source.cr:1:8"
issue.end_location.to_s.should eq "source.cr:1:24" issue.end_location.to_s.should eq "source.cr:1:24"
issue.message.should eq "Use `join(separator) {...}` instead of `map {...}.join(separator)`" issue.message.should eq "Use `join {...}` instead of `map {...}.join`"
end end
end end
end end

View file

@ -1,32 +1,35 @@
module Ameba::Rule::Performance module Ameba::Rule::Performance
# This rule is used to identify usage of `join` calls that follow `map`. # This rule is used to identify usage of `join/sum/product` calls
# that follow `map`.
# #
# For example, this is considered inefficient: # For example, this is considered inefficient:
# #
# ``` # ```
# (1..3).map(&.to_s).join('.') # (1..3).map(&.to_s).join('.')
# (1..3).map(&.*(2)).sum
# ``` # ```
# #
# And can be written as this: # And can be written as this:
# #
# ``` # ```
# (1..3).join('.', &.to_s) # (1..3).join('.', &.to_s)
# (1..3).sum(&.*(2))
# ``` # ```
# #
# YAML configuration example: # YAML configuration example:
# #
# ``` # ```
# Performance/JoinAfterMap # Performance/MapInsteadOfBlock
# Enabled: true # Enabled: true
# ``` # ```
struct JoinAfterMap < Base struct MapInsteadOfBlock < Base
properties do properties do
description "Identifies usage of `join` calls that follow `map`." description "Identifies usage of `join/sum/product` calls that follow `map`."
end end
CALL_NAMES = %w(join sum product)
MAP_NAME = "map" MAP_NAME = "map"
JOIN_NAME = "join" MSG = "Use `%s {...}` instead of `map {...}.%s`"
MSG = "Use `join(separator) {...}` instead of `map {...}.join(separator)`"
def test(source) def test(source)
AST::NodeVisitor.new self, source, skip: [ AST::NodeVisitor.new self, source, skip: [
@ -38,11 +41,12 @@ module Ameba::Rule::Performance
end end
def test(source, node : Crystal::Call) def test(source, node : Crystal::Call)
return unless node.name == JOIN_NAME && (obj = node.obj) return unless node.name.in?(CALL_NAMES) && (obj = node.obj)
return unless obj.is_a?(Crystal::Call) && obj.block return unless obj.is_a?(Crystal::Call) && obj.block
return unless obj.name == MAP_NAME return unless obj.name == MAP_NAME
issue_for obj.name_location, node.name_end_location, MSG issue_for obj.name_location, node.name_end_location,
MSG % {node.name, node.name}
end end
end end
end end