From 73fecfd7c5800d42bfb6025b941a6f5c4f2a10eb Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 16 Feb 2025 01:09:19 +0900 Subject: [PATCH 01/10] Switch back docs version to master --- docs/antora.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/antora.yml b/docs/antora.yml index 3ee5f7e808..21b1815a34 100644 --- a/docs/antora.yml +++ b/docs/antora.yml @@ -2,6 +2,6 @@ name: rubocop-performance title: RuboCop Performance # We always provide version without patch here (e.g. 1.1), # as patch versions should not appear in the docs. -version: '1.24' +version: ~ nav: - modules/ROOT/nav.adoc From 197472cac9541e4742a04a0a2164ba667f78e686 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 18 Feb 2025 15:32:09 +0900 Subject: [PATCH 02/10] Enable `InternalAffairs/CopEnabled` cop Since #490 made RuboCop 1.72+ a runtime requirement, `cop_enabled?` can now be used. --- .rubocop_todo.yml | 6 ----- .../performance/inefficient_hash_search.rb | 3 +-- .../inefficient_hash_search_spec.rb | 24 ++++++++----------- 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 719e5d053f..ba4e372ec6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,12 +6,6 @@ # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -InternalAffairs/CopEnabled: - Exclude: - - 'lib/rubocop/cop/performance/inefficient_hash_search.rb' - # Offense count: 12 InternalAffairs/NodeDestructuring: Exclude: diff --git a/lib/rubocop/cop/performance/inefficient_hash_search.rb b/lib/rubocop/cop/performance/inefficient_hash_search.rb index 99e4e2c0f1..778db82afa 100644 --- a/lib/rubocop/cop/performance/inefficient_hash_search.rb +++ b/lib/rubocop/cop/performance/inefficient_hash_search.rb @@ -84,8 +84,7 @@ def current_method(node) end def use_long_method - preferred_config = config.for_all_cops['Style/PreferredHashMethods'] - preferred_config && preferred_config['EnforcedStyle'] == 'long' && preferred_config['Enabled'] + config.for_enabled_cop('Style/PreferredHashMethods')['EnforcedStyle'] == 'long' end def correct_argument(node) diff --git a/spec/rubocop/cop/performance/inefficient_hash_search_spec.rb b/spec/rubocop/cop/performance/inefficient_hash_search_spec.rb index d7d282bc55..ee68ae6f02 100644 --- a/spec/rubocop/cop/performance/inefficient_hash_search_spec.rb +++ b/spec/rubocop/cop/performance/inefficient_hash_search_spec.rb @@ -138,28 +138,24 @@ def my_include?(key) end context 'when config specifies long hash methods but is not enabled' do - let(:config) do - RuboCop::Config.new( - 'AllCops' => { - 'Style/PreferredHashMethods' => { - 'EnforcedStyle' => 'long', 'Enabled' => false - } + let(:other_cops) do + { + 'Style/PreferredHashMethods' => { + 'EnforcedStyle' => 'long', 'Enabled' => false } - ) + } end it_behaves_like 'correct behavior', :short end context 'when config enforces long hash methods' do - let(:config) do - RuboCop::Config.new( - 'AllCops' => { - 'Style/PreferredHashMethods' => { - 'EnforcedStyle' => 'long', 'Enabled' => true - } + let(:other_cops) do + { + 'Style/PreferredHashMethods' => { + 'EnforcedStyle' => 'long', 'Enabled' => true } - ) + } end it_behaves_like 'correct behavior', :long From d196b537782cac49852666e54ff0c0e399499fc4 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Tue, 25 Feb 2025 18:54:04 +0100 Subject: [PATCH 03/10] [Fix #482] Change `Performance/CollectionLiteralInLoop` to not register offenses for `Array#include?` that are optimized directly in Ruby. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since `include?` on arrays are the only instances I ever run across this cop, I think it makes sense to add special handling for this. On Ruby 3.4 this does no array allocations: ``` require "memory_profiler" class Foo def bar Bar.new end end class Bar def baz end end foo = Foo.new report = MemoryProfiler.report do [1,2].include?(foo.bar.baz) end.pretty_print ``` Also, this code is simply slower on Ruby 3.4: ```rb require "benchmark/ips" local = [301, 302] val = 301 Benchmark.ips do |x| x.report('local') do local.include?(val) end x.report('literal') do [301, 302].include?(val) end x.compare! end ``` ``` $ ruby test.rb ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] Warming up -------------------------------------- local 1.822M i/100ms literal 2.296M i/100ms Calculating ------------------------------------- local 18.235M (± 1.6%) i/s (54.84 ns/i) - 92.906M in 5.096277s literal 22.807M (± 1.5%) i/s (43.85 ns/i) - 114.789M in 5.034289s Comparison: literal: 22806932.0 i/s local: 18235340.7 i/s - 1.25x slower ``` --- ...hange_collection_literal_include_ruby34.md | 1 + .../performance/collection_literal_in_loop.rb | 39 +++++++++-- .../collection_literal_in_loop_spec.rb | 67 +++++++++++++++++++ 3 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 changelog/change_collection_literal_include_ruby34.md diff --git a/changelog/change_collection_literal_include_ruby34.md b/changelog/change_collection_literal_include_ruby34.md new file mode 100644 index 0000000000..0874c9ca7e --- /dev/null +++ b/changelog/change_collection_literal_include_ruby34.md @@ -0,0 +1 @@ +* [#482](https://github.com/rubocop/rubocop-performance/issues/482): Change `Performance/CollectionLiteralInLoop` to not register offenses for `Array#include?` that are optimized directly in Ruby. ([@earlopain][]) diff --git a/lib/rubocop/cop/performance/collection_literal_in_loop.rb b/lib/rubocop/cop/performance/collection_literal_in_loop.rb index f75847a6b1..498048ba6f 100644 --- a/lib/rubocop/cop/performance/collection_literal_in_loop.rb +++ b/lib/rubocop/cop/performance/collection_literal_in_loop.rb @@ -12,6 +12,14 @@ module Performance # You can set the minimum number of elements to consider # an offense with `MinSize`. # + # NOTE: Since Ruby 3.4, certain simple arguments to `Array#include?` are + # optimized directly in Ruby. This avoids allocations without changing the + # code, as such no offense will be registered in those cases. Currently that + # includes: strings, `self`, local variables, instance variables, and method + # calls without arguments. Additionally, any number of methods can be chained: + # `[1, 2, 3].include?(@foo)` and `[1, 2, 3].include?(@foo.bar.baz)` both avoid + # the array allocation. + # # @example # # bad # users.select do |user| @@ -55,6 +63,8 @@ class CollectionLiteralInLoop < Base ARRAY_METHODS = (ENUMERABLE_METHOD_NAMES | NONMUTATING_ARRAY_METHODS).to_set.freeze + ARRAY_INCLUDE_OPTIMIZED_TYPES = %i[str self lvar ivar send].freeze + NONMUTATING_HASH_METHODS = %i[< <= == > >= [] any? assoc compact dig each each_key each_pair each_value empty? eql? fetch fetch_values filter flatten has_key? @@ -80,8 +90,8 @@ class CollectionLiteralInLoop < Base PATTERN def on_send(node) - receiver, method, = *node.children - return unless check_literal?(receiver, method) && parent_is_loop?(receiver) + receiver, method, *arguments = *node.children + return unless check_literal?(receiver, method, arguments) && parent_is_loop?(receiver) message = format(MSG, literal_class: literal_class(receiver)) add_offense(receiver, message: message) @@ -89,12 +99,33 @@ def on_send(node) private - def check_literal?(node, method) + def check_literal?(node, method, arguments) !node.nil? && nonmutable_method_of_array_or_hash?(node, method) && node.children.size >= min_size && - node.recursive_basic_literal? + node.recursive_basic_literal? && + !optimized_array_include?(node, method, arguments) + end + + # Since Ruby 3.4, simple arguments to Array#include? are optimized. + # See https://github.com/ruby/ruby/pull/12123 for more details. + # rubocop:disable Metrics/CyclomaticComplexity + def optimized_array_include?(node, method, arguments) + return false unless target_ruby_version >= 3.4 && node.array_type? && method == :include? + # Disallow include?(1, 2) + return false if arguments.count != 1 + + arg = arguments.first + # Allow `include?(foo.bar.baz.bat)` + while arg.send_type? + return false if arg.arguments.any? # Disallow include?(foo(bar)) + break unless arg.receiver + + arg = arg.receiver + end + ARRAY_INCLUDE_OPTIMIZED_TYPES.include?(arg.type) end + # rubocop:enable Metrics/CyclomaticComplexity def nonmutable_method_of_array_or_hash?(node, method) (node.array_type? && ARRAY_METHODS.include?(method)) || diff --git a/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb b/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb index effbddea5c..4d9bd1e958 100644 --- a/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb +++ b/spec/rubocop/cop/performance/collection_literal_in_loop_spec.rb @@ -269,4 +269,71 @@ RUBY end end + + context 'when Ruby >= 3.4', :ruby34 do + it 'registers an offense for `include?` on a Hash literal' do + expect_offense(<<~RUBY) + each do + { foo: :bar }.include?(:foo) + ^^^^^^^^^^^^^ Avoid immutable Hash literals in loops. It is better to extract it into a local variable or a constant. + end + RUBY + end + + it 'registers an offense for other array methods' do + expect_offense(<<~RUBY) + each do + [1, 2, 3].index(foo) + ^^^^^^^^^ Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant. + end + RUBY + end + + context 'when using an Array literal and calling `include?`' do + [ + '"string"', + 'self', + 'local_variable', + 'method_call', + '@instance_variable' + ].each do |argument| + it "registers no offense when the argument is #{argument}" do + expect_no_offenses(<<~RUBY) + #{'local_variable = 123' if argument == 'local_variable'} + array.all? do |e| + [1, 2, 3].include?(#{argument}) + end + RUBY + end + + it "registers no offense when the argument is #{argument} with method chain" do + expect_no_offenses(<<~RUBY) + #{'local_variable = 123' if argument == 'local_variable'} + array.all? do |e| + [1, 2, 3].include?(#{argument}.call) + end + RUBY + end + + it "registers no offense when the argument is #{argument} with double method chain" do + expect_no_offenses(<<~RUBY) + #{'local_variable = 123' if argument == 'local_variable'} + array.all? do |e| + [1, 2, 3].include?(#{argument}.call.call) + end + RUBY + end + + it "registers an offense when the argument is #{argument} with method chain and arguments" do + expect_offense(<<~RUBY) + #{'local_variable = 123' if argument == 'local_variable'} + array.all? do |e| + [1, 2, 3].include?(#{argument}.call(true)) + ^^^^^^^^^ Avoid immutable Array literals in loops. It is better to extract it into a local variable or a constant. + end + RUBY + end + end + end + end end From 5fa5f1a39a4a9fb39d1c8c8dbf9e1c3861073cf4 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 19 Feb 2025 14:26:28 +0900 Subject: [PATCH 04/10] Use extended `CopsDocumentationGenerator` Follow up https://github.com/rubocop/rubocop/pull/13908. --- tasks/cops_documentation.rake | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tasks/cops_documentation.rake b/tasks/cops_documentation.rake index 52e29f1be7..82587559a9 100644 --- a/tasks/cops_documentation.rake +++ b/tasks/cops_documentation.rake @@ -15,9 +15,7 @@ task update_cops_documentation: :yard_for_generate_documentation do # NOTE: Update `<>` version for docs/modules/ROOT/pages/cops_performance.adoc # when running release tasks. - RuboCop::ConfigLoader.inject_defaults!("#{__dir__}/../config/default.yml") - - CopsDocumentationGenerator.new(departments: deps).call + CopsDocumentationGenerator.new(departments: deps, plugin_name: 'rubocop-performance').call end desc 'Syntax check for the documentation comments' From d8428314abddb604de15759b795cedcca345727a Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 2 Mar 2025 13:20:45 +0900 Subject: [PATCH 05/10] Automate the process of GitHub release creation Follow-up to https://github.com/rubocop/rubocop/blob/master/.github/workflows/github_release.yml. This action will be triggered when a new tag is pushed and will auto-fill the release notes using the relevant file. The `rubocop-performance` repository has essentially the same structure as the `rubocop` repository, so it should work as is. --- .github/workflows/github_release.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .github/workflows/github_release.yml diff --git a/.github/workflows/github_release.yml b/.github/workflows/github_release.yml new file mode 100644 index 0000000000..7fd6dea172 --- /dev/null +++ b/.github/workflows/github_release.yml @@ -0,0 +1,25 @@ +name: Create GitHub Release + +on: + push: + tags: + - "v*" # Trigger when a version tag is pushed (e.g., v1.0.0) + +jobs: + create-release: + runs-on: ubuntu-latest + + permissions: + contents: write + + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Create GitHub Release + uses: ncipollo/release-action@v1 + with: + tag: ${{ github.ref_name }} + name: RuboCop Performance ${{ github.ref_name }} + bodyFile: relnotes/${{ github.ref_name }}.md + token: ${{ secrets.GITHUB_TOKEN }} From 8c368f6a1659318f0f1179e344248c99045d24cb Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 8 Mar 2025 01:33:35 +0900 Subject: [PATCH 06/10] [Fix #492] Fix false positives for `Performance/StringIdentifierArgument` This PR fixes false positives for `Performance/StringIdentifierArgument` when using interpolated string argument. Converting a string with interpolation into a symbol degrades performance. Fixes #492. --- ..._performance_string_identifier_argument.md | 1 + .../performance/string_identifier_argument.rb | 16 +++------ .../string_identifier_argument_spec.rb | 33 ++++++++----------- 3 files changed, 18 insertions(+), 32 deletions(-) create mode 100644 changelog/fix_false_positives_for_performance_string_identifier_argument.md diff --git a/changelog/fix_false_positives_for_performance_string_identifier_argument.md b/changelog/fix_false_positives_for_performance_string_identifier_argument.md new file mode 100644 index 0000000000..834e343ae7 --- /dev/null +++ b/changelog/fix_false_positives_for_performance_string_identifier_argument.md @@ -0,0 +1 @@ +* [#492](https://github.com/rubocop/rubocop-performance/issues/492): Fix false positives for `Performance/StringIdentifierArgument` when using interpolated string argument. ([@koic][]) diff --git a/lib/rubocop/cop/performance/string_identifier_argument.rb b/lib/rubocop/cop/performance/string_identifier_argument.rb index 54c593e0fb..eda22ba9a2 100644 --- a/lib/rubocop/cop/performance/string_identifier_argument.rb +++ b/lib/rubocop/cop/performance/string_identifier_argument.rb @@ -16,19 +16,19 @@ module Performance # send('do_something') # attr_accessor 'do_something' # instance_variable_get('@ivar') - # respond_to?("string_#{interpolation}") # # # good # send(:do_something) # attr_accessor :do_something # instance_variable_get(:@ivar) - # respond_to?(:"string_#{interpolation}") # # # good - these methods don't support namespaced symbols # const_get("#{module_path}::Base") # const_source_location("#{module_path}::Base") # const_defined?("#{module_path}::Base") # + # # good - using a symbol when string interpolation is involved causes a performance regression. + # respond_to?("string_#{interpolation}") # class StringIdentifierArgument < Base extend AutoCorrector @@ -40,8 +40,6 @@ class StringIdentifierArgument < Base protected public public_constant module_function ].freeze - INTERPOLATION_IGNORE_METHODS = %i[const_get const_source_location const_defined?].freeze - TWO_ARGUMENTS_METHOD = :alias_method MULTIPLE_ARGUMENTS_METHODS = %i[ attr_accessor attr_reader attr_writer private private_constant @@ -59,7 +57,7 @@ class StringIdentifierArgument < Base deprecate_constant remove_const ruby2_keywords define_singleton_method instance_variable_defined? instance_variable_get instance_variable_set method public_method public_send remove_instance_variable respond_to? send singleton_method __send__ - ] + COMMAND_METHODS + INTERPOLATION_IGNORE_METHODS).freeze + ] + COMMAND_METHODS).freeze def on_send(node) return if COMMAND_METHODS.include?(node.method_name) && node.receiver @@ -83,13 +81,7 @@ def string_arguments(node) [node.first_argument] end - arguments.compact.filter { |argument| string_argument_compatible?(argument, node) } - end - - def string_argument_compatible?(argument, node) - return true if argument.str_type? - - argument.dstr_type? && INTERPOLATION_IGNORE_METHODS.none? { |method| node.method?(method) } + arguments.compact.filter(&:str_type?) end def register_offense(argument, argument_value) diff --git a/spec/rubocop/cop/performance/string_identifier_argument_spec.rb b/spec/rubocop/cop/performance/string_identifier_argument_spec.rb index 34ad8894fe..6d703a0e3c 100644 --- a/spec/rubocop/cop/performance/string_identifier_argument_spec.rb +++ b/spec/rubocop/cop/performance/string_identifier_argument_spec.rb @@ -44,26 +44,19 @@ RUBY end - if described_class::INTERPOLATION_IGNORE_METHODS.include?(method) - it 'does not register an offense when using string interpolation for `#{method}` method' do - # NOTE: These methods don't support `::` when passing a symbol. const_get('A::B') is valid - # but const_get(:'A::B') isn't. Since interpolated arguments may contain any content these - # cases are not detected as an offense to prevent false positives. - expect_no_offenses(<<~RUBY) - #{method}("\#{module_name}class_name") - RUBY - end - else - it 'registers an offense when using interpolated string argument' do - expect_offense(<<~RUBY, method: method) - #{method}("do_something_\#{var}") - _{method} ^^^^^^^^^^^^^^^^^^^^^ Use `:"do_something_\#{var}"` instead of `"do_something_\#{var}"`. - RUBY - - expect_correction(<<~RUBY) - #{method}(:"do_something_\#{var}") - RUBY - end + it 'does not register an offense when using string interpolation for `#{method}` method' do + # NOTE: These methods don't support `::` when passing a symbol. const_get('A::B') is valid + # but const_get(:'A::B') isn't. Since interpolated arguments may contain any content these + # cases are not detected as an offense to prevent false positives. + expect_no_offenses(<<~RUBY) + #{method}("\#{module_name}class_name") + RUBY + end + + it 'does not register an offense when using interpolated string argument' do + expect_no_offenses(<<~RUBY) + #{method}("do_something_\#{var}") + RUBY end end end From d339b99376afec98bf20a6802bbf0c97fc5f1edd Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Thu, 20 Mar 2025 11:49:15 -0400 Subject: [PATCH 07/10] Fix `Performance/FixedSize` false positive when `count` is called with a `numblock` --- changelog/fix_fix_performance_fixed_size_false.md | 1 + lib/rubocop/cop/performance/fixed_size.rb | 2 +- spec/rubocop/cop/performance/fixed_size_spec.rb | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 changelog/fix_fix_performance_fixed_size_false.md diff --git a/changelog/fix_fix_performance_fixed_size_false.md b/changelog/fix_fix_performance_fixed_size_false.md new file mode 100644 index 0000000000..88cc443de4 --- /dev/null +++ b/changelog/fix_fix_performance_fixed_size_false.md @@ -0,0 +1 @@ +* [#494](https://github.com/rubocop/rubocop-performance/pull/494): Fix `Performance/FixedSize` false positive when `count` is called with a `numblock`. ([@dvandersluis][]) diff --git a/lib/rubocop/cop/performance/fixed_size.rb b/lib/rubocop/cop/performance/fixed_size.rb index 4e2e1d826f..7a50a45c4b 100644 --- a/lib/rubocop/cop/performance/fixed_size.rb +++ b/lib/rubocop/cop/performance/fixed_size.rb @@ -75,7 +75,7 @@ def allowed_argument?(arg) end def allowed_parent?(node) - node&.type?(:casgn, :block) + node&.type?(:casgn, :any_block) end def contains_splat?(node) diff --git a/spec/rubocop/cop/performance/fixed_size_spec.rb b/spec/rubocop/cop/performance/fixed_size_spec.rb index 51331ca64b..ba35c06c03 100644 --- a/spec/rubocop/cop/performance/fixed_size_spec.rb +++ b/spec/rubocop/cop/performance/fixed_size_spec.rb @@ -204,6 +204,14 @@ expect_no_offenses("#{variable}.count { |v| v == 'a' }") end + it 'accepts calling count with a numblock', :ruby27 do + expect_no_offenses("#{variable}.count { _1 == 'a' }") + end + + it 'accepts calling count with an itblock', :ruby34 do + expect_no_offenses("#{variable}.count { it == 'a' }") + end + it 'accepts calling count with a symbol proc' do expect_no_offenses("#{variable}.count(&:any?) ") end From cee374c83188692c29594015292a07d8bdbb5f14 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 27 Mar 2025 01:24:07 +0900 Subject: [PATCH 08/10] Support `it` block parameter in `Performance` cops This PR supports `it` block parameter in `Performance` cops. --- .github/workflows/test.yml | 2 +- .../new_support_itblock_in_performance_cops.md | 1 + lib/rubocop/cop/performance/times_map.rb | 1 + lib/rubocop/cop/performance/zip_without_block.rb | 1 + rubocop-performance.gemspec | 2 +- .../cop/performance/chain_array_allocation_spec.rb | 9 +++++++++ .../cop/performance/redundant_block_call_spec.rb | 8 ++++++++ spec/rubocop/cop/performance/times_map_spec.rb | 13 +++++++++++++ .../cop/performance/zip_without_block_spec.rb | 11 +++++++++++ spec/spec_helper.rb | 4 ++++ 10 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 changelog/new_support_itblock_in_performance_cops.md diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d0a1e8b93e..875cf0538f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -92,7 +92,7 @@ jobs: sed -e "/gem 'rubocop', github: 'rubocop\/rubocop'/d" \ -e "/gem 'rubocop-rspec',/d" -i Gemfile cat << EOF > Gemfile.local - gem 'rubocop', '1.72.1' # Specify the oldest supported RuboCop version + gem 'rubocop', '1.75.0' # Specify the oldest supported RuboCop version EOF - name: set up Ruby uses: ruby/setup-ruby@v1 diff --git a/changelog/new_support_itblock_in_performance_cops.md b/changelog/new_support_itblock_in_performance_cops.md new file mode 100644 index 0000000000..33b6466d29 --- /dev/null +++ b/changelog/new_support_itblock_in_performance_cops.md @@ -0,0 +1 @@ +* [#496](https://github.com/rubocop/rubocop-performance/pull/496): Support `it` block parameter in `Performance` cops. ([@koic][]) diff --git a/lib/rubocop/cop/performance/times_map.rb b/lib/rubocop/cop/performance/times_map.rb index 07623634da..d2740cebd0 100644 --- a/lib/rubocop/cop/performance/times_map.rb +++ b/lib/rubocop/cop/performance/times_map.rb @@ -52,6 +52,7 @@ def on_block(node) check(node) end alias on_numblock on_block + alias on_itblock on_block private diff --git a/lib/rubocop/cop/performance/zip_without_block.rb b/lib/rubocop/cop/performance/zip_without_block.rb index f2815b9912..09c15f12d0 100644 --- a/lib/rubocop/cop/performance/zip_without_block.rb +++ b/lib/rubocop/cop/performance/zip_without_block.rb @@ -28,6 +28,7 @@ class ZipWithoutBlock < Base { (block (call !nil? RESTRICT_ON_SEND) (args (arg _)) (array (lvar _))) (numblock (call !nil? RESTRICT_ON_SEND) 1 (array (lvar _))) + (itblock (call !nil? RESTRICT_ON_SEND) :it (array (lvar _))) } PATTERN diff --git a/rubocop-performance.gemspec b/rubocop-performance.gemspec index e7c2ebd7e3..a33f801774 100644 --- a/rubocop-performance.gemspec +++ b/rubocop-performance.gemspec @@ -32,6 +32,6 @@ Gem::Specification.new do |s| } s.add_dependency('lint_roller', '~> 1.1') - s.add_dependency('rubocop', '>= 1.72.1', '< 2.0') + s.add_dependency('rubocop', '>= 1.75.0', '< 2.0') s.add_dependency('rubocop-ast', '>= 1.38.0', '< 2.0') end diff --git a/spec/rubocop/cop/performance/chain_array_allocation_spec.rb b/spec/rubocop/cop/performance/chain_array_allocation_spec.rb index 8ed057c9e9..5749371567 100644 --- a/spec/rubocop/cop/performance/chain_array_allocation_spec.rb +++ b/spec/rubocop/cop/performance/chain_array_allocation_spec.rb @@ -80,6 +80,15 @@ end end + describe 'when using `select` with block argument after `select` with `it` block', :ruby34, unsupported_on: :parser do + it 'registers an offense' do + expect_offense(<<~RUBY) + model.select { it.foo }.select { |item| item.bar } + ^^^^^^ Use unchained `select` and `select!` (followed by `return array` if required) instead of chaining `select...select`. + RUBY + end + end + describe 'when using `select` with positional arguments after `select`' do it 'does not register an offense' do expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/performance/redundant_block_call_spec.rb b/spec/rubocop/cop/performance/redundant_block_call_spec.rb index 1a7cd624c4..b1f796605e 100644 --- a/spec/rubocop/cop/performance/redundant_block_call_spec.rb +++ b/spec/rubocop/cop/performance/redundant_block_call_spec.rb @@ -133,6 +133,14 @@ def method(&block) RUBY end + it 'accepts when using `block.call` with `it` block argument', :ruby34, unsupported_on: :parser do + expect_no_offenses(<<~RUBY) + def method(&block) + block.call { it.do_something } + end + RUBY + end + it 'accepts another block being passed along with other args' do expect_no_offenses(<<~RUBY) def method(&block) diff --git a/spec/rubocop/cop/performance/times_map_spec.rb b/spec/rubocop/cop/performance/times_map_spec.rb index a70fd62ecc..33ab80531f 100644 --- a/spec/rubocop/cop/performance/times_map_spec.rb +++ b/spec/rubocop/cop/performance/times_map_spec.rb @@ -138,6 +138,19 @@ RUBY end end + + context 'when using `it` parameter', :ruby34, unsupported_on: :parser do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY, method: method) + 4.times.#{method} { it.to_s } + ^^^^^^^^^{method}^^^^^^^^^^^^ Use `Array.new(4)` with a block instead of `.times.#{method}`. + RUBY + + expect_correction(<<~RUBY) + Array.new(4) { it.to_s } + RUBY + end + end end end diff --git a/spec/rubocop/cop/performance/zip_without_block_spec.rb b/spec/rubocop/cop/performance/zip_without_block_spec.rb index f750b4ff06..1987d3e037 100644 --- a/spec/rubocop/cop/performance/zip_without_block_spec.rb +++ b/spec/rubocop/cop/performance/zip_without_block_spec.rb @@ -163,6 +163,17 @@ [1, 2, 3].zip RUBY end + + it 'registers an offense for collect with a numblock', :ruby34, unsupported_on: :parser do + expect_offense(<<~RUBY) + [1, 2, 3].collect { [it] } + ^^^^^^^^^^^^^^^^ Use `zip` without a block argument instead. + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3].zip + RUBY + end end context 'when using select with an array literal' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cd49bf7ca8..b94697a122 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,6 +15,10 @@ # Prism supports Ruby 3.3+ parsing. config.filter_run_excluding unsupported_on: :prism if ENV['PARSER_ENGINE'] == 'parser_prism' + + # With whitequark/parser, RuboCop supports Ruby syntax compatible with 2.0 to 3.3. + config.filter_run_excluding unsupported_on: :parser if ENV['PARSER_ENGINE'] != 'parser_prism' + config.example_status_persistence_file_path = 'spec/examples.txt' config.disable_monkey_patching! config.warnings = true From 9bbcc5263dad2c35255e625e5891c19bc3fba2a2 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 1 Apr 2025 11:45:15 +0900 Subject: [PATCH 09/10] Update Changelog --- CHANGELOG.md | 13 +++++++++++++ .../change_collection_literal_include_ruby34.md | 1 - ...es_for_performance_string_identifier_argument.md | 1 - changelog/fix_fix_performance_fixed_size_false.md | 1 - .../new_support_itblock_in_performance_cops.md | 1 - 5 files changed, 13 insertions(+), 4 deletions(-) delete mode 100644 changelog/change_collection_literal_include_ruby34.md delete mode 100644 changelog/fix_false_positives_for_performance_string_identifier_argument.md delete mode 100644 changelog/fix_fix_performance_fixed_size_false.md delete mode 100644 changelog/new_support_itblock_in_performance_cops.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d8bbc8dc9..ee781ad038 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,19 @@ ## master (unreleased) +### New features + +* [#496](https://github.com/rubocop/rubocop-performance/pull/496): Support `it` block parameter in `Performance` cops. ([@koic][]) + +### Bug fixes + +* [#494](https://github.com/rubocop/rubocop-performance/pull/494): Fix `Performance/FixedSize` false positive when `count` is called with a `numblock`. ([@dvandersluis][]) +* [#492](https://github.com/rubocop/rubocop-performance/issues/492): Fix false positives for `Performance/StringIdentifierArgument` when using interpolated string argument. ([@koic][]) + +### Changes + +* [#482](https://github.com/rubocop/rubocop-performance/issues/482): Change `Performance/CollectionLiteralInLoop` to not register offenses for `Array#include?` that are optimized directly in Ruby. ([@earlopain][]) + ## 1.24.0 (2025-02-16) ### New features diff --git a/changelog/change_collection_literal_include_ruby34.md b/changelog/change_collection_literal_include_ruby34.md deleted file mode 100644 index 0874c9ca7e..0000000000 --- a/changelog/change_collection_literal_include_ruby34.md +++ /dev/null @@ -1 +0,0 @@ -* [#482](https://github.com/rubocop/rubocop-performance/issues/482): Change `Performance/CollectionLiteralInLoop` to not register offenses for `Array#include?` that are optimized directly in Ruby. ([@earlopain][]) diff --git a/changelog/fix_false_positives_for_performance_string_identifier_argument.md b/changelog/fix_false_positives_for_performance_string_identifier_argument.md deleted file mode 100644 index 834e343ae7..0000000000 --- a/changelog/fix_false_positives_for_performance_string_identifier_argument.md +++ /dev/null @@ -1 +0,0 @@ -* [#492](https://github.com/rubocop/rubocop-performance/issues/492): Fix false positives for `Performance/StringIdentifierArgument` when using interpolated string argument. ([@koic][]) diff --git a/changelog/fix_fix_performance_fixed_size_false.md b/changelog/fix_fix_performance_fixed_size_false.md deleted file mode 100644 index 88cc443de4..0000000000 --- a/changelog/fix_fix_performance_fixed_size_false.md +++ /dev/null @@ -1 +0,0 @@ -* [#494](https://github.com/rubocop/rubocop-performance/pull/494): Fix `Performance/FixedSize` false positive when `count` is called with a `numblock`. ([@dvandersluis][]) diff --git a/changelog/new_support_itblock_in_performance_cops.md b/changelog/new_support_itblock_in_performance_cops.md deleted file mode 100644 index 33b6466d29..0000000000 --- a/changelog/new_support_itblock_in_performance_cops.md +++ /dev/null @@ -1 +0,0 @@ -* [#496](https://github.com/rubocop/rubocop-performance/pull/496): Support `it` block parameter in `Performance` cops. ([@koic][]) From 1a7fa7cadd72550a25be1b79ed6d21088570ce39 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 1 Apr 2025 11:45:27 +0900 Subject: [PATCH 10/10] Cut 1.25.0 --- CHANGELOG.md | 2 ++ docs/antora.yml | 2 +- docs/modules/ROOT/pages/cops_performance.adoc | 13 +++++++++++-- lib/rubocop/performance/version.rb | 2 +- relnotes/v1.25.0.md | 16 ++++++++++++++++ 5 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 relnotes/v1.25.0.md diff --git a/CHANGELOG.md b/CHANGELOG.md index ee781ad038..eff2906c2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ ## master (unreleased) +## 1.25.0 (2025-04-01) + ### New features * [#496](https://github.com/rubocop/rubocop-performance/pull/496): Support `it` block parameter in `Performance` cops. ([@koic][]) diff --git a/docs/antora.yml b/docs/antora.yml index 21b1815a34..def2162865 100644 --- a/docs/antora.yml +++ b/docs/antora.yml @@ -2,6 +2,6 @@ name: rubocop-performance title: RuboCop Performance # We always provide version without patch here (e.g. 1.1), # as patch versions should not appear in the docs. -version: ~ +version: '1.25' nav: - modules/ROOT/nav.adoc diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index bae90e109e..20d5fd0dcc 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -425,6 +425,14 @@ to avoid unnecessary allocations on each iteration. You can set the minimum number of elements to consider an offense with `MinSize`. +NOTE: Since Ruby 3.4, certain simple arguments to `Array#include?` are +optimized directly in Ruby. This avoids allocations without changing the +code, as such no offense will be registered in those cases. Currently that +includes: strings, `self`, local variables, instance variables, and method +calls without arguments. Additionally, any number of methods can be chained: +`[1, 2, 3].include?(@foo)` and `[1, 2, 3].include?(@foo.bar.baz)` both avoid +the array allocation. + [#examples-performancecollectionliteralinloop] === Examples @@ -2281,18 +2289,19 @@ and the following examples are parts of it. send('do_something') attr_accessor 'do_something' instance_variable_get('@ivar') -respond_to?("string_#{interpolation}") # good send(:do_something) attr_accessor :do_something instance_variable_get(:@ivar) -respond_to?(:"string_#{interpolation}") # good - these methods don't support namespaced symbols const_get("#{module_path}::Base") const_source_location("#{module_path}::Base") const_defined?("#{module_path}::Base") + +# good - using a symbol when string interpolation is involved causes a performance regression. +respond_to?("string_#{interpolation}") ---- [#performancestringinclude] diff --git a/lib/rubocop/performance/version.rb b/lib/rubocop/performance/version.rb index 24695abafb..64bc31ac63 100644 --- a/lib/rubocop/performance/version.rb +++ b/lib/rubocop/performance/version.rb @@ -4,7 +4,7 @@ module RuboCop module Performance # This module holds the RuboCop Performance version information. module Version - STRING = '1.24.0' + STRING = '1.25.0' def self.document_version STRING.match('\d+\.\d+').to_s diff --git a/relnotes/v1.25.0.md b/relnotes/v1.25.0.md new file mode 100644 index 0000000000..0e26cb278e --- /dev/null +++ b/relnotes/v1.25.0.md @@ -0,0 +1,16 @@ +### New features + +* [#496](https://github.com/rubocop/rubocop-performance/pull/496): Support `it` block parameter in `Performance` cops. ([@koic][]) + +### Bug fixes + +* [#494](https://github.com/rubocop/rubocop-performance/pull/494): Fix `Performance/FixedSize` false positive when `count` is called with a `numblock`. ([@dvandersluis][]) +* [#492](https://github.com/rubocop/rubocop-performance/issues/492): Fix false positives for `Performance/StringIdentifierArgument` when using interpolated string argument. ([@koic][]) + +### Changes + +* [#482](https://github.com/rubocop/rubocop-performance/issues/482): Change `Performance/CollectionLiteralInLoop` to not register offenses for `Array#include?` that are optimized directly in Ruby. ([@earlopain][]) + +[@koic]: https://github.com/koic +[@dvandersluis]: https://github.com/dvandersluis +[@earlopain]: https://github.com/earlopain