From 8af40870e2208612a5af34e19f231032745a7f90 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Mon, 27 Aug 2018 14:23:25 +0200 Subject: [PATCH] Applies rule only when extending ActiveSupport::Concern --- .../cop/prefer_class_methods_over_module.rb | 24 +++++++++++++++++-- .../prefer_class_methods_over_module_spec.rb | 23 +++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/rubocop/cop/prefer_class_methods_over_module.rb b/rubocop/cop/prefer_class_methods_over_module.rb index bd91c663e58..496d2ed249f 100644 --- a/rubocop/cop/prefer_class_methods_over_module.rb +++ b/rubocop/cop/prefer_class_methods_over_module.rb @@ -2,12 +2,14 @@ module RuboCop module Cop - # Enforces the use of 'class_methods' instead of 'module ClassMethods' + # Enforces the use of 'class_methods' instead of 'module ClassMethods' for activesupport concerns. # For more information see: https://gitlab.com/gitlab-org/gitlab-ce/issues/50414 # # @example # # bad # module Foo + # extend ActiveSupport::Concern + # # module ClassMethods # def a_class_method # end @@ -16,6 +18,8 @@ module RuboCop # # # good # module Foo + # extend ActiveSupport::Concern + # # class_methods do # def a_class_method # end @@ -27,8 +31,12 @@ module RuboCop MSG = 'Do not use module ClassMethods, use class_methods block instead.' + def_node_matcher :extend_activesupport_concern?, <<~PATTERN + (:send nil? :extend (:const (:const nil? :ActiveSupport) :Concern)) + PATTERN + def on_module(node) - add_offense(node) if node.defined_module_name == 'ClassMethods' + add_offense(node) if node.defined_module_name == 'ClassMethods' && extends_activesupport_concern?(node) end def autocorrect(node) @@ -39,6 +47,18 @@ module RuboCop private + def extends_activesupport_concern?(node) + container_module(node.parent)&.descendants.any? do |descendant| + extend_activesupport_concern?(descendant) + end + end + + def container_module(node) + node = node.parent until node.type == :module + + node + end + def module_range(node) module_node, _ = *node range_between(node.loc.keyword.begin_pos, module_node.source_range.end_pos) diff --git a/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb b/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb index 64b1f1b948d..527c236eecf 100644 --- a/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb +++ b/spec/rubocop/cop/prefer_class_methods_over_module_spec.rb @@ -10,9 +10,11 @@ describe RuboCop::Cop::PreferClassMethodsOverModule do subject(:cop) { described_class.new } - it 'flags violation when using ClassMethods' do + it 'flags violation when using module ClassMethods' do expect_offense(<<~RUBY) module Foo + extend ActiveSupport::Concern + module ClassMethods ^^^^^^^^^^^^^^^^^^^ Do not use module ClassMethods, use class_methods block instead. def a_class_method @@ -25,6 +27,8 @@ describe RuboCop::Cop::PreferClassMethodsOverModule do it "doesn't flag violation when using class_methods" do expect_no_offenses(<<~RUBY) module Foo + extend ActiveSupport::Concern + class_methods do def a_class_method end @@ -33,9 +37,22 @@ describe RuboCop::Cop::PreferClassMethodsOverModule do RUBY end + it "doesn't flag violation when module is not extending ActiveSupport::Concern" do + expect_no_offenses(<<~RUBY) + module Foo + module ClassMethods + def a_class_method + end + end + end + RUBY + end + it "doesn't flag violation when not using either class_methods or ClassMethods" do expect_no_offenses(<<~RUBY) module Foo + extend ActiveSupport::Concern + def a_method end end @@ -45,6 +62,8 @@ describe RuboCop::Cop::PreferClassMethodsOverModule do it 'autocorrects ClassMethods into class_methods' do source = <<~RUBY module Foo + extend ActiveSupport::Concern + module ClassMethods def a_class_method end @@ -55,6 +74,8 @@ describe RuboCop::Cop::PreferClassMethodsOverModule do expected_source = <<~RUBY module Foo + extend ActiveSupport::Concern + class_methods do def a_class_method end