Skip to content

Commit

Permalink
[Fix #7144] Extend the namespaces definition for Style/Documentation …
Browse files Browse the repository at this point in the history
…cop (#7147)

Extend the definition of namespaces by adding the constants visibility
statement.
No longer report an error for classes and modules that contain constants'
visibility statements.

Co-authored-by: Bozhidar Batsov <bozhidar@batsov.com>
  • Loading branch information
AdrienSdy and bbatsov committed Mar 21, 2020
1 parent b387fda commit 319bee7
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -194,6 +194,7 @@

### Bug fixes

* [#7144](https://github.com/rubocop-hq/rubocop/issues/7144): Fix `Style/Documentation` constant visibility declaration in namespace. ([@AdrienSldy][])
* [#7391](https://github.com/rubocop-hq/rubocop/issues/7391): Support pacman formatter on Windows. ([@laurenball][])
* [#7256](https://github.com/rubocop-hq/rubocop/issues/7256): Fix an error of `Style/RedundantParentheses` on method calls where the first argument begins with a hash literal. ([@halfwhole][])
* [#7263](https://github.com/rubocop-hq/rubocop/issues/7263): Make `Layout/SpaceInsideArrayLiteralBrackets` properly handle tab-indented arrays. ([@buehmann][])
Expand Down Expand Up @@ -300,6 +301,7 @@
* [#7171](https://github.com/rubocop-hq/rubocop/issues/7171): Fix an error for `Style/SafeNavigation` when using `unless nil?` as a safeguarded'. ([@koic][])
* [#7130](https://github.com/rubocop-hq/rubocop/pull/7130): Skip autocorrect in `Style/FormatString` if second argument to `String#%` is a variable. ([@tejasbubane][])
* [#7171](https://github.com/rubocop-hq/rubocop/issues/7171): Fix an error for `Style/SafeNavigation` when using `unless nil?` as a safeguarded'. ([@koic][])
* [#7113](https://github.com/rubocop-hq/rubocop/pull/7113): This PR renames `EnforcedStyle: rails` to `EnabledStyle: outdented_access_modifiers` for `Layout/IndentationConsistency`. ([@koic][])

### Changes

Expand Down
48 changes: 43 additions & 5 deletions lib/rubocop/cop/style/documentation.rb
Expand Up @@ -3,10 +3,11 @@
module RuboCop
module Cop
module Style
# This cop checks for missing top-level documentation of
# classes and modules. Classes with no body are exempt from the
# check and so are namespace modules - modules that have nothing in
# their bodies except classes, other modules, or constant definitions.
# This cop checks for missing top-level documentation of classes and
# modules. Classes with no body are exempt from the check and so are
# namespace modules - modules that have nothing in their bodies except
# classes, other modules, constant definitions or constant visibility
# declarations.
#
# The documentation requirement is annulled if the class or module has
# a "#:nodoc:" comment next to it. Likewise, "#:nodoc: all" does the
Expand All @@ -18,19 +19,52 @@ module Style
# # ...
# end
#
# module Math
# end
#
# # good
# # Description/Explanation of Person class
# class Person
# # ...
# end
#
# # allowed
# # Class without body
# class Person
# end
#
# # Namespace - A namespace can be a class or a module
# # Containing a class
# module Namespace
# # Description/Explanation of Person class
# class Person
# # ...
# end
# end
#
# # Containing constant visibility declaration
# module Namespace
# class Private
# end
#
# private_constant :Private
# end
#
# # Containing constant definition
# module Namespace
# Public = Class.new
# end
#
class Documentation < Cop
include DocumentationComment

MSG = 'Missing top-level %<type>s documentation comment.'

def_node_matcher :constant_definition?, '{class module casgn}'
def_node_search :outer_module, '(const (const nil? _) _)'
def_node_matcher :constant_visibility_declaration?, <<~PATTERN
(send nil? {:public_constant :private_constant} ({sym str} _))
PATTERN

def on_class(node)
return unless node.body
Expand Down Expand Up @@ -59,12 +93,16 @@ def namespace?(node)
return false unless node

if node.begin_type?
node.children.all? { |child| constant_definition?(child) }
node.children.all?(&method(:constant_declaration?))
else
constant_definition?(node)
end
end

def constant_declaration?(node)
constant_definition?(node) || constant_visibility_declaration?(node)
end

def compact_namespace?(node)
node.loc.name.source =~ /::/
end
Expand Down
39 changes: 35 additions & 4 deletions manual/cops_style.md
Expand Up @@ -1320,10 +1320,11 @@ Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChan
--- | --- | --- | --- | ---
Enabled | Yes | No | 0.9 | -

This cop checks for missing top-level documentation of
classes and modules. Classes with no body are exempt from the
check and so are namespace modules - modules that have nothing in
their bodies except classes, other modules, or constant definitions.
This cop checks for missing top-level documentation of classes and
modules. Classes with no body are exempt from the check and so are
namespace modules - modules that have nothing in their bodies except
classes, other modules, constant definitions or constant visibility
declarations.

The documentation requirement is annulled if the class or module has
a "#:nodoc:" comment next to it. Likewise, "#:nodoc: all" does the
Expand All @@ -1337,11 +1338,41 @@ class Person
# ...
end

module Math
end

# good
# Description/Explanation of Person class
class Person
# ...
end

# allowed
# Class without body
class Person
end

# Namespace - A namespace can be a class or a module
# Containing a class
module Namespace
# Description/Explanation of Person class
class Person
# ...
end
end

# Containing constant visibility declaration
module Namespace
class Private
end

private_constant :Private
end

# Containing constant definition
module Namespace
Public = Class.new
end
```

### Configurable attributes
Expand Down
32 changes: 32 additions & 0 deletions spec/rubocop/cop/style/documentation_spec.rb
Expand Up @@ -187,6 +187,38 @@ module Test
RUBY
end

context 'without documentation' do
context 'with non-empty module' do
context 'with constants visibility declaration content' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
module Namespace
class Private
end
private_constant :Private
end
RUBY
end
end
end

context 'with non-empty class' do
context 'with constants visibility declaration content' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class Namespace
class Private
end
private_constant :Private
end
RUBY
end
end
end
end

it 'does not raise an error for an implicit match conditional' do
expect do
inspect_source(<<~RUBY)
Expand Down

0 comments on commit 319bee7

Please sign in to comment.