[Fix rubocop#3696] Add AllowComments option to Lint/EmptyWhen cop

Fixes rubocop#3696 and rubocop#3754. This PR add `AllowComments` option to `Lint/EmptyWhen` cop. This option is enabled by default based on user feedback. It is also the same default as the option of `Lint/SuppressedException` set in rubocop#7805.
koic · May 10, 2020 · f5917d8 · f5917d8
1 parent 0c3d278
commit f5917d8
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 * [#595](https://github.com/rubocop-hq/rubocop/issues/595): Add ERB pre-processing for configuration files. ([@jonas054][])
 * [#7918](https://github.com/rubocop-hq/rubocop/pull/7918): Support autocorrection for `Lint/AmbiguousOperator`. ([@koic][])
 * [#7937](https://github.com/rubocop-hq/rubocop/pull/7937): Support autocorrection for `Style/IfWithSemicolon`. ([@koic][])
+* [#3696](https://github.com/rubocop-hq/rubocop/issues/3696): Add `AllowComments` option to `Lint/EmptyWhen` cop. ([@robotdana][])
 
 ### Bug fixes
 

diff --git a/config/default.yml b/config/default.yml
@@ -1415,7 +1415,9 @@ Lint/EmptyInterpolation:
 Lint/EmptyWhen:
   Description: 'Checks for `when` branches with empty bodies.'
   Enabled: true
+  AllowComments: true
   VersionAdded: '0.45'
+  VersionChanged: '0.83'
 
 Lint/EnsureReturn:
   Description: 'Do not use return in an ensure block.'

diff --git a/lib/rubocop/cop/lint/empty_when.rb b/lib/rubocop/cop/lint/empty_when.rb
@@ -8,26 +8,49 @@ module Lint
       # @example
       #
       #   # bad
-      #
       #   case foo
-      #   when bar then 1
-      #   when baz then # nothing
+      #   when bar
+      #     do_something
+      #   when baz
       #   end
       #
       # @example
       #
       #   # good
+      #   case condition
+      #   when foo
+      #     do_something
+      #   when bar
+      #     nil
+      #   end
       #
-      #   case foo
-      #   when bar then 1
-      #   when baz then 2
+      # @example AllowComments: true (default)
+      #
+      #   # good
+      #   case condition
+      #   when foo
+      #     do_something
+      #   when bar
+      #     # noop
       #   end
+      #
+      # @example AllowComments: false
+      #
+      #   # bad
+      #   case condition
+      #   when foo
+      #     do_something
+      #   when bar
+      #     # do nothing
+      #   end
+      #
       class EmptyWhen < Cop
         MSG = 'Avoid `when` branches without a body.'
 
         def on_case(node)
           node.each_when do |when_node|
             next if when_node.body
+            next if cop_config['AllowComments'] && comment_lines?(node)
 
             add_offense(when_node, location: when_node.source_range)
           end

diff --git a/lib/rubocop/cop/lint/suppressed_exception.rb b/lib/rubocop/cop/lint/suppressed_exception.rb
@@ -73,12 +73,6 @@ def on_resbody(node)
 
           add_offense(node)
         end
-
-        private
-
-        def comment_lines?(node)
-          processed_source[line_range(node)].any? { |line| comment_line?(line) }
-        end
       end
     end
   end

diff --git a/lib/rubocop/cop/style/empty_method.rb b/lib/rubocop/cop/style/empty_method.rb
@@ -90,10 +90,6 @@ def joint(node)
           compact_style? ? '; ' : "\n#{indent}"
         end
 
-        def comment_lines?(node)
-          processed_source[line_range(node)].any? { |line| comment_line?(line) }
-        end
-
         def compact?(node)
           node.single_line?
         end

diff --git a/lib/rubocop/cop/util.rb b/lib/rubocop/cop/util.rb
@@ -17,6 +17,10 @@ def comment_line?(line_source)
         line_source =~ /^\s*#/
       end
 
+      def comment_lines?(node)
+        processed_source[line_range(node)].any? { |line| comment_line?(line) }
+      end
+
       def line_range(node)
         node.first_line..node.last_line
       end

diff --git a/manual/cops_lint.md b/manual/cops_lint.md
@@ -595,28 +595,57 @@ This cop checks for empty interpolation.
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
 --- | --- | --- | --- | ---
-Enabled | Yes | No | 0.45 | -
+Enabled | Yes | No | 0.45 | 0.83
 
 This cop checks for the presence of `when` branches without a body.
 
 ### Examples
 
 ```ruby
 # bad
-
 case foo
-when bar then 1
-when baz then # nothing
+when bar
+  do_something
+when baz
 end
 ```
 ```ruby
 # good
+case condition
+when foo
+  do_something
+when bar
+  nil
+end
+```
+#### AllowComments: true (default)
 
-case foo
-when bar then 1
-when baz then 2
+```ruby
+# good
+case condition
+when foo
+  do_something
+when bar
+  # noop
 end
 ```
+#### AllowComments: false
+
+```ruby
+# bad
+case condition
+when foo
+  do_something
+when bar
+  # do nothing
+end
+```
+
+### Configurable attributes
+
+Name | Default value | Configurable values
+--- | --- | ---
+AllowComments | `true` | Boolean
 
 ## Lint/EnsureReturn
 

diff --git a/spec/rubocop/cop/lint/empty_when_spec.rb b/spec/rubocop/cop/lint/empty_when_spec.rb
@@ -147,4 +147,30 @@
       end
     RUBY
   end
+
+  context 'when `AllowComments: true`' do
+    let(:cop_config) { { 'AllowComments' => true } }
+
+    it_behaves_like 'code without offense', <<~RUBY
+      case condition
+      when foo
+        do_something
+      when bar
+        # do nothing
+      end
+    RUBY
+  end
+
+  context 'when `AllowComments: false`' do
+    let(:cop_config) { { 'AllowComments' => false } }
+
+    it_behaves_like 'code with offense', <<~RUBY
+      case condition
+      when foo
+        do_something
+      when bar
+        # do nothing
+      end
+    RUBY
+  end
 end