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

Add new Lint/MissingSuper cop #8376

Merged
merged 1 commit into from Jul 23, 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 @@ -5,6 +5,7 @@
### New features

* [#8322](https://github.com/rubocop-hq/rubocop/pull/8322): Support autocorrect for `Style/CaseEquality` cop. ([@fatkodima][])
* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): Add new `Lint/MissingSuper` cop. ([@fatkodima][])
fatkodima marked this conversation as resolved.
Show resolved Hide resolved
* [#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][])

### Bug fixes
Expand All @@ -20,6 +21,7 @@

### Changes

* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): `Style/MethodMissingSuper` cop is removed in favor of new `Lint/MissingSuper` cop. ([@fatkodima][])
* [#8350](https://github.com/rubocop-hq/rubocop/pull/8350): Set default max line length to 120 for `Style/MultilineMethodSignature`. ([@koic][])
* [#8338](https://github.com/rubocop-hq/rubocop/pull/8338): **potentially breaking**. Config#for_department now returns only the config specified for that department; the 'Enabled' attribute is no longer calculated. ([@marcandre][])

Expand Down
13 changes: 7 additions & 6 deletions config/default.yml
Expand Up @@ -1556,6 +1556,13 @@ Lint/MissingCopEnableDirective:
# .inf for any size
MaximumRangeSize: .inf

Lint/MissingSuper:
Description: >-
This cop checks for the presence of constructors and lifecycle callbacks
without calls to `super`'.
Enabled: pending
VersionAdded: '0.89'

Lint/MixedRegexpCaptureTypes:
Description: 'Do not mix named captures and numbered captures in a Regexp literal.'
Enabled: pending
Expand Down Expand Up @@ -3219,12 +3226,6 @@ Style/MethodDefParentheses:
- require_no_parentheses
- require_no_parentheses_except_multiline

Style/MethodMissingSuper:
Description: Checks for `method_missing` to call `super`.
StyleGuide: '#no-method-missing'
Enabled: true
VersionAdded: '0.56'

Style/MinMax:
Description: >-
Use `Enumerable#minmax` instead of `Enumerable#min`
Expand Down
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -216,6 +216,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintliteralininterpolation[Lint/LiteralInInterpolation]
* xref:cops_lint.adoc#lintloop[Lint/Loop]
* xref:cops_lint.adoc#lintmissingcopenabledirective[Lint/MissingCopEnableDirective]
* xref:cops_lint.adoc#lintmissingsuper[Lint/MissingSuper]
* xref:cops_lint.adoc#lintmixedregexpcapturetypes[Lint/MixedRegexpCaptureTypes]
* xref:cops_lint.adoc#lintmultiplecomparison[Lint/MultipleComparison]
* xref:cops_lint.adoc#lintnestedmethoddefinition[Lint/NestedMethodDefinition]
Expand Down Expand Up @@ -395,7 +396,6 @@ In the following section you find all available cops:
* xref:cops_style.adoc#stylemethodcallwithoutargsparentheses[Style/MethodCallWithoutArgsParentheses]
* xref:cops_style.adoc#stylemethodcalledondoendblock[Style/MethodCalledOnDoEndBlock]
* xref:cops_style.adoc#stylemethoddefparentheses[Style/MethodDefParentheses]
* xref:cops_style.adoc#stylemethodmissingsuper[Style/MethodMissingSuper]
* xref:cops_style.adoc#styleminmax[Style/MinMax]
* xref:cops_style.adoc#stylemissingelse[Style/MissingElse]
* xref:cops_style.adoc#stylemissingrespondtomissing[Style/MissingRespondToMissing]
Expand Down
50 changes: 50 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -1693,6 +1693,56 @@ x += 1
| Float
|===

== Lint/MissingSuper

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

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

This cop checks for the presence of constructors and lifecycle callbacks
without calls to `super`.

=== Examples

[source,ruby]
----
# bad
class Employee < Person
def initialize(name, salary)
@salary = salary
end
end
# good
class Employee < Person
def initialize(name, salary)
super(name)
@salary = salary
end
end
# bad
class Parent
def self.inherited(base)
do_something
end
end
# good
class Parent
def self.inherited(base)
super
do_something
end
end
----

== Lint/MixedRegexpCaptureTypes

|===
Expand Down
36 changes: 0 additions & 36 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -4815,42 +4815,6 @@ end

* https://rubystyle.guide#method-parens

== Style/MethodMissingSuper

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

| Enabled
| Yes
| No
| 0.56
| -
|===

This cop checks for the presence of `method_missing` without
falling back on `super`.

=== Examples

[source,ruby]
----
#bad
def method_missing(name, *args)
# ...
end
#good
def method_missing(name, *args)
# ...
super
end
----

=== References

* https://rubystyle.guide#no-method-missing

== Style/MinMax

|===
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop.rb
Expand Up @@ -273,6 +273,7 @@
require_relative 'rubocop/cop/lint/literal_in_interpolation'
require_relative 'rubocop/cop/lint/loop'
require_relative 'rubocop/cop/lint/missing_cop_enable_directive'
require_relative 'rubocop/cop/lint/missing_super'
require_relative 'rubocop/cop/lint/mixed_regexp_capture_types'
require_relative 'rubocop/cop/lint/multiple_comparison'
require_relative 'rubocop/cop/lint/nested_method_definition'
Expand Down Expand Up @@ -443,7 +444,6 @@
require_relative 'rubocop/cop/style/method_call_with_args_parentheses/require_parentheses'
require_relative 'rubocop/cop/style/method_called_on_do_end_block'
require_relative 'rubocop/cop/style/method_def_parentheses'
require_relative 'rubocop/cop/style/method_missing_super'
require_relative 'rubocop/cop/style/min_max'
require_relative 'rubocop/cop/style/missing_else'
require_relative 'rubocop/cop/style/missing_respond_to_missing'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cli/command/base.rb
Expand Up @@ -13,6 +13,7 @@ class << self
attr_accessor :command_name

def inherited(subclass)
super
@subclasses << subclass
end

Expand Down
4 changes: 3 additions & 1 deletion lib/rubocop/config_obsoletion.rb
Expand Up @@ -84,7 +84,9 @@ class ConfigObsoletion
'Lint/InvalidCharacterLiteral' => 'it was never being actually triggered',
'Lint/SpaceBeforeFirstArg' =>
'it was a duplicate of `Layout/SpaceBeforeFirstArg`. Please use ' \
'`Layout/SpaceBeforeFirstArg` instead'
'`Layout/SpaceBeforeFirstArg` instead',
'Style/MethodMissingSuper' => 'it has been superseded by `Lint/MissingSuper`. Please use ' \
'`Lint/MissingSuper` instead'
}.map do |cop_name, reason|
[cop_name, "The `#{cop_name}` cop has been removed since #{reason}."]
end
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/base.rb
Expand Up @@ -137,6 +137,7 @@ def external_dependency_checksum
end

def self.inherited(subclass)
super
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😊

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fatkodima Marc-André is smiling for a reason.
There's no super for a class with no parent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I still like adding it, and I was surprise I didn't in the first place.

Registry.global.enlist(subclass)
end

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/force.rb
Expand Up @@ -11,6 +11,7 @@ def self.all
end

def self.inherited(subclass)
super
all << subclass
end

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

module RuboCop
module Cop
module Lint
# This cop checks for the presence of constructors and lifecycle callbacks
# without calls to `super`.
#
# @example
# # bad
# class Employee < Person
# def initialize(name, salary)
# @salary = salary
# end
# end
#
# # good
# class Employee < Person
# def initialize(name, salary)
# super(name)
# @salary = salary
# end
# end
#
# # bad
# class Parent
# def self.inherited(base)
# do_something
# end
# end
#
# # good
# class Parent
# def self.inherited(base)
# super
# do_something
# end
# end
#
class MissingSuper < Base
CONSTRUCTOR_MSG = 'Call `super` to initialize state of the parent class.'
CALLBACK_MSG = 'Call `super` to invoke callback defined in the parent class.'

STATELESS_CLASSES = %w[BasicObject Object].freeze

OBJECT_LIFECYCLE_CALLBACKS = %i[method_missing respond_to_missing?].freeze
CLASS_LIFECYCLE_CALLBACKS = %i[inherited].freeze
MODULE_LIFECYCLE_CALLBACKS = %i[included extended prepended].freeze

def on_def(node)
return unless offender?(node)

if node.method?(:initialize)
add_offense(node, message: CONSTRUCTOR_MSG) if inside_class_with_stateful_parent?(node)
elsif callback_method_def?(node)
add_offense(node, message: CALLBACK_MSG)
end
end

def on_defs(node)
return if !callback_method_def?(node) || contains_super?(node)

add_offense(node, message: CALLBACK_MSG)
end

private

def offender?(node)
(node.method?(:initialize) || callback_method_def?(node)) && !contains_super?(node)
end

def callback_method_def?(node)
method_name = node.method_name

if OBJECT_LIFECYCLE_CALLBACKS.include?(method_name) ||
CLASS_LIFECYCLE_CALLBACKS.include?(method_name)

node.each_ancestor(:class, :sclass, :module).first
elsif MODULE_LIFECYCLE_CALLBACKS.include?(method_name)
node.each_ancestor(:module).first
end
end

def contains_super?(node)
node.each_descendant(:super, :zsuper).any?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any is not the same thing as !.empty?. It will work the same way in most cases, however this will fail if the array contains nil.

[1] pry(main)> [nil].any?
=> false
[2] pry(main)> ![nil].empty?
=> true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, but each_descendant(:super, :zsuper) won't return array with nils - either empty or with Node instances.

end

def inside_class_with_stateful_parent?(node)
class_node = node.each_ancestor(:class).first
class_node&.parent_class && !stateless_class?(class_node.parent_class)
end

def stateless_class?(node)
STATELESS_CLASSES.include?(node.const_name)
end
end
end
end
end
2 changes: 2 additions & 0 deletions lib/rubocop/cop/mixin/enforce_superclass.rb
Expand Up @@ -5,6 +5,8 @@ module Cop
# Common functionality for enforcing a specific superclass
module EnforceSuperclass
def self.included(base)
super

base.def_node_matcher :class_definition, <<~PATTERN
(class (const _ !:#{base::SUPERCLASS}) #{base::BASE_PATTERN} ...)
PATTERN
Expand Down
34 changes: 0 additions & 34 deletions lib/rubocop/cop/style/method_missing_super.rb

This file was deleted.

1 change: 1 addition & 0 deletions lib/rubocop/cop/variable_force/branch.rb
Expand Up @@ -49,6 +49,7 @@ def self.classes
end

def self.inherited(subclass)
super
classes << subclass
end

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/error.rb
Expand Up @@ -12,6 +12,7 @@ class ValidationError < Error; end
# A wrapper to display errored location of analyzed file.
class ErrorWithAnalyzedFileLocation < Error
def initialize(cause:, node:, cop:)
super()
@cause = cause
@cop = cop
@location = node.is_a?(RuboCop::AST::Node) ? node.loc : node
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/formatter/formatter_set.rb
Expand Up @@ -35,6 +35,7 @@ class FormatterSet < Array
end

def initialize(options = {})
super()
@options = options # CLI options
end

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/rake_task.rb
Expand Up @@ -15,6 +15,7 @@ class RakeTask < ::Rake::TaskLib
attr_accessor :name, :verbose, :fail_on_error, :patterns, :formatters, :requires, :options

def initialize(name = :rubocop, *args, &task_block)
super()
setup_ivars(name)

desc 'Run RuboCop' unless ::Rake.application.last_description
Expand Down