From eda9b80de223361b41c8d9b4a6498231e7e05373 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sat, 21 Jan 2023 15:31:50 +0200 Subject: [PATCH] Add new `Performance/Defined` cop --- changelog/new_defined_cop.md | 1 + config/default.yml | 7 ++ lib/rubocop/cop/performance/defined.rb | 80 +++++++++++++++++++ lib/rubocop/cop/performance_cops.rb | 1 + spec/rubocop/cop/performance/defined_spec.rb | 82 ++++++++++++++++++++ 5 files changed, 171 insertions(+) create mode 100644 changelog/new_defined_cop.md create mode 100644 lib/rubocop/cop/performance/defined.rb create mode 100644 spec/rubocop/cop/performance/defined_spec.rb diff --git a/changelog/new_defined_cop.md b/changelog/new_defined_cop.md new file mode 100644 index 0000000000..442996d7ee --- /dev/null +++ b/changelog/new_defined_cop.md @@ -0,0 +1 @@ +* [#325](https://github.com/rubocop/rubocop-performance/issues/325): Add new `Performance/Defined` cop. ([@fatkodima][]) diff --git a/config/default.yml b/config/default.yml index ee6aaad3d8..4eefc0e727 100644 --- a/config/default.yml +++ b/config/default.yml @@ -102,6 +102,13 @@ Performance/Count: VersionAdded: '0.31' VersionChanged: '1.8' +Performance/Defined: + Description: >- + Identifies usages of `class_variable_defined?`, `const_defined?`, + `instance_variable_defined?` that can be replaced with `defined?`. + Enabled: pending + VersionAdded: '<>' + Performance/DeletePrefix: Description: 'Use `delete_prefix` instead of `gsub`.' Enabled: true diff --git a/lib/rubocop/cop/performance/defined.rb b/lib/rubocop/cop/performance/defined.rb new file mode 100644 index 0000000000..6f3a77afa9 --- /dev/null +++ b/lib/rubocop/cop/performance/defined.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Performance + # Identifies usages of `class_variable_defined?`, `const_defined?`, + # `instance_variable_defined?` that can be replaced with `defined?`. + # + # @example + # # bad + # return if class_variable_defined?(:@@foo) + # return if const_defined?(:Foo) + # return if const_defined?(:Foo) + # return if instance_variable_defined?(:@foo) + # + # # good + # return if defined?(@@foo) + # return if defined?(Foo) + # return if defined?(@foo) + # + # # good (explicit receiver) + # obj.instance_variable_defined?(:@foo) + # + # # good (non basic literal expression) + # obj.instance_variable_defined?(ivar_name) + # + # # good (`const_defined?` disallowing inheritance) + # const_defined?(:Foo, false) + # const_defined?(:Foo, inherit) + # + # # bad (as a non condition) + # instance_variable_defined?(:@foo) + # + # # good + # !defined(@foo).nil? + # + class Defined < Base + extend AutoCorrector + + MSG = 'Use `defined?` instead of `%s`.' + + RESTRICT_ON_SEND = %i[class_variable_defined? const_defined? instance_variable_defined?].freeze + + def_node_matcher :defined_candidate?, <<~PATTERN + (send nil? ${:#{RESTRICT_ON_SEND.join(' :')}} $#basic_literal? (true) ?) + PATTERN + + def on_send(node) + defined_candidate?(node) do |method_name, expression_node| + message = format(MSG, bad_method: method_name) + add_offense(node.loc.selector, message: message) do |corrector| + replacement = "defined?(#{expression_node.value})" + replacement = "!#{replacement}.nil?" unless condition?(node) + corrector.replace(node, replacement) + end + end + end + + private + + def basic_literal?(node) + node&.basic_literal? + end + + def condition?(node) + return false unless (parent = non_begin_parent(node)) + + (parent.basic_conditional? && parent.condition == node) || + (parent.or_type? || parent.and_type?) + end + + def non_begin_parent(node) + parent = node.parent + parent = parent.parent while parent&.begin_type? + parent + end + end + end + end +end diff --git a/lib/rubocop/cop/performance_cops.rb b/lib/rubocop/cop/performance_cops.rb index 02a64a6300..22a7f29ce6 100644 --- a/lib/rubocop/cop/performance_cops.rb +++ b/lib/rubocop/cop/performance_cops.rb @@ -16,6 +16,7 @@ require_relative 'performance/concurrent_monotonic_time' require_relative 'performance/constant_regexp' require_relative 'performance/count' +require_relative 'performance/defined' require_relative 'performance/delete_prefix' require_relative 'performance/delete_suffix' require_relative 'performance/detect' diff --git a/spec/rubocop/cop/performance/defined_spec.rb b/spec/rubocop/cop/performance/defined_spec.rb new file mode 100644 index 0000000000..cd224478ab --- /dev/null +++ b/spec/rubocop/cop/performance/defined_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Performance::Defined, :config do + it 'registers an offense when using `#class_variable_defined?`' do + expect_offense(<<~RUBY) + return if class_variable_defined?(:@@foo) + ^^^^^^^^^^^^^^^^^^^^^^^ Use `defined?` instead of `class_variable_defined?`. + RUBY + + expect_correction(<<~RUBY) + return if defined?(@@foo) + RUBY + end + + it 'registers an offense when using `#const_defined?`' do + expect_offense(<<~RUBY) + return if const_defined?(:FOO) + ^^^^^^^^^^^^^^ Use `defined?` instead of `const_defined?`. + RUBY + + expect_correction(<<~RUBY) + return if defined?(FOO) + RUBY + end + + it 'registers an offense when using `#instance_variable_defined?`' do + expect_offense(<<~RUBY) + return if instance_variable_defined?(:@foo) + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `defined?` instead of `instance_variable_defined?`. + RUBY + + expect_correction(<<~RUBY) + return if defined?(@foo) + RUBY + end + + it 'does not register an offense when using `#defined?`' do + expect_no_offenses(<<~RUBY) + defined?(@foo) + RUBY + end + + it 'does not register an offense when using `#_defined?` on explicit receiver' do + expect_no_offenses(<<~RUBY) + x.instance_variable_defined?(:@foo) + RUBY + end + + it 'does not register an offense when using `#_defined?` with non basic literal expression' do + expect_no_offenses(<<~RUBY) + instance_variable_defined?(foo_ivar) + RUBY + end + + it 'does not register an offense when using `#const_defined?` with non `true` `inherit` option' do + expect_no_offenses(<<~RUBY) + const_defined?(:FOO, false) + RUBY + end + + it 'registers an offense when using `#const_defined?` with `true` `inherit` option' do + expect_offense(<<~RUBY) + return if const_defined?(:FOO, true) + ^^^^^^^^^^^^^^ Use `defined?` instead of `const_defined?`. + RUBY + + expect_correction(<<~RUBY) + return if defined?(FOO) + RUBY + end + + it 'registers an offense when using `#_defined?` as a non condition' do + expect_offense(<<~RUBY) + const_defined?(:FOO) + ^^^^^^^^^^^^^^ Use `defined?` instead of `const_defined?`. + RUBY + + expect_correction(<<~RUBY) + !defined?(FOO).nil? + RUBY + end +end