michaeljb commented 1 week ago

Ideas for more cops based on PR comments

[ ] don't use nil?, use truthiness checks instead
[ ] [0] instead of .first
[ ] index { } instead of find_index { }
[ ] size instead of length or count
- https://www.github.com/tobymao/18xx/pull/9293#discussion_r1257785323
- https://www.github.com/tobymao/18xx/pull/9182#discussion_r1193083326 (different object than a collection here, so this count isn't actually the same, but maybe that's an indication the attr should be renamed from count)
- https://www.github.com/tobymao/18xx/pull/7809#discussion_r879039857
- https://www.github.com/tobymao/18xx/pull/7543#discussion_r835836974
- https://www.github.com/tobymao/18xx/pull/7375#discussion_r819312009
[ ] don't use flat_map
- https://www.github.com/tobymao/18xx/pull/9187#discussion_r1199561456
[ ] use reject/select instead of delete_if/filter
[ ] prevent use of built-in Random
- https://www.github.com/tobymao/18xx/pull/8889#discussion_r1117857951
[ ] use empty? instead of none? (if no block given on none?)
- https://www.github.com/tobymao/18xx/pull/8889#discussion_r1119519251
- https://www.github.com/tobymao/18xx/pull/8119#discussion_r931772767
[ ] don't map { }.map { }
- https://www.github.com/tobymao/18xx/pull/8016#discussion_r906837969
[ ] stop inheriting certain base engine classes, this would require a big refactor
- https://www.github.com/tobymao/18xx/pull/6937#discussion_r799134823
- https://www.github.com/tobymao/18xx/pull/6836#discussion_r790399174
[ ] don't have a keyword arg and a **kwargs
- https://www.github.com/tobymao/18xx/pull/6886#discussion_r796241152
[ ] enforce only standard files for games
- https://www.github.com/tobymao/18xx/pull/6865#discussion_r795118316
[ ] empty? instead of !size.positive?
- https://www.github.com/tobymao/18xx/pull/6848#discussion_r793293724

cops that probably already exist

[ ] use parens for function calls
[ ] no puts or print
- https://www.github.com/tobymao/18xx/pull/7609#discussion_r848009981
[ ] redundant parens https://www.rubydoc.info/github/bbatsov/RuboCop/RuboCop/Cop/Style/RedundantParentheses
- https://www.github.com/tobymao/18xx/pull/7276#issuecomment-1048476899
[ ] class structure https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Layout/ClassStructure
- https://www.github.com/tobymao/18xx/pull/6732#discussion_r780728694
[ ] use && not and
- https://www.github.com/tobymao/18xx/pull/6709#discussion_r779232697

other ideas

[ ] mathematical division is one source of errors between Ruby and JS; in ruby int/int returns int while in JS int/int returns a floating point number, so maybe a cop that looks for int / int?

michaeljb commented 1 week ago

For the above list, I looked at comments on PRs from with this search link https://www.github.com/tobymao/18xx/pulls?page=1&q=is%3Apr+commenter%3Atobymao+is%3Aclosed from PR 10720, back through PR 8592 on January 1, 2023

update: Now checked through January 1, 2022, back through PR 6696. Might go through more later, 2021 is on pages 23-60, and 2020 on pages 61-99, both a lot more active than 2022 and 2023

michaeljb commented 1 week ago

Here's a suggestion to use .any? with no block instead of !.compact.empty? -- currently the NoBlocklessAny cop prefers !.compact.entity? https://www.github.com/tobymao/18xx/pull/8424#discussion_r988546410

michaeljb commented 1 week ago

Assuming cop config allows this, probably a lot of these could be handled in one cop where unwanted method names and their suggested replacements are put into the config. For a generalized cop like that, auto-correct probably would not work.

michaeljb commented 1 day ago

enforce ::Set instead of Set:

michaeljb / rubocop-g18xx

Cop ideas #1

Ideas for more cops based on PR comments

cops that probably already exist

other ideas