From 28aafbd4bca903919e9a78718995e4f987582646 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 7 Sep 2023 19:08:38 +0200
Subject: [PATCH] Add and apply Style/InvertibleUnlessCondition rule

This rule was added in rubocop 1.44.0. It's useful to avoid accidental
`unless !condition` clauses.

Note we aren't replacing `unless zero?` with `if nonzero?` because we
never use `nonzero?`; using it sounds like `if !zero?`.

Replacing `unless any?` with `if none?` is only consistent if we also replace
`unless present?` with `if blank?`, so we're also adding this case. For
consistency, we're also replacing `unless blank?` with `if present?`.

We're also simplifying code dealing with `> 0` conditions in order to
make the code (hopefully) easier to understand.

Also for consistency, we're enabling the `Style/InverseMethods` rule,
which follows a similar idea.
---
 .rubocop.yml                                           | 10 ++++++++++
 app/components/budgets/investments/map_component.rb    |  2 +-
 app/controllers/admin/api/stats_controller.rb          |  8 ++++----
 .../admin/local_census_records/imports_controller.rb   |  2 +-
 .../concerns/dashboard/expects_date_range.rb           |  4 ++--
 app/controllers/officing/base_controller.rb            |  2 +-
 app/controllers/proposals_controller.rb                |  4 ++--
 app/helpers/followables_helper.rb                      |  2 +-
 app/models/admin_notification.rb                       |  2 +-
 app/models/concerns/conflictable.rb                    |  4 +---
 app/models/lock.rb                                     |  4 +---
 app/models/machine_learning.rb                         |  2 +-
 app/models/map_location.rb                             |  2 +-
 app/models/poll.rb                                     |  8 ++++----
 app/models/site_customization/image.rb                 |  2 +-
 app/models/tenant.rb                                   |  2 +-
 app/models/user.rb                                     |  2 +-
 lib/active_model/dates.rb                              |  2 +-
 lib/census_api.rb                                      |  2 +-
 lib/remote_census_api.rb                               |  6 +++---
 lib/score_calculator.rb                                |  2 +-
 21 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 3781f9659..8bbad8784 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -637,6 +637,16 @@ Style/IdenticalConditionalBranches:
 Style/IfWithBooleanLiteralBranches:
   Enabled: true
 
+Style/InverseMethods:
+  Enabled: true
+
+Style/InvertibleUnlessCondition:
+  Enabled: true
+  InverseMethods:
+    :blank?: :present?
+    :present?: :blank?
+    :zero?: ~
+
 Style/LineEndConcatenation:
   Enabled: true
 
diff --git a/app/components/budgets/investments/map_component.rb b/app/components/budgets/investments/map_component.rb
index 43253dfed..8046d384a 100644
--- a/app/components/budgets/investments/map_component.rb
+++ b/app/components/budgets/investments/map_component.rb
@@ -22,7 +22,7 @@ class Budgets::Investments::MapComponent < ApplicationComponent
     end
 
     def geozones_data
