The issue
Recently #932 added support for the Rails/Presence cop to start suggesting additional changes. I feel these changes obfuscate the code.
Some of the examples from my code are:
Use (params[:email].presence&.gsub!(/[[:space:]]/, '')) instead of params[:email].gsub!(/[[:space:]]/, '') if params[:email].present?
Use manga_app_scope_params[:android_product_id].presence&.strip instead of manga_app_scope_params[:android_product_id].present? ? manga_app_scope_params[:android_product_id].strip : nil
Use manga_params[:list_price].presence&.strip instead of manga_params[:list_price].present? ? manga_params[:list_price].strip : nil
Use has_upc.presence&.delete('-') instead of has_upc.blank? ? nil : has_upc.delete( '-')
Use has_title.presence&.gsub(/[,.-]/, '') instead of has_title.blank? ? nil : has_title.gsub(/[,.-]/, '')
Use u.dob.presence&.to_s(:dbdate) instead of u.dob.present? ? u.dob.to_s(:dbdate) : nil
Use (item_hash[:series].presence&.map!(&:to_i)) instead of item_hash[:series].map!(&:to_i) if item_hash[:series].present?
Use (target_url.presence&.sub!('scheme', url_scheme)) instead of target_url.sub!('scheme', url_scheme) if target_url.present?
Use attr_hash[:recommended_pb_property_ids].presence&.map(&:to_i) instead of attr_hash[:recommended_pb_property_ids].blank? ? nil : attr_hash[:recommended_pb_property_ids].map(&:to_i)
Use (u.presence&.set_email_as_invalid) instead of u.set_email_as_invalid() if u.present?
Use manga.publication_date.presence&.to_i instead of manga.publication_date.present? ? manga.publication_date.to_i : nil
Use manga.expiration_date.presence&.to_i instead of manga.expiration_date.present? ? manga.expiration_date.to_i : nil
Use (venice_receipt.presence&.original_json_response) instead of venice_receipt.blank? ? nil : venice_receipt.original_json_response
Use (mp.presence&.destroy) instead of mp.destroy if mp.present?
Use user.dob.presence&.to_s(:dbdate) instead of user.dob.present? ? user.dob.to_s(:dbdate) : nil
Use user.dob.presence&.to_s(:dbdatetime) instead of user.dob.present? ? user.dob.to_s(:dbdatetime) : nil
Use r[:fields][:product_types].presence&.first instead of r[:fields][:product_types].blank? ? nil : r[:fields][:product_types].first
Use (sub.presence&.destroy) instead of sub.destroy if sub.present?
Suggested solution
I would like a configuration for this cop to skip over suggestions that require &. to function.
Alternatives considered
- I guess we could get used to it.
- We could disable rubocop inline wherever we feel that rubocop makes the code harder to read.
- We could just disable this cop.
NOTE
The previous examples before the recent update were fine though and are easy to read.
# @example
# # bad
# a.present? ? a : nil
# !a.present? ? nil : a
# a.blank? ? nil : a
# !a.blank? ? a : nil
#
# # good
# a.presence
#
# @example
# # bad
# a.present? ? a : b
# !a.present? ? b : a
# a.blank? ? b : a
# !a.blank? ? a : b
#
# # good
# a.presence || b
The issue
Recently #932 added support for the Rails/Presence cop to start suggesting additional changes. I feel these changes obfuscate the code.
Some of the examples from my code are:
Use
(params[:email].presence&.gsub!(/[[:space:]]/, ''))instead ofparams[:email].gsub!(/[[:space:]]/, '') if params[:email].present?Use
manga_app_scope_params[:android_product_id].presence&.stripinstead ofmanga_app_scope_params[:android_product_id].present? ? manga_app_scope_params[:android_product_id].strip : nilUse
manga_params[:list_price].presence&.stripinstead ofmanga_params[:list_price].present? ? manga_params[:list_price].strip : nilUse
has_upc.presence&.delete('-')instead ofhas_upc.blank? ? nil : has_upc.delete( '-')Use
has_title.presence&.gsub(/[,.-]/, '')instead ofhas_title.blank? ? nil : has_title.gsub(/[,.-]/, '')Use
u.dob.presence&.to_s(:dbdate)instead ofu.dob.present? ? u.dob.to_s(:dbdate) : nilUse
(item_hash[:series].presence&.map!(&:to_i))instead ofitem_hash[:series].map!(&:to_i) if item_hash[:series].present?Use
(target_url.presence&.sub!('scheme', url_scheme))instead oftarget_url.sub!('scheme', url_scheme) if target_url.present?Use
attr_hash[:recommended_pb_property_ids].presence&.map(&:to_i)instead ofattr_hash[:recommended_pb_property_ids].blank? ? nil : attr_hash[:recommended_pb_property_ids].map(&:to_i)Use
(u.presence&.set_email_as_invalid)instead ofu.set_email_as_invalid() if u.present?Use
manga.publication_date.presence&.to_iinstead ofmanga.publication_date.present? ? manga.publication_date.to_i : nilUse
manga.expiration_date.presence&.to_iinstead ofmanga.expiration_date.present? ? manga.expiration_date.to_i : nilUse
(venice_receipt.presence&.original_json_response)instead ofvenice_receipt.blank? ? nil : venice_receipt.original_json_responseUse
(mp.presence&.destroy)instead ofmp.destroy if mp.present?Use
user.dob.presence&.to_s(:dbdate)instead ofuser.dob.present? ? user.dob.to_s(:dbdate) : nilUse
user.dob.presence&.to_s(:dbdatetime)instead ofuser.dob.present? ? user.dob.to_s(:dbdatetime) : nilUse
r[:fields][:product_types].presence&.firstinstead ofr[:fields][:product_types].blank? ? nil : r[:fields][:product_types].firstUse
(sub.presence&.destroy)instead ofsub.destroy if sub.present?Suggested solution
I would like a configuration for this cop to skip over suggestions that require
&.to function.Alternatives considered
NOTE
The previous examples before the recent update were fine though and are easy to read.