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 Style/ClassMethodsDefinitions cop #8381

Merged
merged 1 commit into from Aug 22, 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
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][])
* [#8562](https://github.com/rubocop-hq/rubocop/pull/8562): Add new `Style/KeywordParametersOrder` cop. ([@fatkodima][])
* [#8381](https://github.com/rubocop-hq/rubocop/pull/8381): Add new `Style/ClassMethodsDefinitions` 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][])
Expand Down
10 changes: 10 additions & 0 deletions config/default.yml
Expand Up @@ -2671,6 +2671,16 @@ Style/ClassMethods:
VersionAdded: '0.9'
VersionChanged: '0.20'

Style/ClassMethodsDefinitions:
Description: 'Enforces using `def self.method_name` or `class << self` to define class methods.'
StyleGuide: '#def-self-class-methods'
Enabled: false
VersionAdded: '0.89'
EnforcedStyle: def_self
SupportedStyles:
- def_self
- self_class

Style/ClassVars:
Description: 'Avoid the use of class variables.'
StyleGuide: '#no-class-vars'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -345,6 +345,7 @@ In the following section you find all available cops:
* xref:cops_style.adoc#styleclassandmodulechildren[Style/ClassAndModuleChildren]
* xref:cops_style.adoc#styleclasscheck[Style/ClassCheck]
* xref:cops_style.adoc#styleclassmethods[Style/ClassMethods]
* xref:cops_style.adoc#styleclassmethodsdefinitions[Style/ClassMethodsDefinitions]
* xref:cops_style.adoc#styleclassvars[Style/ClassVars]
* xref:cops_style.adoc#stylecollectionmethods[Style/CollectionMethods]
* xref:cops_style.adoc#stylecolonmethodcall[Style/ColonMethodCall]
Expand Down
91 changes: 91 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -1087,6 +1087,97 @@ end

* https://rubystyle.guide#def-self-class-methods

== Style/ClassMethodsDefinitions

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

| Disabled
| Yes
| Yes
| 0.89
| -
|===

This cop enforces using `def self.method_name` or `class << self` to define class methods.

=== Examples

==== EnforcedStyle: def_self (default)

[source,ruby]
----
# bad
class SomeClass
class << self
attr_accessor :class_accessor
def class_method
# ...
end
end
end
# good
class SomeClass
def self.class_method
# ...
end
class << self
attr_accessor :class_accessor
end
end
# good - contains private method
class SomeClass
class << self
attr_accessor :class_accessor
private
def private_class_method
# ...
end
end
end
----

==== EnforcedStyle: self_class

[source,ruby]
----
# bad
class SomeClass
def self.class_method
# ...
end
end
# good
class SomeClass
class << self
def class_method
# ...
end
end
end
----

=== Configurable attributes

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

| EnforcedStyle
| `def_self`
| `def_self`, `self_class`
|===

=== References

* https://rubystyle.guide#def-self-class-methods

== Style/ClassVars

|===
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop.rb
Expand Up @@ -116,6 +116,7 @@
require_relative 'rubocop/cop/mixin/uncommunicative_name'
require_relative 'rubocop/cop/mixin/unused_argument'
require_relative 'rubocop/cop/mixin/visibility_help'
require_relative 'rubocop/cop/mixin/comments_help' # relies on visibility_help

require_relative 'rubocop/cop/utils/format_string'

Expand Down Expand Up @@ -389,6 +390,7 @@
require_relative 'rubocop/cop/style/class_and_module_children'
require_relative 'rubocop/cop/style/class_check'
require_relative 'rubocop/cop/style/class_methods'
require_relative 'rubocop/cop/style/class_methods_definitions'
require_relative 'rubocop/cop/style/class_vars'
require_relative 'rubocop/cop/style/collection_methods'
require_relative 'rubocop/cop/style/colon_method_call'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/layout/class_structure.rb
Expand Up @@ -134,6 +134,7 @@ module Layout
#
# @see https://rubystyle.guide#consistent-classes
class ClassStructure < Cop
include CommentsHelp
include VisibilityHelp

HUMANIZED_NODE_TYPE = {
Expand Down
54 changes: 54 additions & 0 deletions lib/rubocop/cop/mixin/comments_help.rb
@@ -0,0 +1,54 @@
# frozen_string_literal: true

module RuboCop
module Cop
# Help methods for working with nodes containing comments.
module CommentsHelp
include VisibilityHelp

def source_range_with_comment(node)
begin_pos, end_pos =
if node.def_type?
start_node = find_visibility_start(node) || node
end_node = find_visibility_end(node) || node
[begin_pos_with_comment(start_node),
end_position_for(end_node) + 1]
else
[begin_pos_with_comment(node), end_position_for(node)]
end

Parser::Source::Range.new(buffer, begin_pos, end_pos)
end

private

def end_position_for(node)
end_line = buffer.line_for_position(node.loc.expression.end_pos)
buffer.line_range(end_line).end_pos
end

def begin_pos_with_comment(node)
annotation_line = node.first_line - 1
first_comment = nil

processed_source.comments_before_line(annotation_line)
.reverse_each do |comment|
if comment.location.line == annotation_line
first_comment = comment
annotation_line -= 1
end
end

start_line_position(first_comment || node)
end

def start_line_position(node)
buffer.line_range(node.loc.line).begin_pos - 1
end

def buffer
processed_source.buffer
end
end
end
end
131 changes: 131 additions & 0 deletions lib/rubocop/cop/style/class_methods_definitions.rb
@@ -0,0 +1,131 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop enforces using `def self.method_name` or `class << self` to define class methods.
#
# @example EnforcedStyle: def_self (default)
# # bad
# class SomeClass
# class << self
# attr_accessor :class_accessor
#
# def class_method
# # ...
# end
# end
# end
#
# # good
# class SomeClass
# def self.class_method
# # ...
# end
#
# class << self
# attr_accessor :class_accessor
# end
# end
#
# # good - contains private method
# class SomeClass
# class << self
# attr_accessor :class_accessor
#
# private
#
# def private_class_method
# # ...
# end
# end
# end
#
# @example EnforcedStyle: self_class
# # bad
# class SomeClass
# def self.class_method
# # ...
# end
# end
#
# # good
# class SomeClass
# class << self
# def class_method
# # ...
# end
# end
# end
#
class ClassMethodsDefinitions < Base
include ConfigurableEnforcedStyle
include CommentsHelp
include VisibilityHelp
extend AutoCorrector

MSG = 'Use `%<preferred>s` to define class method.'

def on_sclass(node)
return unless def_self_style?
return unless node.identifier.source == 'self'
return if contains_non_public_methods?(node)

def_nodes(node).each do |def_node|
next unless node_visibility(def_node) == :public

message = format(MSG, preferred: "def self.#{def_node.method_name}")
add_offense(def_node, message: message) do |corrector|
extract_def_from_sclass(def_node, node, corrector)
end
end
end

def on_defs(node)
return if def_self_style?

message = format(MSG, preferred: 'class << self')
add_offense(node, message: message)
end

private

def def_self_style?
style == :def_self
end

def contains_non_public_methods?(sclass_node)
def_nodes(sclass_node).any? { |def_node| node_visibility(def_node) != :public }
end

def def_nodes(sclass_node)
sclass_def = sclass_node.body
return [] unless sclass_def

if sclass_def.def_type?
[sclass_def]
elsif sclass_def.begin_type?
sclass_def.each_child_node(:def).to_a
else
[]
end
end

def extract_def_from_sclass(def_node, sclass_node, corrector)
range = source_range_with_comment(def_node)
source = range.source.sub!(
"def #{def_node.method_name}",
"def self.#{def_node.method_name}"
)

corrector.insert_before(sclass_node, "#{source}\n#{indent(sclass_node)}")
corrector.remove(range)
end

def indent(node)
' ' * node.loc.column
end
end
end
end
end