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

Rubocop update #2768

Merged
merged 6 commits into from
Sep 1, 2021
Merged

Rubocop update #2768

merged 6 commits into from
Sep 1, 2021

Conversation

sonalkr132
Copy link
Member

related #2760

not updating to the most recent versions as that would be too many changes.

Offenses:

app/controllers/reverse_dependencies_controller.rb:13:110: W: Lint/RedundantSafeNavigation: Redundant safe navigation detected.
    @reverse_dependencies = @reverse_dependencies.legacy_search(params[:rdeps_query]) if params[:rdeps_query]&.is_a?(String)
                                                                                                             ^^^^^^^^^^^^^^^
app/controllers/searches_controller.rb:5:33: W: Lint/RedundantSafeNavigation: Redundant safe navigation detected.
    return unless params[:query]&.is_a?(String)
                                ^^^^^^^^^^^^^^^
app/helpers/application_helper.rb:67:42: C: Rails/ContentTag: Use tag instead of content_tag. (rails/rails#25195, https://ap
i.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-content_tag)
    api_key.enabled_scopes.map { |scope| content_tag(:ul, t(".#{scope}"), class: "scopes__list") }.reduce(&:+)
                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/helpers/application_helper.rb:67:100: C: Performance/Sum: Use sum instead of reduce(&:+), unless calling reduce(&:+) on an empty array. (https://b
log.bigbinary.com/2016/11/02/ruby-2-4-introduces-enumerable-sum.html)
    api_key.enabled_scopes.map { |scope| content_tag(:ul, t(".#{scope}"), class: "scopes__list") }.reduce(&:+)
                                                                                                   ^^^^^^^^^^^
app/helpers/rubygems_helper.rb:30:7: C: Rails/ContentTag: Use tag instead of content_tag. (rails/rails#25195, https://api.ru
byonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-content_tag)
      content_tag :p, escape_once(sanitize(text.strip)), nil, false
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/jobs/fastly_log_processor.rb:56:10: C: Performance/CollectionLiteralInLoop: Avoid immutable Array literals in loops. It is better to extract it in
to a local variable or a constant.
      if [200, 304].include?(response_code.to_i) && (match = path.match %r{/gems/(?<path>.+)\.gem})
         ^^^^^^^^^^
app/models/concerns/rubygem_searchable.rb:82:20: C: Rails/SquishedSQLHeredocs: Use <<-SQL.squish instead of <<-SQL. (https://rails.rubystyle.guide/#sq
uished-heredocs)
      conditions = <<-SQL
                   ^^^^^^
