[Fix #3360] Make Style/DocumentationMethod configurable for non-pub…

…lic methods (#3378)

This change allows the cop to be configured to also apply to non-public methods
using:

```
RequireForNonPublicMethods: true
```

It also makes the cop highlight the entire method definition, rather than just
the `def` keyword.

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### New features
 
 * [#2968](https://github.com/bbatsov/rubocop/issues/2968): Add new `Style/DocumentationMethod` cop. ([@sooyang][])
+* [#3360](https://github.com/bbatsov/rubocop/issues/3360): Add `RequireForNonPublicMethods` configuration option to `Style/DocumentationMethod` cop. ([@drenmi][])
 
 ### Bug fixes
 

config/default.yml

@@ -395,6 +395,9 @@ Style/Copyright:
   Notice: '^Copyright (\(c\) )?2[0-9]{3} .+'
   AutocorrectNotice: ''
 
+Style/DocumentationMethod:
+  RequireForNonPublicMethods: false
+
 # Multi-line method chaining should be done with leading dots.
 Style/DotPosition:
   EnforcedStyle: leading

lib/rubocop.rb

@@ -56,6 +56,7 @@
 require 'rubocop/cop/mixin/classish_length' # relies on code_length
 require 'rubocop/cop/mixin/configurable_enforced_style'
 require 'rubocop/cop/mixin/configurable_naming'
+require 'rubocop/cop/mixin/def_node'
 require 'rubocop/cop/mixin/documentation_comment'
 require 'rubocop/cop/mixin/empty_lines_around_body'
 require 'rubocop/cop/mixin/end_keyword_alignment'

lib/rubocop/cop/mixin/def_node.rb

@@ -0,0 +1,28 @@
+# encoding: utf-8
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Common functionality for checking if nodes.
+    module DefNode
+      extend NodePattern::Macros
+
+      NON_PUBLIC_MODIFIERS = %w(private protected).freeze
+
+      def non_public?(node)
+        non_public_modifier?(node.parent) ||
+          preceding_non_public_modifier?(node)
+      end
+
+      def preceding_non_public_modifier?(node)
+        stripped_source_upto(node.loc.line).any? do |line|
+          NON_PUBLIC_MODIFIERS.include?(line)
+        end
+      end
+
+      def_node_matcher :non_public_modifier?, <<-PATTERN
+        (send nil {:private :protected} ({def defs} ...))
+      PATTERN
+    end
+  end
+end

lib/rubocop/cop/mixin/documentation_comment.rb

@@ -6,12 +6,13 @@ module Cop
     # Common functionality for checking documentation.
     module DocumentationComment
       extend NodePattern::Macros
+      include Style::AnnotationComment
 
       def_node_matcher :constant_definition?, '{class module casgn}'
 
       private
 
-      def associated_comment?(node)
+      def documentation_comment?(node)
         preceding_lines = preceding_lines(node)
 
         return false unless preceding_comment?(node, preceding_lines.last)
@@ -21,9 +22,9 @@ def associated_comment?(node)
         end
       end
 
-      def preceding_comment?(node, line)
-        line && preceed?(line, node) &&
-          comment_line?(line.loc.expression.source)
+      def preceding_comment?(n1, n2)
+        n1 && n2 && preceed?(n2, n1) &&
+          comment_line?(n2.loc.expression.source)
       end
 
       def preceding_lines(node)

lib/rubocop/cop/style/documentation.rb

@@ -14,7 +14,6 @@ module Style
       # same for all its children.
       class Documentation < Cop
         include DocumentationComment
-        include AnnotationComment
 
         MSG = 'Missing top-level %s documentation comment.'.freeze
 
@@ -38,7 +37,7 @@ def on_module(node)
 
         def check(node, body, type)
           return if namespace?(body)
-          return if associated_comment?(node) || nodoc_comment?(node)
+          return if documentation_comment?(node) || nodoc_comment?(node)
 
           add_offense(node, :keyword, format(MSG, type))
         end

lib/rubocop/cop/style/documentation_method.rb

@@ -4,54 +4,55 @@ module RuboCop
   module Cop
     module Style
       # This cop checks for missing documentation comment for public methods.
+      # It can optionally be configured to also require documentation for
+      # non-public methods.
       #
       # @example
       #
       #   # bad
       #
-      #   class MyClass
-      #     def method
-      #       puts 'method'
+      #   class Foo
+      #     def bar
+      #       puts baz
       #     end
       #   end
       #
-      #   module MyModule
-      #     def method
-      #       puts 'method'
+      #   module Foo
+      #     def bar
+      #       puts baz
       #     end
       #   end
       #
-      #   def my_class.method
-      #     puts 'method'
+      #   def foo.bar
+      #     puts baz
       #   end
       #
       #   # good
       #
-      #   class MyClass
-      #     # Method Comment
-      #     def method
-      #       puts 'method'
+      #   class Foo
+      #     # Documentation
+      #     def bar
+      #       puts baz
       #     end
       #   end
       #
-      #   module MyModule
-      #     # Method Comment
-      #     def method
-      #       puts 'method'
+      #   module Foo
+      #     # Documentation
+      #     def bar
+      #       puts baz
       #     end
       #   end
       #
-      #   # Method Comment
-      #   def my_class.method
-      #     puts 'method'
+      #   # Documenation
+      #   def foo.bar
+      #     puts baz
       #   end
       class DocumentationMethod < Cop
         include DocumentationComment
-        include AnnotationComment
         include OnMethodDef
+        include DefNode
 
         MSG = 'Missing method documentation comment.'.freeze
-        NON_PUBLIC_MODIFIERS = %w(private protected).freeze
 
         def on_def(node)
           check(node)
@@ -64,30 +65,15 @@ def on_method_def(node, *)
         private
 
         def check(node)
-          return if non_public_method?(node)
-          return if associated_comment?(node)
+          return if non_public?(node) && !require_for_non_public_methods?
+          return if documentation_comment?(node)
 
-          add_offense(node, :keyword, MSG)
+          add_offense(node, :expression, MSG)
         end
 
-        def non_public_method?(node)
-          non_public_modifier?(node.parent) ||
-            preceding_non_public_modifier?(node)
+        def require_for_non_public_methods?
+          cop_config['RequireForNonPublicMethods']
         end
-
-        def preceding_non_public_modifier?(node)
-          stripped_source_upto(node.loc.line).any? do |line|
-            NON_PUBLIC_MODIFIERS.include?(line)
-          end
-        end
-
-        def stripped_source_upto(line)
-          processed_source[0..line].map(&:strip)
-        end
-
-        def_node_matcher :non_public_modifier?, <<-PATTERN
-          (send nil {:private :protected} ({def defs} ...))
-        PATTERN
       end
     end
   end

lib/rubocop/cop/util.rb

@@ -258,6 +258,10 @@ def line_distance(n1, n2)
       def preceed?(n1, n2)
         line_distance(n1, n2) == 1
       end
+
+      def stripped_source_upto(line)
+        processed_source[0..line].map(&:strip)
+      end
     end
   end
 end