-      return unless heading.geozone.present?
+      return if heading.geozone.blank?
 
       [
         {
diff --git a/app/controllers/admin/api/stats_controller.rb b/app/controllers/admin/api/stats_controller.rb
index c882e0e01..c86badceb 100644
--- a/app/controllers/admin/api/stats_controller.rb
+++ b/app/controllers/admin/api/stats_controller.rb
@@ -1,9 +1,9 @@
 class Admin::Api::StatsController < Admin::Api::BaseController
   def show
-    unless params[:event].present? ||
-           params[:visits].present? ||
-           params[:budget_investments].present? ||
-           params[:user_supported_budgets].present?
+    if params[:event].blank? &&
+       params[:visits].blank? &&
+       params[:budget_investments].blank? &&
+       params[:user_supported_budgets].blank?
       return render json: {}, status: :bad_request
     end
 
diff --git a/app/controllers/admin/local_census_records/imports_controller.rb b/app/controllers/admin/local_census_records/imports_controller.rb
index c09313d87..fa4aa5a45 100644
--- a/app/controllers/admin/local_census_records/imports_controller.rb
+++ b/app/controllers/admin/local_census_records/imports_controller.rb
@@ -14,7 +14,7 @@ class Admin::LocalCensusRecords::ImportsController < Admin::LocalCensusRecords::
   private
 
     def local_census_records_import_params
-      return {} unless params[:local_census_records_import].present?
+      return {} if params[:local_census_records_import].blank?
 
       params.require(:local_census_records_import).permit(allowed_params)
     end
diff --git a/app/controllers/concerns/dashboard/expects_date_range.rb b/app/controllers/concerns/dashboard/expects_date_range.rb
index e80137e4a..2be42ebe3 100644
--- a/app/controllers/concerns/dashboard/expects_date_range.rb
+++ b/app/controllers/concerns/dashboard/expects_date_range.rb
@@ -4,13 +4,13 @@ module Dashboard::ExpectsDateRange
   include Dashboard::HasProposal
 
   def start_date(fallback_date = proposal.created_at.to_date)
-    return Date.parse(params[:start_date]) unless params[:start_date].blank?
+    return Date.parse(params[:start_date]) if params[:start_date].present?
 
     fallback_date
   end
 
   def end_date
-    return Date.parse(params[:end_date]) unless params[:end_date].blank?
+    return Date.parse(params[:end_date]) if params[:end_date].present?
 
     Date.current
   end
diff --git a/app/controllers/officing/base_controller.rb b/app/controllers/officing/base_controller.rb
index 6b8730ad8..706ef166c 100644
--- a/app/controllers/officing/base_controller.rb
+++ b/app/controllers/officing/base_controller.rb
@@ -30,7 +30,7 @@ class Officing::BaseController < ApplicationController
     end
 
     def verify_booth
-      return unless current_booth.blank?
+      return if current_booth.present?
 
       booths = current_user.poll_officer.todays_booths
       case booths.count
diff --git a/app/controllers/proposals_controller.rb b/app/controllers/proposals_controller.rb
index 5fb131053..d22836066 100644
--- a/app/controllers/proposals_controller.rb
+++ b/app/controllers/proposals_controller.rb
@@ -155,8 +155,8 @@ class ProposalsController < ApplicationController
     end
 
     def load_featured
-      return unless !@advanced_search_terms && @search_terms.blank? &&
-                    params[:retired].blank? && @current_order != "recommendations"
+      return if @advanced_search_terms || @search_terms.present? ||
+                params[:retired].present? || @current_order == "recommendations"
 
       if Setting["feature.featured_proposals"]
         @featured_proposals = Proposal.not_archived
diff --git a/app/helpers/followables_helper.rb b/app/helpers/followables_helper.rb
index b302c3f55..ab199567f 100644
--- a/app/helpers/followables_helper.rb
+++ b/app/helpers/followables_helper.rb
@@ -8,7 +8,7 @@ module FollowablesHelper
   end
 
   def render_follow(follow)
-    return unless follow.followable.present?
+    return if follow.followable.blank?
 
     followable = follow.followable
     partial = "#{followable_class_name(followable)}_follow"
diff --git a/app/models/admin_notification.rb b/app/models/admin_notification.rb
index 7b5e3b74e..b2357c22d 100644
--- a/app/models/admin_notification.rb
+++ b/app/models/admin_notification.rb
@@ -40,7 +40,7 @@ class AdminNotification < ApplicationRecord
     end
 
     def complete_link_url
-      return unless link.present?
+      return if link.blank?
 
       unless link =~ /\A(http:\/\/|https:\/\/|\/)/
         self.link = "http://#{link}"
diff --git a/app/models/concerns/conflictable.rb b/app/models/concerns/conflictable.rb
index faea52032..10ea9d3eb 100644
--- a/app/models/concerns/conflictable.rb
+++ b/app/models/concerns/conflictable.rb
@@ -2,8 +2,6 @@ module Conflictable
   extend ActiveSupport::Concern
 
   def conflictive?
-    return false unless flags_count > 0
-
-    cached_votes_up / flags_count.to_f < 5
+    flags_count > 0 && cached_votes_up / flags_count.to_f < 5
   end
 end
diff --git a/app/models/lock.rb b/app/models/lock.rb
index 28d4398a7..982bcb0ca 100644
--- a/app/models/lock.rb
+++ b/app/models/lock.rb
@@ -16,9 +16,7 @@ class Lock < ApplicationRecord
   end
 
   def too_many_tries?
-    return false unless tries > 0
-
-    tries % Lock.max_tries == 0
+    tries > 0 && tries % Lock.max_tries == 0
   end
 
   def self.increase_tries(user)
diff --git a/app/models/machine_learning.rb b/app/models/machine_learning.rb
index bfe8bb33d..6341449bc 100644
--- a/app/models/machine_learning.rb
+++ b/app/models/machine_learning.rb
@@ -465,7 +465,7 @@ class MachineLearning
 
     def updated_file?(filename)
       return false unless File.exist? data_folder.join(filename)
-      return true unless previous_modified_date[filename].present?
+      return true if previous_modified_date[filename].blank?
 
       last_modified_date_for(filename) > previous_modified_date[filename]
     end
diff --git a/app/models/map_location.rb b/app/models/map_location.rb
index 7adc6d639..d9165f791 100644
--- a/app/models/map_location.rb
+++ b/app/models/map_location.rb
@@ -26,7 +26,7 @@ class MapLocation < ApplicationRecord
   end
 
   def self.investments_json_data(investments)
-    return [] unless investments.any?
+    return [] if investments.none?
 
     budget_id = investments.first.budget_id
 
diff --git a/app/models/poll.rb b/app/models/poll.rb
index 1ceff3b7b..86c4667fb 100644
--- a/app/models/poll.rb
+++ b/app/models/poll.rb
@@ -149,7 +149,7 @@ class Poll < ApplicationRecord
   end
 
   def date_range
-    unless starts_at.present? && ends_at.present? && starts_at <= ends_at
+    if starts_at.blank? || ends_at.blank? || starts_at > ends_at
       errors.add(:starts_at, I18n.t("errors.messages.invalid_date_range"))
     end
   end
@@ -192,9 +192,9 @@ class Poll < ApplicationRecord
   end
 
   def only_one_active
-    return unless starts_at.present?
-    return unless ends_at.present?
-    return unless Poll.overlaping_with(self).any?
+    return if starts_at.blank?
+    return if ends_at.blank?
+    return if Poll.overlaping_with(self).none?
 
     errors.add(:starts_at, I18n.t("activerecord.errors.messages.another_poll_active"))
   end
diff --git a/app/models/site_customization/image.rb b/app/models/site_customization/image.rb
index ca68c0288..83039ac3c 100644
--- a/app/models/site_customization/image.rb
+++ b/app/models/site_customization/image.rb
@@ -68,7 +68,7 @@ class SiteCustomization::Image < ApplicationRecord
       height = image.metadata[:height]
 
       if name == "logo_header"
-        errors.add(:image, :image_width, required_width: required_width) unless width <= required_width
+        errors.add(:image, :image_width, required_width: required_width) if width > required_width
       else
         errors.add(:image, :image_width, required_width: required_width) unless width == required_width
       end
diff --git a/app/models/tenant.rb b/app/models/tenant.rb
index 36d28b141..08757bb57 100644
--- a/app/models/tenant.rb
+++ b/app/models/tenant.rb
@@ -19,7 +19,7 @@ class Tenant < ApplicationRecord
   end
 
   def self.resolve_host(host)
-    return nil unless Rails.application.config.multitenancy.present?
+    return nil if Rails.application.config.multitenancy.blank?
     return nil if host.blank? || host.match?(Resolv::AddressRegex)
 
     schema = schema_for(host)
diff --git a/app/models/user.rb b/app/models/user.rb
index ecc2833e8..ae195ec54 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -419,7 +419,7 @@ class User < ApplicationRecord
   private
 
     def clean_document_number
-      return unless document_number.present?
+      return if document_number.blank?
 
       self.document_number = document_number.gsub(/[^a-z0-9]+/i, "").upcase
     end
diff --git a/lib/active_model/dates.rb b/lib/active_model/dates.rb
index 9defc62a9..b10dceafa 100644
--- a/lib/active_model/dates.rb
+++ b/lib/active_model/dates.rb
@@ -2,7 +2,7 @@ module ActiveModel::Dates
   def parse_date(field, attrs)
     year, month, day = attrs["#{field}(1i)"], attrs["#{field}(2i)"], attrs["#{field}(3i)"]
 
-    return nil unless day.present? && month.present? && year.present?
+    return nil if day.blank? || month.blank? || year.blank?
 
     Date.new(year.to_i, month.to_i, day.to_i)
   end
diff --git a/lib/census_api.rb b/lib/census_api.rb
index 39ef65c94..09fcbcc87 100644
--- a/lib/census_api.rb
+++ b/lib/census_api.rb
@@ -21,7 +21,7 @@ class CensusApi
     def date_of_birth
       str = data[:datos_habitante][:item][:fecha_nacimiento_string]
       day, month, year = str.match(/(\d\d?)\D(\d\d?)\D(\d\d\d?\d?)/)[1..3]
-      return nil unless day.present? && month.present? && year.present?
+      return nil if day.blank? || month.blank? || year.blank?
 
       Time.zone.local(year.to_i, month.to_i, day.to_i).to_date
     end
diff --git a/lib/remote_census_api.rb b/lib/remote_census_api.rb
index e91de7e12..c5db70365 100644
--- a/lib/remote_census_api.rb
+++ b/lib/remote_census_api.rb
@@ -16,7 +16,7 @@ class RemoteCensusApi
 
     def extract_value(path_value)
       path = parse_response_path(path_value)
-      return nil unless path.present?
+      return nil if path.blank?
 
       @body.dig(*path)
     end
@@ -29,10 +29,10 @@ class RemoteCensusApi
     def date_of_birth
       path_value = Setting["remote_census.response.date_of_birth"]
       str = extract_value(path_value)
-      return nil unless str.present?
+      return nil if str.blank?
 
       day, month, year = str.match(/(\d\d?)\D(\d\d?)\D(\d\d\d?\d?)/)[1..3]
-      return nil unless day.present? && month.present? && year.present?
+      return nil if day.blank? || month.blank? || year.blank?
 
       Time.zone.local(year.to_i, month.to_i, day.to_i).to_date
     end
diff --git a/lib/score_calculator.rb b/lib/score_calculator.rb
index 1bad85c14..179a1efae 100644
--- a/lib/score_calculator.rb
+++ b/lib/score_calculator.rb
@@ -13,7 +13,7 @@ module ScoreCalculator
   end
 
   def self.confidence_score(votes_total, votes_up)
-    return 1 unless votes_total > 0
+    return 1 if votes_total.zero?
 
     votes_total = votes_total.to_f
     votes_up    = votes_up.to_f