Skip to content

Commit

Permalink
Add new Lint/UselessMethodDefinition cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Aug 17, 2020
1 parent a0fac65 commit 4d8eb11
Show file tree
Hide file tree
Showing 8 changed files with 341 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@

* [#8451](https://github.com/rubocop-hq/rubocop/issues/8451): Add new `Style/RedundantSelfAssignment` cop. ([@fatkodima][])
* [#8474](https://github.com/rubocop-hq/rubocop/pull/8474): Add new `Lint/DuplicateRequire` cop. ([@fatkodima][])
* [#8472](https://github.com/rubocop-hq/rubocop/issues/8472): Add new `Lint/UselessMethodDefinition` cop. ([@fatkodima][])
* [#8531](https://github.com/rubocop-hq/rubocop/issues/8531): Add new `Lint/EmptyFile` cop. ([@fatkodima][])

### Bug fixes
Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -1942,6 +1942,13 @@ Lint/UselessElseWithoutRescue:
Enabled: true
VersionAdded: '0.17'

Lint/UselessMethodDefinition:
Description: 'Checks for useless method definitions.'
Enabled: pending
VersionAdded: '0.90'
Safe: false
AllowComments: true

Lint/UselessSetterCall:
Description: 'Checks for useless setter call to a local variable.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -275,6 +275,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintuselessaccessmodifier[Lint/UselessAccessModifier]
* xref:cops_lint.adoc#lintuselessassignment[Lint/UselessAssignment]
* xref:cops_lint.adoc#lintuselesselsewithoutrescue[Lint/UselessElseWithoutRescue]
* xref:cops_lint.adoc#lintuselessmethoddefinition[Lint/UselessMethodDefinition]
* xref:cops_lint.adoc#lintuselesssettercall[Lint/UselessSetterCall]
* xref:cops_lint.adoc#lintvoid[Lint/Void]

Expand Down
71 changes: 71 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -4558,6 +4558,77 @@ else
end
----

== Lint/UselessMethodDefinition

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

| Pending
| No
| Yes (Unsafe)
| 0.90
| -
|===

This cop checks for useless method definitions, specifically: empty constructors
and methods just delegating to `super`.

This cop is marked as unsafe as it can trigger false positives for cases when
an empty constructor just overrides the parent constructor, which is bad anyway.

=== Examples

[source,ruby]
----
# bad
def initialize
end
def method
super
end
# good
def initialize
initialize_internals
end
def method
super
do_something_else
end
----

==== AllowComments: true (default)

[source,ruby]
----
# good
def initialize
# Comment.
end
----

==== AllowComments: false

[source,ruby]
----
# bad
def initialize
# Comment.
end
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| AllowComments
| `true`
| Boolean
|===

== Lint/UselessSetterCall

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -333,6 +333,7 @@
require_relative 'rubocop/cop/lint/useless_access_modifier'
require_relative 'rubocop/cop/lint/useless_assignment'
require_relative 'rubocop/cop/lint/useless_else_without_rescue'
require_relative 'rubocop/cop/lint/useless_method_definition'
require_relative 'rubocop/cop/lint/useless_setter_call'
require_relative 'rubocop/cop/lint/void'

Expand Down
77 changes: 77 additions & 0 deletions lib/rubocop/cop/lint/useless_method_definition.rb
@@ -0,0 +1,77 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks for useless method definitions, specifically: empty constructors
# and methods just delegating to `super`.
#
# This cop is marked as unsafe as it can trigger false positives for cases when
# an empty constructor just overrides the parent constructor, which is bad anyway.
#
# @example
# # bad
# def initialize
# end
#
# def method
# super
# end
#
# # good
# def initialize
# initialize_internals
# end
#
# def method
# super
# do_something_else
# end
#
# @example AllowComments: true (default)
# # good
# def initialize
# # Comment.
# end
#
# @example AllowComments: false
# # bad
# def initialize
# # Comment.
# end
#
class UselessMethodDefinition < Base
extend AutoCorrector

MSG = 'Useless method definition detected.'

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

add_offense(node) { |corrector| corrector.remove(node) }
end
alias on_defs on_def

private

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)
end
end
end
end
end
2 changes: 2 additions & 0 deletions lib/rubocop/formatter/html_formatter.rb
Expand Up @@ -90,9 +90,11 @@ def initialize(files, summary)
end

# Make Kernel#binding public.
# rubocop:disable Lint/UselessMethodDefinition
def binding
super
end
# rubocop:enable Lint/UselessMethodDefinition

def decorated_message(offense)
offense.message.gsub(/`(.+?)`/) do
Expand Down
181 changes: 181 additions & 0 deletions spec/rubocop/cop/lint/useless_method_definition_spec.rb
@@ -0,0 +1,181 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::UselessMethodDefinition, :config do
subject(:cop) { described_class.new(config) }

let(:cop_config) do
{ 'AllowComments' => true }
end

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

expect_correction(<<~RUBY)
class Foo
end
RUBY
end

it 'does not register an offense for constructor with only comments' do
expect_no_offenses(<<~RUBY)
def initialize(arg)
# Comment.
end
RUBY
end

it 'does not register an offense for constructor containing additional code to `super`' do
expect_no_offenses(<<~RUBY)
def initialize(arg)
super
do_something
end
RUBY
end

it 'does not register an offense for empty class level `initialize` method' do
expect_no_offenses(<<~RUBY)
def self.initialize
end
RUBY
end

it 'registers an offense and corrects for method containing only `super` call' do
expect_offense(<<~RUBY)
class Foo
def useful_instance_method
do_something
end
def instance_method
^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super
end
def instance_method_with_args(arg)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super(arg)
end
def self.useful_class_method
do_something
end
def self.class_method
^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super
end
def self.class_method_with_args(arg)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super(arg)
end
class << self
def self.other_useful_class_method
do_something
end
def other_class_method
^^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super
end
def other_class_method_with_args(arg)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Useless method definition detected.
super(arg)
end
end
end
RUBY

expect_correction(<<~RUBY)
class Foo
def useful_instance_method
do_something
end
def self.useful_class_method
do_something
end
class << self
def self.other_useful_class_method
do_something
end
end
end
RUBY
end

it 'does not register an offense for method containing additional code to `super`' do
expect_no_offenses(<<~RUBY)
def method
super
do_something
end
RUBY
end

it 'does not register an offense when `super` arguments differ from method arguments' do
expect_no_offenses(<<~RUBY)
def method1(foo)
super(bar)
end
def method2(foo, bar)
super(bar, foo)
end
RUBY
end

it 'does not register an offense when non-constructor contains only comments' do
expect_no_offenses(<<~RUBY)
def non_constructor
# Comment.
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
end
RUBY
end
end
end

0 comments on commit 4d8eb11

Please sign in to comment.