common_rubocop_rails.yml

require: rubocop-rails

inherit_mode:
  merge:
    - Exclude
    - AllowedPatterns

inherit_from: common_rubocop.yml

AllCops:
  Exclude:
    - 'bin/puma'
    - 'bin/pumactl'
    - 'bin/rails'
    - 'bin/shoryuken'
    - 'bin/sidekiq'
    - 'bin/sidekiqctl'
    - 'bin/spring'
    - 'db/schema.rb'
    - 'db/migrate/**/*'

# Rails project's are not concerned with API docs normally
Style/Documentation:
  Enabled: false

# Rails foreign keys and indexes can get long. We want to ignore our annotation
# comments which are for these entries.
#
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
# URISchemes: http, https
Layout/LineLength:
  AllowedPatterns:
    - '\A#  fk_rails_'
    - '\A#  index_'

Metrics/BlockLength:
  Exclude:
    - 'bin/setup'
    - 'bin/update'
    - 'config/routes.rb'
    - 'spec/rails_helper.rb'

# For our Rails apps several of them use the `respond_to` with `format` blocks
# to handle various mime types (mostly HTML and JSON). Given our `do` / `end`
# block style for non-functional blocks (which includes both `respond_to` and
# `format`) the general method limit of is too small. This also applies to the
# helper methods which define the allowed parameters for the action; especially
# for larger forms.
#
# Here is an example of a minimal controller `update` method which uses the
# `respond_to` style to support just the HTML and JSON formats:
#
#   ```ruby
#   def update
#     respond_to do |format|
#       if @resource.update(resource_params)
#         format.html do
#           redirect_to resources_url, notice: 'Resource was successfully updated.'
#         end
#         format.json do
#           render :show, status: :ok, location: @resource
#         end
#       else
#         format.html do
#           render :edit
#         end
#         format.json do
#           render json: @resource.errors, status: :unprocessable_entity
#         end
#       end
#     end
#   end
#   ```
#
# We do believe that the default size of 10, which is what we explicitly
# configure below so there's no confusion, is a good general limit to help
# encourage a balance between terseness and procedural code. Thus we do not
# want to raise the limit, instead we just want to exclude these controllers.
#
# At this time there is no way for us to exclude just the common controller
# actions / *_params methods so we exclude the entire file.
#
# Configuration parameters: CountComments.
Metrics/MethodLength:
  Max: 10
  Exclude:
    - 'app/controllers/**/*_controller.rb'

# Ignore subclass parent for one off benchmarks
#
# Benchmarks are generally meant to be small and targeted. They often have
# custom model declarations which help direct the focus of the benchmarks.
# These one-off models exist outside of the main Rails app. We don't want this
# cop to run for them because including `ApplicationRecord` is unnecessary in
# these cases and just adds noise to the setup.
Rails/ApplicationRecord:
  Exclude:
    - 'benchmarks/**/*'

# As Rails defaults to creating timestamps you need to go out of your way to
# explicitly have them removed from the table. This cop is almost always a
# false negative so we're disabling it.
#
# Configuration parameters: Include.
# Include: db/migrate/*.rb
Rails/CreateTableWithTimestamps:
  Enabled: false

# This cop looks for uses of default_scope because named scopes are preferred:
# https://rails.rubystyle.guide/#named-scopes
Rails/DefaultScope:
  Enabled: true

# We were originally going to disable this, but after much discussion agreed that enabling
# this cop with AllowReads: true should be relatively painless.
Rails/EnvironmentVariableAccess:
  Enabled: true
  AllowReads: true

# Usage of `find_by` is more expressive of intent than `where.first`. We should
# check all app code, not just the models to improve intent expression.
#
# Since rake tasks often live in `lib` we also check all of lib as well.
#
# We are also disabling the default IgnoreWhereFirst that was added in version
# 2.11
#
# Configuration parameters: Include.
# Include: app/models/**/*.rb
Rails/FindBy:
  Enabled: true
  IgnoreWhereFirst: false
  Include:
    - 'app/**/*.rb'
    - 'lib/**/*.rb'

# This cop enforces that ActiveRecord#find is used instead of where.take!, find_by!, and find_by_id!
# to retrieve a single record by primary key when you expect it to be found.
Rails/FindById:
  Enabled: true

# Usage of `each` for large datasets can be a performance issue; specially a
# drain on system memory. When possible it's better to use `find_each` so that
# chunks of data are evaluated at a time.
#
# We should check all app code, not just the models to help prevent this. Since
# rake tasks often live in `lib` we also check all of lib as well.
#
# Configuration parameters: Include.
# Include: app/models/**/*.rb
Rails/FindEach:
  Enabled: true
  Include:
    - 'app/**/*.rb'
    - 'lib/**/*.rb'

# We understand the trade-offs for using the through model versus a lookup
# table. As such this cop is just noise as it flags only those cases we really
# do want a lookup table.
#
# Configuration parameters: Include.
# Include: app/models/**/*.rb
Rails/HasAndBelongsToMany:
  Enabled: false

# We find the combo `:only` and `:if` readable. While the `:except` and `:if`
# combo is easier to read as a combined proc. As a team we are fine with
# handling this in PR reviews, until such time which Rubocop provides an option
# for us to configure this.
Rails/IgnoredSkipActionFilterOption:
  Enabled: false

