diff --git a/CHANGELOG.md b/CHANGELOG.md index b71dd260f9a..8b3abd34480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index 04d9ea06a3b..68124625536 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -4615,6 +4615,7 @@ an empty constructor just overrides the parent constructor, which is bad anyway. ---- # bad def initialize + super end def method @@ -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 ---- diff --git a/lib/rubocop/cop/lint/useless_method_definition.rb b/lib/rubocop/cop/lint/useless_method_definition.rb index c86e43f6059..147b2d6ca77 100644 --- a/lib/rubocop/cop/lint/useless_method_definition.rb +++ b/lib/rubocop/cop/lint/useless_method_definition.rb @@ -12,6 +12,7 @@ module Lint # @example # # bad # def initialize + # super # end # # def method @@ -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 @@ -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 @@ -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 diff --git a/spec/rubocop/cop/lint/useless_method_definition_spec.rb b/spec/rubocop/cop/lint/useless_method_definition_spec.rb index ab52c9870b4..a9b0c20537e 100644 --- a/spec/rubocop/cop/lint/useless_method_definition_spec.rb +++ b/spec/rubocop/cop/lint/useless_method_definition_spec.rb @@ -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 @@ -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) @@ -122,6 +120,8 @@ def self.other_useful_class_method #{trailing_whitespace} #{trailing_whitespace} + + #{trailing_whitespace} end end RUBY @@ -145,6 +145,10 @@ def method1(foo) def method2(foo, bar) super(bar, foo) end + + def method3(foo, bar) + super() + end RUBY end @@ -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