Skip to content

Commit

Permalink
Add Safety section to documentation
Browse files Browse the repository at this point in the history
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
renawatson68 committed Oct 11, 2021
1 parent 8034ba6 commit 90fdbec
Show file tree
Hide file tree
Showing 21 changed files with 209 additions and 61 deletions.
3 changes: 3 additions & 0 deletions .yardopts
@@ -0,0 +1,3 @@
--markup markdown
--hide-void-return
--tag safety:"Cop Safety Information"
130 changes: 106 additions & 24 deletions docs/modules/ROOT/pages/cops_performance.adoc
Expand Up @@ -15,6 +15,11 @@
This cop is used to identify usages of `ancestors.include?` and
change them to use `<=` instead.

=== Safety

This cop is unsafe because it can't tell whether the receiver is a class or an object.
e.g. the false positive was for `Nokogiri::XML::Node#ancestors`.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -48,7 +53,10 @@ This cop identifies places where slicing arrays with semi-infinite ranges
can be replaced by `Array#take` and `Array#drop`.
This cop was created due to a mistake in microbenchmark and hence is disabled by default.
Refer https://github.com/rubocop/rubocop-performance/pull/175#issuecomment-731892717
This cop is also unsafe for string slices because strings do not have `#take` and `#drop` methods.

=== Safety

This cop is unsafe for string slices because strings do not have `#take` and `#drop` methods.

=== Examples

Expand Down Expand Up @@ -227,11 +235,14 @@ conditions can be true for any given condition. A likely scenario for
this defining a higher level when condition to override a condition
that is inside of the splat expansion.

This is not a guaranteed performance improvement. If the data being
processed by the `case` condition is normalized in a manner that favors
hitting a condition in the splat expansion, it is possible that
moving the splat condition to the end will use more memory,
=== Safety

This cop is not unsafe auto-correction because it is not a guaranteed
performance improvement. If the data being processed by the `case` condition is
normalized in a manner that favors hitting a condition in the splat expansion,
it is possible that moving the splat condition to the end will use more memory,
and run slightly slower.
See for more details: https://github.com/rubocop/rubocop/pull/6163

=== Examples

Expand Down Expand Up @@ -292,6 +303,9 @@ end

This cop identifies places where a case-insensitive string comparison
can better be implemented using `casecmp`.

=== Safety

This cop is unsafe because `String#casecmp` and `String#casecmp?` behave
differently when using Non-ASCII characters.

Expand Down Expand Up @@ -505,18 +519,28 @@ This cop is used to identify usages of `count` on an `Enumerable` that
follow calls to `select`, `find_all`, `filter` or `reject`. Querying logic can instead be
passed to the `count` call.

`ActiveRecord` compatibility:
=== Safety

This cop is unsafe because it has known compatibility issues with `ActiveRecord` and other
frameworks. ActiveRecord's `count` ignores the block that is passed to it.
`ActiveRecord` will ignore the block that is passed to `count`.
Other methods, such as `select`, will convert the association to an
array and then run the block on the array. A simple work around to
make `count` work with a block is to call `to_a.count {...}`.

Example:
`Model.where(id: [1, 2, 3]).select { |m| m.method == true }.size`
For example:

becomes:
[source,ruby]
----
`Model.where(id: [1, 2, 3]).select { |m| m.method == true }.size`
----

`Model.where(id: [1, 2, 3]).to_a.count { |m| m.method == true }`
becomes:

[source,ruby]
----
`Model.where(id: [1, 2, 3]).to_a.count { |m| m.method == true }`
----

=== Examples

Expand Down Expand Up @@ -556,14 +580,17 @@ In Ruby 2.5, `String#delete_prefix` has been added.

This cop identifies places where `gsub(/\Aprefix/, '')` and `sub(/\Aprefix/, '')`
can be replaced by `delete_prefix('prefix')`.
It is marked as unsafe by default because `Pathname` has `sub` but not `delete_prefix`.

This cop has `SafeMultiline` configuration option that `true` by default because
`^prefix` is unsafe as it will behave incompatible with `delete_prefix`
for receiver is multiline string.

The `delete_prefix('prefix')` method is faster than `gsub(/\Aprefix/, '')`.

=== Safety

This cop is unsafe because `Pathname` has `sub` but not `delete_prefix`.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -628,14 +655,17 @@ In Ruby 2.5, `String#delete_suffix` has been added.

This cop identifies places where `gsub(/suffix\z/, '')` and `sub(/suffix\z/, '')`
can be replaced by `delete_suffix('suffix')`.
It is marked as unsafe by default because `Pathname` has `sub` but not `delete_suffix`.

This cop has `SafeMultiline` configuration option that `true` by default because
`suffix$` is unsafe as it will behave incompatible with `delete_suffix?`
for receiver is multiline string.

The `delete_suffix('suffix')` method is faster than `gsub(/suffix\z/, '')`.

