Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Use described_class" when not possible #59

Closed
aldesantis opened this issue Jan 13, 2016 · 8 comments
Closed

"Use described_class" when not possible #59

aldesantis opened this issue Jan 13, 2016 · 8 comments
Assignees

Comments

@aldesantis
Copy link

RSpec.describe MyConcern do
  before(:all) do
    Temping.create :temp_model do
      include MyConcern
    end
  end

  # ...
end

Reports this offense:

Use described_class instead of MyConcern

But described_class is not defined inside the block, so I can't use it.

@aldesantis aldesantis changed the title 'Use described_class' when not possible "Use described_class" when not possible Jan 13, 2016
@gmalette
Copy link

@Emile-Filteau

@leomao10
Copy link

leomao10 commented Feb 1, 2016

I ran into same issue, here is my code:

RSpec.describe MyModule do
  controller(ApplicationController) do
    include MyModule # rubocop:disable RSpec/DescribedClass

    def index
      my_module_method
      render text: 'Hello World'
    end
  end

controller block don't have described_class method inside

@andyw8
Copy link
Collaborator

andyw8 commented Apr 29, 2016

When you provide a constant to describe, RSpec expects it to be a class name: https://www.relishapp.com/rspec/rspec-core/v/3-4/docs/metadata/described-class

So IMO passing a module is incorrect, and this should be written in an different way, e.g.
RSpec.describe "include MyConcern".

@backus
Copy link
Collaborator

backus commented Aug 3, 2016

I'm looking into fixing this. Basically, the issue here is blocks which are executed using instance_exec and therefore change the value of self within the closure. There are two non-ideal options here:

  1. Disable this cop inside of all non-rspec blocks
  2. Blacklist a handful of common instance_exec'd use cases from ruby core and/or ruby lib (maybe just Class.new { ... }, Module.new { ... })

I am leaning towards the second because I think instance_exec should mainly be avoided and disable is fine in this context. This would fix some cases but it would mean that I would close this as WONTFIX for @alessandro1997's and @leomao10's examples.

Does anyone have any better ideas?

@backus
Copy link
Collaborator

backus commented Aug 3, 2016

cc @pavlo-vavruk @madwort and @mknapik on the comment above ^. I assume the 👍s mean you also want your use cases fixed

@backus
Copy link
Collaborator

backus commented Aug 3, 2016

Another option would be to add an IgnoreClosure option which I would ship as disabled by default. This would disable this cop inside all non-rspec blocks as I mentioned above

backus added a commit that referenced this issue Aug 3, 2016
backus added a commit that referenced this issue Aug 3, 2016
@backus backus closed this as completed in 9bc28e2 Aug 3, 2016
@backus
Copy link
Collaborator

backus commented Aug 3, 2016

I've released 1.6.0 with a SkipBlocks option that you can enable

@madwort
Copy link

madwort commented Aug 3, 2016

Hi @backus yes, the 👍 was because I'd also experienced the auto-correction changing valid code to invalid code in this way. I've just tested 1.6.0 and can confirm that the cop now ignores the relevant bit of code in our project. Thanks for your efforts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants