Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #8431] Add Safety section to documentation for all cops that are Safe: false or SafeAutoCorrect: false #10094

Merged
merged 2 commits into from Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .yardopts
@@ -1,2 +1,3 @@
--markup markdown
--hide-void-return
--tag safety:"Cop Safety Information"
@@ -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
5 changes: 5 additions & 0 deletions lib/rubocop/cop/generator.rb
Expand Up @@ -24,6 +24,11 @@ module %<department>s
# `SupportedStyle` and unique configuration, there needs to be examples.
# Examples must have valid Ruby syntax. Do not use upticks.
#
# @safety
# Delete this section if the cop is not unsafe (`Safe: false` or
# `SafeAutoCorrect: false`), or use it to explain how the cop is
# unsafe.
#
# @example EnforcedStyle: bar (default)
# # Description of the `bar` style.
#
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