diff --git a/CHANGELOG.md b/CHANGELOG.md index 717ba3a4b22e..3b2faafcd2ed 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/config/default.yml b/config/default.yml index c36d4466418e..4fec7a131697 100644 --- a/config/default.yml +++ b/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: diff --git a/lib/rubocop/cop/style/module_function.rb b/lib/rubocop/cop/style/module_function.rb index cd9299693ab1..d99f4e7b11d8 100644 --- a/lib/rubocop/cop/style/module_function.rb +++ b/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 diff --git a/spec/rubocop/cop/style/module_function_spec.rb b/spec/rubocop/cop/style/module_function_spec.rb index f58095ca6007..504e24f0412d 100644 --- a/spec/rubocop/cop/style/module_function_spec.rb +++ b/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