Skip to content

Commit

Permalink
Improve the messaging for Style/Documentation to be more clear abou…
Browse files Browse the repository at this point in the history
…t what class/module needs documentation.
  • Loading branch information
dvandersluis committed Aug 27, 2021
1 parent 536d986 commit af1e633
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 38 deletions.
1 change: 1 addition & 0 deletions changelog/change_improve_the_messaging_for.md
@@ -0,0 +1 @@
* [#10051](https://github.com/rubocop/rubocop/pull/10051): Improve the messaging for `Style/Documentation` to be more clear about what class/module needs documentation. ([@dvandersluis][])
25 changes: 20 additions & 5 deletions lib/rubocop/cop/style/documentation.rb
Expand Up @@ -71,8 +71,9 @@ module Style
#
class Documentation < Base
include DocumentationComment
include RangeHelp

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

# @!method constant_definition?(node)
def_node_matcher :constant_definition?, '{class module casgn}'
Expand All @@ -88,23 +89,25 @@ class Documentation < Base
def on_class(node)
return unless node.body

check(node, node.body, :class)
check(node, node.body)
end

def on_module(node)
check(node, node.body, :module)
check(node, node.body)
end

private

def check(node, body, type)
def check(node, body)
return if namespace?(body)
return if documentation_comment?(node)
return if constant_allowed?(node)
return if nodoc_self_or_outer_module?(node)
return if macro_only?(body)

add_offense(node.loc.keyword, message: format(MSG, type: type))
range = range_between(node.loc.expression.begin_pos, node.loc.name.end_pos)
message = format(MSG, type: node.type, identifier: identifier(node))
add_offense(range, message: message)
end

def nodoc_self_or_outer_module?(node)
Expand Down Expand Up @@ -165,6 +168,18 @@ def nodoc(node)
def allowed_constants
@allowed_constants ||= cop_config.fetch('AllowedConstants', []).map(&:intern)
end

def identifier(node)
# Get the fully qualified identifier for a class/module
nodes = [node, *node.each_ancestor(:class, :module)]
nodes.reverse_each.flat_map { |n| qualify_const(n.identifier) }.join('::')
end

def qualify_const(node)
return if node.nil?

[qualify_const(node.namespace), node.short_name].compact
end
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/fixtures/html_formatter/expected.html
Expand Up @@ -439,10 +439,10 @@ <h1 class="title">RuboCop Inspection Report</h1>
<div class="meta">
<span class="location">Line #1</span>
<span class="severity convention">convention:</span>
<span class="message">Style/Documentation: Missing top-level class documentation comment.</span>
<span class="message">Style/Documentation: Missing top-level documentation comment for <code>class BooksController</code>.</span>
</div>

<pre><code><span class="highlight convention">class</span> BooksController &lt; ApplicationController</code></pre>
<pre><code><span class="highlight convention">class BooksController</span> &lt; ApplicationController</code></pre>

</div>

Expand Down Expand Up @@ -603,10 +603,10 @@ <h1 class="title">RuboCop Inspection Report</h1>
<div class="meta">
<span class="location">Line #1</span>
<span class="severity convention">convention:</span>
<span class="message">Style/Documentation: Missing top-level class documentation comment.</span>
<span class="message">Style/Documentation: Missing top-level documentation comment for <code>class Book</code>.</span>
</div>

<pre><code><span class="highlight convention">class</span> Book &lt; ActiveRecord::Base</code></pre>
<pre><code><span class="highlight convention">class Book</span> &lt; ActiveRecord::Base</code></pre>

</div>

Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -830,7 +830,7 @@ def func
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
C: 2: 1: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
C: 3: 1: Style/Documentation: Missing top-level class documentation comment.
C: 3: 1: Style/Documentation: Missing top-level documentation comment for class A.
W: 4: 3: [Corrected] Lint/RedundantCopDisableDirective: Unnecessary disabling of Metrics/MethodLength.
C: 5: 3: [Corrected] Layout/IndentationWidth: Use 2 (not 6) spaces for indentation.
W: 5: 22: [Corrected] Lint/RedundantCopEnableDirective: Unnecessary enabling of Metrics/MethodLength.
Expand Down Expand Up @@ -1247,7 +1247,7 @@ class Dsl
RUBY
e = abs('example.rb')
expect($stdout.string).to eq(<<~RESULT)
#{e}:1:1: C: Style/Documentation: Missing top-level class documentation comment.
#{e}:1:1: C: Style/Documentation: Missing top-level documentation comment for `class Dsl`.
#{e}:2:1: C: [Corrected] Layout/AccessModifierIndentation: Indent access modifiers like `private`.
#{e}:3:7: C: [Corrected] Style/WordArray: Use `%w` or `%W` for an array of words.
#{e}:3:21: C: [Corrected] Style/TrailingCommaInArrayLiteral: Avoid comma after the last item of an array.
Expand Down
70 changes: 43 additions & 27 deletions spec/rubocop/cop/style/documentation_spec.rb
Expand Up @@ -9,8 +9,8 @@

it 'registers an offense for non-empty class' do
expect_offense(<<~RUBY)
class My_Class
^^^^^ Missing top-level class documentation comment.
class MyClass
^^^^^^^^^^^^^ Missing top-level documentation comment for `class MyClass`.
def method
end
end
Expand All @@ -22,8 +22,8 @@ def method
# Copyright 2014
# Some company
class My_Class
^^^^^ Missing top-level class documentation comment.
class MyClass
^^^^^^^^^^^^^ Missing top-level documentation comment for `class MyClass`.
def method
end
end
Expand All @@ -32,8 +32,8 @@ def method

it 'registers an offense for non-namespace' do
expect_offense(<<~RUBY)
module My_Class
^^^^^^ Missing top-level module documentation comment.
module MyModule
^^^^^^^^^^^^^^^ Missing top-level documentation comment for `module MyModule`.
def method
end
end
Expand All @@ -45,15 +45,15 @@ def method
# explanation.
expect_offense(<<~RUBY)
module Test
^^^^^^ Missing top-level module documentation comment.
^^^^^^^^^^^ Missing top-level documentation comment for `module Test`.
end
RUBY
end

it 'accepts non-empty class with documentation' do
expect_no_offenses(<<~RUBY)
# class comment
class My_Class
class MyClass
def method
end
end
Expand All @@ -63,8 +63,8 @@ def method
it 'registers an offense for non-empty class with annotation comment' do
expect_offense(<<~RUBY)
# OPTIMIZE: Make this faster.
class My_Class
^^^^^ Missing top-level class documentation comment.
class MyClass
^^^^^^^^^^^^^ Missing top-level documentation comment for `class MyClass`.
def method
end
end
Expand All @@ -74,8 +74,8 @@ def method
it 'registers an offense for non-empty class with directive comment' do
expect_offense(<<~RUBY)
# rubocop:disable Style/For
class My_Class
^^^^^ Missing top-level class documentation comment.
class MyClass
^^^^^^^^^^^^^ Missing top-level documentation comment for `class MyClass`.
def method
end
end
Expand All @@ -85,8 +85,8 @@ def method
it 'registers offense for non-empty class with frozen string comment' do
expect_offense(<<~RUBY)
# frozen_string_literal: true
class My_Class
^^^^^ Missing top-level class documentation comment.
class MyClass
^^^^^^^^^^^^^ Missing top-level documentation comment for `class MyClass`.
def method
end
end
Expand All @@ -96,8 +96,8 @@ def method
it 'registers an offense for non-empty class with encoding comment' do
expect_offense(<<~RUBY)
# encoding: ascii-8bit
class My_Class
^^^^^ Missing top-level class documentation comment.
class MyClass
^^^^^^^^^^^^^ Missing top-level documentation comment for `class MyClass`.
def method
end
end
Expand All @@ -108,7 +108,7 @@ def method
expect_no_offenses(<<~RUBY)
# OPTIMIZE: Make this faster.
# Class comment.
class My_Class
class MyClass
def method
end
end
Expand All @@ -129,7 +129,7 @@ def initialize
it 'accepts non-empty module with documentation' do
expect_no_offenses(<<~RUBY)
# class comment
module My_Class
module MyModule
def method
end
end
Expand All @@ -138,7 +138,7 @@ def method

it 'accepts empty class without documentation' do
expect_no_offenses(<<~RUBY)
class My_Class
class MyClass
end
RUBY
end
Expand Down Expand Up @@ -236,7 +236,7 @@ module Foo
it 'registers offense for macro with other methods' do
expect_offense(<<~RUBY)
module Foo
^^^^^^ Missing top-level module documentation comment.
^^^^^^^^^^ Missing top-level documentation comment for `module Foo`.
extend B
include C
Expand All @@ -251,7 +251,7 @@ def foo; end
expect do
expect_offense(<<~RUBY)
class Test
^^^^^ Missing top-level class documentation comment.
^^^^^^^^^^ Missing top-level documentation comment for `class Test`.
if //
end
end
Expand All @@ -263,7 +263,7 @@ class Test
expect_offense(<<~RUBY)
module A # The A Module
class B
^^^^^ Missing top-level class documentation comment.
^^^^^^^ Missing top-level documentation comment for `class A::B`.
C = 1
def method
end
Expand All @@ -275,7 +275,7 @@ def method
it 'registers an offense for compact-style nested module' do
expect_offense(<<~RUBY)
module A::B
^^^^^^ Missing top-level module documentation comment.
^^^^^^^^^^^ Missing top-level documentation comment for `module A::B`.
C = 1
def method
end
Expand All @@ -286,14 +286,30 @@ def method
it 'registers an offense for compact-style nested class' do
expect_offense(<<~RUBY)
class A::B
^^^^^ Missing top-level class documentation comment.
^^^^^^^^^^ Missing top-level documentation comment for `class A::B`.
C = 1
def method
end
end
RUBY
end

it 'registers an offense for a deeply nested class' do
expect_offense(<<~RUBY)
module A::B
module C
class D
class E::F
^^^^^^^^^^ Missing top-level documentation comment for `class A::B::C::D::E::F`.
def method
end
end
end
end
end
RUBY
end

context 'sparse and trailing comments' do
%w[class module].each do |keyword|
it "ignores comments after #{keyword} node end" do
Expand All @@ -312,7 +328,7 @@ def method
expect_offense(<<~RUBY, keyword: keyword)
module TestModule
%{keyword} Test
^{keyword} Missing top-level #{keyword} documentation comment.
^{keyword}^^^^^ Missing top-level documentation comment for `#{keyword} TestModule::Test`.
def method
end
# sparse comment
Expand Down Expand Up @@ -348,7 +364,7 @@ def method
module TestModule #:nodoc:
TEST = 20
%{keyword} Test
^{keyword} Missing top-level #{keyword} documentation comment.
^{keyword}^^^^^ Missing top-level documentation comment for `#{keyword} TestModule::Test`.
def method
end
end
Expand Down Expand Up @@ -387,7 +403,7 @@ def method
module TestModule #:nodoc:
TEST = 20
class Test < Parent
^^^^^ Missing top-level class documentation comment.
^^^^^^^^^^ Missing top-level documentation comment for `class TestModule::Test`.
def method
end
end
Expand Down

0 comments on commit af1e633

Please sign in to comment.