Rubocop-rspec: DescribeClass raises violation incorrectly when Rspec.describe is used

[armahillo]

I have a class:

class Foo
# ...
end

tested by:

require './lib/foo.rb'

Rspec.describe Foo do
# ...
end

and it raises this violation:

spec/foo_spec.rb:5:16: C: RSpec/DescribeClass: The first argument to describe should be the class or module being tested.
 Rspec.describe Foo do
                ^^^

but changing the spec to:

require './lib/foo.rb'

describe Foo do
# ...
end

clears the violation.

It's not a huge issue, but were I to be the type to disable_monkey_patching! in my spec_helper.rb this would create an unnecessary problem.

[pirj]

May I ask you to provide more context @armahillo, it seems to detect the offence at line 5, what are the first four lines?

PS Monkey patching will be removed in RSpec 4, so probably it's the way to go in any case.

[armahillo]

@pirj Certainly!

Lines 1 & 2 were just a constant that I had defined:

DEMOFILE = 'spec/support/foo.txt'
# this line was blank

[pirj]

What is the version of rubocop-rspec you are using?

I've tried to reproduce on master:

# spec/a_spec.rb
DEMOFILE = 'spec/support/foo.txt'

require './lib/foo.rb'

RSpec.describe Foo do
  describe do
    it {}
  end
end

$ rubocop --only RSpec/DescribeClass spec/a_spec.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

[coding-bunny]

running into the same problem:

 bundle show rubocop-rspec
/home/olivar/.rvm/gems/ruby-2.3.0/gems/rubocop-rspec-1.33.0

This is the test:

require "rails_helper"

::RSpec.describe ReplicatorService do
  subject(:replicator_service) { described_class.instance }

  let(:style) { create(:style) }
  let(:name) { "Copy of #{style.name}" }
  let(:email) { "[email protected]" }

  describe "#clone_style" do
    it "raises an ArgumentError when the provided name is already in use" do
      expect { replicator_service.clone_style(id: style.id, name: style.name, email: email) }.to(
        raise_error(::ArgumentError, "There is already a Style with the given name: #{style.name}")
      )
    end

    it "raises an ArgumentError when no email is provided" do
      expect { replicator_service.clone_style(id: style.id, name: name, email: "") }.to(
        raise_error(::ArgumentError, "An email is required to perform this operation")
      )
    end

    it "raises an ArgumentError when an invalid ID is provided" do
      expect { replicator_service.clone_style(id: -1, name: name, email: email) }.to(
        raise_error(::ArgumentError, "Cannot copy a non-existing Style: -1")
      )
    end

    it "schedules a CopyStyleJob" do
      expect(::CopyStyleJob).to(receive(:perform_later).with(style, name, email))

      replicator_service.clone_style(id: style.id, name: name, email: email)
    end
  end
end

And Rubocop complains about line 3
However if I replace the top with RSpec.describe ReplicatorService do it works.....

[pirj]

I can reproduce it.

But why would you use ::RSpec instead of RSpec, @coding-bunny ?

The fix is trivial:

# lib/rubocop/rspec/language.rb
- RSPEC = '{(const nil? :RSpec) nil?}'
+ RSPEC = '{(const { nil? cbase } :RSpec) nil?}'

(plus a change to spec/rubocop/rspec/language/selector_set_spec.rb).

@bquorning @Darhazer WDYT?

PS Just noticed an incorrect spelling of RSpec in your code @armahillo, it's Rspec. Does this even work? For me:

NameError:
  uninitialized constant Rspec
  Did you mean?  RSpec

[coding-bunny]

@pirj We use absolute paths for most classes to prevent loading issues/clashing with code.
And it works fine for pretty much everything, except this file was behaving oddly.....and others got accepted by Rubocop for some reason.

it's...odd, but the work around is sufficient for us right now.
Just wanted to raise awareness to the behavior for this Cop

[pirj]

We can definitely address this edge case since being precautious to loading issues is not something uncommon.

@armahillo I can reproduce the issue with Rspec, but it's not the case if I change it to RSpec. Can you please provide more details on how Rspec (with second letter lowercase) is defined?

[mikegee]

Rspec was removed in favor of RSpec in 3.0. https://github.com/rspec/rspec-core/blob/master/Changelog.md#300beta1--2013-11-07

[pirj]

Nice find @mikegee 👍

Are we in a position to support pre-3.0 RSpec?

Fun fact: just spotted Rspec in the README.

[pirj]

RSpec Style Guide explicitly states that:

This guide assumes you are using RSpec 3 or later.

Since we're moving towards cohesion and coherence with the guide, WDYT of dropping support for pre-RSpec 3.0 constructs if there are such left?

[Darhazer]

Since we're moving towards cohesion and coherence with the guide, WDYT of dropping support for pre-RSpec 3.0 constructs if there are such left?

supporting pre-RSpec 3.0 things is a no-goal and forcing migration, as long as RSpec have a mechanism on itself, is a no-goal as well. We may need to clarify further in the no-goals section of the readme.