Skip to content

Commit

Permalink
Add Safety section to documentation
Browse files Browse the repository at this point in the history
Follow up to Follow up to rubocop/rubocop#10094.

This PR adds safety section to documentation for all cops that are
`Safe: false` or `SafeAutoCorrect: false`.
  • Loading branch information
koic committed Oct 11, 2021
1 parent 1ca074e commit c24c5b8
Show file tree
Hide file tree
Showing 26 changed files with 244 additions and 47 deletions.
3 changes: 3 additions & 0 deletions .yardopts
@@ -0,0 +1,3 @@
--markup markdown
--hide-void-return
--tag safety:"Cop Safety Information"
127 changes: 118 additions & 9 deletions docs/modules/ROOT/pages/cops_rails.adoc
Expand Up @@ -78,6 +78,11 @@ skip_after_filter :do_stuff
Checks that ActiveRecord aliases are not used. The direct method names
are more clear and easier to read.

=== Safety

This cop is unsafe because custom `update_attributes` method call was changed to
`update` but the method name remained same in the method definition.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -311,6 +316,11 @@ after_update_commit :log_update_action

This cop checks that controllers subclass ApplicationController.

=== Safety

This cop's autocorrection is unsafe because a controller class which inherits
`ActionController::Base` directly.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -340,6 +350,11 @@ end

This cop checks that jobs subclass ApplicationJob with Rails 5.0.

=== Safety

This cop's autocorrection is unsafe because an Active Job class which inherits
`ActiveJob::Base` directly.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -369,6 +384,11 @@ end

This cop checks that mailers subclass ApplicationMailer with Rails 5.0.

=== Safety

This cop's autocorrection is unsafe because an Action Mailer class which inherits
`ActionMailer::Base` directly.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -398,6 +418,11 @@ end

This cop checks that models subclass ApplicationRecord with Rails 5.0.

=== Safety

This cop's autocorrection is unsafe because an Active Record class which inherits
`ActiveRecord::Base` directly.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -432,6 +457,16 @@ quoted asterisk (e.g. <tt>`my_model`.`*`</tt>). This causes the
database to look for a column named <tt>`*`</tt> (or `"*"`) as opposed
to expanding the column list as one would likely expect.

=== Safety

This cop is unsafe because it checks for `["*"]`.

[source,ruby]
---
hash = { "*" => true }
hash["*"] # This would get corrected to `hash[Arel.star]`
---

=== Examples

[source,ruby]
Expand Down Expand Up @@ -637,15 +672,17 @@ end
This cop checks for code that can be written with simpler conditionals
using `Object#blank?` defined by Active Support.

This cop is marked as unsafe auto-correction, because `' '.empty?` returns false,
but `' '.blank?` returns true. Therefore, auto-correction is not compatible
if the receiver is a non-empty blank string, tab, or newline meta characters.

Interaction with `Style/UnlessElse`:
The configuration of `NotPresent` will not produce an offense in the
context of `unless else` if `Style/UnlessElse` is inabled. This is
to prevent interference between the auto-correction of the two cops.

=== Safety

This cop is unsafe auto-correction, because `' '.empty?` returns false,
but `' '.blank?` returns true. Therefore, auto-correction is not compatible
if the receiver is a non-empty blank string, tab, or newline meta characters.

=== Examples

==== NilOrEmpty: true (default)
Expand Down Expand Up @@ -1187,6 +1224,11 @@ This cop checks dynamic `find_by_*` methods.
Use `find_by` instead of dynamic method.
See. https://rails.rubystyle.guide#find_by

=== Safety

This cop is unsafe because it may detect user-defined `find_by_xxx` method.
It does not work as `find_by(xxx: xxx)`.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -2407,6 +2449,8 @@ end
This cop checks that methods specified in the filter's `only` or
`except` options are defined within the same class or module.

=== Safety

