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

Rails/HttpStatus not working after requiring rubocop-rspec #611

Closed
a14m opened this issue May 14, 2018 · 15 comments
Closed

Rails/HttpStatus not working after requiring rubocop-rspec #611

a14m opened this issue May 14, 2018 · 15 comments

Comments

@a14m
Copy link

a14m commented May 14, 2018

After adding require: rubocop-rspec to the .rubocop.yml, the Rails/HttpStatus doesn't check the code, only the specs

first reported here: rubocop/rubocop#5738 (comment)

rubocop-rspec 1.25.1

@anthony-robin
Copy link
Contributor

anthony-robin commented May 26, 2018

I think there is a conflict of cops name between rubocop and rubocop-rspec as Rails/HttpStatus is defined in both gems with the same name and rubocop-rspec might override folders to inspect.

I'm not sure how we can fix that 🤔

@flyerhzm
Copy link

could we rename Rails/HttpStatus to RSpec/Rails/HttpStatus?

@dmytro-savochkin
Copy link

Since this is still a problem and it's hard to find out how to fix this maybe it would be better to rename RSpec/Rails/HttpStatus to RSpec/StatusHttp? Yes, it might be ugly but at least this won't have any conflicts...

@ixti
Copy link

ixti commented Sep 19, 2019

Any news/plans on fixing this?

@ixti
Copy link

ixti commented Sep 19, 2019

Right now I have to switch orders of requires to check both cops worked...

@ixti
Copy link

ixti commented Oct 27, 2019

I have fixed this issue locally with following hacks:

# file: rubocop/fixes/http_status.rb
# frozen_string_literal: true

require "rubocop/cop/rails/http_status"
require "rubocop/cop/rspec/rails/http_status"

RuboCop::Cop::Cop.registry.instance_eval do
  [
    RuboCop::Cop::Rails::HttpStatus,
    RuboCop::Cop::RSpec::Rails::HttpStatus
  ].each do |cop|
    @registry.delete cop.badge
    @departments[cop.department].delete cop
    @cops_by_cop_name[cop.cop_name].delete cop
  end
end

module RuboCop
  module Cop
    module RSpec
      module Rails
        class HttpStatus < Cop
          def self.badge
            *departament, cop_name = name.split("::").last(3)
            Badge.new(departament.join("/"), cop_name)
          end
        end
      end
    end
  end
end

[RuboCop::Cop::Rails::HttpStatus, RuboCop::Cop::RSpec::Rails::HttpStatus]
  .each { |cop| RuboCop::Cop::Cop.registry.enlist cop }

# file: rubocop/fixes/http_status.yml

RSpec/Rails/HttpStatus:
  Description: Enforces use of symbolic or numeric value to describe HTTP status.
  Enabled: true
  EnforcedStyle: symbolic
  SupportedStyles:
  - numeric
  - symbolic
  StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/HttpStatus

# file: .rubocop.yml

require:
  - ./rubocop/fixes/http_status.rb

inherit:
  - ./rubocop/fixes/http_status.yml

@ixti
Copy link

ixti commented Oct 27, 2019

I've added initial work to fix this issue:

ixti@ac8f048

@pirj
Copy link
Member

pirj commented Oct 27, 2019

@ixti the code required to support sub-departments looks non-trivial, I'm not sure if it will have a warm welcome in RuboCop.

WDYT of renaming this cop to RSpecRails/HttpStatus? have_http_status is an RSpec-Rails' matcher anyway.
This naming seems to be more consistent with other departments (Capybara, FactoryBot).

@ixti
Copy link

ixti commented Oct 27, 2019

@pirj Yeah, I thought that it won't be welcomed :D It was more like a "what about" example to have some input ideas on. And I totally love your renaming idea.

ixti added a commit to ixti/rubocop-rspec that referenced this issue Oct 27, 2019
@ixti
Copy link

ixti commented Oct 27, 2019

@pirj opened a PR with renaming (#834).

@haines
Copy link

haines commented May 6, 2020

Did someone end up opening an issue to fix this on the RuboCop side (#834 (comment))? I can't find one.

@pirj
Copy link
Member

pirj commented May 26, 2020

@haines Unfortunately not. Would you like to handle this?
References:
#834 (comment)
rubocop/rubocop#5251

@haines
Copy link

haines commented May 27, 2020

Opened rubocop/rubocop#8044

@pirj
Copy link
Member

pirj commented May 27, 2020

Thanks @haines !

@pirj
Copy link
Member

pirj commented Nov 6, 2020

This should be fixed in #1019 that is part of the release 2.0.

@pirj pirj closed this as completed Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants