Merge pull request #157 from nevir/feature/ExpectActual

Add RSpec/ExpectActual cop

CHANGELOG.md

@@ -9,6 +9,7 @@
 * Add `CustomIncludeMethods` configuration option for `EmptyExampleGroup`. ([@backus][])
 * Add `NestedGroups` cop for detecting excessive example group nesting. ([@backus][])
 * Add `MaxNesting` configuration option for `NestedGroups` cop. ([@backus][])
+* Add `ExpectActual` cop for detecting literal values within `expect(...)`. ([@backus][])
 
 ## 1.6.0 (2016-08-03)
 

config/default.yml

@@ -29,6 +29,10 @@ RSpec/EmptyExampleGroup:
   Enabled: true
   CustomIncludeMethods: []
 
+RSpec/ExpectActual:
+  Description: 'Checks for `expect(...)` with literal values'
+  Enabled: true
+
 RSpec/MultipleDescribes:
   Description: 'Checks for multiple top level describes.'
   Enabled: true

lib/rubocop-rspec.rb

@@ -17,6 +17,7 @@
 require 'rubocop/cop/rspec/empty_example_group'
 require 'rubocop/cop/rspec/example_length'
 require 'rubocop/cop/rspec/example_wording'
+require 'rubocop/cop/rspec/expect_actual'
 require 'rubocop/cop/rspec/file_path'
 require 'rubocop/cop/rspec/focus'
 require 'rubocop/cop/rspec/instance_variable'

lib/rubocop/cop/rspec/expect_actual.rb

@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Checks for literal values within `expect(...)`
+      #
+      # @example
+      #   # bad
+      #   expect(5).to eq(price)
+      #   expect(/foo/).to eq(pattern)
+      #   expect("John").to eq(name)
+      #
+      #   # good
+      #   expect(price).to eq(5)
+      #   expect(pattern).to eq(/foo/)
+      #   expect(name).to eq("John")
+      #
+      class ExpectActual < Cop
+        MSG = 'Provide the actual you are testing to `expect(...)`'.freeze
+
+        SIMPLE_LITERALS = %i(
+          true
+          false
+          nil
+          int
+          float
+          str
+          sym
+          complex
+          rational
+          regopt
+        ).freeze
+
+        COMPLEX_LITERALS = %i(
+          array
+          hash
+          pair
+          irange
+          erange
+          regexp
+        ).freeze
+
+        def_node_matcher :expect, '(send _ :expect $_)'
+
+        def on_send(node)
+          expect_literal(node) do |argument|
+            add_offense(argument, :expression)
+          end
+        end
+
+        private
+
+        # This is not implement using a NodePattern because it seems
+        # to not be able to match against an explicit (nil) sexp
+        def expect_literal(node)
+          return unless (argument = expect(node))
+
+          yield(argument) if literal?(argument)
+        end
+
+        def literal?(node)
+          simple_literal?(node) || complex_literal?(node)
+        end
+
+        def simple_literal?(node)
+          SIMPLE_LITERALS.include?(node.type)
+        end
+
+        def complex_literal?(node)
+          COMPLEX_LITERALS.include?(node.type) &&
+            node.each_child_node.all?(&method(:literal?))
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/rspec/expect_actual_spec.rb

@@ -0,0 +1,136 @@
+# frozen_string_literal: true
+
+describe RuboCop::Cop::RSpec::ExpectActual do
+  subject(:cop) { described_class.new }
+
+  it 'flags numeric literal values within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect(123).to eq(bar)
+                 ^^^ Provide the actual you are testing to `expect(...)`
+          expect(12.3).to eq(bar)
+                 ^^^^ Provide the actual you are testing to `expect(...)`
+          expect(1i).to eq(bar)
+                 ^^ Provide the actual you are testing to `expect(...)`
+          expect(1r).to eq(bar)
+                 ^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'flags boolean literal values within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect(true).to eq(bar)
+                 ^^^^ Provide the actual you are testing to `expect(...)`
+          expect(false).to eq(bar)
+                 ^^^^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'flags string and symbol literal values within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect("foo").to eq(bar)
+                 ^^^^^ Provide the actual you are testing to `expect(...)`
+          expect(:foo).to eq(bar)
+                 ^^^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'flags literal nil value within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect(nil).to eq(bar)
+                 ^^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'does not flag dynamic values within expect(...)' do
+    expect_no_violations(<<-'RUBY')
+      describe Foo do
+        it 'uses expect correctly' do
+          expect(foo).to eq(bar)
+          expect("foo#{baz}").to eq(bar)
+          expect(:"foo#{baz}").to  eq(bar)
+        end
+      end
+    RUBY
+  end
+
+  it 'flags arrays containing only literal values within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect([123]).to eq(bar)
+                 ^^^^^ Provide the actual you are testing to `expect(...)`
+          expect([[123]]).to eq(bar)
+                 ^^^^^^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'flags hashes containing only literal values within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect(foo: 1, bar: 2).to eq(bar)
+                 ^^^^^^^^^^^^^^ Provide the actual you are testing to `expect(...)`
+          expect(foo: 1, bar: [{}]).to eq(bar)
+                 ^^^^^^^^^^^^^^^^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'flags ranges containing only literal values within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect(1..2).to eq(bar)
+                 ^^^^ Provide the actual you are testing to `expect(...)`
+          expect(1...2).to eq(bar)
+                 ^^^^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'flags regexps containing only literal values within expect(...)' do
+    expect_violation(<<-RUBY)
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect(/foo|bar/).to eq(bar)
+                 ^^^^^^^^^ Provide the actual you are testing to `expect(...)`
+        end
+      end
+    RUBY
+  end
+
+  it 'does not flag complex values with dynamic parts within expect(...)' do
+    expect_no_violations(<<-'RUBY')
+      describe Foo do
+        it 'uses expect incorrectly' do
+          expect([foo]).to eq(bar)
+          expect([[foo]]).to eq(bar)
+          expect(foo: 1, bar: foo).to eq(bar)
+          expect(1..foo).to eq(bar)
+          expect(1...foo).to eq(bar)
+          expect(/foo|#{bar}/).to eq(bar)
+        end
+      end
+    RUBY
+  end
+end