Merge pull request #339 from Shopify/rubocop

Add Rubocop

.rubocop.yml

@@ -0,0 +1,36 @@
+require:
+ - rubocop-performance
+
+inherit_gem:
+ rubocop-shopify: rubocop.yml
+
+AllCops:
+ Exclude:
+ - 'spec/**/*'
+ SuggestExtensions: false
+
+Style/MethodCallWithoutArgsParentheses:
+ Enabled: false
+
+Style/StringLiterals:
+ Enabled: false
+
+Layout/EmptyLineAfterGuardClause:
+ Enabled: false
+
+Layout/LineLength:
+ Max: 120
+ AllowedPatterns:
+ - '^\s*#'
+
+Style/ClassVars:
+ Enabled: false
+
+Style/ClassMethodsDefinitions:
+ Enabled: false
+
+Naming/FileName:
+ Enabled: false
+
+Style/SpecialGlobalVars:
+ Enabled: false

Gemfile

@@ -1,7 +1,10 @@
 # frozen_string_literal: true
+
 source "https://rubygems.org"
 
 gem "pry-byebug", require: false
-gem 'rubocop', require: false
+gem "rubocop", require: false
+gem "rubocop-shopify", ">=2.8.0", require: false
+gem "rubocop-performance", require: false
 
 gemspec

Gemfile.lock

@@ -223,6 +223,11 @@ GEM
  unicode-display_width (>= 2.4.0, < 3.0)
  rubocop-ast (1.32.3)
  parser (>= 3.3.1.0)
+ rubocop-performance (1.22.1)
+ rubocop (>= 1.48.1, < 2.0)
+ rubocop-ast (>= 1.31.1, < 2.0)
+ rubocop-shopify (2.15.1)
+ rubocop (~> 1.51)
  ruby-progressbar (1.13.0)
  securerandom (0.3.1)
  simplecov (0.22.0)
@@ -265,6 +270,8 @@ DEPENDENCIES
  rails (~> 7.2)
  rspec (~> 3.2)
  rubocop
+ rubocop-performance
+ rubocop-shopify (>= 2.8.0)
  shopify-money!
  simplecov
  sqlite3

Rakefile

@@ -15,7 +15,7 @@ begin
 rescue Bundler::BundlerError => e
  $stderr.puts e.message
  $stderr.puts "Run `bundle install` to install missing gems"
- exit e.status_code
+ exit(e.status_code)
 end
 require 'rake'
 
@@ -30,7 +30,7 @@ RSpec::Core::RakeTask.new(:rcov) do |spec|
  spec.rcov = true
 end
 
-task :default => :spec
+task default: :spec
 
 require 'rake/task'
 RDoc::Task.new do |rdoc|

lib/money.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 require_relative 'money/version'
 require_relative 'money/parser/fuzzy'
 require_relative 'money/helpers'

lib/money/allocator.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 require 'delegate'
 
 class Money
@@ -107,7 +108,7 @@ def allocate(splits, strategy = :roundrobin)
  # Money.new(100).allocate_max_amounts([Money.new(5), Money.new(2)])
  # #=> [Money.new(5), Money.new(2)]
  def allocate_max_amounts(maximums)
- allocation_currency = extract_currency(maximums + [self.__getobj__])
+ allocation_currency = extract_currency(maximums + [__getobj__])
  maximums = maximums.map { |max| max.to_money(allocation_currency) }
  maximums_total = maximums.reduce(Money.new(0, allocation_currency), :+)
 
@@ -116,16 +117,16 @@ def allocate_max_amounts(maximums)
  Money.rational(max_amount, maximums_total)
  end
 
- total_allocatable = [maximums_total.subunits, self.subunits].min
+ total_allocatable = [maximums_total.subunits, subunits].min
 
  subunits_amounts, left_over = amounts_from_splits(1, splits, total_allocatable)
  subunits_amounts.map! { |amount| amount[:whole_subunits] }
 
  subunits_amounts.each_with_index do |amount, index|
- break unless left_over > 0
+ break if left_over <= 0
 
  max_amount = maximums[index].value * allocation_currency.subunit_to_unit
- next unless amount < max_amount
+ next if amount >= max_amount
 
  left_over -= 1
  subunits_amounts[index] += 1
@@ -137,9 +138,12 @@ def allocate_max_amounts(maximums)
  private
 
  def extract_currency(money_array)