You can technically specify methods of superclass or methods added by
mixins on the filter, but these can confuse developers. If you specify
methods that are defined in other classes or modules, you should
Expand Down Expand Up @@ -2563,6 +2607,11 @@ This cop enforces that mailer names end with `Mailer` suffix.
Without the `Mailer` suffix it isn't immediately apparent what's a mailer
and which views are related to the mailer.

=== Safety

This cop's autocorrection is unsafe bacause Renaming a constant is
always an unsafe operation.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -2658,7 +2707,9 @@ match 'photos/:id', to: 'photos#show', via: :all
This cop enforces the use of `collection.exclude?(obj)`
over `!collection.include?(obj)`.

It is marked as unsafe by default because false positive will occur for
=== Safety

This cop is unsafe because false positive will occur for
a receiver object that do not have `exclude?` method. (e.g. `IPAddr`)

=== Examples
Expand Down Expand Up @@ -2769,6 +2820,11 @@ scope :chronological, -> { order(created_at: :asc) }

This cop checks for the use of output calls like puts and print

=== Safety

This cop's autocorrection is unsafe because depending on the Rails log level configuration,
changing from `puts` to `Rails.logger.debug` could result in no output being shown.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -2886,6 +2942,14 @@ Using `pluck` followed by `first` creates an intermediate array, which
`pick` avoids. When called on an Active Record relation, `pick` adds a
limit to the query so that only one value is fetched from the database.

=== Safety

This cop is unsafe because pluck is defined on both `ActiveRecord::Relation` and `Enumerable`,
whereas `pick` is only defined on `ActiveRecord::Relation` in Rails 6.0. This will be addressed
in Rails 6.1 via rails/rails#38760, at which point the cop will be safe.

See: https://github.com/rubocop/rubocop-rails/pull/249

=== Examples

[source,ruby]
Expand Down Expand Up @@ -2952,6 +3016,10 @@ Post.published.pluck(:title)

This cop enforces the use of `ids` over `pluck(:id)` and `pluck(primary_key)`.

=== Safety

This cop is unsafe if the receiver object is not an Active Record object.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -2995,11 +3063,13 @@ 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.

This cop has two different enforcement modes. When the EnforcedStyle
is conservative (the default) then only calls to `pluck` on a constant
This cop has two different enforcement modes. When the `EnforcedStyle`
is `conservative` (the default) then only calls to `pluck` on a constant
(i.e. a model class) in the `where` is used as offenses.

When the EnforcedStyle is aggressive then all calls to `pluck` in the
=== Safety

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.
Expand Down Expand Up @@ -3231,6 +3301,11 @@ following conditions:
* The task does not need application code.
* The task invokes the `:environment` task.

=== Safety

Probably not a problem in most cases, but it is likely that calling `:environment` task
will break a behavior.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -3533,7 +3608,10 @@ end

This cop checks if the value of the option `class_name`, in
the definition of a reflection is a string.
It is marked as unsafe because it cannot be determined whether

=== Safety

This cop is unsafe because it cannot be determined whether
constant or method return value specified to `class_name` is a string.

=== Examples
Expand Down Expand Up @@ -4206,10 +4284,20 @@ foo&.bar { |e| e.baz }
This cop checks to make sure safe navigation isn't used with `blank?` in
a conditional.

=== Safety

While the safe navigation operator is generally a good idea, when
checking `foo&.blank?` in a conditional, `foo` being `nil` will actually
do the opposite of what the author intends.

For example:

[source,ruby]
----
foo&.blank? #=> nil
foo.blank? #=> true
----

=== Examples

[source,ruby]
Expand Down Expand Up @@ -4257,6 +4345,11 @@ that behavior can be turned off with `AllowImplicitReturn: false`.
You can permit receivers that are giving false positives with
`AllowedReceivers: []`

=== Safety

This cop's autocorrection is unsafe because custom `update` method call was changed to `update!`
but the method name remained same in the method definition.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -4552,6 +4645,9 @@ user.touch
|===

Checks SQL heredocs to use `.squish`.

