Skip to content

Commit

Permalink
[Fixes #8626] Fix false negatives for Lint/UselessMethodDefinition.
Browse files Browse the repository at this point in the history
Note that not calling `super` at all from `initialize` is left
to `Lint/MissingSuper` cop.
  • Loading branch information
marcandre authored and mergify[bot] committed Sep 10, 2020
1 parent d0bdb7c commit 92d3fc6
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 86 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -26,6 +26,7 @@
* [#8688](https://github.com/rubocop-hq/rubocop/issues/8688): Mark `Style/GlobalStdStream` as unsafe autocorrection. ([@marcandre][])
* [#8642](https://github.com/rubocop-hq/rubocop/issues/8642): Fix a false negative for `Style/SpaceInsideHashLiteralBraces` when a correct empty hash precedes the incorrect hash. ([@dvandersluis][])
* [#8683](https://github.com/rubocop-hq/rubocop/issues/8683): Make naming cops work with non-ascii characters. ([@tejasbubane][])
* [#8626](https://github.com/rubocop-hq/rubocop/issues/8626): Fix false negatives for `Lint/UselessMethodDefinition`. ([@marcandre][])

### Changes

Expand Down
27 changes: 4 additions & 23 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -4615,6 +4615,7 @@ an empty constructor just overrides the parent constructor, which is bad anyway.
----
# bad
def initialize
super
end
def method
Expand All @@ -4628,32 +4629,12 @@ end
# good
def initialize
initialize_internals
end
def method
super
do_something_else
end
----

==== AllowComments: true (default)

[source,ruby]
----
# good
def initialize
# Comment.
initialize_internals
end
----
==== AllowComments: false

[source,ruby]
----
# bad
def initialize
# Comment.
def method(*args)
super(:extra_arg, *args)
end
----

Expand Down
45 changes: 14 additions & 31 deletions lib/rubocop/cop/lint/useless_method_definition.rb
Expand Up @@ -12,6 +12,7 @@ module Lint
# @example
# # bad
# def initialize
# super
# end
#
# def method
Expand All @@ -25,24 +26,12 @@ module Lint
#
# # good
# def initialize
# initialize_internals
# end
#
# def method
# super
# do_something_else
# end
#
# @example AllowComments: true (default)
# # good
# def initialize
# # Comment.
# initialize_internals
# end
#
# @example AllowComments: false
# # bad
# def initialize
# # Comment.
# def method(*args)
# super(:extra_arg, *args)
# end
#
class UselessMethodDefinition < Base
Expand All @@ -52,8 +41,7 @@ class UselessMethodDefinition < Base

def on_def(node)
return if optional_args?(node)
return unless (constructor?(node) && empty_constructor?(node)) ||
delegating?(node.body, node)
return unless delegating?(node.body, node)

add_offense(node) { |corrector| corrector.remove(node) }
end
Expand All @@ -65,21 +53,16 @@ def optional_args?(node)
node.arguments.any? { |arg| arg.optarg_type? || arg.kwoptarg_type? }
end

def empty_constructor?(node)
return false if node.body
return false if cop_config['AllowComments'] && comment_lines?(node)

true
end

def constructor?(node)
node.def_type? && node.method?(:initialize)
end

def delegating?(node, def_node)
return false unless node&.super_type? || node&.zsuper_type?

!node.arguments? || node.arguments.map(&:source) == def_node.arguments.map(&:source)
if node.nil?
false
elsif node.zsuper_type?
true
elsif node.super_type?
node.arguments.map(&:source) == def_node.arguments.map(&:source)
else
false
end
end
end
end
Expand Down
45 changes: 13 additions & 32 deletions spec/rubocop/cop/lint/useless_method_definition_spec.rb
Expand Up @@ -7,20 +7,13 @@
{ 'AllowComments' => true }
end

it 'registers an offense and corrects for empty constructor' do
expect_offense(<<~RUBY)
it 'does not register an offense for empty constructor' do
expect_no_offenses(<<~RUBY)
class Foo
def initialize(arg1, arg2)
^^^^^^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
end
end
RUBY

expect_correction(<<~RUBY)
class Foo
#{trailing_whitespace}
end
RUBY
end

it 'does not register an offense for constructor with only comments' do
Expand Down Expand Up @@ -88,6 +81,11 @@ def other_class_method
super
end
def other_class_method_with_parens
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super()
end
def other_class_method_with_args(arg)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super(arg)
Expand Down Expand Up @@ -122,6 +120,8 @@ def self.other_useful_class_method
#{trailing_whitespace}
#{trailing_whitespace}
#{trailing_whitespace}
end
end
RUBY
Expand All @@ -145,6 +145,10 @@ def method1(foo)
def method2(foo, bar)
super(bar, foo)
end
def method3(foo, bar)
super()
end
RUBY
end

Expand All @@ -171,27 +175,4 @@ def non_constructor
end
RUBY
end

context 'when AllowComments is false' do
let(:cop_config) do
{ 'AllowComments' => false }
end

it 'registers an offense when constructor contains only comments' do
expect_offense(<<~RUBY)
class Foo
def initialize
^^^^^^^^^^^^^^ Useless method definition detected.
# Comment.
end
end
RUBY

expect_correction(<<~RUBY)
class Foo
#{trailing_whitespace}
end
RUBY
end
end
end

0 comments on commit 92d3fc6

Please sign in to comment.