From af1e633b0ef2641a386eb7dba9267f44ff512aba Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Thu, 26 Aug 2021 22:25:08 -0400 Subject: [PATCH] Improve the messaging for `Style/Documentation` to be more clear about what class/module needs documentation. --- changelog/change_improve_the_messaging_for.md | 1 + lib/rubocop/cop/style/documentation.rb | 25 +++++-- spec/fixtures/html_formatter/expected.html | 8 +-- spec/rubocop/cli/autocorrect_spec.rb | 4 +- spec/rubocop/cop/style/documentation_spec.rb | 70 ++++++++++++------- 5 files changed, 70 insertions(+), 38 deletions(-) create mode 100644 changelog/change_improve_the_messaging_for.md diff --git a/changelog/change_improve_the_messaging_for.md b/changelog/change_improve_the_messaging_for.md new file mode 100644 index 00000000000..96a12a7a564 --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/style/documentation.rb b/lib/rubocop/cop/style/documentation.rb index 26335cded44..f912211198b 100644 --- a/lib/rubocop/cop/style/documentation.rb +++ b/lib/rubocop/cop/style/documentation.rb @@ -71,8 +71,9 @@ module Style # class Documentation < Base include DocumentationComment + include RangeHelp - MSG = 'Missing top-level %s documentation comment.' + MSG = 'Missing top-level documentation comment for `%s %s`.' # @!method constant_definition?(node) def_node_matcher :constant_definition?, '{class module casgn}' @@ -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) @@ -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 diff --git a/spec/fixtures/html_formatter/expected.html b/spec/fixtures/html_formatter/expected.html index 7ae5c86df47..6ede0aa320f 100644 --- a/spec/fixtures/html_formatter/expected.html +++ b/spec/fixtures/html_formatter/expected.html @@ -439,10 +439,10 @@

RuboCop Inspection Report

Line #1 – convention: - Style/Documentation: Missing top-level class documentation comment. + Style/Documentation: Missing top-level documentation comment for class BooksController.
- 
class BooksController < ApplicationController
+ 
class BooksController < ApplicationController
@@ -603,10 +603,10 @@

RuboCop Inspection Report

Line #1 – convention: - Style/Documentation: Missing top-level class documentation comment. + Style/Documentation: Missing top-level documentation comment for class Book.
- 
class Book < ActiveRecord::Base
+ 
class Book < ActiveRecord::Base
diff --git a/spec/rubocop/cli/autocorrect_spec.rb b/spec/rubocop/cli/autocorrect_spec.rb index 543e9698b8c..1d1bf52b2be 100644 --- a/spec/rubocop/cli/autocorrect_spec.rb +++ b/spec/rubocop/cli/autocorrect_spec.rb @@ -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. @@ -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. diff --git a/spec/rubocop/cop/style/documentation_spec.rb b/spec/rubocop/cop/style/documentation_spec.rb index 00c0a22fbb2..2d8bc509b08 100644 --- a/spec/rubocop/cop/style/documentation_spec.rb +++ b/spec/rubocop/cop/style/documentation_spec.rb @@ -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 @@ -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 @@ -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 @@ -45,7 +45,7 @@ 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 @@ -53,7 +53,7 @@ module Test it 'accepts non-empty class with documentation' do expect_no_offenses(<<~RUBY) # class comment - class My_Class + class MyClass def method end end @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -286,7 +286,7 @@ 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 @@ -294,6 +294,22 @@ def method 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 @@ -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 @@ -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 @@ -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