Skip to content

Commit

Permalink
[Fix #8431] Add @safety section to all cops that are Safe: false
Browse files Browse the repository at this point in the history
…or `SafeAutoCorrect: false`.
  • Loading branch information
dvandersluis authored and bbatsov committed Sep 20, 2021
1 parent 04d1c5c commit 28ae02a
Show file tree
Hide file tree
Showing 68 changed files with 520 additions and 146 deletions.
@@ -0,0 +1 @@
* [#8431](https://github.com/rubocop/rubocop/issues/8431): Add `Safety` section to documentation for all cops that are `Safe: false` or `SafeAutoCorrect: false`. ([@dvandersluis][])
2 changes: 1 addition & 1 deletion config/default.yml
Expand Up @@ -3708,7 +3708,7 @@ Style/InPatternThen:
Style/InfiniteLoop:
Description: >-
Use Kernel#loop for infinite loops.
This cop is unsafe in the body may raise a `StopIteration` exception.
This cop is unsafe if the body may raise a `StopIteration` exception.
Safe: false
StyleGuide: '#infinite-loop'
Enabled: true
Expand Down
14 changes: 7 additions & 7 deletions lib/rubocop/cop/lint/ambiguous_range.rb
Expand Up @@ -10,19 +10,20 @@ module Lint
# explicit by requiring parenthesis around complex range boundaries (anything
# that is not a basic literal: numerics, strings, symbols, etc.).
#
# NOTE: The cop auto-corrects by wrapping the entire boundary in parentheses, which
# makes the outcome more explicit but is possible to not be the intention of the
# programmer. For this reason, this cop's auto-correct is marked as unsafe (it
# will not change the behaviour of the code, but will not necessarily match the
# intent of the program).
#
# This cop can be configured with `RequireParenthesesForMethodChains` in order to
# specify whether method chains (including `self.foo`) should be wrapped in parens
# by this cop.
#
# NOTE: Regardless of this configuration, if a method receiver is a basic literal
# value, it will be wrapped in order to prevent the ambiguity of `1..2.to_a`.
#
# @safety
# The cop auto-corrects by wrapping the entire boundary in parentheses, which
# makes the outcome more explicit but is possible to not be the intention of the
# programmer. For this reason, this cop's auto-correct is unsafe (it will not
# change the behaviour of the code, but will not necessarily match the
# intent of the program).
#
# @example
# # bad
# x || 1..2
Expand Down Expand Up @@ -55,7 +56,6 @@ module Lint
#
# # good
# (a.foo)..(b.bar)
#
class AmbiguousRange < Base
extend AutoCorrector

Expand Down
11 changes: 9 additions & 2 deletions lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb
Expand Up @@ -17,9 +17,16 @@ module Lint
# always be the same (`x - x` will always be 0; `x / x` will always be 1), and
# thus are legitimate offenses.
#
# This cop is marked as unsafe as it does not consider side effects when calling methods
# and thus can generate false positives:
# @safety
# This cop is unsafe as it does not consider side effects when calling methods
# and thus can generate false positives, for example:
#
# [source,ruby]
# ----
# if wr.take_char == '\0' && wr.take_char == '\0'
# # ...
# end
# ----
#
# @example
# # bad
Expand Down
5 changes: 5 additions & 0 deletions lib/rubocop/cop/lint/boolean_symbol.rb
Expand Up @@ -6,6 +6,11 @@ module Lint
# This cop checks for `:true` and `:false` symbols.
# In most cases it would be a typo.
#
# @safety
# Autocorrection is unsafe for this cop because code relying
# on `:true` or `:false` symbols will break when those are
# changed to actual booleans.
#
# @example
#
# # bad
Expand Down
25 changes: 24 additions & 1 deletion lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb
Expand Up @@ -3,7 +3,7 @@
module RuboCop
module Cop
module Lint
# This cop checks constructors for disjunctive assignments that should
# This cop checks constructors for disjunctive assignments (`||=`) that should
# be plain assignments.
#
# So far, this cop is only concerned with disjunctive assignment of
Expand All @@ -12,6 +12,29 @@ module Lint
# In ruby, an instance variable is nil until a value is assigned, so the
# disjunction is unnecessary. A plain assignment has the same effect.
#
# @safety
# This cop is unsafe because it can register a false positive when a
# method is redefined in a subclass that calls super. For example:
#
# [source,ruby]
# ----
# class Base
# def initialize
# @config ||= 'base'
# end
# end
#
# class Derived < Base
# def initialize
# @config = 'derived'
# super
# end
# end
# ----
#
# Without the disjunctive assignment, `Derived` will be unable to override
# the value for `@config`.
#
# @example
# # bad
# def initialize
Expand Down
15 changes: 12 additions & 3 deletions lib/rubocop/cop/lint/hash_compare_by_identity.rb
Expand Up @@ -3,10 +3,19 @@
module RuboCop
module Cop
module Lint
# Prefer using `Hash#compare_by_identity` than using `object_id` for hash keys.
# Prefer using `Hash#compare_by_identity` rather than using `object_id`
# for hash keys.
#
# This cop is marked as unsafe as a hash possibly can contain other keys
# besides `object_id`s.
# This cop looks for hashes being keyed by objects' `object_id`, using
# one of these methods: `key?`, `has_key?`, `fetch`, `[]` and `[]=`.
#
# @safety
# This cop is unsafe. Although unlikely, the hash could store both object
# ids and other values that need be compared by value, and thus
# could be a false positive.
#
# Furthermore, this cop cannot guarantee that the receiver of one of the
# methods (`key?`, etc.) is actually a hash.
#
# @example
# # bad
Expand Down
5 changes: 5 additions & 0 deletions lib/rubocop/cop/lint/interpolation_check.rb
Expand Up @@ -5,6 +5,11 @@ module Cop
module Lint
# This cop checks for interpolation in a single quoted string.
#
# @safety
# This cop is generally safe, but is marked as unsafe because
# it is possible to actually intentionally have text inside
# `#{...}` in a single quoted string.
#
# @example
#
# # bad
Expand Down
7 changes: 4 additions & 3 deletions lib/rubocop/cop/lint/loop.rb
Expand Up @@ -5,9 +5,10 @@ module Cop
module Lint
# This cop checks for uses of `begin...end while/until something`.
#
# The cop is marked as unsafe because behaviour can change in some cases, including
# if a local variable inside the loop body is accessed outside of it, or if the
# loop body raises a `StopIteration` exception (which `Kernel#loop` rescues).
# @safety
# The cop is unsafe because behaviour can change in some cases, including
# if a local variable inside the loop body is accessed outside of it, or if the
# loop body raises a `StopIteration` exception (which `Kernel#loop` rescues).
#
# @example
#
Expand Down
6 changes: 5 additions & 1 deletion lib/rubocop/cop/lint/non_deterministic_require_order.rb
Expand Up @@ -14,7 +14,11 @@ module Lint
# `Dir.glob` and `Dir[]` sort globbed results by default in Ruby 3.0.
# So all bad cases are acceptable when Ruby 3.0 or higher are used.
#
# This cop will be deprecated and removed when supporting only Ruby 3.0 and higher.
# NOTE: This cop will be deprecated and removed when supporting only Ruby 3.0 and higher.
#
# @safety
# This cop is unsafe in the case where sorting files changes existing
# expected behaviour.
#
# @example
#
Expand Down
5 changes: 5 additions & 0 deletions lib/rubocop/cop/lint/number_conversion.rb
Expand Up @@ -18,6 +18,11 @@ module Lint
# cop by default). Similarly, Rails' duration methods do not work well
# with `Integer()` and can be ignored with `IgnoredMethods`.
#
# @safety
# Autocorrection is unsafe because it is not guaranteed that the
# replacement `Kernel` methods are able to properly handle the
# input if it is not a standard class.
#
# @example
#
# # bad
Expand Down
6 changes: 4 additions & 2 deletions lib/rubocop/cop/lint/or_assignment_to_constant.rb
Expand Up @@ -9,8 +9,10 @@ module Lint
# should always be the same. If constants are assigned in multiple
# locations, the result may vary depending on the order of `require`.
#
# Also, if you already have such an implementation, auto-correction may
# change the result.
# @safety
# This cop is unsafe because code that is already conditionally
# assigning a constant may have its behaviour changed by
# auto-correction.
#
# @example
#
Expand Down
17 changes: 17 additions & 0 deletions lib/rubocop/cop/lint/out_of_range_regexp_ref.rb
Expand Up @@ -6,6 +6,23 @@ module Lint
# This cops looks for references of Regexp captures that are out of range
# and thus always returns nil.
#
# @safety
# This cop is unsafe because it is naive in how it determines what
# references are available based on the last encountered regexp, but
# it cannot handle some cases, such as conditional regexp matches, which
# leads to false positives, such as:
#
# [source,ruby]
# ----
# foo ? /(c)(b)/ =~ str : /(b)/ =~ str
# do_something if $2
# # $2 is defined for the first condition but not the second, however
# # the cop will mark this as an offense.
# ----
#
# This might be a good indication of code that should be refactored,
# however.
#
# @example
#
# /(foo)bar/ =~ 'foobar'
Expand Down
10 changes: 10 additions & 0 deletions lib/rubocop/cop/lint/percent_string_array.rb
Expand Up @@ -9,6 +9,16 @@ module Lint
# example, mistranslating an array of literals to percent string notation)
# rather than meant to be part of the resulting strings.
#
# @safety
# The cop is unsafe because the correction changes the values in the array
# and that might have been done purposely.
#
# [source,ruby]
# ----
# %w('foo', "bar") #=> ["'foo',", '"bar"']
# %w(foo bar) #=> ['foo', 'bar']
# ----
#
# @example
#
# # bad
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/lint/raise_exception.rb
Expand Up @@ -13,6 +13,10 @@ module Lint
# `Exception`. Alternatively, make `Exception` a fully qualified class
# name with an explicit namespace.
#
# @safety
# This cop is unsafe because it will change the exception class being
# raised, which is a change in behaviour.
#
# @example
# # bad
# raise Exception, 'Error message here'
Expand Down
9 changes: 5 additions & 4 deletions lib/rubocop/cop/lint/redundant_safe_navigation.rb
Expand Up @@ -7,13 +7,14 @@ module Lint
# `instance_of?`, `kind_of?`, `is_a?`, `eql?`, `respond_to?`, and `equal?` methods
# are checked by default. These are customizable with `AllowedMethods` option.
#
# This cop is marked as unsafe, because auto-correction can change the
# return type of the expression. An offending expression that previously
# could return `nil` will be auto-corrected to never return `nil`.
#
# In the example below, the safe navigation operator (`&.`) is unnecessary
# because `NilClass` has methods like `respond_to?` and `is_a?`.
#
# @safety
# This cop is unsafe, because auto-correction can change the return type of
# the expression. An offending expression that previously could return `nil`
# will be auto-corrected to never return `nil`.
#
# @example
# # bad
# do_something if attrs&.respond_to?(:[])
Expand Down
11 changes: 8 additions & 3 deletions lib/rubocop/cop/lint/unexpected_block_arity.rb
Expand Up @@ -13,14 +13,19 @@ module Lint
# Keyword arguments (including `**kwargs`) do not get counted towards
# this, as they are not used by the methods in question.
#
# NOTE: This cop matches for method names only and hence cannot tell apart
# methods with same name in different classes.
#
# Method names and their expected arity can be configured like this:
#
# [source,yaml]
# ----
# Methods:
# inject: 2
# reduce: 2
# ----
#
# @safety
# This cop matches for method names only and hence cannot tell apart
# methods with same name in different classes, which may lead to a
# false positive.
#
# @example
# # bad
Expand Down
5 changes: 3 additions & 2 deletions lib/rubocop/cop/lint/useless_method_definition.rb
Expand Up @@ -6,8 +6,9 @@ module Lint
# This cop checks for useless method definitions, specifically: empty constructors
# and methods just delegating to `super`.
#
# This cop is marked as unsafe as it can trigger false positives for cases when
# an empty constructor just overrides the parent constructor, which is bad anyway.
# @safety
# This cop is unsafe as it can register false positives for cases when an empty
# constructor just overrides the parent constructor, which is bad anyway.
#
# @example
# # bad
Expand Down
11 changes: 7 additions & 4 deletions lib/rubocop/cop/lint/useless_setter_call.rb
Expand Up @@ -5,11 +5,14 @@ module Cop
module Lint
# This cop checks for setter call to local variable as the final
# expression of a function definition.
# Its auto-correction is marked as unsafe because return value will be changed.
#
# NOTE: There are edge cases in which the local variable references a
# value that is also accessible outside the local scope. This is not
# detected by the cop, and it can yield a false positive.
# @safety
# There are edge cases in which the local variable references a
# value that is also accessible outside the local scope. This is not
# detected by the cop, and it can yield a false positive.
#
# As well, auto-correction is unsafe because the method's
# return value will be changed.
#
# @example
#
Expand Down
5 changes: 3 additions & 2 deletions lib/rubocop/cop/lint/useless_times.rb
Expand Up @@ -7,8 +7,9 @@ module Lint
# (when the integer <= 0) or that will only ever yield once
# (`1.times`).
#
# This cop is marked as unsafe as `times` returns its receiver, which
# is *usually* OK, but might change behavior.
# @safety
# This cop is unsafe as `times` returns its receiver, which is
# *usually* OK, but might change behavior.
#
# @example
# # bad
Expand Down
9 changes: 5 additions & 4 deletions lib/rubocop/cop/naming/memoized_instance_variable_name.rb
Expand Up @@ -14,6 +14,11 @@ module Naming
# convention that is used to implicitly indicate that an ivar should not
# be set or referenced outside of the memoization method.
#
# @safety
# This cop relies on the pattern `@instance_var ||= ...`,
# but this is sometimes used for other purposes than memoization
# so this cop is considered unsafe.
#
# @example EnforcedStyleForLeadingUnderscores: disallowed (default)
# # bad
# # Method foo is memoized using an instance variable that is
Expand Down Expand Up @@ -139,10 +144,6 @@ module Naming
# define_method(:foo) do
# @_foo ||= calculate_expensive_thing
# end
#
# This cop relies on the pattern `@instance_var ||= ...`,
# but this is sometimes used for other purposes than memoization
# so this cop is considered unsafe.
class MemoizedInstanceVariableName < Base
include ConfigurableEnforcedStyle

Expand Down
15 changes: 8 additions & 7 deletions lib/rubocop/cop/security/json_load.rb
Expand Up @@ -6,13 +6,14 @@ module Security
# This cop checks for the use of JSON class methods which have potential
# security issues.
#
# Autocorrect is disabled by default because it's potentially dangerous.
# If using a stream, like `JSON.load(open('file'))`, it will need to call
# `#read` manually, like `JSON.parse(open('file').read)`.
# If reading single values (rather than proper JSON objects), like
# `JSON.load('false')`, it will need to pass the `quirks_mode: true`
# option, like `JSON.parse('false', quirks_mode: true)`.
# Other similar issues may apply.
# @safety
# Autocorrect is disabled by default because it's potentially dangerous.
# If using a stream, like `JSON.load(open('file'))`, it will need to call
# `#read` manually, like `JSON.parse(open('file').read)`.
# If reading single values (rather than proper JSON objects), like
# `JSON.load('false')`, it will need to pass the `quirks_mode: true`
# option, like `JSON.parse('false', quirks_mode: true)`.
# Other similar issues may apply.
#
# @example
# # bad
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/security/open.rb
Expand Up @@ -11,6 +11,10 @@ module Security
# the argument of `Kernel#open` and `URI.open`. It would be better to use
# `File.open`, `IO.popen` or `URI.parse#open` explicitly.
#
# @safety
# This cop could register false positives if `open` is redefined
# in a class and then used without a receiver in that class.
#
# @example
# # bad
# open(something)
Expand Down

0 comments on commit 28ae02a

Please sign in to comment.