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

Improve RSpec/DescribedClass cop #1741

Closed
ydakuka opened this issue Nov 16, 2023 · 8 comments · Fixed by #1806 or #1825
Closed

Improve RSpec/DescribedClass cop #1741

ydakuka opened this issue Nov 16, 2023 · 8 comments · Fixed by #1806 or #1825

Comments

@ydakuka
Copy link

ydakuka commented Nov 16, 2023

Describe the solution you'd like

RSpec.describe Site do
  subject(:site) { create(:site, value: value) }

  # bad
  let(:value) { Site::MAX_VALUE }
  # good
  let(:value) { described_class::MAX_VALUE }

  specify do
    expect(site).not_to be_nil
  end
end

Rubocop

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop -V
1.57.2 (using Parser 3.2.2.4, rubocop-ast 1.29.0, running on ruby 2.7.8) [x86_64-linux]
  - rubocop-capybara 2.19.0
  - rubocop-factory_bot 2.24.0
  - rubocop-performance 1.19.1
  - rubocop-rails 2.22.0
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.25.0
  - rubocop-thread_safety 0.5.1
@ydah
Copy link
Member

ydah commented Nov 21, 2023

We don't know if it is a class name or a constant name? For example, if we use Foo::A or Foo::HTML, we don't know whether A or HTML is really a class name or a constant name.

@pirj
Copy link
Member

pirj commented Nov 21, 2023

Do we care if the A const is assigned a class or a value? We only care about Foo here, and want to detect cases where it is used as a namespace like Foo::A, right?

@ydah
Copy link
Member

ydah commented Nov 21, 2023

Ah, I see. I was mistaken.

@timon
Copy link

timon commented Mar 1, 2024

This change makes a lot of our CI pipelines fail, with typical example being a gem autogenerated by bundle:

 RSpec.describe MyNewGem do 
   it 'has a version number' do 
     expect(MyNewGem::VERSION).not_to be_nil 
   end 
 end 

For things like these I would prefer full name for verbosity and explicitness.
Is there a way to configure this cop to still accept old style of referencing namespaced constants, besides having an engineer to go through this and manually putting a disable comment? (Yes, I'm aware of autocorrect — no, I don't want to autocorrect these)

@bquorning
Copy link
Collaborator

There’s the EnforcedStyle: explicit configuration option, as documented in https://docs.rubocop.org/rubocop-rspec/cops_rspec.html#rspecdescribedclass. There is not currently an option to enforce different styles for “stand-alone constants” (e.g. MyNewGem) vs namespaces (e.g. MyNewGem::VERSION).

@timon
Copy link

timon commented Mar 1, 2024

There is not currently an option to enforce different styles for “stand-alone constants”

:(

We prefer described_class in general. As it is not possible to make exceptions for nested constants I think we still have to go with autocorrect and change them to be described_class::VERSION

@bquorning
Copy link
Collaborator

…or temporarily disable the cop, or wait with the upgrade until @ydah or @ydakuka comments on this issue. 😄

@dduugg
Copy link
Contributor

dduugg commented Mar 1, 2024

I would also like a configuration option to disable this change, as it breaks Sorbet type checking, which doesn't support dynamic constant references: https://sorbet.org/docs/error-reference#5001

ydah added a commit to rubocop/rubocop-rspec_rails that referenced this issue Mar 27, 2024
ydah added a commit to rubocop/rubocop-rspec_rails that referenced this issue Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment