diff --git a/CHANGELOG.md b/CHANGELOG.md index 62aeb9126bd..7f2dbecfe59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * [#3893](https://github.com/bbatsov/rubocop/issues/3893): Add a new configuration, `IncludeActiveSupportAliases`, to `Performance/DoublStartEndWith`. This configuration will check for ActiveSupport's `starts_with?` and `ends_with?`. ([@rrosenblum][]) * [#3889](https://github.com/bbatsov/rubocop/pull/3889): Add new `Style/EmptyLineAfterMagicComment` cop. ([@backus][]) * [#3800](https://github.com/bbatsov/rubocop/issues/3800): Make `Style/EndOfLine` configurable with `lf`, `crlf`, and `native` (default) styles. ([@jonas054][]) +* [#3936](https://github.com/bbatsov/rubocop/issues/3936): Add new `Style/MixinGrouping` cop. ([@drenmi][]) ### Changes diff --git a/config/default.yml b/config/default.yml index 0979587d2ff..878c8e2e340 100644 --- a/config/default.yml +++ b/config/default.yml @@ -728,6 +728,16 @@ Style/MethodName: - snake_case - camelCase +# Checks the grouping of mixins (`include`, `extend`, `prepend`) in `class` and +# `module` bodies. +Style/MixinGrouping: + EnforcedStyle: separated + SupportedStyles: + # separated: each mixed in module goes in a separate statement. + # grouped: mixed in modules are grouped into a single statement. + - separated + - grouped + Style/ModuleFunction: EnforcedStyle: module_function SupportedStyles: diff --git a/config/enabled.yml b/config/enabled.yml index d4fdd02902e..4bf66df47be 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -426,6 +426,11 @@ Style/MethodMissing: StyleGuide: '#no-method-missing' Enabled: true +Style/MixinGrouping: + Description: 'Checks for grouping of mixins in `class` and `module` bodies.' + StyleGuide: '#mixin-grouping' + Enabled: true + Style/ModuleFunction: Description: 'Checks for usage of `extend self` in modules.' StyleGuide: '#module-function' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 530f98062f0..f2ce83c7438 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -311,6 +311,7 @@ require 'rubocop/cop/style/method_name' require 'rubocop/cop/style/method_missing' require 'rubocop/cop/style/missing_else' +require 'rubocop/cop/style/mixin_grouping' require 'rubocop/cop/style/module_function' require 'rubocop/cop/style/multiline_array_brace_layout' require 'rubocop/cop/style/multiline_assignment_layout' diff --git a/lib/rubocop/cop/style/mixin_grouping.rb b/lib/rubocop/cop/style/mixin_grouping.rb new file mode 100644 index 00000000000..038df48de6c --- /dev/null +++ b/lib/rubocop/cop/style/mixin_grouping.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop checks for grouping of mixins in `class` and `module` bodies. + # By default it enforces mixins to be placed in separate declarations, + # but it can be configured to enforce grouping them in one declaration. + # + # @example + # + # EnforcedStyle: separated (default) + # + # @bad + # class Foo + # include Bar, Qox + # end + # + # @good + # class Foo + # include Bar + # include Qox + # end + # + # EnforcedStyle: grouped + # + # @bad + # class Foo + # extend Bar + # extend Qox + # end + # + # @good + # class Foo + # extend Bar, Qox + # end + class MixinGrouping < Cop + include ConfigurableEnforcedStyle + + MIXIN_METHODS = [:extend, :include, :prepend].freeze + MSG = 'Put `%s` mixins in %s.'.freeze + + def on_send(node) + _reciever, method_name, *_args = *node + + return unless MIXIN_METHODS.include?(method_name) + + check(node) + end + + private + + def check(send_node) + if separated_style? + check_separated_style(send_node) + else + check_grouped_style(send_node) + end + end + + def check_grouped_style(send_node) + return unless sibling_mixins?(send_node) + + add_offense(send_node, :expression) + end + + def check_separated_style(send_node) + _reciever, _method_name, *args = *send_node + + return if args.one? + + add_offense(send_node, :expression) + end + + def sibling_mixins?(send_node) + siblings = send_node.parent.each_child_node(:send) + .reject { |sibling| sibling == send_node } + + siblings.any? do |sibling_node| + sibling_node.method_name == send_node.method_name + end + end + + def message(send_node) + suffix = + separated_style? ? 'separate statements' : 'a single statement' + + format(MSG, send_node.method_name, suffix) + end + + def grouped_style? + style == :grouped + end + + def separated_style? + style == :separated + end + end + end + end +end diff --git a/lib/rubocop/formatter/html_formatter.rb b/lib/rubocop/formatter/html_formatter.rb index 40d53b2c66c..af4b8bcb75b 100644 --- a/lib/rubocop/formatter/html_formatter.rb +++ b/lib/rubocop/formatter/html_formatter.rb @@ -61,7 +61,8 @@ def render_html # This class provides helper methods used in the ERB template. class ERBContext - include PathUtil, TextUtil + include PathUtil + include TextUtil SEVERITY_COLORS = { refactor: Color.new(0xED, 0x9C, 0x28, 1.0), diff --git a/lib/rubocop/formatter/simple_text_formatter.rb b/lib/rubocop/formatter/simple_text_formatter.rb index 6319730be6a..5b53410b32c 100644 --- a/lib/rubocop/formatter/simple_text_formatter.rb +++ b/lib/rubocop/formatter/simple_text_formatter.rb @@ -9,7 +9,8 @@ module Formatter # Offenses are displayed at compact form - just the # location of the problem and the associated message. class SimpleTextFormatter < BaseFormatter - include Colorizable, PathUtil + include Colorizable + include PathUtil COLOR_FOR_SEVERITY = { refactor: :yellow, @@ -90,7 +91,8 @@ def message(offense) # A helper class for building the report summary text. class Report - include Colorizable, TextUtil + include Colorizable + include TextUtil def initialize(file_count, offense_count, correction_count, rainbow) @file_count = file_count diff --git a/manual/cops.md b/manual/cops.md index 1db2d694c47..583549cf7ec 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -308,6 +308,7 @@ In the following section you find all available cops: * [Style/MethodMissing](cops_style.md#stylemethodmissing) * [Style/MethodName](cops_style.md#stylemethodname) * [Style/MissingElse](cops_style.md#stylemissingelse) +* [Style/MixinGrouping](cops_style.md#stylemixingrouping) * [Style/ModuleFunction](cops_style.md#stylemodulefunction) * [Style/MultilineArrayBraceLayout](cops_style.md#stylemultilinearraybracelayout) * [Style/MultilineAssignmentLayout](cops_style.md#stylemultilineassignmentlayout) diff --git a/manual/cops_style.md b/manual/cops_style.md index 888a28d361e..d697991964f 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -2765,6 +2765,54 @@ EnforcedStyle | both SupportedStyles | if, case, both +## Style/MixinGrouping + +Enabled by default | Supports autocorrection +--- | --- +Enabled | No + +This cop checks for grouping of mixins in `class` and `module` bodies. +By default it enforces mixins to be placed in separate declarations, +but it can be configured to enforce grouping them in one declaration. + +### Example + +```ruby +EnforcedStyle: separated (default) + +# bad +class Foo + include Bar, Qox +end + +# good +class Foo + include Bar + include Qox +end + +EnforcedStyle: grouped + +# bad +class Foo + extend Bar + extend Qox +end + +# good +class Foo + extend Bar, Qox +end +``` + +### Important attributes + +Attribute | Value +--- | --- +EnforcedStyle | separated +SupportedStyles | separated, grouped + + ## Style/ModuleFunction Enabled by default | Supports autocorrection diff --git a/spec/rubocop/cop/style/mixin_grouping_spec.rb b/spec/rubocop/cop/style/mixin_grouping_spec.rb new file mode 100644 index 00000000000..85e8ed555d1 --- /dev/null +++ b/spec/rubocop/cop/style/mixin_grouping_spec.rb @@ -0,0 +1,212 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe RuboCop::Cop::Style::MixinGrouping, :config do + subject(:cop) { described_class.new(config) } + + before do + inspect_source(cop, source) + end + + shared_examples 'code with offense' do |code, expected| + context "when checking #{code}" do + let(:source) { code } + + it 'registers an offense' do + expect(cop.offenses.size).to eq(offenses) + expect(cop.messages).to eq([message] * offenses) + end + + if expected + it 'auto-corrects' do + expect(autocorrect_source(cop, code)).to eq(expected) + end + else + it 'does not auto-correct' do + expect(autocorrect_source(cop, code)).to eq(code) + end + end + end + end + + shared_examples 'code without offense' do |code| + let(:source) { code } + + it 'does not register an offense' do + expect(cop.offenses).to be_empty + end + end + + context 'when configured with separated style' do + let(:cop_config) { { 'EnforcedStyle' => 'separated' } } + + let(:offenses) { 1 } + + context 'when using `include`' do + let(:message) { 'Put `include` mixins in separate statements.' } + + context 'with several mixins in one call' do + it_behaves_like 'code with offense', + ['class Foo', + ' include Bar, Qux', + 'end'].join("\n") + end + + context 'with several mixins in separate calls' do + it_behaves_like 'code without offense', + ['class Foo', + ' include Bar', + ' include Qux', + 'end'].join("\n") + end + end + + context 'when using `extend`' do + let(:message) { 'Put `extend` mixins in separate statements.' } + + context 'with several mixins in one call' do + it_behaves_like 'code with offense', + ['class Foo', + ' extend Bar, Qux', + 'end'].join("\n") + end + + context 'with several mixins in separate calls' do + it_behaves_like 'code without offense', + ['class Foo', + ' extend Bar', + ' extend Qux', + 'end'].join("\n") + end + end + + context 'when using `prepend`' do + let(:message) { 'Put `prepend` mixins in separate statements.' } + + context 'with several mixins in one call' do + it_behaves_like 'code with offense', + ['class Foo', + ' prepend Bar, Qux', + 'end'].join("\n") + end + + context 'with several mixins in separate calls' do + it_behaves_like 'code without offense', + ['class Foo', + ' prepend Bar', + ' prepend Qux', + 'end'].join("\n") + end + end + + context 'when using a mix of diffent methods' do + context 'with some calls having several mixins' do + let(:message) { 'Put `include` mixins in separate statements.' } + + it_behaves_like 'code with offense', + ['class Foo', + ' include Bar, Baz', + ' extend Qux', + 'end'].join("\n") + end + + context 'with all calls having one mixin' do + it_behaves_like 'code without offense', + ['class Foo', + ' include Bar', + ' prepend Baz', + ' extend Baz', + 'end'].join("\n") + end + end + end + + context 'when configured with grouped style' do + let(:cop_config) { { 'EnforcedStyle' => 'grouped' } } + + context 'when using include' do + context 'with several mixins in one call' do + let(:offenses) { 3 } + let(:message) { 'Put `include` mixins in a single statement.' } + + it_behaves_like 'code with offense', + ['class Foo', + ' include Bar', + ' include Baz', + ' include Qux', + 'end'].join("\n") + end + + context 'with several mixins in separate calls' do + it_behaves_like 'code without offense', + ['class Foo', + ' include Bar, Qux', + 'end'].join("\n") + end + end + + context 'when using `extend`' do + context 'with several mixins in one call' do + let(:offenses) { 2 } + let(:message) { 'Put `extend` mixins in a single statement.' } + + it_behaves_like 'code with offense', + ['class Foo', + ' extend Bar', + ' extend Baz', + 'end'].join("\n") + end + + context 'with several mixins in separate calls' do + it_behaves_like 'code without offense', + ['class Foo', + ' extend Bar, Qux', + 'end'].join("\n") + end + end + + context 'when using `prepend`' do + context 'with several mixins in one call' do + let(:offenses) { 2 } + let(:message) { 'Put `prepend` mixins in a single statement.' } + + it_behaves_like 'code with offense', + ['class Foo', + ' prepend Bar', + ' prepend Baz', + 'end'].join("\n") + end + + context 'with several mixins in separate calls' do + it_behaves_like 'code without offense', + ['class Foo', + ' prepend Bar, Qux', + 'end'].join("\n") + end + end + + context 'when using a mix of diffent methods' do + context 'with some duplicated mixin methods' do + let(:offenses) { 2 } + let(:message) { 'Put `include` mixins in a single statement.' } + + it_behaves_like 'code with offense', + ['class Foo', + ' include Bar', + ' include Baz', + ' extend Baz', + 'end'].join("\n") + end + + context 'with all different mixin methods' do + it_behaves_like 'code without offense', + ['class Foo', + ' include Bar', + ' prepend Baz', + ' extend Baz', + 'end'].join("\n") + end + end + end +end