Merge pull request #259 from backus/feature/RepeatedDescription

Detect repeated descriptions in example groups

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * 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][])
 * Add `SingleArgumentMessageChain` cop for recommending use of `receive` instead of `receive_message_chain` where possible. ([@bquorning][])
+* Add `RepeatedDescription` cop for detecting repeated example descriptions within example groups. ([@backus][])
 
 ## 1.8.0 (2016-10-27)
 

config/default.yml

@@ -137,6 +137,10 @@ RSpec/NotToNot:
   - to_not
   Enabled: true
 
+RSpec/RepeatedDescription:
+  Enabled: true
+  Description: Check for repeated description strings in example groups.
+
 RSpec/SingleArgumentMessageChain:
   Description: Checks that chains of messages contain more than one element.
   Enabled: true

lib/rubocop-rspec.rb

@@ -40,6 +40,7 @@
 require 'rubocop/cop/rspec/named_subject'
 require 'rubocop/cop/rspec/nested_groups'
 require 'rubocop/cop/rspec/not_to_not'
+require 'rubocop/cop/rspec/repeated_description'
 require 'rubocop/cop/rspec/single_argument_message_chain'
 require 'rubocop/cop/rspec/subject_stub'
 require 'rubocop/cop/rspec/verified_doubles'

lib/rubocop/cop/rspec/repeated_description.rb

@@ -0,0 +1,87 @@
+module RuboCop
+  module Cop
+    module RSpec
+      # Check for repeated description strings in example groups.
+      #
+      # @example
+      #
+      #     # bad
+      #     RSpec.describe User do
+      #       it 'is valid' do
+      #         # ...
+      #       end
+      #
+      #       it 'is valid' do
+      #         # ...
+      #       end
+      #     end
+      #
+      #     # good
+      #     RSpec.describe User do
+      #       it 'is valid when first and last name are present' do
+      #         # ...
+      #       end
+      #
+      #       it 'is valid when last name only is present' do
+      #         # ...
+      #       end
+      #     end
+      #
+      class RepeatedDescription < Cop
+        def_node_matcher :example, <<-PATTERN
+          (block $(send _ #{Examples::ALL.node_pattern_union} str ...) ...)
+        PATTERN
+
+        # @!method scope_change?(node)
+        #
+        #   Detect if the node is an example group or shared example
+        #
+        #   Selectors which indicate that we should stop searching
+        #
+        def_node_matcher :scope_change?,
+                         (ExampleGroups::ALL + SharedGroups::ALL).block_pattern
+
+        MSG = "Don't repeat descriptions within an example group.".freeze
+
+        def on_block(node)
+          return unless example_group?(node)
+
+          repeated_descriptions(node).each do |repeated_hook|
+            add_offense(repeated_hook, :expression)
+          end
+        end
+
+        private
+
+        # Select examples in the current scope with repeated description strings
+        def repeated_descriptions(node)
+          examples_in_scope(node)    # Select examples in example group
+            .group_by(&:method_args) # Group examples by description string
+            .values                  # Reduce to array of grouped examples
+            .reject(&:one?)          # Reject groups with only one example
+            .flatten                 # Flatten down to array of offending nodes
+        end
+
+        def examples_in_scope(node, &block)
+          return to_enum(__method__, node) unless block_given?
+
+          node.each_child_node { |child| find_examples(child, &block) }
+        end
+
+        # Recursively search for examples within the current scope
+        #
+        # Searches node for examples and halts when a scope change is detected
+        #
+        # @param node [RuboCop::Node] node to recursively search for examples
+        #
+        # @yield [RuboCop::Node] discovered example nodes
+        def find_examples(node, &block)
+          return if scope_change?(node)
+
+          example(node, &block)
+          examples_in_scope(node, &block)
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/rspec/described_class_spec.rb

@@ -160,7 +160,7 @@ module MyModle
     RUBY
   end
 
-  it 'does not flag violations within a scope change' do
+  it 'does not flag violations within a class scope change' do
     expect_no_violations(<<-RUBY)
       describe MyNamespace::MyClass do
         before do
@@ -172,7 +172,7 @@ class Foo
     RUBY
   end
 
-  it 'does not flag violations within a scope change' do
+  it 'does not flag violations within a hook scope change' do
     expect_no_violations(<<-RUBY)
       describe do
         before do

spec/rubocop/cop/rspec/implicit_expect_spec.rb

@@ -30,7 +30,7 @@
       expect_no_violations('it { is_expected.to_not be_truthy }')
     end
 
-    it 'approves of is_expected.to_not' do
+    it 'approves of is_expected.not_to' do
       expect_no_violations('it { is_expected.not_to be_truthy }')
     end
 

spec/rubocop/cop/rspec/repeated_description_spec.rb

@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+describe RuboCop::Cop::RSpec::RepeatedDescription do
+  subject(:cop) { described_class.new }
+
+  it 'registers an offense for repeated descriptions' do
+    expect_violation(<<-RUBY)
+      describe 'doing x' do
+        it "does x" do
+        ^^^^^^^^^^^ Don't repeat descriptions within an example group.
+        end
+
+        it "does x" do
+        ^^^^^^^^^^^ Don't repeat descriptions within an example group.
+        end
+      end
+    RUBY
+  end
+
+  it 'registers offense for repeated descriptions separated by a context' do
+    expect_violation(<<-RUBY)
+      describe 'doing x' do
+        it "does x" do
+        ^^^^^^^^^^^ Don't repeat descriptions within an example group.
+        end
+
+        context 'during some use case' do
+          it "does x" do
+            # this should be fine
+          end
+        end
+
+        it "does x" do
+        ^^^^^^^^^^^ Don't repeat descriptions within an example group.
+        end
+      end
+    RUBY
+  end
+
+  it 'ignores descriptions repeated in a shared context' do
+    expect_no_violations(<<-RUBY)
+      describe 'doing x' do
+        it "does x" do
+        end
+
+        shared_context 'shared behavior' do
+          it "does x" do
+          end
+        end
+      end
+    RUBY
+  end
+
+  it 'ignores repeated descriptions in a nested context' do
+    expect_no_violations(<<-RUBY)
+      describe 'doing x' do
+        it "does x" do
+        end
+
+        context 'in a certain use case' do
+          it "does x" do
+          end
+        end
+      end
+    RUBY
+  end
+
+  it 'does not flag tests which do not contain description strings' do
+    expect_no_violations(<<-RUBY)
+      describe 'doing x' do
+        it { foo }
+        it { bar }
+      end
+    RUBY
+  end
+end