Skip to content

Commit

Permalink
Add new Lint/RedundantSafeNavigation cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Oct 4, 2020
1 parent 269ac4c commit 0203a76
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions config/default.yml
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -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]
Expand Down
39 changes: 39 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -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

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'
Expand Down
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions 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
2 changes: 1 addition & 1 deletion lib/rubocop/rspec/cop_helper.rb
Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions 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

0 comments on commit 0203a76

Please sign in to comment.