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.

Sign up for GitHub

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

Jump to bottom

Improve the messaging for Style/Documentation #10051

Merged
merged 1 commit into from Aug 27, 2021
Merged

Improve the messaging for Style/Documentation #10051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/change_improve_the_messaging_for.md
View file
Open in desktop
@@ -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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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