rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
802 stars 257 forks source link

Cop idea: Named Placeholder cop #1084

Open ydakuka opened 1 year ago

ydakuka commented 1 year ago

Reference: https://rails.rubystyle.guide/#named-placeholder

Actual behavior

I have the code:

# frozen_string_literal: true

class User < ApplicationRecord
  def method
    Client.where(
      'orders_count >= ? AND country_code = ?',
      params[:min_orders_count], params[:country_code]
    )
  end
end

I will run rubocop and not get any offences:

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop app/models/user.rb
Inspecting 1 file
.

1 file inspected, no offenses detected

Rubocop

ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop -V
1.55.1 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 2.7.8) [x86_64-linux]
  - rubocop-capybara 2.18.0
  - rubocop-factory_bot 2.23.1
  - rubocop-performance 1.18.0
  - rubocop-rails 2.20.2
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.23.0
  - rubocop-thread_safety 0.5.1
fatkodima commented 1 year ago

I doubt this is better for parameters less than 3, for example. This cop probably needs a minimum count configuration.

koic commented 1 year ago

The style guide rule indicates the following:

Consider using named placeholders instead of positional placeholders when you have more than 1 placeholder in your query.

The more places where the ? placeholder is used, the more meaningful naming becomes. So, unless there's significant user feedback, the configuration might be unnecessary.