diff --git a/CHANGELOG.md b/CHANGELOG.md index 28fe55d..937a165 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### New features +* [#128](https://github.com/rubocop-hq/rubocop-performance/pull/128): Add new `Performance/ReverseFirst` 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 daf9d8c..84393e3 100644 --- a/config/default.yml +++ b/config/default.yml @@ -196,6 +196,11 @@ Performance/ReverseEach: Enabled: true VersionAdded: '0.30' +Performance/ReverseFirst: + Description: 'Use `last(n).reverse` instead of `reverse.first(n)`.' + Enabled: true + VersionAdded: '1.7' + Performance/Size: Description: >- Use `size` instead of `count` for counting diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index fded315..481188c 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -27,6 +27,7 @@ * xref:cops_performance.adoc#performanceredundantsortblock[Performance/RedundantSortBlock] * xref:cops_performance.adoc#performanceregexpmatch[Performance/RegexpMatch] * xref:cops_performance.adoc#performancereverseeach[Performance/ReverseEach] +* xref:cops_performance.adoc#performancereversefirst[Performance/ReverseFirst] * xref:cops_performance.adoc#performancesize[Performance/Size] * xref:cops_performance.adoc#performancesortreverse[Performance/SortReverse] * xref:cops_performance.adoc#performancesqueeze[Performance/Squeeze] diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index 942ff5f..6b84a1c 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -1181,6 +1181,34 @@ change them to use `reverse_each` instead. * https://github.com/JuanitoFatas/fast-ruby#enumerablereverseeach-vs-enumerablereverse_each-code +== Performance/ReverseFirst + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Enabled +| Yes +| Yes +| 1.7 +| - +|=== + +This cop identifies places where `reverse.first(n)` and `reverse.first` +can be replaced by `last(n).reverse` and `last`. + +=== Examples + +[source,ruby] +---- +# bad +array.reverse.first(5) +array.reverse.first + +# good +array.last(5).reverse +array.last +---- + == Performance/Size |=== diff --git a/lib/rubocop/cop/performance/reverse_first.rb b/lib/rubocop/cop/performance/reverse_first.rb new file mode 100644 index 0000000..d01c9da --- /dev/null +++ b/lib/rubocop/cop/performance/reverse_first.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # This cop identifies places where `reverse.first(n)` and `reverse.first` + # can be replaced by `last(n).reverse` and `last`. + # + # @example + # + # # bad + # array.reverse.first(5) + # array.reverse.first + # + # # good + # array.last(5).reverse + # array.last + # + class ReverseFirst < Cop + include RangeHelp + + MSG = 'Use `%s` instead of `%s`.' + + def_node_matcher :reverse_first_candidate?, <<~PATTERN + (send $(send _ :reverse) :first (int _)?) + PATTERN + + def on_send(node) + reverse_first_candidate?(node) do |receiver| + range = correction_range(receiver, node) + message = build_message(node) + + add_offense(node, location: range, message: message) + end + end + + def autocorrect(node) + reverse_first_candidate?(node) do |receiver| + range = correction_range(receiver, node) + replacement = build_good_method(node) + + lambda do |corrector| + corrector.replace(range, replacement) + end + end + end + + private + + def correction_range(receiver, node) + range_between(receiver.loc.selector.begin_pos, node.loc.expression.end_pos) + end + + def build_message(node) + good_method = build_good_method(node) + bad_method = build_bad_method(node) + format(MSG, good_method: good_method, bad_method: bad_method) + end + + def build_good_method(node) + if node.arguments? + "last(#{node.arguments.first.source}).reverse" + else + 'last' + end + end + + def build_bad_method(node) + if node.arguments? + "reverse.first(#{node.arguments.first.source})" + else + 'reverse.first' + end + end + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 7dd5c1f..9fd291a 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -27,6 +27,7 @@ require_relative 'performance/redundant_sort_block' require_relative 'performance/regexp_match' require_relative 'performance/reverse_each' +require_relative 'performance/reverse_first' require_relative 'performance/size' require_relative 'performance/sort_reverse' require_relative 'performance/squeeze' diff --git a/spec/rubocop/cop/performance/reverse_first_spec.rb b/spec/rubocop/cop/performance/reverse_first_spec.rb new file mode 100644 index 0000000..7009303 --- /dev/null +++ b/spec/rubocop/cop/performance/reverse_first_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::ReverseFirst do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when using `#reverse.first(5)`' do + expect_offense(<<~RUBY) + array.reverse.first(5) + ^^^^^^^^^^^^^^^^ Use `last(5).reverse` instead of `reverse.first(5)`. + RUBY + + expect_correction(<<~RUBY) + array.last(5).reverse + RUBY + end + + it 'registers an offense and corrects when using `#reverse.first`' do + expect_offense(<<~RUBY) + array.reverse.first + ^^^^^^^^^^^^^ Use `last` instead of `reverse.first`. + RUBY + + expect_correction(<<~RUBY) + array.last + RUBY + end + + it 'does not register an offense when `#reverse` is not followed by `#first`' do + expect_no_offenses(<<~RUBY) + array.reverse + RUBY + + expect_no_offenses(<<~RUBY) + array.reverse.last(5) + RUBY + end +end