=== Safety

Some SQL syntax (e.g. PostgreSQL comments and functions) requires newlines
to be preserved in order to work, thus auto-correction for this cop is not safe.

Expand Down Expand Up @@ -4617,6 +4713,10 @@ then only use of `Time.zone` is allowed.
When EnforcedStyle is 'flexible' then it's also allowed
to use `Time#in_time_zone`.

=== Safety

This cop is unsafe because it may change handling time.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -4743,6 +4843,8 @@ as the cop cannot distinguish between calls to pluck on an
ActiveRecord::Relation vs a call to pluck on an
ActiveRecord::Associations::CollectionProxy.

=== Safety

This cop is unsafe because the behavior may change depending on the
database collation.
Autocorrect is disabled by default for this cop since it may generate
Expand Down Expand Up @@ -4993,6 +5095,11 @@ validates :foo, uniqueness: true
This cop identifies places where manually constructed SQL
in `where` can be replaced with `where(attribute: value)`.

=== Safety

This cop's autocorrection is unsafe because is may change SQL.
See: https://github.com/rubocop/rubocop-rails/issues/403

=== Examples

[source,ruby]
Expand Down Expand Up @@ -5036,6 +5143,8 @@ then the cop enforces `exists?(...)` over `where(...).exists?`.
When EnforcedStyle is 'where' then the cop enforces
`where(...).exists?` over `exists?(...)`.

=== Safety

This cop is unsafe for auto-correction because the behavior may change on the following case:

[source,ruby]
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/rails/active_record_aliases.rb
Expand Up @@ -6,6 +6,10 @@ module Rails
# Checks that ActiveRecord aliases are not used. The direct method names
# are more clear and easier to read.
#
# @safety
# This cop is unsafe because custom `update_attributes` method call was changed to
# `update` but the method name remained same in the method definition.
#
# @example
# #bad
# Book.update_attributes!(author: 'Alice')
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/rails/application_controller.rb
Expand Up @@ -5,6 +5,10 @@ module Cop
module Rails
# This cop checks that controllers subclass ApplicationController.
#
# @safety
# This cop's autocorrection is unsafe because a controller class which inherits
# `ActionController::Base` directly.
#
# @example
#
# # good
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/rails/application_job.rb
Expand Up @@ -5,6 +5,10 @@ module Cop
module Rails
# This cop checks that jobs subclass ApplicationJob with Rails 5.0.
#
# @safety
# This cop's autocorrection is unsafe because an Active Job class which inherits
# `ActiveJob::Base` directly.
#
# @example
#
# # good
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/rails/application_mailer.rb
Expand Up @@ -5,6 +5,10 @@ module Cop
module Rails
# This cop checks that mailers subclass ApplicationMailer with Rails 5.0.
#
# @safety
# This cop's autocorrection is unsafe because an Action Mailer class which inherits
# `ActionMailer::Base` directly.
#
# @example
#
# # good
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/rails/application_record.rb
Expand Up @@ -5,6 +5,10 @@ module Cop
module Rails
# This cop checks that models subclass ApplicationRecord with Rails 5.0.
#
# @safety
# This cop's autocorrection is unsafe because an Active Record class which inherits
# `ActiveRecord::Base` directly.
#
# @example
#
# # good
Expand Down
9 changes: 9 additions & 0 deletions lib/rubocop/cop/rails/arel_star.rb
Expand Up @@ -10,6 +10,15 @@ module Rails
# database to look for a column named <tt>`*`</tt> (or `"*"`) as opposed
# to expanding the column list as one would likely expect.
#
# @safety
# This cop is unsafe because it checks for `["*"]`.
#
# [source,ruby]
# ---
# hash = { "*" => true }
# hash["*"] # This would get corrected to `hash[Arel.star]`
# ---
#
# @example
# # bad
# MyTable.arel_table["*"]
Expand Down

0 comments on commit c24c5b8

Please sign in to comment.