diff --git a/CHANGELOG.md b/CHANGELOG.md index f23968b4059..218c8a786ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) diff --git a/config/default.yml b/config/default.yml index 996db611a84..46a5dd5e0ae 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index a0f0e78aecd..5738dfbad31 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 0c09547b3ff..dc14a2c7bb0 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -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 |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 48d9cdc1bb7..96231f293be 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' @@ -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' diff --git a/lib/rubocop/cop/layout/class_structure.rb b/lib/rubocop/cop/layout/class_structure.rb index 3fa226199cd..e60c9f7401c 100644 --- a/lib/rubocop/cop/layout/class_structure.rb +++ b/lib/rubocop/cop/layout/class_structure.rb @@ -134,6 +134,7 @@ module Layout # # @see https://rubystyle.guide#consistent-classes class ClassStructure < Cop + include CommentsHelp include VisibilityHelp HUMANIZED_NODE_TYPE = { diff --git a/lib/rubocop/cop/mixin/comments_help.rb b/lib/rubocop/cop/mixin/comments_help.rb new file mode 100644 index 00000000000..76db20f2080 --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/style/class_methods_definitions.rb b/lib/rubocop/cop/style/class_methods_definitions.rb new file mode 100644 index 00000000000..8c2589e9695 --- /dev/null +++ b/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 `%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 diff --git a/spec/rubocop/cop/style/class_methods_definitions_spec.rb b/spec/rubocop/cop/style/class_methods_definitions_spec.rb new file mode 100644 index 00000000000..111be4f8517 --- /dev/null +++ b/spec/rubocop/cop/style/class_methods_definitions_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::ClassMethodsDefinitions, :config do + context 'when EnforcedStyle is def_self' do + let(:cop_config) do + { 'EnforcedStyle' => 'def_self' } + end + + it 'registers an offense and corrects when defining class methods with `class << self`' do + expect_offense(<<~RUBY) + class A + class << self + attr_reader :two + + def three + ^^^^^^^^^ Use `def self.three` to define class method. + end + end + end + RUBY + + expect_correction(<<~RUBY) + class A + + def self.three + end + + class << self + attr_reader :two + end + end + RUBY + end + + it 'correctly handles methods with annotation comments' do + expect_offense(<<~RUBY) + class A + class << self + attr_reader :one + + # Multiline + # comment. + def two + ^^^^^^^ Use `def self.two` to define class method. + end + end + end + RUBY + + expect_correction(<<~RUBY) + class A + + # Multiline + # comment. + def self.two + end + + class << self + attr_reader :one + end + end + RUBY + end + + it 'does not register an offense when `class << self` contains non public methods' do + expect_no_offenses(<<~RUBY) + class A + class << self + def one + end + + private + + def one + end + end + end + RUBY + end + + it 'does not register an offense when defining class methods with `def self.method`' do + expect_no_offenses(<<~RUBY) + class A + def self.one + end + end + RUBY + end + + it 'does not register an offense when defining singleton methods using `self << object`' do + expect_no_offenses(<<~RUBY) + class A + class << not_self + def one + end + end + end + RUBY + end + end + + context 'when EnforcedStyle is self_class' do + let(:cop_config) do + { 'EnforcedStyle' => 'self_class' } + end + + it 'registers an offense when defining class methods with `def self.method`' do + expect_offense(<<~RUBY) + class A + def self.one + ^^^^^^^^^^^^ Use `class << self` to define class method. + end + end + RUBY + end + + it 'does not register an offense when defining class methods with `class << self`' do + expect_no_offenses(<<~RUBY) + class A + class << self + def one + end + end + end + RUBY + end + end +end