Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new Lint/TopLevelReturnWithArgument cop #8377

Merged
merged 18 commits into from Jul 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@
* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): Add new `Lint/MissingSuper` cop. ([@fatkodima][])
* [#8339](https://github.com/rubocop-hq/rubocop/pull/8339): Add `Config#for_badge` as an efficient way to get a cop's config merged with its department's. ([@marcandre][])
* [#5067](https://github.com/rubocop-hq/rubocop/issues/5067): Add new `Style/StringConcatenation` cop. ([@fatkodima][])
* [#7425](https://github.com/rubocop-hq/rubocop/pull/7425): Add new `Lint/TopLevelReturnWithArgument` cop. ([@iamravitejag][])

### Bug fixes

Expand Down Expand Up @@ -4717,3 +4718,4 @@
[@CamilleDrapier]: https://github.com/CamilleDrapier
[@shekhar-patil]: https://github.com/shekhar-patil
[@knejad]: https://github.com/knejad
[@iamravitejag]: https://github.com/iamravitejag
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -1820,6 +1820,11 @@ Lint/ToJSON:
Enabled: true
VersionAdded: '0.66'

Lint/TopLevelReturnWithArgument:
Description: 'This cop detects top level return statements with argument.'
Enabled: 'pending'
VersionAdded: '0.89'

Lint/UnderscorePrefixedVariableName:
Description: 'Do not use prefix `_` for a variable that is used.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -256,6 +256,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintsuppressedexception[Lint/SuppressedException]
* xref:cops_lint.adoc#lintsyntax[Lint/Syntax]
* xref:cops_lint.adoc#linttojson[Lint/ToJSON]
* xref:cops_lint.adoc#linttoplevelreturnwithargument[Lint/TopLevelReturnWithArgument]
* xref:cops_lint.adoc#lintunderscoreprefixedvariablename[Lint/UnderscorePrefixedVariableName]
* xref:cops_lint.adoc#lintunifiedinteger[Lint/UnifiedInteger]
* xref:cops_lint.adoc#lintunreachablecode[Lint/UnreachableCode]
Expand Down
24 changes: 24 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -3462,6 +3462,30 @@ def to_json(*_args)
end
----

== Lint/TopLevelReturnWithArgument

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| No
| 0.89
| -
|===

This cop checks for top level return with arguments. If there is a
top-level return statement with an argument, then the argument is
always ignored. This is detected automatically since Ruby 2.7.

=== Examples

[source,ruby]
----
# Detected since Ruby 2.7
return 1 # 1 is always ignored.
----

== Lint/UnderscorePrefixedVariableName

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -313,6 +313,7 @@
require_relative 'rubocop/cop/lint/suppressed_exception'
require_relative 'rubocop/cop/lint/syntax'
require_relative 'rubocop/cop/lint/to_json'
require_relative 'rubocop/cop/lint/top_level_return_with_argument'
require_relative 'rubocop/cop/lint/underscore_prefixed_variable_name'
require_relative 'rubocop/cop/lint/unified_integer'
require_relative 'rubocop/cop/lint/unreachable_code'
Expand Down
34 changes: 34 additions & 0 deletions lib/rubocop/cop/lint/top_level_return_with_argument.rb
@@ -0,0 +1,34 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks for top level return with arguments. If there is a
# top-level return statement with an argument, then the argument is
# always ignored. This is detected automatically since Ruby 2.7.
#
# @example
#
# # Detected since Ruby 2.7
# return 1 # 1 is always ignored.
class TopLevelReturnWithArgument < Cop
# This cop works by validating the ancestors of the return node. A
# top-level return node's ancestors should not be of block, def, or
# defs type.

MSG = 'Top level return with argument detected.'

def on_return(return_node)
add_offense(return_node) if return_node.arguments? && ancestors_valid?(return_node)
end

private

def ancestors_valid?(return_node)
prohibited_ancestors = return_node.each_ancestor(:block, :def, :defs)
prohibited_ancestors.none?
end
end
end
end
end
127 changes: 127 additions & 0 deletions spec/rubocop/cop/lint/top_level_return_with_argument_spec.rb
@@ -0,0 +1,127 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::TopLevelReturnWithArgument, :config do
context 'Code segment with only top-level return statement' do
it 'Expects no offense from the return without arguments' do
expect_no_offenses(<<~RUBY)
return
RUBY
end

it 'Expects offense from the return with arguments' do
expect_offense(<<~RUBY)
return 1, 2, 3
^^^^^^^^^^^^^^ Top level return with argument detected.
RUBY
end

it 'Expects multiple offenses from the return with arguments statements' do
expect_offense(<<~RUBY)
return 1, 2, 3
^^^^^^^^^^^^^^ Top level return with argument detected.

return 1, 2, 3
^^^^^^^^^^^^^^ Top level return with argument detected.

return 1, 2, 3
^^^^^^^^^^^^^^ Top level return with argument detected.
RUBY
end
end

context 'Code segment with block level returns other than the top-level return' do
it 'Expects no offense from the return without arguments' do
expect_no_offenses(<<~RUBY)
foo

[1, 2, 3, 4, 5].each { |n| return n }

return

bar
RUBY
end

it 'Expects offense from the return with arguments' do
expect_offense(<<~RUBY)
foo

[1, 2, 3, 4, 5].each { |n| return n }

return 1, 2, 3
^^^^^^^^^^^^^^ Top level return with argument detected.

bar
RUBY
end
end

context 'Code segment with method-level return statements' do
it 'Expects offense when method-level & top-level return co-exist' do
expect_offense(<<~RUBY)
def method
return 'Hello World'
end

return 1, 2, 3
^^^^^^^^^^^^^^ Top level return with argument detected.
RUBY
end
end

context 'Code segment with inline if along with top-level return' do
it 'Expects no offense from the return without arguments' do
expect_no_offenses(<<~RUBY)
foo

return if 1 == 1

bar

def method
return "Hello World" if 1 == 1
end
RUBY
end

it 'Expects multiple offense from the return with arguments' do
expect_offense(<<~RUBY)
foo
return 1, 2, 3 if 1 == 1
^^^^^^^^^^^^^^ Top level return with argument detected.
bar
return 2
^^^^^^^^ Top level return with argument detected.
return 3
^^^^^^^^ Top level return with argument detected.

def method
return "Hello World" if 1 == 1
end
RUBY
end
end

context 'Code segment containing semi-colon separated statements' do
it 'Expects an offense from the return with arguments and multi-line code' do
expect_offense(<<~RUBY)
foo

if a == b; warn 'hey'; return 42; end
^^^^^^^^^ Top level return with argument detected.

bar
RUBY
end

it 'Expects no offense from the return with arguments and multi-line code' do
expect_no_offenses(<<~RUBY)
foo

if a == b; warn 'hey'; return; end

bar
RUBY
end
end
end