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.

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

Favor #public_send over #send using Rubocop's cop #2618

Merged
merged 2 commits into from
Sep 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ Style/HashSyntax:
Enabled: true
EnforcedStyle: ruby19_no_mixed_keys

Style/Send:
Enabled: true

Layout/MultilineMethodCallIndentation:
Enabled: true
EnforcedStyle: indented
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/contents_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def edit
def update
params.each do |name, value|
if ContentConfig.has_preference?(name) || ContentConfig.has_attachment?(name)
ContentConfig.send("#{name}=", value)
ContentConfig.public_send("#{name}=", value)
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/inventory_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class InventoryItemsController < ResourceController
# we can authorise #create using an object with required attributes
def build_resource
if parent_data.present?
parent.send(controller_name).build
parent.public_send(controller_name).build
else
model_class.new(params[object_name]) # This line changed
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/product_import_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def process_data(method)
@importer = ProductImport::ProductImporter.new(File.new(params[:filepath]), spree_current_user, start: params[:start], end: params[:end], settings: params[:settings])

begin
@importer.send("#{method}_entries")
@importer.public_send("#{method}_entries")
rescue StandardError => e
render json: e.message, response: 500
return false
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/admin/resource_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ def new_object_url(options = {})

def edit_object_url(object, options = {})
if parent_data.present?
main_app.send "edit_admin_#{model_name}_#{object_name}_url", parent, object, options
main_app.public_send "edit_admin_#{model_name}_#{object_name}_url", parent, object, options
else
main_app.send "edit_admin_#{object_name}_url", object, options
main_app.public_send "edit_admin_#{object_name}_url", object, options
end
end

def object_url(object = nil, options = {})
target = object ? object : @object
if parent_data.present?
main_app.send "admin_#{model_name}_#{object_name}_url", parent, target, options
main_app.public_send "admin_#{model_name}_#{object_name}_url", parent, target, options
else
main_app.send "admin_#{object_name}_url", target, options
main_app.public_send "admin_#{object_name}_url", target, options
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/enterprises_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def reset_distributor(order, distributor)

def reset_user_and_customer(order)
order.associate_user!(spree_current_user) if order.user.blank? || order.email.blank?
order.send(:associate_customer) if order.customer.nil? # Only associates existing customers
order.__send__(:associate_customer) if order.customer.nil? # Only associates existing customers
end

def reset_order_cycle(order, distributor)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/shop_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def changeable_orders_alert
private

def filtered_json(products_json)
if applicator.send(:rules).any?
if applicator.rules.any?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this one public. If we use it from outside, it's part of applicator's public API

filter(products_json)
else
products_json
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ def edit
@preferences_general << :bugherd_api_key
end
end
GeneralSettingsController.send(:prepend, GeneralSettingsEditPreferences)
GeneralSettingsController.prepend(GeneralSettingsEditPreferences)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Admin
# Only show payment methods that user has access to and sort by distributor name
# ! Redundant code copied from Spree::Admin::ResourceController with modifications marked
def collection
return parent.send(controller_name) if parent_data.present?
return parent.public_send(controller_name) if parent_data.present?
collection = if model_class.respond_to?(:accessible_by) &&
!current_ability.has_block?(params[:action], model_class)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def fire