=== Safety

This cop is unsafe because `Pathname` has `sub` but not `delete_suffix`.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -705,6 +735,12 @@ chained to `select`, `find_all` or `filter` and change them to use
own meaning. Correcting ActiveRecord methods with this cop should be
considered unsafe.

=== Safety

This cop is unsafe because is has known compatibility issues with `ActiveRecord` and other
frameworks. `ActiveRecord` does not implement a `detect` method and `find` has its own
meaning. Correcting `ActiveRecord` methods with this cop should be considered unsafe.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -787,6 +823,12 @@ This cop has `SafeMultiline` configuration option that `true` by default because
`end$` is unsafe as it will behave incompatible with `end_with?`
for receiver is multiline string.

=== Safety

This will change to a new method call which isn't guaranteed to be on the
object. Switching these methods has to be done with knowledge of the types
of the variables which rubocop doesn't have.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -968,6 +1010,10 @@ performs a faster O(1) search for the key.
both perform an O(n) search through all of the values, calling `values`
allocates a new array while using `value?` does not.

=== Safety

This cop is unsafe because it can't tell whether the receiver is a hash object.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -1054,15 +1100,18 @@ NOTE: Required Ruby version: 2.7
In Ruby 2.7, `Enumerable#filter_map` has been added.

This cop identifies places where `map { ... }.compact` can be replaced by `filter_map`.
It is marked as unsafe auto-correction by default because `map { ... }.compact`
that is not compatible with `filter_map`.

[source,ruby]
----
[true, false, nil].compact #=> [true, false]
[true, false, nil].filter_map(&:itself) #=> [true]
----

=== Safety

This cop's autocorrection is unsafe because `map { ... }.compact` that is not
compatible with `filter_map`.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -1130,6 +1179,11 @@ This could have an effect on performance,
especially in case of single-threaded
applications with multiple `OpenStruct` instantiations.

=== Safety

This cop is unsafe because `OpenStruct.new` and `Struct.new`
are not equivalent.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -1168,8 +1222,10 @@ item in a `Range` to see if a specified item is there. In contrast,
end points of the `Range`. In a great majority of cases, this is what
is wanted.

This cop is `Safe: false` by default because `Range#include?` (or `Range#member?`) and
`Range#cover?` are not equivalent behaviour.
=== Safety

This cop is unsafe because `Range#include?` (or `Range#member?`) and `Range#cover?`
are not equivalent behaviour.

=== Examples

Expand Down Expand Up @@ -1250,8 +1306,10 @@ and `Enumerable#none?` are compared with `===` or similar methods in block.
By default, `Object#===` behaves the same as `Object#==`, but this
behavior is appropriately overridden in subclass. For example,
`Range#===` returns `true` when argument is within the range.
Therefore, It is marked as unsafe by default because `===` and `==`
do not always behave the same.

=== Safety

This cop is unsafe because `===` and `==` do not always behave the same.

=== Examples

Expand Down Expand Up @@ -1319,7 +1377,9 @@ This cop identifies places where `Hash#merge!` can be replaced by
You can set the maximum number of key-value pairs to consider
an offense with `MaxKeyValuePairs`.

This cop is marked as unsafe because RuboCop cannot determine if the
=== Safety

This cop is unsafe because RuboCop cannot determine if the
receiver of `merge!` is actually a hash or not.

=== Examples
Expand Down Expand Up @@ -1783,6 +1843,12 @@ This cop has `SafeMultiline` configuration option that `true` by default because
`^start` is unsafe as it will behave incompatible with `start_with?`
for receiver is multiline string.

=== Safety

This will change to a new method call which isn't guaranteed to be on the
object. Switching these methods has to be done with knowledge of the types
of the variables which rubocop doesn't have.

=== Examples

[source,ruby]
Expand Down Expand Up @@ -1854,6 +1920,8 @@ for receiver is multiline string.
This cop identifies unnecessary use of a regex where
`String#include?` would suffice.

=== Safety

This cop's offenses are not safe to auto-correct if a receiver is nil.

=== Examples
Expand Down Expand Up @@ -1996,6 +2064,19 @@ This cop checks for .times.map calls.
In most cases such calls can be replaced
with an explicit array creation.

=== Safety

This cop's autocorrection is unsafe because `Integer#times` does nothing if receiver is 0
or less. However, `Array.new` raises an error if argument is less than 0.

For example:

[source,ruby]
----
-1.times{} # does nothing
Array.new(-1) # ArgumentError: negative array size
----

=== Examples

[source,ruby]
Expand Down Expand Up @@ -2037,11 +2118,12 @@ In Ruby 2.3 or later, use unary plus operator to unfreeze a string
literal instead of `String#dup` and `String.new`.
Unary plus operator is faster than `String#dup`.

