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.
This commit is contained in:
Javi Martín
2023-09-07 19:08:38 +02:00
parent 21ca96ae1c
commit 28aafbd4bc
21 changed files with 40 additions and 34 deletions

View File

@@ -637,6 +637,16 @@ Style/IdenticalConditionalBranches:
Style/IfWithBooleanLiteralBranches: Style/IfWithBooleanLiteralBranches:
Enabled: true Enabled: true
Style/InverseMethods:
Enabled: true
Style/InvertibleUnlessCondition:
Enabled: true
InverseMethods:
:blank?: :present?
:present?: :blank?
:zero?: ~
Style/LineEndConcatenation: Style/LineEndConcatenation:
Enabled: true Enabled: true

View File

@@ -22,7 +22,7 @@ class Budgets::Investments::MapComponent < ApplicationComponent
end end
def geozones_data def geozones_data
return unless heading.geozone.present? return if heading.geozone.blank?
[ [
{ {

View File

@@ -1,9 +1,9 @@
class Admin::Api::StatsController < Admin::Api::BaseController class Admin::Api::StatsController < Admin::Api::BaseController
def show def show
unless params[:event].present? || if params[:event].blank? &&
params[:visits].present? || params[:visits].blank? &&
params[:budget_investments].present? || params[:budget_investments].blank? &&
params[:user_supported_budgets].present? params[:user_supported_budgets].blank?
return render json: {}, status: :bad_request return render json: {}, status: :bad_request
end end

View File

@@ -14,7 +14,7 @@ class Admin::LocalCensusRecords::ImportsController < Admin::LocalCensusRecords::
private private
def local_census_records_import_params 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) params.require(:local_census_records_import).permit(allowed_params)
end end

View File

@@ -4,13 +4,13 @@ module Dashboard::ExpectsDateRange
include Dashboard::HasProposal include Dashboard::HasProposal
def start_date(fallback_date = proposal.created_at.to_date) 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 fallback_date
end end
def end_date 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 Date.current
end end

View File

@@ -30,7 +30,7 @@ class Officing::BaseController < ApplicationController
end end
def verify_booth def verify_booth
return unless current_booth.blank? return if current_booth.present?
booths = current_user.poll_officer.todays_booths booths = current_user.poll_officer.todays_booths
case booths.count case booths.count

View File

@@ -155,8 +155,8 @@ class ProposalsController < ApplicationController
end end
def load_featured def load_featured
return unless !@advanced_search_terms && @search_terms.blank? && return if @advanced_search_terms || @search_terms.present? ||
params[:retired].blank? && @current_order != "recommendations" params[:retired].present? || @current_order == "recommendations"
if Setting["feature.featured_proposals"] if Setting["feature.featured_proposals"]
@featured_proposals = Proposal.not_archived @featured_proposals = Proposal.not_archived

View File

@@ -8,7 +8,7 @@ module FollowablesHelper
end end
def render_follow(follow) def render_follow(follow)
return unless follow.followable.present? return if follow.followable.blank?
followable = follow.followable followable = follow.followable
partial = "#{followable_class_name(followable)}_follow" partial = "#{followable_class_name(followable)}_follow"

View File

@@ -40,7 +40,7 @@ class AdminNotification < ApplicationRecord
end end
def complete_link_url def complete_link_url
return unless link.present? return if link.blank?
unless link =~ /\A(http:\/\/|https:\/\/|\/)/ unless link =~ /\A(http:\/\/|https:\/\/|\/)/
self.link = "http://#{link}" self.link = "http://#{link}"

View File

@@ -2,8 +2,6 @@ module Conflictable
extend ActiveSupport::Concern extend ActiveSupport::Concern
def conflictive? def conflictive?
return false unless flags_count > 0 flags_count > 0 && cached_votes_up / flags_count.to_f < 5
cached_votes_up / flags_count.to_f < 5
end end
end end

View File

@@ -16,9 +16,7 @@ class Lock < ApplicationRecord
end end
def too_many_tries? def too_many_tries?
return false unless tries > 0 tries > 0 && tries % Lock.max_tries == 0
tries % Lock.max_tries == 0
end end
def self.increase_tries(user) def self.increase_tries(user)

View File

@@ -465,7 +465,7 @@ class MachineLearning
def updated_file?(filename) def updated_file?(filename)
return false unless File.exist? data_folder.join(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] last_modified_date_for(filename) > previous_modified_date[filename]
end end

View File

@@ -26,7 +26,7 @@ class MapLocation < ApplicationRecord
end end
def self.investments_json_data(investments) def self.investments_json_data(investments)
return [] unless investments.any? return [] if investments.none?
budget_id = investments.first.budget_id budget_id = investments.first.budget_id

View File

@@ -149,7 +149,7 @@ class Poll < ApplicationRecord
end end
def date_range 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")) errors.add(:starts_at, I18n.t("errors.messages.invalid_date_range"))
end end
end end
@@ -192,9 +192,9 @@ class Poll < ApplicationRecord
end end
def only_one_active def only_one_active
return unless starts_at.present? return if starts_at.blank?
return unless ends_at.present? return if ends_at.blank?
return unless Poll.overlaping_with(self).any? return if Poll.overlaping_with(self).none?
errors.add(:starts_at, I18n.t("activerecord.errors.messages.another_poll_active")) errors.add(:starts_at, I18n.t("activerecord.errors.messages.another_poll_active"))
end end

View File

@@ -68,7 +68,7 @@ class SiteCustomization::Image < ApplicationRecord
height = image.metadata[:height] height = image.metadata[:height]
if name == "logo_header" 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 else
errors.add(:image, :image_width, required_width: required_width) unless width == required_width errors.add(:image, :image_width, required_width: required_width) unless width == required_width
end end

View File

@@ -19,7 +19,7 @@ class Tenant < ApplicationRecord
end end
def self.resolve_host(host) 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) return nil if host.blank? || host.match?(Resolv::AddressRegex)
schema = schema_for(host) schema = schema_for(host)

View File

@@ -419,7 +419,7 @@ class User < ApplicationRecord
private private
def clean_document_number 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 self.document_number = document_number.gsub(/[^a-z0-9]+/i, "").upcase
end end

View File

@@ -2,7 +2,7 @@ module ActiveModel::Dates
def parse_date(field, attrs) def parse_date(field, attrs)
year, month, day = attrs["#{field}(1i)"], attrs["#{field}(2i)"], attrs["#{field}(3i)"] 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) Date.new(year.to_i, month.to_i, day.to_i)
end end

View File

@@ -21,7 +21,7 @@ class CensusApi
def date_of_birth def date_of_birth
str = data[:datos_habitante][:item][:fecha_nacimiento_string] 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] 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 Time.zone.local(year.to_i, month.to_i, day.to_i).to_date
end end

View File

@@ -16,7 +16,7 @@ class RemoteCensusApi
def extract_value(path_value) def extract_value(path_value)
path = parse_response_path(path_value) path = parse_response_path(path_value)
return nil unless path.present? return nil if path.blank?
@body.dig(*path) @body.dig(*path)
end end
@@ -29,10 +29,10 @@ class RemoteCensusApi
def date_of_birth def date_of_birth
path_value = Setting["remote_census.response.date_of_birth"] path_value = Setting["remote_census.response.date_of_birth"]
str = extract_value(path_value) 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] 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 Time.zone.local(year.to_i, month.to_i, day.to_i).to_date
end end

View File

@@ -13,7 +13,7 @@ module ScoreCalculator
end end
def self.confidence_score(votes_total, votes_up) 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_total = votes_total.to_f
votes_up = votes_up.to_f votes_up = votes_up.to_f