From 3c1e47a394cc05cb002699b1de0495fd00312d7e Mon Sep 17 00:00:00 2001 From: fatkodima Date: Tue, 9 Jun 2020 13:15:00 +0300 Subject: [PATCH] Add new `Performance/Sum` cop --- CHANGELOG.md | 1 + config/default.yml | 5 + docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_performance.adoc | 30 ++++ lib/rubocop/cop/performance/sum.rb | 135 ++++++++++++++++++ lib/rubocop/cop/performance_cops.rb | 1 + spec/rubocop/cop/performance/sum_spec.rb | 63 ++++++++ 7 files changed, 236 insertions(+) create mode 100644 lib/rubocop/cop/performance/sum.rb create mode 100644 spec/rubocop/cop/performance/sum_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 8901a06f9b..6d2c2c9f33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### New features +* [#137](https://github.com/rubocop-hq/rubocop-performance/pull/137): Add new `Performance/Sum` cop. ([@fatkodima][]) * [#132](https://github.com/rubocop-hq/rubocop-performance/issues/132): Add new `Performance/RedundantSortBlock` cop. ([@fatkodima][]) * [#125](https://github.com/rubocop-hq/rubocop-performance/pull/125): Support `Array()` and `Hash()` methods for `Performance/Size` cop. ([@fatkodima][]) * [#124](https://github.com/rubocop-hq/rubocop-performance/pull/124): Add new `Performance/Squeeze` cop. ([@fatkodima][]) diff --git a/config/default.yml b/config/default.yml index daf9d8c99a..0f85534758 100644 --- a/config/default.yml +++ b/config/default.yml @@ -242,6 +242,11 @@ Performance/StringReplacement: Enabled: true VersionAdded: '0.33' +Performance/Sum: + Description: 'Use `sum` instead of a custom array summation.' + Enabled: true + VersionAdded: '1.7' + Performance/TimesMap: Description: 'Checks for .times.map calls.' AutoCorrect: false diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index fded315504..6850f9325f 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -33,6 +33,7 @@ * xref:cops_performance.adoc#performancestartwith[Performance/StartWith] * xref:cops_performance.adoc#performancestringinclude[Performance/StringInclude] * xref:cops_performance.adoc#performancestringreplacement[Performance/StringReplacement] +* xref:cops_performance.adoc#performancesum[Performance/Sum] * xref:cops_performance.adoc#performancetimesmap[Performance/TimesMap] * xref:cops_performance.adoc#performanceunfreezestring[Performance/UnfreezeString] * xref:cops_performance.adoc#performanceuridefaultparser[Performance/UriDefaultParser] diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index 942ff5f07a..cffa5f692c 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -1440,6 +1440,36 @@ This cop identifies places where `gsub` can be replaced by * https://github.com/JuanitoFatas/fast-ruby#stringgsub-vs-stringtr-code +== Performance/Sum + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Enabled +| Yes +| Yes +| 1.7 +| - +|=== + +This cop identifies places where custom array summation +can be replaced by `sum` method. + +=== Examples + +[source,ruby] +---- +# bad +[1, 2, 3].inject(:+) +[1, 2, 3].reduce(10, :+) +[1, 2, 3].reduce { |acc, elem| acc + elem } + +# good +[1, 2, 3].sum +[1, 2, 3].sum(10) +[1, 2, 3].sum +---- + == Performance/TimesMap |=== diff --git a/lib/rubocop/cop/performance/sum.rb b/lib/rubocop/cop/performance/sum.rb new file mode 100644 index 0000000000..a0a3a9e78c --- /dev/null +++ b/lib/rubocop/cop/performance/sum.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # This cop identifies places where custom array summation + # can be replaced by `sum` method. + # + # @example + # # bad + # [1, 2, 3].inject(:+) + # [1, 2, 3].reduce(10, :+) + # [1, 2, 3].reduce { |acc, elem| acc + elem } + # + # # good + # [1, 2, 3].sum + # [1, 2, 3].sum(10) + # [1, 2, 3].sum + # + class Sum < Cop + include RangeHelp + + MSG = 'Use `%s` instead of `%s`.' + + def_node_matcher :sum_candidate?, <<~PATTERN + (send _ ${:inject :reduce} $_init ? (sym :+)) + PATTERN + + def_node_matcher :sum_with_block_candidate?, <<~PATTERN + (block + $(send _ {:inject :reduce} $_init ?) + (args (arg $_acc) (arg $_elem)) + $send) + PATTERN + + def_node_matcher :acc_plus_elem?, <<~PATTERN + (send (lvar %1) :+ (lvar %2)) + PATTERN + alias elem_plus_acc? acc_plus_elem? + + def on_send(node) + sum_candidate?(node) do |method, init| + range = sum_method_range(node) + message = build_method_message(method, init) + + add_offense(node, location: range, message: message) + end + end + + def on_block(node) + sum_with_block_candidate?(node) do |send, init, var_acc, var_elem, body| + if acc_plus_elem?(body, var_acc, var_elem) || elem_plus_acc?(body, var_elem, var_acc) + range = sum_block_range(send, node) + message = build_block_message(send, init, var_acc, var_elem, body) + + add_offense(node, location: range, message: message) + end + end + end + + def autocorrect(node) + if (matches = sum_candidate?(node)) + _, init = *matches + range = sum_method_range(node) + elsif (matches = sum_with_block_candidate?(node)) + send, init, = matches + range = sum_block_range(send, node) + else + return + end + + unless init.empty? + lambda do |corrector| + replacement = build_good_method(init) + corrector.replace(range, replacement) + end + end + end + + private + + def sum_method_range(node) + range_between(node.loc.selector.begin_pos, node.loc.end.end_pos) + end + + def sum_block_range(send, node) + range_between(send.loc.selector.begin_pos, node.loc.end.end_pos) + end + + def build_method_message(method, init) + good_method = build_good_method(init) + bad_method = build_method_bad_method(init, method) + format(MSG, good_method: good_method, bad_method: bad_method) + end + + def build_block_message(send, init, var_acc, var_elem, body) + good_method = build_good_method(init) + bad_method = build_block_bad_method(send.method_name, init, var_acc, var_elem, body) + format(MSG, good_method: good_method, bad_method: bad_method) + end + + def build_good_method(init) + good_method = 'sum' + + unless init.empty? + init = init.first + good_method += "(#{init.source})" if init.source.to_i != 0 + end + good_method + end + + def build_method_bad_method(init, method) + bad_method = "#{method}(" + unless init.empty? + init = init.first + bad_method += "#{init.source}, " + end + bad_method += ':+)' + bad_method + end + + def build_block_bad_method(method, init, var_acc, var_elem, body) + bad_method = method.to_s + + unless init.empty? + init = init.first + bad_method += "(#{init.source})" + end + bad_method += " { |#{var_acc}, #{var_elem}| #{body.source} }" + bad_method + end + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 7dd5c1f079..66efe97c2a 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -33,6 +33,7 @@ require_relative 'performance/start_with' require_relative 'performance/string_include' require_relative 'performance/string_replacement' +require_relative 'performance/sum' require_relative 'performance/times_map' require_relative 'performance/unfreeze_string' require_relative 'performance/uri_default_parser' diff --git a/spec/rubocop/cop/performance/sum_spec.rb b/spec/rubocop/cop/performance/sum_spec.rb new file mode 100644 index 0000000000..5a5d927b9f --- /dev/null +++ b/spec/rubocop/cop/performance/sum_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::Sum do + subject(:cop) { described_class.new } + + %i[inject reduce].each do |method| + it "registers an offense and corrects when using `array.#{method}(10, :+)`" do + source = "array.#{method}(10, :+)" + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + expect(cop.highlights).to eq(["#{method}(10, :+)"]) + + new_source = autocorrect_source(source) + expect(new_source).to eq('array.sum(10)') + end + + it "registers an offense and corrects when using `array.#{method}(10) { |acc, elem| acc + elem }`" do + source = "array.#{method}(10) { |acc, elem| acc + elem }" + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + expect(cop.highlights).to eq(["#{method}(10) { |acc, elem| acc + elem }"]) + + new_source = autocorrect_source(source) + expect(new_source).to eq('array.sum(10)') + end + + it "registers an offense and corrects when using `array.#{method}(10) { |acc, elem| elem + acc }`" do + source = "array.#{method}(10) { |acc, elem| elem + acc }" + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + expect(cop.highlights).to eq(["#{method}(10) { |acc, elem| elem + acc }"]) + + new_source = autocorrect_source(source) + expect(new_source).to eq('array.sum(10)') + end + + it 'does not autocorrect when initial value is not provided' do + source = "array.#{method}(:+)" + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + expect(cop.highlights).to eq(["#{method}(:+)"]) + + new_source = autocorrect_source(source) + expect(new_source).to eq(source) + end + + it 'does not register an offense when block does not implement summation' do + source = "array.#{method} { |acc, elem| elem * 2 }" + inspect_source(source) + expect(cop.offenses.size).to eq(0) + end + + it 'does not register an offense when using `sum`' do + expect_no_offenses(<<~RUBY) + array.sum + RUBY + end + end +end