- currencies = money_array.lazy.select { |money| money.is_a?(Money) }.reject(&:no_currency?).map(&:currency).to_a.uniq
+ currencies = money_array.lazy.select do |money|
+ money.is_a?(Money)
+ end.reject(&:no_currency?).map(&:currency).to_a.uniq
  if currencies.size > 1
- raise ArgumentError, "operation not permitted for Money objects with different currencies #{currencies.join(', ')}"
+ raise ArgumentError,
+ "operation not permitted for Money objects with different currencies #{currencies.join(", ")}"
  end
  currencies.first || NULL_CURRENCY
  end
@@ -154,8 +158,8 @@ def amounts_from_splits(allocations, splits, subunits_to_split = subunits)
  fractional_subunits = (subunits_to_split * ratio / allocations.to_r).to_f - whole_subunits
  left_over -= whole_subunits
  {
- :whole_subunits => whole_subunits,
- :fractional_subunits => fractional_subunits
+ whole_subunits: whole_subunits,
+ fractional_subunits: fractional_subunits,
  }
  end
 
@@ -172,7 +176,7 @@ def all_rational?(splits)
  # the next whole number. Similarly, given the input [9.1, 5.5, 3.9] the correct ranking is *still* 2, 1, 0. This
  # is because 3.9 is nearer to 4 than 9.1 is to 10.
  def rank_by_nearest(amounts)
- amounts.each_with_index.sort_by{ |amount, i| 1 - amount[:fractional_subunits] }.map(&:last)
+ amounts.each_with_index.sort_by { |amount, _i| 1 - amount[:fractional_subunits] }.map(&:last)
  end
  end
 end

lib/money/core_extensions.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 # Allows Writing of 100.to_money for +Numeric+ types
 # 100.to_money => #<Money @cents=10000>
 # 100.37.to_money => #<Money @cents=10037>
@@ -25,7 +26,7 @@ def to_money(currency = nil)
  return Money.new(self, currency)
  end
 
- return Money.new(0, currency) if self.empty?
+ return Money.new(0, currency) if empty?
 
  Money::Parser::Fuzzy.parse(self, currency).tap do |money|
  old_value = money.value

lib/money/currency.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 require "money/currency/loader"
 
 class Money
@@ -27,8 +28,16 @@ def currencies
  end
  end
 
- attr_reader :iso_code, :iso_numeric, :name, :smallest_denomination, :subunit_symbol,
- :subunit_to_unit, :minor_units, :symbol, :disambiguate_symbol, :decimal_mark
+ attr_reader :iso_code,
+ :iso_numeric,
+ :name,
+ :smallest_denomination,
+ :subunit_symbol,
+ :subunit_to_unit,
+ :minor_units,
+ :symbol,
+ :disambiguate_symbol,
+ :decimal_mark
 
  def initialize(currency_iso)
  data = self.class.currencies[currency_iso]
@@ -51,7 +60,7 @@ def eql?(other)
  end
 
  def hash
- [ self.class, iso_code ].hash
+ [self.class, iso_code].hash
  end
 
  def compatible?(other)

lib/money/currency/loader.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 require 'yaml'
 
 class Money
@@ -9,9 +10,9 @@ def load_currencies
  currency_data_path = File.expand_path("../../../../config", __FILE__)
 
  currencies = {}
- currencies.merge! YAML.load_file("#{currency_data_path}/currency_historic.yml")
- currencies.merge! YAML.load_file("#{currency_data_path}/currency_non_iso.yml")
- currencies.merge! YAML.load_file("#{currency_data_path}/currency_iso.yml")
+ currencies.merge!(YAML.load_file("#{currency_data_path}/currency_historic.yml"))
+ currencies.merge!(YAML.load_file("#{currency_data_path}/currency_non_iso.yml"))
+ currencies.merge!(YAML.load_file("#{currency_data_path}/currency_iso.yml"))
  deep_deduplicate!(currencies)
  end
 

lib/money/deprecations.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 Money.class_eval do
  ACTIVE_SUPPORT_DEFINED = defined?(ActiveSupport)
 

lib/money/errors.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 class Money
  class Error < StandardError
  end

lib/money/helpers.rb

@@ -1,9 +1,10 @@
 # frozen_string_literal: true
+
 require 'bigdecimal'
 
 class Money
  module Helpers
- module_function
+ extend self
 
  DECIMAL_ZERO = BigDecimal(0).freeze
  MAX_DECIMAL = 21

lib/money/money.rb

@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 require 'forwardable'
 require 'json'
 
