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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Style/ClassAndModuleChildren style: namespace #9887

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
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
@@ -0,0 +1 @@
* [#9887](https://github.com/rubocop/rubocop/pull/9887): Add namespace style to Style/ClassAndModuleChildren. ([@markburns][])
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -3230,6 +3230,7 @@ Style/ClassAndModuleChildren:
EnforcedStyle: nested
SupportedStyles:
- nested
- namespace
- compact

Style/ClassCheck:
Expand Down
207 changes: 203 additions & 4 deletions lib/rubocop/cop/style/class_and_module_children.rb
Expand Up @@ -30,16 +30,29 @@ module Style
# end
#
# The compact style is only forced for classes/modules with one child.
#
# @example EnforcedStyle: namespaced
# # good
# # combine namespacing modules as much as possible,
# # but keep the constant definition on its own line
# module Foo::Bar::Baz::Qux::Quux::Quuz::Corge::Grault::Garply
# module Waldo
# end
# end
class ClassAndModuleChildren < Base
include ConfigurableEnforcedStyle
include RangeHelp
extend AutoCorrector

NESTED_MSG = 'Use nested module/class definitions instead of compact style.'
NAMESPACING_MSG = 'Keep namespacing module in compact style.'
NAMESPACED_MODULE_MSG = 'Keep module definition separate from namespacing module.'
NAMESPACED_CLASS_MSG = 'Keep class definition separate from namespacing module.'
USING_CLASS_AS_NAMESPACE_MSG = 'Don\'t use classes as a namespace.'
COMPACT_MSG = 'Use compact module/class definition instead of nested style.'

def on_class(node)
return if node.parent_class && style != :nested
return if skip_compact?(node)

check_style(node, node.body)
end
Expand Down Expand Up @@ -90,6 +103,11 @@ def add_trailing_end(corrector, node, padding)
corrector.replace(node.loc.end, replacement)
end

def namespace_definition(corrector, node)
compact_node(corrector, node)
remove_end(corrector, node.body)
end

def compact_definition(corrector, node)
compact_node(corrector, node)
remove_end(corrector, node.body)
Expand Down Expand Up @@ -148,9 +166,12 @@ def indent_width
def check_style(node, body)
return if node.identifier.children[0]&.cbase_type?

if style == :nested
case style
when :nested
check_nested_style(node)
else
when :namespace
check_namespaced_style(node)
when :compact
check_compact_style(node, body)
end
end
Expand All @@ -163,6 +184,71 @@ def check_nested_style(node)
end
end

def check_namespaced_style(node)
return if already_warned_about_class_as_namespace?(node)

if LowestLevelConstantDefinition.check(node)
check_class_or_module_definition_namespacing(node)
else
check_namespacing_node?(node)
end
end

def check_namespacing_node?(node)
return if class_as_namespace(node)
return if NamespacingModule.check(node)
return if LowestLevelConstantDefinition.check(node)
return if already_warned_about_namespace?(node)

warn_about_namespace(node)
add_offense(node.loc.name, message: NAMESPACING_MSG) do |corrector|
namespace_definition(corrector, node)
end

nil
end

def class_as_namespace(node)
return false unless UsingClassAsNamespace.check(node)

warn_about_class_as_namespace(node)
add_offense(node.loc.name, message: USING_CLASS_AS_NAMESPACE_MSG)

true
end

def warn_about_namespace(node)
already_warned_about_namespace.add node
end

def already_warned_about_namespace?(node)
node.ancestors.any? { |n| already_warned_about_namespace.include?(n) }
end

def already_warned_about_namespace
@already_warned_about_namespace ||= Set.new
end

def already_warned_about_class_as_namespace?(node)
node.ancestors.any? { |n| already_warned_about_class_as_namespace.include?(n) }
end

def warn_about_class_as_namespace(node)
already_warned_about_class_as_namespace.add node
end

def already_warned_about_class_as_namespace
@already_warned_about_class_as_namespace ||= Set.new
end

def check_class_or_module_definition_namespacing(node)
return true if DefinitionNamespacedCorrectly.check(node)

message = node.class_type? ? NAMESPACED_CLASS_MSG : NAMESPACED_MODULE_MSG

add_offense(node.loc.name, message: message)
end

def check_compact_style(node, body)
parent = node.parent
return if parent&.class_type? || parent&.module_type?
Expand All @@ -175,18 +261,131 @@ def check_compact_style(node, body)
end

def autocorrect(corrector, node)
return if node.class_type? && node.parent_class && style != :nested
return if node.class_type? && skip_compact?(node)

nest_or_compact(corrector, node)
end

def skip_compact?(node)
node.parent_class && style == :compact
end

def needs_compacting?(body)
body && %i[module class].include?(body.type)
end

def compact_node_name?(node)
/::/.match?(node.identifier.source)
end

# A base class for some checks performed whilst checking modules and their children
class CheckBase
def self.check(node)
new(node).check
end

attr_reader :node

def initialize(node)
@node = node
end

def constant_has_no_colon?
node.children.first.loc.double_colon.nil?
end

def class_or_module_children?
node.descendants.any? { |d| d.module_type? || d.class_type? }
end

def class_or_module
@class_or_module ||= node.class_type? || node.module_type?
end

def top_level_constant
class_or_module && empty_parent?
end

def empty_parent?
node.parent.nil? || (begin_block? && node.parent.parent.nil?)
end

def non_namespaced_top_level_constant
top_level_constant && constant_has_no_colon?
end

def begin_block?
node.parent&.begin_type?
end
end

private_constant :CheckBase

# Checks that a module is correctly namespaced
class NamespacingModule < CheckBase
def check
non_namespaced_children? &&
((node.module_type? && non_namespaced_top_level_constant) ||
correctly_namespaced_top_level_module)
end

def non_namespaced_children?
module_without_double_colon_and_without_module_descendants?(node) &&
no_module_or_class_grandchildren?
end

def no_module_or_class_grandchildren?
node.children.all? do |n|
n.descendants.none? do |g|
g.module_type? || g.class_type?
end
end
end

def module_without_double_colon_and_without_module_descendants?(node)
module_descendants = node.descendants.select(&:module_type?)

module_descendants.all? do |n|
module_without_double_colon_and_without_module_descendants?(n)
end
end

def correctly_namespaced_top_level_module
top_level_constant && node.module_type? && class_or_module_children?
end
end

private_constant :NamespacingModule

# Checks that a constant node is at the lowest level of the AST constant hierarchy
class LowestLevelConstantDefinition < CheckBase
def check
class_or_module && !class_or_module_children?
end
end

private_constant :LowestLevelConstantDefinition

# Checks that a leaf node constant is within the correct namespace
class DefinitionNamespacedCorrectly < CheckBase
def check
non_namespaced_top_level_constant || parent_ok? && constant_has_no_colon?
end

def parent_ok?
node.parent && node.parent.defined_module_name == node.parent_module_name
end
end

private_constant :DefinitionNamespacedCorrectly

# Checks that a class is not being used as a namespace
class UsingClassAsNamespace < CheckBase
def check
node.class_type? && class_or_module_children?
end
end
private_constant :UsingClassAsNamespace
end
end
end
Expand Down