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.

Sign up for GitHub

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

Jump to bottom

Cop idea: Style/CompactBlank #562

Closed
benjaminwols opened this issue Sep 24, 2021 · 1 comment · Fixed by #598
Closed

Cop idea: Style/CompactBlank #562

benjaminwols opened this issue Sep 24, 2021 · 1 comment · Fixed by #598
Labels
feature request Request for new functionality

Comments

@benjaminwols
Copy link

benjaminwols commented Sep 24, 2021 •
edited
Loading

Rails added compact_blank in Rails 6.1: https://apidock.com/rails/Enumerable/compact_blank

Would you be open to a cop to enforce the usage of compact_blank over reject(&:blank?)
Similar to https://github.com/rubocop/rubocop/blob/master/lib/rubocop/cop/style/collection_compact.rb

See also: https://andycroll.com/ruby/compact_blank-remove-empty-strings-from-array-and-hash/ for some examples

bad

array.reject { |e| e.blank? }
array.reject { |e| e.empty? }

good

array.compact_blank

bad

hash.reject! { |k, v| v.blank? }
hash.reject! { |k, v| v.empty? }

good

hash.compact_blank!
@koic koic added the feature request Request for new functionality label Sep 24, 2021
koic added a commit to koic/rubocop-rails that referenced this issue Dec 20, 2021
@koic
[Fix rubocop#562] Add new Rails/CompactBlank cop
f03f4e5 
This cop checks for places where custom logic on rejection blanks
from arrays and hashes can be replaced with `compact_blank`.

```ruby
# bad
collection.reject { |e| e.blank? }
collection.reject { |e| e.empty? }

# good
collection.compact_blank

# bad
collection.reject! { |e| e.blank? }
collection.reject! { |e| e.empty? }

# good
collection.compact_blank!
```
koic added a commit to koic/rubocop-rails that referenced this issue Dec 20, 2021
@koic
[Fix rubocop#562] Add new Rails/CompactBlank cop
401297a 
Fixes rubocop#562.

This cop checks for places where custom logic on rejection blanks
from arrays and hashes can be replaced with `compact_blank`.

```ruby
# bad
collection.reject { |e| e.blank? }
collection.reject { |e| e.empty? }

# good
collection.compact_blank

# bad
collection.reject! { |e| e.blank? }
collection.reject! { |e| e.empty? }

# good
collection.compact_blank!
```
@koic koic mentioned this issue Dec 20, 2021
Merged
10 tasks
@pirj
Copy link
Member

pirj commented Dec 20, 2021

Just for the reference rails/rails#36412

koic added a commit to koic/rubocop-rails that referenced this issue Dec 21, 2021
@koic
[Fix rubocop#562] Add new Rails/CompactBlank cop
e001bc0 
Fixes rubocop#562.

This cop checks for places where custom logic on rejection blanks
from arrays and hashes can be replaced with `compact_blank`.

```ruby
# bad
collection.reject { |e| e.blank? }
collection.reject { |e| e.empty? }

# good
collection.compact_blank

# bad
collection.reject! { |e| e.blank? }
collection.reject! { |e| e.empty? }

# good
collection.compact_blank!
```
koic added a commit to koic/rubocop-rails that referenced this issue Dec 21, 2021
@koic
[Fix rubocop#562] Add new Rails/CompactBlank cop
c7f39c4 
Fixes rubocop#562.

This cop checks for places where custom logic on rejection blanks
from arrays and hashes can be replaced with `compact_blank`.

```ruby
# bad
collection.reject { |e| e.blank? }
collection.reject { |e| e.empty? }

# good
collection.compact_blank

# bad
collection.reject! { |e| e.blank? }
collection.reject! { |e| e.empty? }

# good
collection.compact_blank!
```
@koic koic closed this as completed in #598 Dec 23, 2021
koic added a commit that referenced this issue Dec 23, 2021
@koic
Merge pull request #598 from koic/add_new_rails_compact_blank_cop
a80ac3b 
[Fix #562] Add new `Rails/CompactBlank` cop
notapatch added a commit to unboxed/bops that referenced this issue Nov 30, 2022
@notapatch
Prefer compact_blank over writing by hand
630ba80 
- Rails 6.1 introduced a consistent interface for removing
  blank from collections

PR rails/rails#36412

> I frequently find myself having to .compact but for blank. which means
> on an array reject(&:blank?) (this is fine), or,
> on a hash .reject { |_k, v| v.blank? } which is slightly more
> frustrating and i usually write it as .reject(&:blank?) first and am
> confused when it's trying to check if the keys are blank.

> I've added the analagous .compact_blank! where there's a reject! to
> build on (there's also a reject! in Set, but there's no other core_ext
> touching Set so i've left that alone)

Rubocp rubocop/rubocop-rails#562
notapatch added a commit to unboxed/bops that referenced this issue Dec 1, 2022
@notapatch
Prefer compact_blank over writing by hand
e6c09b4 
- Rails 6.1 introduced a consistent interface for removing
  blank from collections

PR rails/rails#36412

> I frequently find myself having to .compact but for blank. which means
> on an array reject(&:blank?) (this is fine), or,
> on a hash .reject { |_k, v| v.blank? } which is slightly more
> frustrating and i usually write it as .reject(&:blank?) first and am
> confused when it's trying to check if the keys are blank.

> I've added the analagous .compact_blank! where there's a reject! to
> build on (there's also a reject! in Set, but there's no other core_ext
> touching Set so i've left that alone)

Rubocop rubocop/rubocop-rails#562
 - Rails/CompactBlank
notapatch added a commit to unboxed/bops that referenced this issue Dec 1, 2022
@notapatch
Prefer compact_blank over writing by hand
75f6ab6 
- Rails 6.1 introduced a consistent interface for removing
  blank from collections

PR rails/rails#36412

> I frequently find myself having to .compact but for blank. which means
> on an array reject(&:blank?) (this is fine), or,
> on a hash .reject { |_k, v| v.blank? } which is slightly more
> frustrating and i usually write it as .reject(&:blank?) first and am
> confused when it's trying to check if the keys are blank.

> I've added the analagous .compact_blank! where there's a reject! to
> build on (there's also a reject! in Set, but there's no other core_ext
> touching Set so i've left that alone)

Rubocop rubocop/rubocop-rails#562
 - Rails/CompactBlank
notapatch added a commit to unboxed/bops that referenced this issue Dec 2, 2022
@notapatch
Rubocop-rails updated
7dd17b1 
- Rails/ActionOrder

  Enforces consistent ordering of the standard Rails RESTful
  controller actions.

  The cop is configurable and can enforce any ordering of the
  standard actions. All other methods are ignored. So, the
  actions specified in ExpectedOrder should be defined before
  actions not specified.

  Rails/ActionOrder:
   ExpectedOrder:
     - index
     - show
     - new
     - edit
     - create
     - update
     - destroy

- Rails/CompactBlank
  Prefer compact_blank over writing by hand
  Rubocop rubocop/rubocop-rails#562

  - Rails 6.1 introduced a consistent interface for removing
    blank from collections

  - PR rails/rails#36412

    > I frequently find myself having to .compact but for blank.
    > which means on an array reject(&:blank?) (this is fine),
    > or, on a hash .reject { |_k, v| v.blank? } which is
    > slightly more frustrating and i usually write it as
    > .reject(&:blank?) first and am confused when it's trying
    > to check if the keys are blank.

    > I've added the analagous .compact_blank! where there's a
    > reject! to build on (there's also a reject! in Set, but
    > there's no other core_ext touching Set so i've left that
    > alone)

- Rails/DuplicateScope: Multiple scopes share this same where clause

  - in this case referenced only seemed to be used once (in the
    scope below it - 'for_display').
  - the other scope "referenced_in_decision_notice" is used around
    the application
  - keeping "referenced_in_decision_notice" and removing
    "referenced"

- Rails/DurationArithmetic

  Duration methods replace adding and subtracting with current
  time
  https://docs.rubocop.org/rubocop-rails/cops_rails.html#railsdurationarithmetic

  Bad:
    Time.current - 1.minute
    Time.zone.now + 2.days

  Good:
    1.minute.ago
    2.days.from_now

- Rails/FilePath
  Identifies usages of file path joining process to use
  Rails.root.join clause. It is used to add uniformity when
  joining paths.

  # bad
  Rails.root.join('app', 'models', 'goober')
  File.join(Rails.root, 'app/models/goober')
  "#{Rails.root}/app/models/goober"

  # good
  Rails.root.join('app/models/goober')

- Rails/PublicPath
  Favor Rails.public_path over Rails.root with 'public'

- Rails/RootPathnameMethod
  Use Rails.root IO methods instead of passing it to File

- bundle exec rubocop --regenerate-todo
  Uptodate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request Request for new functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #562] Add new Rails/CompactBlank cop koic/rubocop-rails
3 participants
@pirj @koic @benjaminwols