From afa86c3cc54775ae6490d2fbe8b1a27d945d4bf5 Mon Sep 17 00:00:00 2001 From: Richard Stewart Date: Fri, 25 Sep 2020 11:03:21 +0300 Subject: [PATCH] Add new `Performance/BlockGivenWithExplicitBlock` cop --- CHANGELOG.md | 4 + config/default.yml | 5 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_performance.adoc | 37 ++++++++++ .../block_given_with_explicit_block.rb | 52 +++++++++++++ lib/rubocop/cop/performance_cops.rb | 1 + .../block_given_with_explicit_block_spec.rb | 73 +++++++++++++++++++ 7 files changed, 173 insertions(+) create mode 100644 lib/rubocop/cop/performance/block_given_with_explicit_block.rb create mode 100644 spec/rubocop/cop/performance/block_given_with_explicit_block_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 188ab69..ea6dc4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#173](https://github.com/rubocop-hq/rubocop-performance/pull/173): Add new `Performance/BlockGivenWithExplicitBlock` cop. ([@fatkodima][]) + ### Changes * [#181](https://github.com/rubocop-hq/rubocop-performance/pull/181): Change default configuration for `Performance/CollectionLiteralInLoop` to `Enabled: 'pending'`. ([@ghiculescu][]) diff --git a/config/default.yml b/config/default.yml index 7e1b4af..db14ebe 100644 --- a/config/default.yml +++ b/config/default.yml @@ -17,6 +17,11 @@ Performance/BindCall: Enabled: true VersionAdded: '1.6' +Performance/BlockGivenWithExplicitBlock: + Description: 'Check block argument explicitly instead of using `block_given?`.' + Enabled: pending + VersionAdded: '1.9' + Performance/Caller: Description: >- Use `caller(n..n)` instead of `caller`. diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 451a45d..32046a5 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -15,6 +15,7 @@ Performance cops optimization analysis for your projects. * xref:cops_performance.adoc#performanceancestorsinclude[Performance/AncestorsInclude] * xref:cops_performance.adoc#performancebigdecimalwithnumericargument[Performance/BigDecimalWithNumericArgument] * xref:cops_performance.adoc#performancebindcall[Performance/BindCall] +* xref:cops_performance.adoc#performanceblockgivenwithexplicitblock[Performance/BlockGivenWithExplicitBlock] * xref:cops_performance.adoc#performancecaller[Performance/Caller] * xref:cops_performance.adoc#performancecasewhensplat[Performance/CaseWhenSplat] * xref:cops_performance.adoc#performancecasecmp[Performance/Casecmp] diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index 02d8ea7..88a654b 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -93,6 +93,43 @@ umethod.bind(obj).(foo, bar) umethod.bind_call(obj, foo, bar) ---- +== Performance/BlockGivenWithExplicitBlock + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 1.9 +| - +|=== + +This cop identifies unnecessary use of a `block_given?` where explicit check +of block argument would suffice. + +=== Examples + +[source,ruby] +---- +# bad +def method(&block) + do_something if block_given? +end + +# good +def method(&block) + do_something if block +end + +# good - block is reassigned +def method(&block) + block ||= -> { do_something } + warn "Using default ..." unless block_given? + # ... +end +---- + == Performance/Caller |=== diff --git a/lib/rubocop/cop/performance/block_given_with_explicit_block.rb b/lib/rubocop/cop/performance/block_given_with_explicit_block.rb new file mode 100644 index 0000000..a25ac19 --- /dev/null +++ b/lib/rubocop/cop/performance/block_given_with_explicit_block.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # This cop identifies unnecessary use of a `block_given?` where explicit check + # of block argument would suffice. + # + # @example + # # bad + # def method(&block) + # do_something if block_given? + # end + # + # # good + # def method(&block) + # do_something if block + # end + # + # # good - block is reassigned + # def method(&block) + # block ||= -> { do_something } + # warn "Using default ..." unless block_given? + # # ... + # end + # + class BlockGivenWithExplicitBlock < Base + extend AutoCorrector + + RESTRICT_ON_SEND = %i[block_given?].freeze + MSG = 'Check block argument explicitly instead of using `block_given?`.' + + def_node_matcher :reassigns_block_arg?, '`(lvasgn %1 ...)' + + def on_send(node) + def_node = node.each_ancestor(:def, :defs).first + return unless def_node + + block_arg = def_node.arguments.find(&:blockarg_type?) + return unless block_arg + + block_arg_name = block_arg.loc.name.source.to_sym + return if reassigns_block_arg?(def_node, block_arg_name) + + add_offense(node) do |corrector| + corrector.replace(node, block_arg_name) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index b898112..e1e55c7 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -6,6 +6,7 @@ require_relative 'performance/ancestors_include' require_relative 'performance/big_decimal_with_numeric_argument' require_relative 'performance/bind_call' +require_relative 'performance/block_given_with_explicit_block' require_relative 'performance/caller' require_relative 'performance/case_when_splat' require_relative 'performance/casecmp' diff --git a/spec/rubocop/cop/performance/block_given_with_explicit_block_spec.rb b/spec/rubocop/cop/performance/block_given_with_explicit_block_spec.rb new file mode 100644 index 0000000..ccc8ed3 --- /dev/null +++ b/spec/rubocop/cop/performance/block_given_with_explicit_block_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::BlockGivenWithExplicitBlock do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when using `block_given?` in an instance method with block arg' do + expect_offense(<<~RUBY) + def method(x, &block) + do_something if block_given? + ^^^^^^^^^^^^ Check block argument explicitly instead of using `block_given?`. + end + RUBY + + expect_correction(<<~RUBY) + def method(x, &block) + do_something if block + end + RUBY + end + + it 'registers an offense and corrects when using `block_given?` in a class method with block arg' do + expect_offense(<<~RUBY) + def self.method(x, &block) + do_something if block_given? + ^^^^^^^^^^^^ Check block argument explicitly instead of using `block_given?`. + end + RUBY + + expect_correction(<<~RUBY) + def self.method(x, &block) + do_something if block + end + RUBY + end + + it 'registers an offense and corrects when using `block_given?` in a method with non standard block arg name' do + expect_offense(<<~RUBY) + def method(x, &myblock) + do_something if block_given? + ^^^^^^^^^^^^ Check block argument explicitly instead of using `block_given?`. + end + RUBY + + expect_correction(<<~RUBY) + def method(x, &myblock) + do_something if myblock + end + RUBY + end + + it 'does not register an offense when using `block_given?` in a method without block arg' do + expect_no_offenses(<<~RUBY) + def method(x) + do_something if block_given? + end + RUBY + end + + it 'does not register an offense when block arg is reassigned' do + expect_no_offenses(<<~RUBY) + def method(a, &block) + block ||= -> {} + do_something if block_given? + end + RUBY + end + + it 'does not register an offense when `block_given?` is used outside of a method' do + expect_no_offenses(<<~RUBY) + do_something if block_given? + RUBY + end +end