Skip to content

Commit

Permalink
[Fix #11293] Fix a false negative for Style/Documentation
Browse files Browse the repository at this point in the history
Fixes #11293.

This PR fixes a false negative for `Style/Documentation`
when using custom macro.

The built-in macros (e.g. `include`, `extend`, `prepend`) and
DSL like `belongs_to` should be treated differently.
Only `include`, `extend`, and `prepend` will be needed to resolve #8788.
  • Loading branch information
koic committed Dec 16, 2022
1 parent 413c7d8 commit bac3f74
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 18 deletions.
1 change: 1 addition & 0 deletions changelog/fix_false_negative_for_style_documentation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11293](https://github.com/rubocop/rubocop/issues/11293): Fix a false negative for `Style/Documentation` when using macro. ([@koic][])
14 changes: 10 additions & 4 deletions lib/rubocop/cop/style/documentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ class Documentation < Base
(send nil? {:public_constant :private_constant} ({sym str} _))
PATTERN

# @!method include_statement?(node)
def_node_matcher :include_statement?, <<~PATTERN
(send nil? {:include :extend :prepend} const)
PATTERN

def on_class(node)
return unless node.body

Expand All @@ -103,7 +108,7 @@ def check(node, body)
return if documentation_comment?(node)
return if constant_allowed?(node)
return if nodoc_self_or_outer_module?(node)
return if macro_only?(body)
return if include_statement_only?(body)

range = range_between(node.loc.expression.begin_pos, node.loc.name.end_pos)
message = format(MSG, type: node.type, identifier: identifier(node))
Expand All @@ -115,9 +120,10 @@ def nodoc_self_or_outer_module?(node)
(compact_namespace?(node) && nodoc_comment?(outer_module(node).first))
end

def macro_only?(body)
(body.respond_to?(:macro?) && body.macro?) ||
(body.respond_to?(:children) && body.children&.all? { |child| macro_only?(child) })
def include_statement_only?(body)
return true if include_statement?(body)

body.respond_to?(:children) && body.children.all? { |node| include_statement_only?(node) }
end

def namespace?(node)
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/formatter.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

module RuboCop
# The bootstrap module for formatter.
# @api private
module Formatter
require_relative 'formatter/text_util'

Expand Down
17 changes: 14 additions & 3 deletions spec/fixtures/html_formatter/expected.html
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,14 @@ <h1 class="title">RuboCop Inspection Report</h1>
<div class="infobox">
<div class="total">
3 files inspected,
22 offenses detected:
23 offenses detected:
</div>
<ul class="offenses-list">


<li>
<a href="#offense_app/controllers/application_controller.rb">
app/controllers/application_controller.rb - 1 offense
app/controllers/application_controller.rb - 2 offenses
</a>
</li>

Expand All @@ -400,9 +400,20 @@ <h1 class="title">RuboCop Inspection Report</h1>

<div class="offense-box" id="offense_app/controllers/application_controller.rb">
<div class="box-title-placeholder"><h3>&nbsp;</h3></div>
<div class="box-title"><h3>app/controllers/application_controller.rb - 1 offense</h3></div>
<div class="box-title"><h3>app/controllers/application_controller.rb - 2 offenses</h3></div>
<div class="offense-reports">

<div class="report">
<div class="meta">
<span class="location">Line #1</span>
<span class="severity convention">convention:</span>
<span class="message">Style/Documentation: Missing top-level documentation comment for <code>class ApplicationController</code>.</span>
</div>

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

</div>

<div class="report">
<div class="meta">
<span class="location">Line #1</span>
Expand Down
10 changes: 8 additions & 2 deletions spec/fixtures/markdown_formatter/expected.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
# RuboCop Inspection Report

4 files inspected, 22 offenses detected:
4 files inspected, 23 offenses detected:

### app/controllers/application_controller.rb - (2 offenses)
* **Line # 1 - convention:** Style/Documentation: Missing top-level documentation comment for `class ApplicationController`.

```rb
class ApplicationController < ActionController::Base
```

### app/controllers/application_controller.rb - (1 offense)
* **Line # 1 - convention:** Style/FrozenStringLiteralComment: Missing frozen string literal comment.

```rb
Expand Down
40 changes: 33 additions & 7 deletions spec/rubocop/cop/style/documentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,26 +244,52 @@ class Private
end
end

context 'macro-only class' do
it 'does not register offense with single macro' do
it 'registers offense with custom macro' do
expect_offense(<<~RUBY)
class Foo < ApplicationRecord
^^^^^^^^^ Missing top-level documentation comment for `class Foo`.
belongs_to :bar
end
RUBY
end

context 'include statement-only class' do
it 'does not register offense with single `include` statements' do
expect_no_offenses(<<~RUBY)
module Foo
include Bar
end
RUBY
end

it 'does not register offense with single `extend` statements' do
expect_no_offenses(<<~RUBY)
module Foo
extend Bar
end
RUBY
end

it 'does not register offense with multiple macros' do
it 'does not register offense with single `prepend` statements' do
expect_no_offenses(<<~RUBY)
module Foo
extend A
extend B
include C
prepend Bar
end
RUBY
end

it 'does not register offense with multiple include macros' do
expect_no_offenses(<<~RUBY)
module Foo
include A
include B
extend C
prepend D
end
RUBY
end

it 'registers offense for macro with other methods' do
it 'registers offense for include statement with other methods' do
expect_offense(<<~RUBY)
module Foo
^^^^^^^^^^ Missing top-level documentation comment for `module Foo`.
Expand Down
4 changes: 2 additions & 2 deletions tasks/spec_runner.rake
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ require 'rspec/core'
require 'test_queue'
require 'test_queue/runner/rspec'

# Add `failed_examples` into `TestQueue::Worker` so we can keep
# track of the output for re-running failed examples from RSpec.
module TestQueue
# Add `failed_examples` into `TestQueue::Worker` so we can keep
# track of the output for re-running failed examples from RSpec.
class Worker
attr_accessor :failed_examples
end
Expand Down

0 comments on commit bac3f74

Please sign in to comment.