Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #3936] Add new Style/MixinGrouping cop #3982

Merged
merged 1 commit into from Jan 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
10 changes: 10 additions & 0 deletions config/default.yml
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions config/enabled.yml
Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'
Expand Down
101 changes: 101 additions & 0 deletions 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
3 changes: 2 additions & 1 deletion lib/rubocop/formatter/html_formatter.rb
Expand Up @@ -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),
Expand Down
6 changes: 4 additions & 2 deletions lib/rubocop/formatter/simple_text_formatter.rb
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Expand Up @@ -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)
Expand Down
48 changes: 48 additions & 0 deletions manual/cops_style.md
Expand Up @@ -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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include Bar, Qox

==

include Qox
include Bar

include Qox
end

EnforcedStyle: grouped

# bad
class Foo
extend Bar
extend Qox
end

# good
class Foo
extend Bar, Qox

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include Bar
include Qox

==

include Qox, Bar

end
```

### Important attributes

Attribute | Value
--- | ---
EnforcedStyle | separated
SupportedStyles | separated, grouped


## Style/ModuleFunction

Enabled by default | Supports autocorrection
Expand Down