NOTE: `String.new` (without operator) is not exactly the same as `+''`.
These differ in encoding. `String.new.encoding` is always `ASCII-8BIT`.
However, `(+'').encoding` is the same as script encoding(e.g. `UTF-8`).
Therefore, auto-correction is unsafe.
So, if you expect `ASCII-8BIT` encoding, disable this cop.
=== Safety

This cop's autocorrection is unsafe because `String.new` (without operator) is not
exactly the same as `+''`. These differ in encoding. `String.new.encoding` is always
`ASCII-8BIT`. However, `(+'').encoding` is the same as script encoding(e.g. `UTF-8`).
if you expect `ASCII-8BIT` encoding, disable this cop.

=== Examples

Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/performance/ancestors_include.rb
Expand Up @@ -6,6 +6,10 @@ module Performance
# This cop is used to identify usages of `ancestors.include?` and
# change them to use `<=` instead.
#
# @safety
# This cop is unsafe because it can't tell whether the receiver is a class or an object.
# e.g. the false positive was for `Nokogiri::XML::Node#ancestors`.
#
# @example
# # bad
# A.ancestors.include?(B)
Expand Down
Expand Up @@ -7,7 +7,9 @@ module Performance
# can be replaced by `Array#take` and `Array#drop`.
# This cop was created due to a mistake in microbenchmark and hence is disabled by default.
# Refer https://github.com/rubocop/rubocop-performance/pull/175#issuecomment-731892717
# This cop is also unsafe for string slices because strings do not have `#take` and `#drop` methods.
#
# @safety
# This cop is unsafe for string slices because strings do not have `#take` and `#drop` methods.
#
# @example
# # bad
Expand Down
12 changes: 7 additions & 5 deletions lib/rubocop/cop/performance/case_when_splat.rb
Expand Up @@ -17,11 +17,13 @@ module Performance
# this defining a higher level when condition to override a condition
# that is inside of the splat expansion.
#
# This is not a guaranteed performance improvement. If the data being
# processed by the `case` condition is normalized in a manner that favors
# hitting a condition in the splat expansion, it is possible that
# moving the splat condition to the end will use more memory,
# and run slightly slower.
# @safety
# This cop is not unsafe auto-correction because it is not a guaranteed
# performance improvement. If the data being processed by the `case` condition is
# normalized in a manner that favors hitting a condition in the splat expansion,
# it is possible that moving the splat condition to the end will use more memory,
# and run slightly slower.
# See for more details: https://github.com/rubocop/rubocop/pull/6163
#
# @example
# # bad
Expand Down
6 changes: 4 additions & 2 deletions lib/rubocop/cop/performance/casecmp.rb
Expand Up @@ -5,8 +5,10 @@ module Cop
module Performance
# This cop identifies places where a case-insensitive string comparison
# can better be implemented using `casecmp`.
# This cop is unsafe because `String#casecmp` and `String#casecmp?` behave
# differently when using Non-ASCII characters.
#
# @safety
# This cop is unsafe because `String#casecmp` and `String#casecmp?` behave
# differently when using Non-ASCII characters.
#
# @example
# # bad
Expand Down
35 changes: 22 additions & 13 deletions lib/rubocop/cop/performance/count.rb
Expand Up @@ -7,6 +7,28 @@ module Performance
# follow calls to `select`, `find_all`, `filter` or `reject`. Querying logic can instead be
# passed to the `count` call.
#
# @safety
# This cop is unsafe because it has known compatibility issues with `ActiveRecord` and other
# frameworks. ActiveRecord's `count` ignores the block that is passed to it.
# `ActiveRecord` will ignore the block that is passed to `count`.
# Other methods, such as `select`, will convert the association to an
# array and then run the block on the array. A simple work around to
# make `count` work with a block is to call `to_a.count {...}`.
#
# For example:
#
# [source,ruby]
# ----
# `Model.where(id: [1, 2, 3]).select { |m| m.method == true }.size`
# ----
#
# becomes:
#
# [source,ruby]
# ----
# `Model.where(id: [1, 2, 3]).to_a.count { |m| m.method == true }`
# ----
#
# @example
# # bad
# [1, 2, 3].select { |e| e > 2 }.size
Expand All @@ -24,19 +46,6 @@ module Performance
# [1, 2, 3].count { |e| e < 2 && e.even? }
# Model.select('field AS field_one').count
# Model.select(:value).count
#
# `ActiveRecord` compatibility:
# `ActiveRecord` will ignore the block that is passed to `count`.
# Other methods, such as `select`, will convert the association to an
# array and then run the block on the array. A simple work around to
# make `count` work with a block is to call `to_a.count {...}`.
#
# Example:
# `Model.where(id: [1, 2, 3]).select { |m| m.method == true }.size`
#
# becomes:
#
# `Model.where(id: [1, 2, 3]).to_a.count { |m| m.method == true }`
class Count < Base
include RangeHelp
extend AutoCorrector
Expand Down

0 comments on commit 90fdbec

Please sign in to comment.