[Fix rubocop#4508] Added return nil cop

CHANGELOG.md

@@ -22,6 +22,7 @@
 * [#3965](https://github.com/bbatsov/rubocop/issues/3965): Add new `Style/DoublePipeEquals` cop. ([@donjar][])
 * Make `rake new_cop` create parent directories if they do not already exist. ([@highb][])
 * [#4368](https://github.com/bbatsov/rubocop/issues/4368): Make `Performance/HashEachMethod` inspect send nodes with any receiver. ([@gohdaniel15][])
+* [#4508](https://github.com/bbatsov/rubocop/issues/4508): Add new `Style/ReturnNil` cop. ([@donjar][])
 
 ### Bug fixes
 

config/default.yml

@@ -1154,6 +1154,12 @@ Style/RegexpLiteral:
   # are found in the regexp string.
   AllowInnerSlashes: false
 
+Style/ReturnNil:
+  EnforcedStyle: return
+  SupportedStyles:
+    - return
+    - return_nil
+
 Style/SafeNavigation:
   # Safe navigation may cause a statement to start returning `nil` in addition
   # to whatever it used to return.

config/disabled.yml

@@ -100,6 +100,10 @@ Style/OptionHash:
   Description: "Don't use option hashes when you can use keyword arguments."
   Enabled: false
 
+Style/ReturnNil:
+  Description: 'Use return instead of return nil.'
+  Enabled: false
+
 Style/Send:
   Description: 'Prefer `Object#__send__` or `Object#public_send` to `send`, as `send` may overlap with existing methods.'
   StyleGuide: '#prefer-public-send'

lib/rubocop.rb

@@ -431,6 +431,7 @@
 require 'rubocop/cop/style/redundant_conditional'
 require 'rubocop/cop/style/regexp_literal'
 require 'rubocop/cop/style/rescue_modifier'
+require 'rubocop/cop/style/return_nil'
 require 'rubocop/cop/style/safe_navigation'
 require 'rubocop/cop/style/self_assignment'
 require 'rubocop/cop/style/semicolon'

lib/rubocop/cop/style/return_nil.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop enforces consistency between 'return nil' and 'return'.
+      #
+      # Supported styles are: return, return_nil.
+      #
+      # @example
+      #
+      #   # EnforcedStyle: return (default)
+      #
+      #   # bad
+      #   def foo(arg)
+      #     return nil if arg
+      #   end
+      #
+      #   # good
+      #   def foo(arg)
+      #     return if arg
+      #   end
+      #
+      #   # EnforcedStyle: return_nil
+      #
+      #   # bad
+      #   def foo(arg)
+      #     return if arg
+      #   end
+      #
+      #   # good
+      #   def foo(arg)
+      #     return nil if arg
+      #   end
+      class ReturnNil < Cop
+        include ConfigurableEnforcedStyle
+
+        RETURN_MSG = 'Use `return` instead of `return nil`.'.freeze
+        RETURN_NIL_MSG = 'Use `return nil` instead of `return`.'.freeze
+
+        def_node_matcher :return_node?, '(return)'
+        # TODO: fix (return nil) on the NodePattern class
+        # def_node_matcher :return_nil_node?, '(return nil)'
+
+        def on_return(node)
+          # Check Lint/NonLocalExitFromIterator first before this cop
+          node.each_ancestor(:block, :def, :defs) do |n|
+            break if scoped_node?(n)
+
+            send_node, args_node, _body_node = *n
+
+            # if a proc is passed to `Module#define_method` or
+            # `Object#define_singleton_method`, `return` will not cause a
+            # non-local exit error
+            break if define_method?(send_node)
+
+            next if args_node.children.empty?
+
+            return nil if chained_send?(send_node)
+          end
+
+          add_offense(node) unless correct_style?(node)
+        end
+
+        private
+
+        def autocorrect(node)
+          lambda do |corrector|
+            corrected = style == :return ? 'return' : 'return nil'
+            corrector.replace(node.source_range, corrected)
+          end
+        end
+
+        def message(_node)
+          style == :return ? RETURN_MSG : RETURN_NIL_MSG
+        end
+
+        def correct_style?(node)
+          style == :return && !return_nil_node?(node) ||
+            style == :return_nil && !return_node?(node)
+        end
+
+        def return_nil_node?(node)
+          !node.children.empty? && node.children.first.nil_type?
+        end
+
+        def scoped_node?(node)
+          node.def_type? || node.defs_type? || node.lambda?
+        end
+
+        def_node_matcher :chained_send?, '(send !nil ...)'
+        def_node_matcher :define_method?, <<-PATTERN
+          (send _ {:define_method :define_singleton_method} _)
+        PATTERN
+      end
+    end
+  end
+end

manual/cops.md

@@ -439,6 +439,7 @@ In the following section you find all available cops:
 * [Style/RedundantSelf](cops_style.md#styleredundantself)
 * [Style/RegexpLiteral](cops_style.md#styleregexpliteral)
 * [Style/RescueModifier](cops_style.md#stylerescuemodifier)
+* [Style/ReturnNil](cops_style.md#stylereturnnil)
 * [Style/SafeNavigation](cops_style.md#stylesafenavigation)
 * [Style/SelfAssignment](cops_style.md#styleselfassignment)
 * [Style/Semicolon](cops_style.md#stylesemicolon)

manual/cops_style.md

@@ -3240,6 +3240,51 @@ This cop checks for uses of rescue in its modifier form.
 
 * [https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers](https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers)
 
+## Style/ReturnNil
+
+Enabled by default | Supports autocorrection
+--- | ---
+Disabled | Yes
+
+This cop enforces consistency between 'return nil' and 'return'.
+
+Supported styles are: return, return_nil.
+
+### Example
+
+```ruby
+# EnforcedStyle: return (default)
+
+# bad
+def foo(arg)
+  return nil if arg
+end
+
+# good
+def foo(arg)
+  return if arg
+end
+
+# EnforcedStyle: return_nil
+
+# bad
+def foo(arg)
+  return if arg
+end
+
+# good
+def foo(arg)
+  return nil if arg
+end
+```
+
+### Important attributes
+
+Attribute | Value
+--- | ---
+EnforcedStyle | return
+SupportedStyles | return, return_nil
+
 ## Style/SafeNavigation
 
 Enabled by default | Supports autocorrection

spec/rubocop/cop/style/return_nil_spec.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+describe RuboCop::Cop::Style::ReturnNil do
+  subject(:cop) { described_class.new(config) }
+
+  context 'when enforced style is `return`' do
+    let(:config) do
+      RuboCop::Config.new(
+        'Style/ReturnNil' => {
+          'EnforcedStyle' => 'return',
+          'SupportedStyles' => %w[return return_nil]
+        }
+      )
+    end
+
+    it 'registers an offense for return nil' do
+      expect_offense(<<-RUBY.strip_indent)
+        return nil
+        ^^^^^^^^^^ Use `return` instead of `return nil`.
+      RUBY
+    end
+
+    it 'auto-corrects `return nil` into `return`' do
+      expect(autocorrect_source('return nil')).to eq 'return'
+    end
+
+    it 'does not register an offense for return' do
+      expect_no_offenses('return')
+    end
+
+    it 'does not register an offense for returning others' do
+      expect_no_offenses('return 2')
+    end
+
+    it 'does not register an offense for return nil from iterators' do
+      expect_no_offenses(<<-RUBY)
+        loop do
+          return if x
+        end
+      RUBY
+    end
+  end
+
+  context 'when enforced style is `return_nil`' do
+    let(:config) do
+      RuboCop::Config.new(
+        'Style/ReturnNil' => {
+          'EnforcedStyle' => 'return_nil',
+          'SupportedStyles' => %w[return return_nil]
+        }
+      )
+    end
+
+    it 'registers an offense for return' do
+      expect_offense(<<-RUBY.strip_indent)
+        return
+        ^^^^^^ Use `return nil` instead of `return`.
+      RUBY
+    end
+
+    it 'auto-corrects `return` into `return nil`' do
+      expect(autocorrect_source('return')).to eq 'return nil'
+    end
+
+    it 'does not register an offense for return nil' do
+      expect_no_offenses('return nil')
+    end
+
+    it 'does not register an offense for returning others' do
+      expect_no_offenses('return 2')
+    end
+  end
+end