(PUP-11993) Even more Style cops! #9284

AriaXLi · 2024-03-06T23:52:12Z

No description provided.

This commit enables the Style/SelfAssignment cop and fixes 13 autocorrectable fixes.

kenyon · 2024-03-07T00:41:55Z

typo in PR title

This commit moves Style/PreferredHashMethods from .rubocop_todo.yml to .rubocop.yml to indefinitely disable the cop. This is because enabling this cop causes 11 failures in rb_tree_map_spec.rb that relate to core functions in trees, like deleting a node and returning nil when a key can't be found.

acceptance/hosts.yaml

classes.pp

test.patch

testing.pp

joshcooper

I stopped copy/pasting my comment, but it applies to most of these changes. I think rescue ... => e is preferable because it doesn't rely on global state (which isn't thread safe) and it avoids requiring the 'English'.

lib/puppet/file_serving/fileset.rb

lib/puppet/provider/package/ports.rb

lib/puppet/provider/package/portupgrade.rb

lib/puppet/provider/package/rpm.rb

lib/puppet/provider/service/base.rb

lib/puppet/provider/service/daemontools.rb

This commit enables the Style/SpecialGlobalVars cop and fixes 41 (unsafe) autocorrectable offenses. Additionally, this commit includes => e when rescuing so there is a local reference to the exception and ERROR_INFO is not needed.

joshcooper

I think these are useful and should be enabled:

Style/StringLiteralsInInterpolation - Easy to mix up single and double quotes in interpolated strings. Might be good to enable this
Style/StructInheritance - need to update SearchPathElement = Struct.new(:name, :type)
Style/SymbolLiteral - hard to remember the rule for when symbols must be quoted
Style/SymbolProc - more streamline to say map(&:thing)
Style/TrailingCommaInArguments - the presence of trailing commas in arguments gets confusing when there are required keyword arguments
Style/TrailingCommaInArrayLiteral - the presence of trailing commas in arrays look like typos
Style/UnlessElse - unless [condition] else [condition] is confusing
Style/UnpackFirst - more streamline way to get the first array element
Style/VariableInterpolation - good to consistently use brackets when interpolating
Style/WhileUntilDo - I didn't know do was optional
Style/WordArray - using %w[ ] does improve readability

I think we can skip these:

Style/StderrPuts - puppet defines a warn method so changing $stderr.puts could break something
Style/StringConcatenation - avoiding lots of small string allocations is good, but lots of changes, maybe later
Style/StringLiterals - I have a hate/like relationship with single quoted strings. Lots of changes here. Maybe later?
Style/SymbolArray - Lots of changes, maybe later?
Style/TernaryParentheses - adding parentheses isn't necessarily bad if (a || b) ? c : d)
Style/TrailingCommaInHashLiteral - puppet code recommends trailing commas for later diffing
Style/TrailingUnderscoreVariable - I think this is a bad cop
Style/TrivialAccessors - using attr_reader is good, but I'm not sure how rubocop handles the documentation above the existing method?
Style/WhileUntilModifier - seems pedantic
Style/WhenThen - I prefer semicolons instead of then
Style/YodaCondition - the order doesn't matter in ruby like it does in other languages
Style/ZeroLengthPredicate - meh

This commit enables the Style/StructInheritance cop and fixes 5 autocorrectable offenses.

This commit enables the Style/StringLiteralsInInterpolation cop and fixes 23 autocorrectable offenses.

AriaXLi · 2024-03-08T00:33:45Z

I think these are useful and should be enabled:

* Style/StringLiteralsInInterpolation - Easy to mix up single and double quotes in interpolated strings. Might be good to enable this

* Style/StructInheritance - need to update `SearchPathElement = Struct.new(:name, :type)`

* Style/SymbolLiteral - hard to remember the rule for when symbols must be quoted

* Style/SymbolProc - more streamline to say `map(&:thing)`

* Style/TrailingCommaInArguments - the presence of trailing commas in arguments gets confusing when there are required keyword arguments

* Style/TrailingCommaInArrayLiteral - the presence of trailing commas in arrays look like typos

* Style/UnlessElse - `unless [condition] else [condition]` is confusing

* Style/UnpackFirst - more streamline way to get the first array element

* Style/VariableInterpolation - good to consistently use brackets when interpolating

* Style/WhileUntilDo - I didn't know `do` was optional

* Style/WordArray - using `%w[ ]` does improve readability

I think we can skip these:

* Style/StderrPuts - puppet defines a `warn` method so changing $stderr.puts could break something

* Style/StringConcatenation - avoiding lots of small string allocations is good, but lots of changes, maybe later

* Style/StringLiterals - I have a hate/like relationship with single quoted strings. Lots of changes here. Maybe later?

* Style/SymbolArray - Lots of changes, maybe later?

* Style/TernaryParentheses - adding parentheses isn't necessarily bad `if (a || b) ? c : d`)

* Style/TrailingCommaInHashLiteral - puppet code recommends trailing commas for later diffing

* Style/TrailingUnderscoreVariable - I think this is a bad cop

* Style/TrivialAccessors - using `attr_reader` is good, but I'm not sure how rubocop handles the documentation above the existing method?

* Style/WhileUntilModifier - seems pedantic

* Style/WhenThen - I prefer semicolons instead of `then`

* Style/YodaCondition - the order doesn't matter in ruby like it does in other languages

* Style/ZeroLengthPredicate - meh

Thank you, this is incredibly helpful! The comments help a lot on how you determine what's a helpful cop and what isnt.

This commit enables the Style/SymbolLiteral cop and fixes 1 autocorrectable offense.

This commit enables the Style/SymbolProc cop and fixes 125 (unsafe) autocorrectable offenses.

This commit enables the Style/TrailingCommaInArguments cop and fixes 17 autocorrectable offenses.

This commit enables the Style/TrailingCommaInArrayLiteral cop and fixes 16 autocorrectable offenses.

This commit enables the Style/UnlessElse cop and fixes 13 autocorrectable offenses.

This commit enables the Style/UnpackFirst cop and fixes 5 autocorrectable offenses.

This commit enables the Style/VariableInterpolation cop and fixes 1 autocorrectable offense.

This commit enables the Style/WhileUntilDo cop and fixes 24 autocorrectable offenses.

This commit enables the Style/WordArray cop and fixes 41 autocorrectable offenses.

joshcooper · 2024-03-08T22:57:25Z

jenkins please test this

(PUP-11993) Style/SelfAssignment

131c4ab

This commit enables the Style/SelfAssignment cop and fixes 13 autocorrectable fixes.

AriaXLi requested review from a team as code owners March 6, 2024 23:52

AriaXLi force-pushed the PUP-11993/more_style_cops branch from da9b4f2 to 131c4ab Compare March 7, 2024 00:31

AriaXLi changed the title ~~(PUP-11993) Even more Syle cops!~~ (PUP-11993) Even more Style cops! Mar 7, 2024

AriaXLi marked this pull request as draft March 7, 2024 16:57

AriaXLi force-pushed the PUP-11993/more_style_cops branch 2 times, most recently from e59471b to 21d8383 Compare March 7, 2024 22:40