Skip to content

Commit

Permalink
[Fix rubocop#4153] Add a new cop Lint/ReturnInVoidContext
Browse files Browse the repository at this point in the history
  • Loading branch information
Harold Simpson authored and pocke committed Jun 5, 2017
1 parent 8f37b7f commit d153550
Show file tree
Hide file tree
Showing 7 changed files with 201 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

* [#4448](https://github.com/bbatsov/rubocop/pull/4448): Add new `TapFormatter`. ([@cyberdelia][])
* Add new `Style/HeredocDelimiters` cop. ([@drenmi][])
* [#4153](https://github.com/bbatsov/rubocop/issues/4153): New cop `Lint/ReturnInVoidContext` checks for the use of a return with a value in a context where it will be ignored. ([@harold-s][])

### Bug fixes

Expand Down Expand Up @@ -2827,3 +2828,4 @@
[@yhirano55]: https://github.com/yhirano55
[@hoshinotsuyoshi]: https://github.com/hoshinotsuyoshi
[@timrogers]: https://github.com/timrogers
[@harold-s]: https://github.com/harold-s
4 changes: 4 additions & 0 deletions config/enabled.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,10 @@ Lint/UselessElseWithoutRescue:
Description: 'Checks for useless `else` in `begin..end` without `rescue`.'
Enabled: true

Lint/ReturnInVoidContext:
Description: 'Checks for return in void context'
Enabled: true

Lint/UselessSetterCall:
Description: 'Checks for useless setter call to a local variable.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@
require 'rubocop/cop/lint/require_parentheses'
require 'rubocop/cop/lint/rescue_exception'
require 'rubocop/cop/lint/rescue_type'
require 'rubocop/cop/lint/return_in_void_context'
require 'rubocop/cop/lint/safe_navigation_chain'
require 'rubocop/cop/lint/script_permission'
require 'rubocop/cop/lint/shadowed_exception'
Expand Down
62 changes: 62 additions & 0 deletions lib/rubocop/cop/lint/return_in_void_context.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks for the use of a return with a value in a context
# where it the value will be ignored. (initialize and setter methods)
#
# @example
#
# # bad
# def initialize
# foo
# return :qux if bar?
# baz
# end
#
# def foo=(bar)
# return 42
# end
#
# # good
# def initialize
# foo
# return if bar?
# baz
# end
#
# def foo=(bar)
# return
# end
#
class ReturnInVoidContext < Cop
MSG = 'Do not return a value in `%s`.'.freeze
def on_return(return_node)
method_name = method_name(return_node)
return unless method_name && return_node.descendants.any? &&
useless_return_method?(method_name)

add_offense(return_node, :keyword, format(message, method_name))
end

private

def method_name(return_node)
method_node = return_node.each_ancestor(:block, :def, :defs).first
return nil unless method_node.type == :def
method_node.children.first
end

def method_setter?(method_name)
method_name.to_s.end_with?('=') &&
!%i[!= == === >= <=].include?(method_name)
end

def useless_return_method?(method_name)
method_name == :initialize || method_setter?(method_name)
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ In the following section you find all available cops:
* [Lint/RequireParentheses](cops_lint.md#lintrequireparentheses)
* [Lint/RescueException](cops_lint.md#lintrescueexception)
* [Lint/RescueType](cops_lint.md#lintrescuetype)
* [Lint/ReturnInVoidContext](cops_lint.md#lintreturninvoidcontext)
* [Lint/SafeNavigationChain](cops_lint.md#lintsafenavigationchain)
* [Lint/ScriptPermission](cops_lint.md#lintscriptpermission)
* [Lint/ShadowedException](cops_lint.md#lintshadowedexception)
Expand Down
35 changes: 35 additions & 0 deletions manual/cops_lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,41 @@ rescue NameError
end
```

## Lint/ReturnInVoidContext

Enabled by default | Supports autocorrection
--- | ---
Enabled | No

This cop checks for the use of a return with a value in a context
where it the value will be ignored. (initialize and setter methods)

# good
def initialize
foo
return if bar?
baz
end

def foo=(bar)
return
end

### Example

```ruby
# bad
def initialize
foo
return :qux if bar?
baz
end

def foo=(bar)
return 42
end
```

## Lint/SafeNavigationChain

Enabled by default | Supports autocorrection
Expand Down
96 changes: 96 additions & 0 deletions spec/rubocop/cop/lint/return_in_void_context_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# frozen_string_literal: true

describe RuboCop::Cop::Lint::ReturnInVoidContext do
subject(:cop) { described_class.new }

shared_examples 'registers an offense' do |message|
it 'registers an offense' do
inspect_source(cop, source)

expect(cop.offenses.size).to eq(1)
expect(cop.messages).to eq([message])
end
end

shared_examples 'does not registers an offense' do
it 'does not registers an offense' do
inspect_source(cop, source)
expect(cop.offenses.size).to eq(0)
end
end

context 'with an initialize method containing a return with a value' do
let(:source) do
['class A',
' def initialize',
' return :qux if bar?',
' end',
'end']
end
it_behaves_like 'registers an offense',
'Do not return a value in `initialize`.'
end

context 'with an initialize method containing a return without a value' do
let(:source) do
['class A',
' def initialize',
' return if bar?',
' end',
'end']
end

it_behaves_like 'does not registers an offense'
end

context 'with a setter method containing a return with a value' do
let(:source) do
['class A',
' def foo=(bar)',
' return 42',
' end',
'end']
end

it_behaves_like 'registers an offense',
'Do not return a value in `foo=`.'
end

context 'with a setter method containing a return without a value' do
let(:source) do
['class A',
' def foo=(bar)',
' return',
' end',
'end']
end

it_behaves_like 'does not registers an offense'
end

context 'with a non initialize method containing a return' do
let(:source) do
['class A',
' def bar',
' foo',
' return :qux if bar?',
' foo',
' end',
'end']
end
it_behaves_like 'does not registers an offense'
end

context 'with a class method called initialize containing a return' do
let(:source) do
['class A',
' def self.initialize',
' foo',
' return :qux if bar?',
' foo',
' end',
'end']
end
it_behaves_like 'does not registers an offense'
end
end

0 comments on commit d153550

Please sign in to comment.