@@ -9,6 +10,7 @@ class Money
  NULL_CURRENCY = NullCurrency.new.freeze
 
  attr_reader :value, :currency
+
  def_delegators :@value, :zero?, :nonzero?, :positive?, :negative?, :to_i, :to_f, :hash
 
  class ReverseOperationProxy
@@ -38,6 +40,7 @@ def *(other)
  class << self
  extend Forwardable
  attr_accessor :config
+
  def_delegators :@config, :default_currency, :default_currency=
 
  def configure
@@ -105,13 +108,11 @@ def current_currency=(currency)
  # I18n.with_locale and ActiveSupport's Time.use_zone. This won't affect
  # instances being created with explicitly set currency.
  def with_currency(new_currency)
- begin
- old_currency = Money.current_currency
- Money.current_currency = new_currency
- yield
- ensure
- Money.current_currency = old_currency
- end
+ old_currency = Money.current_currency
+ Money.current_currency = new_currency
+ yield
+ ensure
+ Money.current_currency = old_currency
  end
 
  private
@@ -127,10 +128,12 @@ def new_from_money(amount, currency)
  return amount
  end
 
- msg = "Money.new(Money.new(amount, #{amount.currency}), #{currency}) is changing the currency of an existing money object"
+ msg = "Money.new(Money.new(amount, #{amount.currency}), #{currency}) " \
+ "is changing the currency of an existing money object"
+
  if Money.config.legacy_deprecations
  Money.deprecate("#{msg}. A Money::IncompatibleCurrencyError will raise in the next major release")
- return Money.new(amount.value, currency)
+ Money.new(amount.value, currency)
  else
  raise Money::IncompatibleCurrencyError, msg
  end
@@ -195,19 +198,19 @@ def -(other)
  end
  end
 
- def *(numeric)
- raise ArgumentError, "Money objects can only be multiplied by a Numeric" unless numeric.is_a?(Numeric)
+ def *(other)
+ raise ArgumentError, "Money objects can only be multiplied by a Numeric" unless other.is_a?(Numeric)
 
- return self if numeric == 1
- Money.new(value.to_r * numeric, currency)
+ return self if other == 1
+ Money.new(value.to_r * other, currency)
  end
 
- def /(numeric)
+ def /(other)
  raise "[Money] Dividing money objects can lose pennies. Use #split instead"
  end
 
  def inspect
- "#<#{self.class} value:#{self} currency:#{self.currency}>"
+ "#<#{self.class} value:#{self} currency:#{currency}>"
  end
 
  def ==(other)
@@ -239,8 +242,10 @@ def to_money(new_currency = nil)
  return Money.new(value, new_currency)
  end
 
- ensure_compatible_currency(Helpers.value_to_currency(new_currency),
- "to_money is attempting to change currency of an existing money object from #{currency} to #{new_currency}")
+ ensure_compatible_currency(
+ Helpers.value_to_currency(new_currency),
+ "to_money is attempting to change currency of an existing money object from #{currency} to #{new_currency}",
+ )
 
  self
  end
@@ -261,7 +266,7 @@ def to_fs(style = nil)
 
  rounded_value = value.round(units)
  if units == 0
- sprintf("%d", rounded_value)
+ format("%d", rounded_value)
  else
  formatted = rounded_value.to_s("F")
  decimal_digits = formatted.size - formatted.index(".") - 1
@@ -303,7 +308,7 @@ def floor
  Money.new(floor, currency)
  end
 
- def round(ndigits=0)
+ def round(ndigits = 0)
  round = value.round(ndigits)
  return self if round == value
  Money.new(round, currency)
@@ -364,13 +369,13 @@ def calculate_splits(num)
  def clamp(min, max)
  raise ArgumentError, 'min cannot be greater than max' if min > max
 
- clamped_value = min if self.value < min
- clamped_value = max if self.value > max
+ clamped_value = min if value < min
+ clamped_value = max if value > max
 
  if clamped_value.nil?
  self
  else
- Money.new(clamped_value, self.currency)
+ Money.new(clamped_value, currency)
  end
  end
 
@@ -379,8 +384,10 @@ def clamp(min, max)
  def arithmetic(other)
  case other
  when Money
- ensure_compatible_currency(other.currency,
- "mathematical operation not permitted for Money objects with different currencies #{other.currency} and #{currency}.")
+ desc = "mathematical operation not permitted for Money objects with different currencies " \
+ "#{other.currency} and #{currency}."
+
+ ensure_compatible_currency(other.currency, desc)
  yield(other)
 
  when Numeric, String