diff --git a/changelog/change_add_safety_section_to_all_cops_that_are.md b/changelog/change_add_safety_section_to_all_cops_that_are.md new file mode 100644 index 00000000000..d32d7ddd0d6 --- /dev/null +++ b/changelog/change_add_safety_section_to_all_cops_that_are.md @@ -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][]) diff --git a/config/default.yml b/config/default.yml index 6bf19087db2..02c9f49f55e 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/lib/rubocop/cop/lint/ambiguous_range.rb b/lib/rubocop/cop/lint/ambiguous_range.rb index eb4b106544c..e384b6f1130 100644 --- a/lib/rubocop/cop/lint/ambiguous_range.rb +++ b/lib/rubocop/cop/lint/ambiguous_range.rb @@ -10,12 +10,6 @@ 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. @@ -23,6 +17,13 @@ module Lint # 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 @@ -55,7 +56,6 @@ module Lint # # # good # (a.foo)..(b.bar) - # class AmbiguousRange < Base extend AutoCorrector diff --git a/lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb b/lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb index 1af3b74ef0d..5c042276946 100644 --- a/lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb +++ b/lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb @@ -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 diff --git a/lib/rubocop/cop/lint/boolean_symbol.rb b/lib/rubocop/cop/lint/boolean_symbol.rb index e8fd68d7ac5..02732f8a3db 100644 --- a/lib/rubocop/cop/lint/boolean_symbol.rb +++ b/lib/rubocop/cop/lint/boolean_symbol.rb @@ -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 diff --git a/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb b/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb index f74c0799ecf..3b07c24d5f0 100644 --- a/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb +++ b/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb @@ -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 @@ -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 diff --git a/lib/rubocop/cop/lint/hash_compare_by_identity.rb b/lib/rubocop/cop/lint/hash_compare_by_identity.rb index f98c850223c..5305e48277d 100644 --- a/lib/rubocop/cop/lint/hash_compare_by_identity.rb +++ b/lib/rubocop/cop/lint/hash_compare_by_identity.rb @@ -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 diff --git a/lib/rubocop/cop/lint/interpolation_check.rb b/lib/rubocop/cop/lint/interpolation_check.rb index 3012358db3f..8fc44b9c9f0 100644 --- a/lib/rubocop/cop/lint/interpolation_check.rb +++ b/lib/rubocop/cop/lint/interpolation_check.rb @@ -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 diff --git a/lib/rubocop/cop/lint/loop.rb b/lib/rubocop/cop/lint/loop.rb index 8254b09f768..e65b36bbe1f 100644 --- a/lib/rubocop/cop/lint/loop.rb +++ b/lib/rubocop/cop/lint/loop.rb @@ -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 # diff --git a/lib/rubocop/cop/lint/non_deterministic_require_order.rb b/lib/rubocop/cop/lint/non_deterministic_require_order.rb index 36005e38c09..1fba59f8278 100644 --- a/lib/rubocop/cop/lint/non_deterministic_require_order.rb +++ b/lib/rubocop/cop/lint/non_deterministic_require_order.rb @@ -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 # diff --git a/lib/rubocop/cop/lint/number_conversion.rb b/lib/rubocop/cop/lint/number_conversion.rb index a61bc973d3b..fd9e1b7871d 100644 --- a/lib/rubocop/cop/lint/number_conversion.rb +++ b/lib/rubocop/cop/lint/number_conversion.rb @@ -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 diff --git a/lib/rubocop/cop/lint/or_assignment_to_constant.rb b/lib/rubocop/cop/lint/or_assignment_to_constant.rb index 3681ae86706..e7b7c877267 100644 --- a/lib/rubocop/cop/lint/or_assignment_to_constant.rb +++ b/lib/rubocop/cop/lint/or_assignment_to_constant.rb @@ -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 # diff --git a/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb index 6275013b689..23395e3e6a4 100644 --- a/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb +++ b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb @@ -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' diff --git a/lib/rubocop/cop/lint/percent_string_array.rb b/lib/rubocop/cop/lint/percent_string_array.rb index bf84e1a1c73..ea5eef58e4c 100644 --- a/lib/rubocop/cop/lint/percent_string_array.rb +++ b/lib/rubocop/cop/lint/percent_string_array.rb @@ -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 diff --git a/lib/rubocop/cop/lint/raise_exception.rb b/lib/rubocop/cop/lint/raise_exception.rb index 00eb4ee1c2b..8711d8d14a6 100644 --- a/lib/rubocop/cop/lint/raise_exception.rb +++ b/lib/rubocop/cop/lint/raise_exception.rb @@ -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' diff --git a/lib/rubocop/cop/lint/redundant_safe_navigation.rb b/lib/rubocop/cop/lint/redundant_safe_navigation.rb index 2835b6038b6..771d4a959de 100644 --- a/lib/rubocop/cop/lint/redundant_safe_navigation.rb +++ b/lib/rubocop/cop/lint/redundant_safe_navigation.rb @@ -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?(:[]) diff --git a/lib/rubocop/cop/lint/unexpected_block_arity.rb b/lib/rubocop/cop/lint/unexpected_block_arity.rb index e679ab3d91c..f1288d72979 100644 --- a/lib/rubocop/cop/lint/unexpected_block_arity.rb +++ b/lib/rubocop/cop/lint/unexpected_block_arity.rb @@ -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 diff --git a/lib/rubocop/cop/lint/useless_method_definition.rb b/lib/rubocop/cop/lint/useless_method_definition.rb index 3139c20b8c2..0d9cf969f1c 100644 --- a/lib/rubocop/cop/lint/useless_method_definition.rb +++ b/lib/rubocop/cop/lint/useless_method_definition.rb @@ -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 diff --git a/lib/rubocop/cop/lint/useless_setter_call.rb b/lib/rubocop/cop/lint/useless_setter_call.rb index 51b775bf7c4..0c95c7dcdd3 100644 --- a/lib/rubocop/cop/lint/useless_setter_call.rb +++ b/lib/rubocop/cop/lint/useless_setter_call.rb @@ -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 # diff --git a/lib/rubocop/cop/lint/useless_times.rb b/lib/rubocop/cop/lint/useless_times.rb index 6aa5b3f35da..ec9125effbd 100644 --- a/lib/rubocop/cop/lint/useless_times.rb +++ b/lib/rubocop/cop/lint/useless_times.rb @@ -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 diff --git a/lib/rubocop/cop/naming/memoized_instance_variable_name.rb b/lib/rubocop/cop/naming/memoized_instance_variable_name.rb index 087267bd12d..6668aec9763 100644 --- a/lib/rubocop/cop/naming/memoized_instance_variable_name.rb +++ b/lib/rubocop/cop/naming/memoized_instance_variable_name.rb @@ -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 @@ -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 diff --git a/lib/rubocop/cop/security/json_load.rb b/lib/rubocop/cop/security/json_load.rb index b9a6e6fada9..afc859d97fc 100644 --- a/lib/rubocop/cop/security/json_load.rb +++ b/lib/rubocop/cop/security/json_load.rb @@ -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 diff --git a/lib/rubocop/cop/security/open.rb b/lib/rubocop/cop/security/open.rb index 76c505bc9a2..ce0203b63b4 100644 --- a/lib/rubocop/cop/security/open.rb +++ b/lib/rubocop/cop/security/open.rb @@ -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) diff --git a/lib/rubocop/cop/security/yaml_load.rb b/lib/rubocop/cop/security/yaml_load.rb index a3cf4b56226..16dba0650bf 100644 --- a/lib/rubocop/cop/security/yaml_load.rb +++ b/lib/rubocop/cop/security/yaml_load.rb @@ -7,6 +7,10 @@ module Security # potential security issues leading to remote code execution when # loading from an untrusted source. # + # @safety + # The behaviour of the code might change depending on what was + # in the YAML payload, since `YAML.safe_load` is more restrictive. + # # @example # # bad # YAML.load("--- foo") diff --git a/lib/rubocop/cop/style/and_or.rb b/lib/rubocop/cop/style/and_or.rb index 35352912b88..7371be01239 100644 --- a/lib/rubocop/cop/style/and_or.rb +++ b/lib/rubocop/cop/style/and_or.rb @@ -7,9 +7,10 @@ module Style # `||` instead. It can be configured to check only in conditions or in # all contexts. # - # It is marked as unsafe auto-correction because it may change the - # operator precedence between logical operators (`&&` and `||`) and - # semantic operators (`and` and `or`). + # @safety + # Auto-correction is unsafe because there is a different operator precedence + # between logical operators (`&&` and `||`) and semantic operators (`and` and `or`), + # and that might change the behaviour. # # @example EnforcedStyle: always # # bad diff --git a/lib/rubocop/cop/style/array_coercion.rb b/lib/rubocop/cop/style/array_coercion.rb index c4845c87225..ecfcb41d226 100644 --- a/lib/rubocop/cop/style/array_coercion.rb +++ b/lib/rubocop/cop/style/array_coercion.rb @@ -5,9 +5,27 @@ module Cop module Style # This cop enforces the use of `Array()` instead of explicit `Array` check or `[*var]`. # - # This cop is disabled by default because false positive will occur if - # the argument of `Array()` is not an array (e.g. Hash, Set), - # an array will be returned as an incompatibility result. + # The cop is disabled by default due to safety concerns. + # + # @safety + # This cop is unsafe because a false positive may occur if + # the argument of `Array()` is (or could be) nil or depending + # on how the argument is handled by `Array()` (which can be + # different than just wrapping the argument in an array). + # + # For example: + # + # [source,ruby] + # ---- + # [nil] #=> [nil] + # Array(nil) #=> [] + # + # [{a: 'b'}] #= [{a: 'b'}] + # Array({a: 'b'}) #=> [[:a, 'b']] + # + # [Time.now] #=> [#