# Because we have a transition method also called void, we do this to avoid conflicts.
event = "void_transaction" if event == "void"
if @payment.send("#{event}!")
if @payment.public_send("#{event}!")
flash[:success] = t(:payment_updated)
else
flash[:error] = t(:cannot_perform_operation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ def load_resource
end
end

Spree::Admin::ResourceController.send(:prepend, AuthorizeOnLoadResource)
Spree::Admin::ResourceController.prepend(AuthorizeOnLoadResource)
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module Admin
# Sort shipping methods by distributor name
# ! Code copied from Spree::Admin::ResourceController with two added lines
def collection
return parent.send(controller_name) if parent_data.present?
return parent.public_send(controller_name) if parent_data.present?

collection = if model_class.respond_to?(:accessible_by) &&
!current_ability.has_block?(params[:action], model_class)
Expand Down
6 changes: 1 addition & 5 deletions app/helpers/angular_form_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ def ng_fields_for(record_name, *args, &block)
end

def ng_text_field(method, options = {})
# @object_name --> "enterprise_fee_set"
# @fields_for_record_name --> :collection
# @object.send(@fields_for_record_name).first.class.to_s.underscore --> enterprise_fee

value = "{{ #{angular_model(method)} }}"
options.reverse_merge!({'id' => angular_id(method)})

Expand Down Expand Up @@ -46,6 +42,6 @@ def angular_id(method)
end

def angular_model(method)
"#{@object.send(@fields_for_record_name).first.class.to_s.underscore}.#{method}"
"#{@object.public_send(@fields_for_record_name).first.class.to_s.underscore}.#{method}"
end
end
2 changes: 1 addition & 1 deletion app/helpers/angular_form_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def ng_options_for_select(container, angular_field=nil)

def ng_options_from_collection_for_select(collection, value_method, text_method, angular_field)
options = collection.map do |element|
[element.send(text_method), element.send(value_method)]
[element.public_send(text_method), element.public_send(value_method)]
end

ng_options_for_select(options, angular_field)
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def ng_form_for(name, *args, &block)
# spree.foo_path in any view rendered from non-spree-namespaced controllers.
def method_missing(method, *args, &block)
if (method.to_s.end_with?('_path') || method.to_s.end_with?('_url')) && spree.respond_to?(method)
spree.send(method, *args)
spree.public_send(method, *args)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails' URL helpers are public

else
super
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/column_preference.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class ColumnPreference < ActiveRecord::Base

def self.for(user, action_name)
stored_preferences = where(user_id: user.id, action_name: action_name)
default_preferences = send("#{action_name}_columns")
default_preferences = __send__("#{action_name}_columns")
filter(default_preferences, user, action_name)
default_preferences.each_with_object([]) do |(column_name, default_attributes), preferences|
stored_preference = stored_preferences.find_by_column_name(column_name)
Expand All @@ -37,7 +37,7 @@ def self.for(user, action_name)
private

def self.valid_columns_for(action_name)
send("#{action_name}_columns").keys.map(&:to_s)
__send__("#{action_name}_columns").keys.map(&:to_s)
end

def self.known_actions
Expand Down
2 changes: 1 addition & 1 deletion app/models/model_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def initialize(klass, collection, attributes={}, reject_if=nil, delete_if=nil)
@collection = attributes[:collection] if attributes[:collection]

attributes.each do |name, value|
send("#{name}=", value)
public_send("#{name}=", value)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the setters of a model are meant to be public

end
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/product_import/entry_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ def assign_defaults(object, entry)
when 'overwrite_all'
object.assign_attributes(attribute => setting['value'])
when 'overwrite_empty'
if object.send(attribute).blank? || ((attribute == 'on_hand' || attribute == 'count_on_hand') && entry.on_hand_nil)
if object.public_send(attribute).blank? || ((attribute == 'on_hand' || attribute == 'count_on_hand') && entry.on_hand_nil)
object.assign_attributes(attribute => setting['value'])
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/product_import/entry_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def tax_and_shipping_validation(entry, type, category, index)
return if category.blank?

if index.key? category
entry.send("#{type}_category_id=", index[category])
entry.public_send("#{type}_category_id=", index[category])
else
mark_as_invalid(entry, attribute: "#{type}_category", error: I18n.t('admin.product_import.model.not_found'))
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/product_import/spreadsheet_entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def assign_units(attrs)

units.converted_attributes.each do |attr, value|
if respond_to?("#{attr}=")
send("#{attr}=", value) unless non_product_attributes.include?(attr)
public_send("#{attr}=", value) unless non_product_attributes.include?(attr)
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions app/models/proxy_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ProxyOrder < ActiveRecord::Base
def state
# NOTE: the order is important here
%w(canceled paused pending cart).each do |state|
return state if send("#{state}?")
return state if __send__("#{state}?")
end
order.state
end
Expand All @@ -32,7 +32,7 @@ def cancel
return false unless order_cycle.orders_close_at.andand > Time.zone.now
transaction do
update_column(:canceled_at, Time.zone.now)
order.send('cancel') if order
order.cancel if order
true
end
end
Expand All @@ -41,7 +41,7 @@ def resume
return false unless order_cycle.orders_close_at.andand > Time.zone.now
transaction do
update_column(:canceled_at, nil)
order.send('resume') if order
order.resume if order
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these two methods are public

true
end
end
Expand Down
6 changes: 3 additions & 3 deletions app/models/spree/calculator/default_tax_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ def compute_order(order)

# Added this block, finds relevant fees for each line_item, calculates the tax on them, and returns the total tax
per_item_fees_total = order.line_items.sum do |line_item|
calculator.send(:per_item_enterprise_fee_applicators_for, line_item.variant)
calculator.per_item_enterprise_fee_applicators_for(line_item.variant)
.select { |applicator| (!applicator.enterprise_fee.inherits_tax_category && applicator.enterprise_fee.tax_category == rate.tax_category) ||
(applicator.enterprise_fee.inherits_tax_category && line_item.product.tax_category == rate.tax_category)
(applicator.enterprise_fee.inherits_tax_category && line_item.product.tax_category == rate.tax_category)
}
.sum { |applicator| applicator.enterprise_fee.compute_amount(line_item) }
end

# Added this block, finds relevant fees for whole order, calculates the tax on them, and returns the total tax
per_order_fees_total = calculator.send(:per_order_enterprise_fee_applicators_for, order)
per_order_fees_total = calculator.per_order_enterprise_fee_applicators_for(order)
.select { |applicator| applicator.enterprise_fee.tax_category == rate.tax_category }
.sum { |applicator| applicator.enterprise_fee.compute_amount(order) }

Expand Down
2 changes: 1 addition & 1 deletion app/models/spree/preferences/file_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def self.preference(name, type, *args)

def get_preference(key)
if !has_preference?(key) && has_attachment?(key)
send key
public_send key
else
super key
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def paused?
def state
# NOTE: the order is important here
%w(canceled paused pending ended).each do |state|
return state if send("#{state}?")
return state if __send__("#{state}?")
end
"active"
end
Expand Down
2 changes: 1 addition & 1 deletion app/serializers/api/admin/enterprise_fee_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def calculator_settings

result = nil

options[:controller].send(:with_format, :html) do
options[:controller].__send__(:with_format, :html) do
result = options[:controller].render_to_string :partial => 'admin/enterprise_fees/calculator_settings', :locals => {:enterprise_fee => object}
end

Expand Down
4 changes: 2 additions & 2 deletions app/services/order_syncer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def relevant_address_attrs

def addresses_match?(order_address, subscription_address)
relevant_address_attrs.all? do |attr|
order_address[attr] == subscription_address.send("#{attr}_was") ||
order_address[attr] == subscription_address.public_send("#{attr}_was") ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AR's dirty attributes API is public

order_address[attr] == subscription_address[attr]
end
end
Expand All @@ -101,7 +101,7 @@ def ship_address_updatable?(order)
# address on the order matches the shop's address
def force_ship_address_required?(order)
return false unless shipping_method.require_ship_address?
distributor_address = order.send(:address_from_distributor)
distributor_address = order.__send__(:address_from_distributor)
relevant_address_attrs.all? do |attr|
order.ship_address[attr] == distributor_address[attr]
end
Expand Down
6 changes: 3 additions & 3 deletions lib/discourse/single_sign_on.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def self.parse(payload, sso_secret = nil)
if BOOLS.include? k
val = ["true", "false"].include?(val) ? val == "true" : nil
end
sso.send("#{k}=", val)
sso.public_send("#{k}=", val)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each of these K are defined as public attr_accessor in line 11

end

decoded_hash.each do |k,v|
Expand Down Expand Up @@ -87,9 +87,9 @@ def payload
def unsigned_payload
payload = {}
ACCESSORS.each do |k|
next if (val = send k) == nil
next if (val = public_send k) == nil

payload[k] = val
payload[k] = val
end

if @custom_fields
Expand Down
22 changes: 11 additions & 11 deletions lib/open_food_network/address_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def initialize(*args)
args.each do |arg|
type = types[arg.class]
next unless type
send("#{type}=", arg)
public_send("#{type}=", arg)
end
end

Expand All @@ -24,16 +24,6 @@ def ship_address
customer_preferred_ship_address || user_preferred_ship_address || fallback_ship_address
end

private

def types
{
String => "email",
Customer => "customer",
Spree::User => "user"
}
end

def email=(arg)
@email ||= arg
end
Expand All @@ -46,6 +36,16 @@ def user=(arg)
@user ||= arg
end

private

def types
{
String => "email",
Customer => "customer",
Spree::User => "user"
}
end

def customer_preferred_bill_address
customer.andand.bill_address
end
Expand Down
Loading