Make Style/ModuleFunction configurable with different styles (rubocop…

…#3173)

The two supported styles are: module_function and extend_self.

Offenses are not auto-corrected since the two styles have
slightly different semantics.

The default style still recommends `module_function` instead
of `extend self`.

CHANGELOG.md

@@ -7,6 +7,7 @@
 * [#2927](https://github.com/bbatsov/rubocop/issues/2927): Add autocorrect for `Rails/Validation` cop. ([@neodelf][])
 * [#3135](https://github.com/bbatsov/rubocop/pull/3135): Add new `Rails/OutputSafety` cop. ([@josh][])
 * [#3164](https://github.com/bbatsov/rubocop/pull/3164): Add [Fastlane](https://fastlane.tools/)'s Fastfile to the default Includes. ([@jules2689][])
+* [#3173](https://github.com/bbatsov/rubocop/pull/3173): Make `Style/ModuleFunction` configurable with `module_function` and `extend_self` styles. ([@tjwp][])
 
 ### Bug fixes
 

config/default.yml

@@ -642,6 +642,12 @@ Style/MethodName:
     - snake_case
     - camelCase
 
+Style/ModuleFunction:
+  EnforcedStyle: module_function
+  SupportedStyles:
+    - module_function
+    - extend_self
+
 Style/MultilineArrayBraceLayout:
   EnforcedStyle: symmetrical
   SupportedStyles:

lib/rubocop/cop/style/module_function.rb

@@ -4,26 +4,48 @@
 module RuboCop
   module Cop
     module Style
-      # This cops checks for use of `extend self` in a module.
+      # This cops checks for use of `extend self` or `module_function` in a
+      # module.
+      #
+      # Supported styles are: module_function, extend_self.
       #
       # @example
       #
+      #   # Good if EnforcedStyle is module_function
       #   module Test
-      #     extend self
+      #     module_function
+      #     ...
+      #   end
       #
+      #   # Good if EnforcedStyle is extend_self
+      #   module Test
+      #     extend self
       #     ...
-      # end
+      #   end
+      #
+      # These offenses are not auto-corrected since there are different
+      # implications to each approach.
       class ModuleFunction < Cop
-        MSG = 'Use `module_function` instead of `extend self`.'.freeze
+        include ConfigurableEnforcedStyle
+
+        MODULE_FUNCTION_MSG = 'Use `module_function` instead of `extend self`.'
+                              .freeze
+        EXTEND_SELF_MSG = 'Use `extend self` instead of `module_function`.'
+                          .freeze
 
-        TARGET_NODE = s(:send, nil, :extend, s(:self))
+        MODULE_FUNCTION_NODE = s(:send, nil, :module_function)
+        EXTEND_SELF_NODE = s(:send, nil, :extend, s(:self))
 
         def on_module(node)
           _name, body = *node
           return unless body && body.type == :begin
 
           body.children.each do |body_node|
-            add_offense(body_node, :expression) if body_node == TARGET_NODE
+            if style == :module_function && body_node == EXTEND_SELF_NODE
+              add_offense(body_node, :expression, MODULE_FUNCTION_MSG)
+            elsif style == :extend_self && body_node == MODULE_FUNCTION_NODE
+              add_offense(body_node, :expression, EXTEND_SELF_MSG)
+            end
           end
         end
       end

spec/rubocop/cop/style/module_function_spec.rb

@@ -3,23 +3,51 @@
 
 require 'spec_helper'
 
-describe RuboCop::Cop::Style::ModuleFunction do
-  subject(:cop) { described_class.new }
-
-  it 'registers an offense for extend self in module' do
-    inspect_source(cop,
-                   ['module Test',
-                    '  extend self',
-                    '  def test; end',
-                    'end'])
-    expect(cop.offenses.size).to eq(1)
+describe RuboCop::Cop::Style::ModuleFunction, :config do
+  subject(:cop) { described_class.new(config) }
+
+  context 'when enforced style is `module_function`' do
+    let(:cop_config) { { 'EnforcedStyle' => 'module_function' } }
+
+    it 'registers an offense for `extend self` in a module' do
+      inspect_source(cop,
+                     ['module Test',
+                      '  extend self',
+                      '  def test; end',
+                      'end'])
+      expect(cop.messages).to eq([described_class::MODULE_FUNCTION_MSG])
+      expect(cop.highlights).to eq(['extend self'])
+    end
+
+    it 'accepts `extend self` in a class' do
+      inspect_source(cop,
+                     ['class Test',
+                      '  extend self',
+                      'end'])
+      expect(cop.offenses).to be_empty
+    end
   end
 
-  it 'accepts extend self in class' do
-    inspect_source(cop,
-                   ['class Test',
-                    '  extend self',
-                    'end'])
-    expect(cop.offenses).to be_empty
+  context 'when enforced style is `extend_self`' do
+    let(:cop_config) { { 'EnforcedStyle' => 'extend_self' } }
+
+    it 'registers an offense for `module_function` without an argument' do
+      inspect_source(cop,
+                     ['module Test',
+                      '  module_function',
+                      '  def test; end',
+                      'end'])
+      expect(cop.messages).to eq([described_class::EXTEND_SELF_MSG])
+      expect(cop.highlights).to eq(['module_function'])
+    end
+
+    it 'accepts module_function with an argument' do
+      inspect_source(cop,
+                     ['module Test',
+                      '  def test; end',
+                      '  module_function :test',
+                      'end'])
+      expect(cop.offenses).to be_empty
+    end
   end
 end