From a8ab7210f264ad8932dade59f26ecde12300e42a Mon Sep 17 00:00:00 2001
From: John Backus <johncbackus@gmail.com>
Date: Mon, 2 Jan 2017 14:29:01 -0800
Subject: [PATCH] Fix bad NestedGroups top level describe behavior

The changes in bd88c07 changes the node pattern for NestedGroups from

    (block (send nil [SELECTORS UNION PATTERN] ...) (args) ...)

to

    (block (send _ [SELECTORS UNION PATTERN] ...) ...)

which led to issue #270 being reported. While this did introduce a
change in behavior, it helped reveal an inconsistency in the
implementation. With the change mentioned above, the following code
generated an offense when it didn't in 1.8.0

    RSpec.describe Foo do
      context 'bar' do
        it 'baz' do
          expect(foo).to_not be(1)
          expect(foo).not_to be(1)
        end
      end

      describe '#qux' do
        context 'when quuz' do
          it 'zyxxy' do
            pass
          end
        end
      end
    end

the following nearly equivalent code *did* generate an offense in 1.8
though:

    describe Foo do
      context 'bar' do
        it 'baz' do
          expect(foo).to_not be(1)
          expect(foo).not_to be(1)
        end
      end

      describe '#qux' do
        context 'when quuz' do
          it 'zyxxy' do
            pass
          end
        end
      end
    end

The problem was that the previous node pattern would only start
counting nesting at the first bare `describe` or `context` and would
not count `RSpec.describe`. It was also probably much less efficient
since it was searching through all nodes inside every example group
instead of only searching from top level describes.

This change fixes #270 by starting a search at the top level describe
and properly handle `RSpec.describe` like `describe`.

This change also convinced me that the current default for NestedGroups
is too aggressive. Almost all of the specs that I write use
`RSpec.describe` which means that rubocop-rspec was more lenient for my
tests. The default MaxNesting is now 3 instead of 2.
---
 CHANGELOG.md                                 | 3 +++
 config/default.yml                           | 2 +-
 lib/rubocop/cop/rspec/nested_groups.rb       | 9 +++------
 spec/rubocop/cop/rspec/nested_groups_spec.rb | 9 ++++-----
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f5eaab08c..491d60302 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## Master (unreleased)
 
+* Fix unintentional regression change in `NestedGroups` reported in #270. ([@backus][])
+* Change `MaxNesting` for `NestedGroups` from 2 to 3. ([@backus][])
+
 ## 1.9.0 (2016-12-29)
 
 * Add `MessageSpies` cop for enforcing consistent style of either `expect(...).to have_received` or `expect(...).to receive`, intended as a replacement for the `MessageExpectation` cop. ([@bquorning][])
diff --git a/config/default.yml b/config/default.yml
index 997ff0485..2d331dc30 100644
--- a/config/default.yml
+++ b/config/default.yml
@@ -127,7 +127,7 @@ RSpec/NamedSubject:
 RSpec/NestedGroups:
   Description: Checks for nested example groups.
   Enabled: true
-  MaxNesting: 2
+  MaxNesting: 3
 
 RSpec/NotToNot:
   Description: Checks for consistent method usage for negating expectations.
diff --git a/lib/rubocop/cop/rspec/nested_groups.rb b/lib/rubocop/cop/rspec/nested_groups.rb
index fb246661a..826be1d98 100644
--- a/lib/rubocop/cop/rspec/nested_groups.rb
+++ b/lib/rubocop/cop/rspec/nested_groups.rb
@@ -91,11 +91,8 @@ class NestedGroups < Cop
 
         def_node_search :find_contexts, ExampleGroups::ALL.block_pattern
 
-        def on_block(node)
-          describe, = described_constant(node)
-          return unless describe
-
-          find_nested_contexts(node) do |context|
+        def on_top_level_describe(node, _)
+          find_nested_contexts(node.parent) do |context|
             add_offense(context.children.first, :expression)
           end
         end
@@ -113,7 +110,7 @@ def find_nested_contexts(node, nesting: 1, &block)
         end
 
         def max_nesting
-          Integer(cop_config.fetch('MaxNesting', 2))
+          Integer(cop_config.fetch('MaxNesting', 3))
         end
       end
     end
diff --git a/spec/rubocop/cop/rspec/nested_groups_spec.rb b/spec/rubocop/cop/rspec/nested_groups_spec.rb
index 25c90d88f..629c6c46c 100644
--- a/spec/rubocop/cop/rspec/nested_groups_spec.rb
+++ b/spec/rubocop/cop/rspec/nested_groups_spec.rb
@@ -8,7 +8,6 @@
       describe MyClass do
         context 'when foo' do
           context 'when bar' do
-          ^^^^^^^^^^^^^^^^^^ Maximum example group nesting exceeded
             context 'when baz' do
             ^^^^^^^^^^^^^^^^^^ Maximum example group nesting exceeded
             end
@@ -17,7 +16,6 @@
 
         context 'when qux' do
           context 'when norf' do
-          ^^^^^^^^^^^^^^^^^^^ Maximum example group nesting exceeded
           end
         end
       end
@@ -35,14 +33,15 @@ class MyThingy
     RUBY
   end
 
-  context 'when MaxNesting is configured as 3' do
-    let(:cop_config) { { 'MaxNesting' => '3' } }
+  context 'when MaxNesting is configured as 2' do
+    let(:cop_config) { { 'MaxNesting' => '2' } }
 
-    it 'only flags third level of nesting' do
+    it 'flags two levels of nesting' do
       expect_violation(<<-RUBY)
         describe MyClass do
           context 'when foo' do
             context 'when bar' do
+            ^^^^^^^^^^^^^^^^^^ Maximum example group nesting exceeded
               context 'when baz' do
               ^^^^^^^^^^^^^^^^^^ Maximum example group nesting exceeded
               end