From 0203a7699f14747ac21db77b9ffba949c681c66c Mon Sep 17 00:00:00 2001 From: fatkodima Date: Fri, 2 Oct 2020 19:18:20 +0300 Subject: [PATCH] Add new `Lint/RedundantSafeNavigation` cop --- CHANGELOG.md | 1 + config/default.yml | 15 ++++++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_lint.adoc | 39 ++++++++++++++ lib/rubocop.rb | 1 + .../empty_lines_around_attribute_accessor.rb | 2 +- .../cop/lint/redundant_safe_navigation.rb | 45 ++++++++++++++++ lib/rubocop/rspec/cop_helper.rb | 2 +- .../lint/redundant_safe_navigation_spec.rb | 54 +++++++++++++++++++ 9 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 lib/rubocop/cop/lint/redundant_safe_navigation.rb create mode 100644 spec/rubocop/cop/lint/redundant_safe_navigation_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 897aada697d..f4095c554c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### New features * [#8796](https://github.com/rubocop-hq/rubocop/pull/8796): Add new `Lint/HashCompareByIdentity` cop. ([@fatkodima][]) +* [#8668](https://github.com/rubocop-hq/rubocop/pull/8668): Add new `Lint/RedundantSafeNavigation` cop. ([@fatkodima][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index d5eccb35cf4..019e6f2af92 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1753,6 +1753,21 @@ Lint/RedundantRequireStatement: Enabled: true VersionAdded: '0.76' +Lint/RedundantSafeNavigation: + Description: 'Checks for redundant safe navigation calls.' + Enabled: pending + VersionAdded: '0.93' + Safe: false + IgnoredMethods: + - to_c + - to_f + - to_i + - to_r + - rationalize + - public_send + - send + - __send__ + Lint/RedundantSplatExpansion: Description: 'Checks for splat unnecessarily being called on literals.' Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 77f491efdc9..241cdc9d16e 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -246,6 +246,7 @@ In the following section you find all available cops: * xref:cops_lint.adoc#lintredundantcopdisabledirective[Lint/RedundantCopDisableDirective] * xref:cops_lint.adoc#lintredundantcopenabledirective[Lint/RedundantCopEnableDirective] * xref:cops_lint.adoc#lintredundantrequirestatement[Lint/RedundantRequireStatement] +* xref:cops_lint.adoc#lintredundantsafenavigation[Lint/RedundantSafeNavigation] * xref:cops_lint.adoc#lintredundantsplatexpansion[Lint/RedundantSplatExpansion] * xref:cops_lint.adoc#lintredundantstringcoercion[Lint/RedundantStringCoercion] * xref:cops_lint.adoc#lintredundantwithindex[Lint/RedundantWithIndex] diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index 616201840a6..4d90bd50937 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2903,6 +2903,45 @@ require 'thread' require 'unloaded_feature' ---- +== Lint/RedundantSafeNavigation + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| No +| Yes (Unsafe) +| 0.93 +| - +|=== + +This cop checks for redundant safe navigation calls. +It is marked as unsafe, because it can produce code that returns +non `nil` while `nil` result is expected on `nil` receiver. + +=== Examples + +[source,ruby] +---- +# bad +attrs&.respond_to?(:[]) +foo&.dup&.inspect + +# good +attrs.respond_to?(:[]) +foo.dup.inspect +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| IgnoredMethods +| `to_c`, `to_f`, `to_i`, `to_r`, `rationalize`, `public_send`, `send`, `__send__` +| Array +|=== + == Lint/RedundantSplatExpansion |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 5dc90731237..cd91abac695 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -308,6 +308,7 @@ require_relative 'rubocop/cop/lint/redundant_cop_disable_directive' require_relative 'rubocop/cop/lint/redundant_cop_enable_directive' require_relative 'rubocop/cop/lint/redundant_require_statement' +require_relative 'rubocop/cop/lint/redundant_safe_navigation' require_relative 'rubocop/cop/lint/redundant_splat_expansion' require_relative 'rubocop/cop/lint/redundant_string_coercion' require_relative 'rubocop/cop/lint/redundant_with_index' diff --git a/lib/rubocop/cop/layout/empty_lines_around_attribute_accessor.rb b/lib/rubocop/cop/layout/empty_lines_around_attribute_accessor.rb index 10fd01dbde6..25930b28363 100644 --- a/lib/rubocop/cop/layout/empty_lines_around_attribute_accessor.rb +++ b/lib/rubocop/cop/layout/empty_lines_around_attribute_accessor.rb @@ -88,7 +88,7 @@ def next_line_empty?(line) end def require_empty_line?(node) - return false unless node&.respond_to?(:type) + return false unless node.respond_to?(:type) !allow_alias?(node) && !attribute_or_allowed_method?(node) end diff --git a/lib/rubocop/cop/lint/redundant_safe_navigation.rb b/lib/rubocop/cop/lint/redundant_safe_navigation.rb new file mode 100644 index 00000000000..c0d79962f95 --- /dev/null +++ b/lib/rubocop/cop/lint/redundant_safe_navigation.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop checks for redundant safe navigation calls. + # It is marked as unsafe, because it can produce code that returns + # non `nil` while `nil` result is expected on `nil` receiver. + # + # @example + # # bad + # attrs&.respond_to?(:[]) + # foo&.dup&.inspect + # + # # good + # attrs.respond_to?(:[]) + # foo.dup.inspect + # + class RedundantSafeNavigation < Base + include IgnoredMethods + include RangeHelp + extend AutoCorrector + + MSG = 'Redundant safe navigation detected.' + + NIL_METHODS = nil.methods.to_set.freeze + + def on_csend(node) + return unless check_method?(node.method_name) + + range = range_between(node.loc.dot.begin_pos, node.source_range.end_pos) + add_offense(range) do |corrector| + corrector.replace(node.loc.dot, '.') + end + end + + private + + def check_method?(method_name) + NIL_METHODS.include?(method_name) && !ignored_method?(method_name) + end + end + end + end +end diff --git a/lib/rubocop/rspec/cop_helper.rb b/lib/rubocop/rspec/cop_helper.rb index 133a3ae9bec..bcf8bb491a4 100644 --- a/lib/rubocop/rspec/cop_helper.rb +++ b/lib/rubocop/rspec/cop_helper.rb @@ -26,7 +26,7 @@ def inspect_source(source, file = nil) end def parse_source(source, file = nil) - if file&.respond_to?(:write) + if file.respond_to?(:write) file.write(source) file.rewind file = file.path diff --git a/spec/rubocop/cop/lint/redundant_safe_navigation_spec.rb b/spec/rubocop/cop/lint/redundant_safe_navigation_spec.rb new file mode 100644 index 00000000000..8f4be21ad99 --- /dev/null +++ b/spec/rubocop/cop/lint/redundant_safe_navigation_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::RedundantSafeNavigation, :config do + let(:cop_config) do + { 'IgnoredMethods' => [] } + end + + it 'registers an offense and corrects when `.&` is used for `nil` method' do + expect_offense(<<~RUBY) + foo&.respond_to?(:bar) + ^^^^^^^^^^^^^^^^^^^ Redundant safe navigation detected. + RUBY + + expect_correction(<<~RUBY) + foo.respond_to?(:bar) + RUBY + end + + it 'registers an offense and corrects when multiple `.&` are used for `nil` methods' do + expect_offense(<<~RUBY) + foo.do_something&.dup&.inspect + ^^^^^ Redundant safe navigation detected. + ^^^^^^^^^ Redundant safe navigation detected. + RUBY + + expect_correction(<<~RUBY) + foo.do_something.dup.inspect + RUBY + end + + it 'does not register an offense when using non-nil method with `.&`' do + expect_no_offenses(<<~RUBY) + foo.&do_something + RUBY + end + + it 'does not register an offense when using `nil` method without `.&`' do + expect_no_offenses(<<~RUBY) + foo.dup + RUBY + end + + context 'when AllowedMethods is set' do + let(:cop_config) do + { 'IgnoredMethods' => ['to_f'] } + end + + it 'does not register an offense when using ignored `nil` method with `.&`' do + expect_no_offenses(<<~RUBY) + foo&.to_f + RUBY + end + end +end