# We do not care about this check due to its lack of configuration.
#
# Some of the team finds the naming of this method is more confusing than using
# `each_with_object`. We all agree the other examples are bad and should not be
# used:
#
#     # OK for us
#     [1, 2, 3].each_with_object({}) { |el, h| h[foo(el)] = el }
#
#     # Bad
#     [1, 2, 3].to_h { |el| [foo(el), el] }
#     [1, 2, 3].map { |el| [foo(el), el] }.to_h
#     Hash[[1, 2, 3].collect { |el| [foo(el), el] }]
#
# If this check supports configuration in the future so that we can allow
# `each_with_object` then we'll turn it back on.
Rails/IndexBy:
  Enabled: false

# We find the name of this method to be very confusing. We'd prefer this method
# is never used.
Rails/IndexWith:
  Enabled: false

# This would be extremely hard to implement with our app
Rails/I18nLocaleTexts:
  Enabled: false

# This cop enforces the use of ids over pluck(:id) and pluck(primary_key).
# https://rails.rubystyle.guide/#ids
Rails/PluckId:
  Enabled: true

# This cop identifies places where pluck is used in where query methods and can be replaced with
# select. Since pluck is an eager method and hits the database immediately, using select helps to
# avoid additional database queries.
#
# When the EnforcedStyle is aggressive then all calls to pluck in the where is used as offenses.
# This may lead to false positives as the cop cannot replace to select between calls to pluck on an
# ActiveRecord::Relation instance vs a call to pluck on an Array instance.
Rails/PluckInWhere:
  EnforcedStyle: aggressive

# The ActiveSupport monkey patches for `present?` are nearly all defined as:
#
#     !blank?
#
# For most of us `unless blank?` reads just as easily as `if present?`.
# Sometimes contextually, it can read better depending on the branch logic and
# surrounding context. As `if present?` requires an additional negation and
# method call it is technically slower. In the general case the perf difference
# isn't much but in some cases it matters. Thus, we are not enforcing changing
# `unless blank?` to `if present?` and are leaving it up to the context to
# decide which is a better fit.
#
# Cop supports --auto-correct.
# Configuration parameters: NotNilAndNotEmpty, NotBlank, UnlessBlank.
Rails/Present:
  UnlessBlank: false

# We prefer you use the attribute readers and writes. For those special cases
# where the intent is really to interact with the raw underlying attribute we
# prefer `read_attribute` and `write_attribute`; as this makes the intent
# explicit. Ideally we'd never use the hash like accessor `[:attr]`.
#
# We disable this cop because it is not configurable.
#
# Configuration parameters: Include.
# Include: app/models/**/*.rb
Rails/ReadWriteAttribute:
  Enabled: false

# Brakeman currently stops us from doing unscoped ActiveRecord lookups
Rails/RedundantActiveRecordAllMethod:
  Enabled: false

# Enabling this because it is disabled by default and we want it.
Rails/ReversibleMigrationMethodDefinition:
  Enabled: true

# This ensures we do not ignore potential validation issues in the code. Doing
# so can lead to strange and surprising bugs where records are expected to
# be created, or be modified, but are not.
#
#     # If author is a new record the book may not be created since the FK is
#     # invalid. Perhaps omitting other fields, maybe new required fields, is
#     # an oversight in the book creation as well.
#     author.save
#     Book.create(author: author)
#
# Or side effects are expected to occur but they do not:
#
#     # This is a condensed default Rails scaffold controller for `destroy`.
#     #
#     # Once a `has_many` or `has_one` associations is added which specifies
#     # `dependent: :restrict_with_error` this no longer behaves as expected.
#     # Given such associations are often added much later in time errors in
#     # this action are an all to common oversight in Rails.
#     def destroy
#       @book.destroy
#       respond_to do |format|
#         format.html do
#           redirect_to books_url, notice: 'Book was successfully destroyed.'
#         end
#       end
#     end
#
# Configuration parameters: AllowImplicitReturn, AllowedReceivers.
Rails/SaveBang:
  Enabled: true

# This cop enforces that short forms of I18n methods are used: t instead of translate and l instead
# of localize. We want this because it's a pain to use the full method names over and over in view
# code. When the EnforcedStyle is aggressive then all translate and localize calls without a
# receiver are added as offenses.
Rails/ShortI18n:
  EnforcedStyle: aggressive

# According to the Rails docs while the following methods skip validations they
# only update the specified (single) attribute reducing risks. We'd rather not
# warn for those cases:
#
#   - decrement!
#   - increment!
#   - touch
#
# We are not excluding `toggle!` because it's more likely that a flag may have
# validations associated with it (or is used by other validations).
#
# See:
#
#   - http://api.rubyonrails.org/classes/ActiveRecord/CounterCache/ClassMethods.html
#   - http://api.rubyonrails.org/classes/ActiveRecord/Persistence.html
#   - http://api.rubyonrails.org/classes/ActiveRecord/Relation.html
#
# Configuration parameters: ForbiddenMethods, AllowedMethods.
# ForbiddenMethods: decrement!, decrement_counter, increment!, increment_counter, toggle!, touch, update_all, update_attribute, update_column, update_columns, update_counters
Rails/SkipsModelValidations:
  AllowedMethods:
    - 'decrement!'
    - 'increment!'
    - 'touch'

# We don't want to be forced to use squish in SQL or JSON heredocs (especially
# in specs).
Rails/SquishedSQLHeredocs:
  Enabled: false

# Rails uses compact style by default so we're disabling this with a :hammer:
# for things likely to be generated by Rails (i.e. most things in app).
#
# Configuration parameters: AutoCorrect, EnforcedStyle.
# SupportedStyles: nested, compact
Style/ClassAndModuleChildren:
  Exclude:
    - 'app/**/*'