From e5340cb53af5ed7e60fc20c5fd4596e9c6a2fb44 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 23 Jun 2017 16:37:02 +1000 Subject: [PATCH] Rubocop: Correct Lint/UselessAssignment offences, disable for /spec in main config --- .rubocop.yml | 8 ++++++++ .rubocop_todo.yml | 4 ---- app/controllers/checkout_controller.rb | 4 ++-- app/controllers/discourse_sso_controller.rb | 1 - .../spree/admin/reports_controller_decorator.rb | 2 -- app/helpers/admin/image_settings_helper.rb | 4 ++-- app/jobs/update_account_invoices.rb | 2 +- app/models/cart.rb | 2 -- app/models/spree/preference_decorator.rb | 2 +- app/serializers/api/product_serializer.rb | 2 +- lib/open_food_network/lettuce_share_report.rb | 5 ++--- lib/open_food_network/order_cycle_management_report.rb | 2 -- lib/open_food_network/sales_tax_report.rb | 2 +- lib/tasks/karma.rake | 2 +- 14 files changed, 19 insertions(+), 23 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index c1d63f476e..28cee15a0f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -34,6 +34,14 @@ Style/StringLiterals: Lint/AmbiguousBlockAssociation: Enabled: false +# Heaps of offences (> 100) in specs, mostly in situations where two or more +# instances of a model are required, but only one is referenced. Difficult to +# fix without making the spec look messy or rewriting it. +# Should definitely fix at some point. +Lint/UselessAssignment: + Exclude: + - spec/**/* + # No alternative to this one until we upgrade to Rails 4 and can use #find_by Rails/DynamicFindBy: Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0eafe78d15..d6bd7afbd4 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -154,10 +154,6 @@ Lint/UselessAccessModifier: - 'lib/open_food_network/reports/bulk_coop_report.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' -# Offense count: 143 -Lint/UselessAssignment: - Enabled: false - # Offense count: 340 Lint/Void: Exclude: diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index c878d3b7b6..37c8bca206 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -167,8 +167,8 @@ class CheckoutController < Spree::CheckoutController customer_preferred_bill_address, customer_preferred_ship_address = @order.customer.bill_address, @order.customer.ship_address if @order.customer - @order.bill_address ||= customer_preferred_bill_address ||= preferred_bill_address || last_used_bill_address || Spree::Address.default - @order.ship_address ||= customer_preferred_ship_address ||= preferred_ship_address || last_used_ship_address || Spree::Address.default + @order.bill_address ||= customer_preferred_bill_address || preferred_bill_address || last_used_bill_address || Spree::Address.default + @order.ship_address ||= customer_preferred_ship_address || preferred_ship_address || last_used_ship_address || Spree::Address.default end def after_payment diff --git a/app/controllers/discourse_sso_controller.rb b/app/controllers/discourse_sso_controller.rb index 0b067f6c92..a0da1299a9 100644 --- a/app/controllers/discourse_sso_controller.rb +++ b/app/controllers/discourse_sso_controller.rb @@ -30,7 +30,6 @@ class DiscourseSsoController < ApplicationController def sso_url secret = discourse_sso_secret! - discourse_url = discourse_url! sso = Discourse::SingleSignOn.parse(request.query_string, secret) sso.email = spree_current_user.email sso.username = spree_current_user.login diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index f5c5ab96e3..88f28bb9ee 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -101,7 +101,6 @@ Spree::Admin::ReportsController.class_eval do # -- Build Report with Order Grouper @report = OpenFoodNetwork::OrderCycleManagementReport.new spree_current_user, params @table = @report.table_items - csv_file_name = "#{params[:report_type]}_#{timestamp}.csv" render_report(@report.header, @table, params[:csv], "order_cycle_management_#{timestamp}.csv") end @@ -126,7 +125,6 @@ Spree::Admin::ReportsController.class_eval do @report = OpenFoodNetwork::PackingReport.new spree_current_user, params order_grouper = OpenFoodNetwork::OrderGrouper.new @report.rules, @report.columns @table = order_grouper.table(@report.table_items) - csv_file_name = "#{params[:report_type]}_#{timestamp}.csv" render_report(@report.header, @table, params[:csv], "packing_#{timestamp}.csv") end diff --git a/app/helpers/admin/image_settings_helper.rb b/app/helpers/admin/image_settings_helper.rb index 10c34fe372..e337161298 100644 --- a/app/helpers/admin/image_settings_helper.rb +++ b/app/helpers/admin/image_settings_helper.rb @@ -5,12 +5,12 @@ module Admin end def admin_image_settings_geometry_from_style(style) - geometry, format = admin_image_settings_split_style style + geometry, _format = admin_image_settings_split_style style geometry end def admin_image_settings_format_from_style(style) - geometry, format = admin_image_settings_split_style style + _geometry, format = admin_image_settings_split_style style format end diff --git a/app/jobs/update_account_invoices.rb b/app/jobs/update_account_invoices.rb index 16112ef870..669c708757 100644 --- a/app/jobs/update_account_invoices.rb +++ b/app/jobs/update_account_invoices.rb @@ -37,7 +37,7 @@ class UpdateAccountInvoices if billable_periods.any? oldest_enterprise = billable_periods.first.enterprise address = oldest_enterprise.address.dup - first, space, last = (oldest_enterprise.contact || "").partition(' ') + first, _space, last = (oldest_enterprise.contact || "").partition(' ') address.update_attributes(phone: oldest_enterprise.phone) if oldest_enterprise.phone.present? address.update_attributes(firstname: first, lastname: last) if first.present? && last.present? account_invoice.order.update_attributes(bill_address: address, ship_address: address) diff --git a/app/models/cart.rb b/app/models/cart.rb index fee1a25a44..ecd1cfb94c 100644 --- a/app/models/cart.rb +++ b/app/models/cart.rb @@ -3,8 +3,6 @@ class Cart < ActiveRecord::Base belongs_to :user, :class_name => Spree.user_class def add_variant variant_id, quantity, distributor, order_cycle, currency - variant = Spree::Variant.find(variant_id) - order = create_or_find_order_for_distributor distributor, order_cycle, currency @populator = Spree::OrderPopulator.new(order, currency) diff --git a/app/models/spree/preference_decorator.rb b/app/models/spree/preference_decorator.rb index c3ab3f9a94..adb2194f2d 100644 --- a/app/models/spree/preference_decorator.rb +++ b/app/models/spree/preference_decorator.rb @@ -21,7 +21,7 @@ module Spree def enterprise enterprise_id = key.match(product_selection_from_inventory_only_regex)[1] - enterprise = Enterprise.find enterprise_id + Enterprise.find(enterprise_id) end def product_selection_from_inventory_only_regex diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb index 7b371c5be1..6e3121702b 100644 --- a/app/serializers/api/product_serializer.rb +++ b/app/serializers/api/product_serializer.rb @@ -56,7 +56,7 @@ class Api::CachedProductSerializer < ActiveModel::Serializer #return a sanitized html description def description_html - d = sanitize(object.description, options = {tags: "p, b, strong, em, i"}) + d = sanitize(object.description, tags: "p, b, strong, em, i") d.to_s.html_safe end diff --git a/lib/open_food_network/lettuce_share_report.rb b/lib/open_food_network/lettuce_share_report.rb index 43fe8529c5..d1d16b2582 100644 --- a/lib/open_food_network/lettuce_share_report.rb +++ b/lib/open_food_network/lettuce_share_report.rb @@ -42,15 +42,14 @@ module OpenFoodNetwork tax_category = variant.product.tax_category if tax_category && tax_category.tax_rates.present? tax_rate = tax_category.tax_rates.first - line_item = mock_line_item(variant, tax_category) + line_item = mock_line_item(variant) tax_rate.calculator.compute line_item else 0 end end - def mock_line_item(variant, tax_category) - product = OpenStruct.new tax_category: tax_category + def mock_line_item(variant) line_item = Spree::LineItem.new quantity: 1 line_item.define_singleton_method(:product) { variant.product } line_item.define_singleton_method(:price) { variant.price } diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb index b4fb9a6f32..2e992f2ee4 100644 --- a/lib/open_food_network/order_cycle_management_report.rb +++ b/lib/open_food_network/order_cycle_management_report.rb @@ -40,7 +40,6 @@ module OpenFoodNetwork def payment_method_row(order) ba = order.billing_address - da = order.distributor.andand.address [ba.firstname, ba.lastname, order.distributor.andand.name, @@ -56,7 +55,6 @@ module OpenFoodNetwork def delivery_row(order) sa = order.shipping_address - da = order.distributor.andand.address [sa.firstname, sa.lastname, order.distributor.andand.name, diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 2d85411fa1..23842d5415 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -93,7 +93,7 @@ module OpenFoodNetwork def shipping_cost_for(order) shipping_cost = order.adjustments.find_by_label("Shipping").andand.amount - shipping_cost = shipping_cost.nil? ? 0.0 : shipping_cost + shipping_cost.nil? ? 0.0 : shipping_cost end def tax_included_in(line_item) diff --git a/lib/tasks/karma.rake b/lib/tasks/karma.rake index 54e47e9110..27aca7d5a6 100644 --- a/lib/tasks/karma.rake +++ b/lib/tasks/karma.rake @@ -29,7 +29,7 @@ namespace :karma do def application_spec_files sprockets = Rails.application.assets sprockets.append_path Rails.root.join("spec/javascripts") - files = Rails.application.assets.find_asset("application_spec.js").to_a.map {|e| e.pathname.to_s } + Rails.application.assets.find_asset("application_spec.js").to_a.map {|e| e.pathname.to_s } end def unit_js(files)