(PUP-2950) rubocop - Style/AndOR enabled for the entire puppet #2900
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vrthra
changed the title
|
CLA signed by all contributors. |
vrthra
changed the title
…ovided a layer
Without this patch,
```
unless diagnostics.errors?
@layering_config = data['layers'] or default_layers
@scheme_extensions = (data['extensions'] and
data['extensions']['scheme_handlers'] or
default_scheme_extensions)
else
@layering_config = []
@scheme_extensions = {}
end
```
This would have resulted in @layering_config not being assigned a value if no
layers was set. This is clearly not intented. This patch updates the behavior
so that `@layering_config` is assigned default_layers instead
(by changing `or` to `||`)
This commit also adds test cases to verify that this case is caught.
This commits enables `AndOr` cop only in lib/puppet/pops directory. This cop bans the use of `and` and `or` keywords in favor of `&&` and `||`. We have chosen this directory as a test case to evaluate the utility of enabling `AndOr` on the complete codebase. We inherit from the base rubocop file, so all the other cops remain enabled. At a future date, when `AndOr` is enabled systemwide, this file may be removed (if no other specialization exists on pops directory). It changes all code that uses `and` and `or` keywords to use `&&` and `||` instead. Note that this commit only fixes the lib/puppet/pops directory. Further, for ease of verification of correctness, redundant parenthesis has been used around the expressions thus converted where there was felt to be a possibility of confusion.
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. This commit enables the `AndOr` cop
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. Rubocop: autocorrect
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
This is part of a series of commits that follows the same pattern. These
checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop
so that future use of `and` and `or` is banned (and instead `&&` and `||` is
encouraged)
For the ease of modification, and to ensure that the possible errors remain a
minimum, we have attempted to make the changes as consistent to each other as
possible, and have also given preference to explicit parenthesis where doing so
helped.
Our approach has been to
- Let rubocopop autocorrect where possible
- Look for regular expression patterns that could catch the maximum number of
violations without ambiguity, and apply these to the code, and then manually
verify that the changes have indeed been correct (and correct where
necessary).
- Once such patterns were exhausted, the remaining changes were done by hand.
REGEX USED:
Replacing only the simplest pattern
s#^( *)if +([^ ]+) +and +([^ ]+)$#$1if $2 && $3#g
s#^( *)if +([^ ]+) +or +([^ ]+)$#$1if $2 || $3#g
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: Replacing only the simplest patterns 's#^(.*) if +([^ ]+) +or +([^ ]+)$#$1 if $2 || $3#g' 's#^(.*) if +([^ ]+) +and +([^ ]+)$#$1 if $2 && $3#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: Replacing only the pattern 's#^(.*) if +([^ ]+) +([!=~+-<>]+) +([^ ]+) and ([^ ]+) ([!=~+-><]+) +([^ \n]+)$#$1 if ($2 $3 $4) && ($5 $6 $7)#g' 's#^(.*) if +([^ ]+) +([!=~+-<>]+) +([^ ]+) or ([^ ]+) ([!=~+-><]+) +([^ \n]+)$#$1 if ($2 $3 $4) || ($5 $6 $7)#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: 's#^(.*) if +([^ ]+) +([!=~+-<>]+) +([^ ]+) or ([^ \n]+)$#$1 if ($2 $3 $4) || $5#g' 's#^(.*) if +([^ ]+) or ([^ ]+) ([!=~+-><]+) +([^ \n]+)$#$1 if $2 || ($3 $4 $5)#g' 's#^(.*) if +([^ ]+) +([!=~+-<>]+) +([^ ]+) and ([^ \n]+)$#$1 if ($2 $3 $4) && $5#g' 's#^(.*) if +([^ ]+) and ([^ ]+) ([!=~+-><]+) +([^ \n]+)$#$1 if $2 && ($3 $4 $5)#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: Only patterns 's#(.*) +unless +([^ ]+) +or +([^ ]+)$#$1 unless $2 || $3#g' 's#(.*) +unless +([^ ]+) +and +([^ ]+)$#$1 unless $2 && $3#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: '%s#if ([^ ]+) and ([^ ]+) then#if $1 && $2 then#g' '%s#if ([^ ]+) or ([^ ]+) then#if $1 || $2 then#g'
This is part of a series of commits that follows the same pattern. These
checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop
so that future use of `and` and `or` is banned (and instead `&&` and `||` is
encouraged)
For the ease of modification, and to ensure that the possible errors remain a
minimum, we have attempted to make the changes as consistent to each other as
possible, and have also given preference to explicit parenthesis where doing so
helped.
Our approach has been to
- Let rubocopop autocorrect where possible
- Look for regular expression patterns that could catch the maximum number of
violations without ambiguity, and apply these to the code, and then manually
verify that the changes have indeed been correct (and correct where
necessary).
- Once such patterns were exhausted, the remaining changes were done by hand.
REGEX USED:
replacing only patterns
's#([ ]*)if +([^ ]+) +and +([^ ]+)$#${1}if $2 && $3#g'
's#([ ]*)if +([^ ]+) +or +([^ ]+)$#${1}if $2 || $3#g'
's#([ ]*)if +([^ ]+) +and not +([^ ]+)$#${1}if $2 && !$3#g'
's#([ ]*)if not +([^ ]+) +and not +([^ ]+)$#${1}if !$2 && !$3#g'
's#([ ]*)if not +([^ ]+) +and +([^ ]+)$#${1}if !$2 && $3#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: Replacing only patterns 's#^([^ ]+) +and +([^ ]+)$#$1 && $2#g' 's#^([^ ]+) +or +([^ ]+)$#$1 || $2#g' 's#^( *[^ ]+) +and +([^ ]+)$#$1 && $2#g' 's#^( *[^ ]+) +or +([^ ]+)$#$1 || $2#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: '%s#if ([^ ]+) and ([^ ]+)#if $1 and $2#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: '%s#(.*) if ([^ ]+) and ([^ ]+) #$1 if $2 and $3 #g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: '%s#(.*) if ([^ ]+) ([=!~]+) and ([^ ]+) #$1 if ($2) && $3 #g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: 's#^( *)([^ ]+) +or raise (.*)$#$1$2 || (raise $3)#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: 's#^( *)([^ ]+) +or return (.*)$#$1$2 || (return $3)#g'
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED: by hand
This is part of a series of commits that follows the same pattern. These checkins remove use of `and` and `or` keywords, and enable the Style/AndOr cop so that future use of `and` and `or` is banned (and instead `&&` and `||` is encouraged) For the ease of modification, and to ensure that the possible errors remain a minimum, we have attempted to make the changes as consistent to each other as possible, and have also given preference to explicit parenthesis where doing so helped. Our approach has been to - Let rubocopop autocorrect where possible - Look for regular expression patterns that could catch the maximum number of violations without ambiguity, and apply these to the code, and then manually verify that the changes have indeed been correct (and correct where necessary). - Once such patterns were exhausted, the remaining changes were done by hand. REGEX USED:
|
I'm concerned about the complete removal of Take the following example: - execute_prerun_command or return nil
+ execute_prerun_command || (return nil)I think that the latter is less clear than the former. Moreover, removing the parentheses is an illegal expression: The flow control operators have a lower precedence for a specific reason - when explicitly used for flow control they behave correctly. Their very low precedence makes them suitable for this specific use case. As we've seen using flow control operators in conditional expressions can do surprising things, which is why we certainly should strip them from conditionals. However removing the flow control operators and replacing them with boolean operators will flip around the problem - instead of precedence in conditionals screwing us up, we'll have precedence in flow control doing unexpected things. This will remove one class of errors but introduce another. We can continue forward with removing flow control operators for the sake of consistency and unambiguity, but doing this means that we should remove the use of flow control style operations entirely. Instead of using |
|
General comment - the title of the commit should indicate a bit more about the changes it's introducing instead of having |
|
@adrienthebo it actually does indicate what has been changed. The regular expressions are too big to put on the commit line, so instead the specifics of the regular expressions have been put in the body. Here is the format of the commit line I adopted: (PUP-2950) Enable rubocop Style/AndOr in lib/ %HOW% where nothing is said if it was an automatic regular expression change, (no auto) indicates it was done by hand, (fix spacing) for removing spaces between operator ! and expression, (autocorrect) for letting rubocop do the correction, and (final fixup) for fixes for issues I found at the end. |
|
@adrienthebo the thing about |
|
@vrthra yes, but my concern is that with the parens the code looks very unusual, and without them we run into the syntax error. By trying to outright ban the logical composition operators we have to use decorated and non-intuitive syntax instead, which I think is a loss rather than a gain. |
|
Closing this to rework |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes were partially automated by regular expression search and replace (about 400 out of 800 changes). The changes were further manually inspected to ensure that the changes were indeed sane.