app/models/gem_download.rb:79:81: C: Rails/Pick: Prefer pick(:count) over pluck(:count).first. (https://rails.rubystyle.guide#pick)
      count = GemDownload.where(rubygem_id: rubygem_id, version_id: version_id).pluck(:count).first
                                                                                ^^^^^^^^^^^^^^^^^^^
app/models/ownership.rb:13:26: C: Rails/WhereNot: Use where.not(confirmed_at: nil) instead of manually constructing negated SQL in where. (https://rai
ls.rubystyle.guide/#hash-conditions)
  scope :confirmed, -> { where("confirmed_at IS NOT NULL") }
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/ownership.rb:14:28: C: Rails/WhereEquals: Use where(confirmed_at: nil) instead of manually constructing SQL. (https://rails.rubystyle.guide
/#hash-conditions)
  scope :unconfirmed, -> { where("confirmed_at IS NULL") }
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/rubygem.rb:32:3: C: Rails/ActiveRecordCallbacksOrder: after_create is supposed to appear before before_destroy. (https://rails.rubystyle.gu
ide/#callbacks-order)
  after_create :create_gem_download
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/rubygem.rb:151:28: C: Rails/WhereExists: Prefer exists?(user: user) over where(user: user).exists?.
    unconfirmed_ownerships.where(user: user).exists?
                           ^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/rubygem.rb:212:17: C: Performance/ConstantRegexp: Extract this regexp into a constant, memoize it, or append an /o option to its options.
    name.remove(/[^#{Patterns::ALLOWED_CHARACTERS}]/)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/rubygem.rb:326:11: C: Performance/ConstantRegexp: Extract this regexp into a constant, memoize it, or append an /o option to its options.
    elsif /\A[#{Regexp.escape(Patterns::SPECIAL_CHARACTERS)}]+/.match?(name)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/sendgrid_event.rb:25:15: C: Rails/WhereExists: Prefer exists?(sendgrid_id: payload[:sg_event_id]) over where(sendgrid_id: payload[:sg_event
_id]).exists?.
    return if where(sendgrid_id: payload[:sg_event_id]).exists?
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/user.rb:32:3: C: Rails/ActiveRecordCallbacksOrder: before_save is supposed to appear before before_destroy. (https://rails.rubystyle.guide/
  before_save :generate_confirmation_token, if: :will_save_change_to_unconfirmed_email?
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/user.rb:271:10: C: Rails/WhereExists: Prefer exists?(email: unconfirmed_email) over where(email: unconfirmed_email).exists?.
    User.where(email: unconfirmed_email).exists?
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/version.rb:9:43: C: Rails/RedundantForeignKey: Specifying the default value for foreign_key is redundant.
  belongs_to :pusher, class_name: "User", foreign_key: "pusher_id", inverse_of: false, optional: true
                                          ^^^^^^^^^^^^^^^^^^^^^^^^
app/models/version.rb:12:3: C: Rails/ActiveRecordCallbacksOrder: before_validation is supposed to appear before before_save. (https://rails.rubystyle.
guide/#callbacks-order)
  before_validation :full_nameify!
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/version.rb:19:88: C: Performance/ConstantRegexp: Extract this regexp into a constant, memoize it, or append an /o option to its options.
  validates :number, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: /\A#{Gem::Version::VERSION_PATTERN}\z/ }
                                                                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/version.rb:44:3: C: Rails/ActiveRecordCallbacksOrder: after_create is supposed to appear before after_save. (https://rails.rubystyle.guide/
  after_create :create_gem_download
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/models/version.rb:132:16: C: Rails/SquishedSQLHeredocs: Use <<-SQL.squish instead of <<-SQL. (https://rails.rubystyle.guide/#squished-heredocs)
    subquery = <<-SQL
               ^^^^^^
app/models/version.rb:142:16: C: Rails/SquishedSQLHeredocs: Use <<-SQL.squish instead of <<-SQL. (https://rails.rubystyle.guide/#squished-heredocs)
    subquery = <<-SQL
               ^^^^^^
config/initializers/gem_version_monkeypatch.rb:10:3: W: Lint/UselessMethodDefinition: Useless method definition detected.
  def self.new(version) ...
  ^^^^^^^^^^^^^^^^^^^^^
config/initializers/rack_attack.rb:81:3: C: Style/CombinableLoops: Combine this loop with the previous loop.
  EXP_BACKOFF_LEVELS.each do |level| ...
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
config/routes.rb:14:19: C: Performance/ConstantRegexp: Extract this regexp into a constant, memoize it, or append an /o option to its options.
          number: /#{Gem::Version::VERSION_PATTERN}(?=\.json\z)|#{Gem::Version::VERSION_PATTERN}(?=\.yaml\z)|#{Gem::Version::VERSION_PATTERN}/
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/gemcutter/middleware/redirector.rb:12:10: C: Rails/NegateInclude: Use .exclude? and remove the negation part. (https://rails.rubystyle.guide#exclu
de)
      if !allowed_hosts.include?(request.host) && request.path !~ %r{^/api|^/internal} && request.host !~ /docs/
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test/functional/owners_controller_test.rb:93:44: C: Rails/WhereNot: Use where.not(last_error: nil) instead of manually constructing negated SQL in whe
re. (https://rails.rubystyle.guide/#hash-conditions)
              assert_equal 0, Delayed::Job.where("last_error is not null").count
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test/functional/stats_controller_test.rb:62:17: C: Rails/Pluck: Prefer pluck(:style) over map { |h| h[:style] }. (https://rails.rubystyle.guide#pluck)
        element.map { |h| h[:style] }.each do |width|
                ^^^^^^^^^^^^^^^^^^^^^
test/functional/users_controller_test.rb:33:62: C: Rails/Pick: Prefer pick(:handle) over pluck(:handle).first. (https://rails.rubystyle.guide#pick)
        assert_equal "foo", User.where(email: "foo@bar.com").pluck(:handle).first
                                                             ^^^^^^^^^^^^^^^^^^^^
test/functional/users_controller_test.rb:38:69: C: Rails/Pick: Prefer pick(:api_key) over pluck(:api_key).first. (https://rails.rubystyle.guide#pick)
        assert_not_equal "nonono", User.where(email: "foo@bar.com").pluck(:api_key).first
                                                                    ^^^^^^^^^^^^^^^^^^^^^
test/unit/gem_download_test.rb:84:33: C: Performance/Sum: Use sum instead of inject(0, :+). (https://blog.bigbinary.com/2016/11/02/ruby-2-4-introduces-enumerable-sum.html)
          total_count = @counts.inject(0, :+)
                                ^^^^^^^^^^^^^

273 files inspected, 32 offenses detected, 30 offenses auto-correctable
needed to ingore:
config/initializers/gem_version_monkeypatch.rb:10:3: W: Lint/UselessMethodDefinition: Useless method definition detected.
Error:
OwnerTest#test_adding_owner_via_UI_with_email:
ActionView::Template::Error: wrong number of arguments (given 4, expected 1..2)
    app/helpers/rubygems_helper.rb:30:in `simple_markup'
    app/views/rubygems/show.html.erb:19
    app/controllers/rubygems_controller.rb:28:in `show'
    lib/clearance_backdoor.rb:9:in `call'
    test/integration/owner_test.rb:201:in `visit_ownerships_page'
    test/integration/owner_test.rb:17:in `block in <class:OwnerTest>'
@@ -79,7 +79,7 @@ def as_indexed_json(_options = {}) # rubocop:disable Metrics/MethodLength
end

def self.legacy_search(query)
conditions = <<-SQL
conditions = <<-SQL.squish
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about to use <<~?

@@ -139,7 +139,7 @@ def self.new_pushed_versions(limit = 5)
end

def self.just_updated(limit = 5)
subquery = <<-SQL
subquery = <<~SQL.squish
Copy link
Member

@simi simi Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the squish still needed? The whole idea of replacing <<- with <<~ was to avoid squish at all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is complaining:

app/models/version.rb:132:16: C: Rails/SquishedSQLHeredocs: Use <<~SQL.squish instead of <<~SQL. (https://rails.rubystyle.guide/#squished-heredocs)
    subquery = <<~SQL
               ^^^^^^

I think the squiggly operator only removes the indentation. squish also move everything to a single line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, ok, sorry for confusion

@sonalkr132 sonalkr132 merged commit 8e6bb76 into rubygems:master Sep 1, 2021
@sonalkr132 sonalkr132 deleted the rubocop-update branch September 1, 2021 09:25
@sonalkr132 sonalkr132 temporarily deployed to staging September 6, 2021 05:23 Inactive
@sonalkr132 sonalkr132 temporarily deployed to production September 6, 2021 17:05 Inactive
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 this pull request may close these issues.

2 participants