Merge pull request #1155 from sunny/eql-returns-false-if-currency-differs

yukideluxe · web-flow · commit 956ece2fe611 · 2025-07-30T12:23:45.000+02:00
Warn when using `#eql?` on zero amounts with different currencies
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,14 @@
 - Add Zimbabwe Gold (ZWG) currency
 - Update thousands_separator for CHF
 - Add Caribbean Guilder (XCG) as replacement for Netherlands Antillean Gulden (ANG)
+- Add `Money.strict_eql_compare = true` so that comparing zero amounts with different currencies using `Money#eql?` returns `false`
+    ```rb
+    Money.new(0, "USD").eql?(Money.new(0, "EUR")) #=> true
+    #> [DEPRECATION] Comparing 0 USD with 0 EUR using `#eql?` will return false…
+
+    Money.strict_eql_compare = true
+    Money.new(0, "USD").eql?(Money.new(0, "EUR")) #=> false
+    ```
 - Add `Money#to_nearest_cash_value` to return a rounded Money instance to the smallest denomination
 - Deprecate `Money#round_to_nearest_cash_value` in favor of calling `to_nearest_cash_value.fractional`
 - Add `Money::Currency#cents_based?` to check if currency is cents-based
diff --git a/lib/money/money.rb b/lib/money/money.rb
@@ -146,7 +146,27 @@ class << self
     #   Used to specify precision for converting Rational to BigDecimal
     #
     #   @return [Integer]
-    attr_accessor :default_formatting_rules, :default_infinite_precision, :conversion_precision
+    #
+    # @!attribute [rw] strict_eql_compare
+    #    Use this to specify how +Money#eql?+ behaves. Opt-in to the new
+    #    behavior by setting this to +true+ and disable warnings when comparing
+    #    zero amounts with different currencies.
+    #
+    #    @example
+    #      Money.strict_eql_compare = false # (default)
+    #      Money.new(0, "USD").eql?(Money.new(0, "EUR")) # => true
+    #      # => [DEPRECATION] warning
+    #
+    #      Money.strict_eql_compare = true
+    #      Money.new(0, "USD").eql?(Money.new(0, "EUR")) # => false
+    #
+    #    @return [Boolean]
+    #
+    #    @see Money#eql
+    attr_accessor :default_formatting_rules,
+                  :default_infinite_precision,
+                  :conversion_precision,
+                  :strict_eql_compare
     attr_reader :use_i18n, :locale_backend
     attr_writer :default_bank
   end
@@ -238,6 +258,10 @@ def self.setup_defaults
 
     # Default the conversion of Rationals precision to 16
     self.conversion_precision = 16
+
+    # Defaults to the deprecated behavior where
+    # `Money.new(0, "USD").eql?(Money.new(0, "EUR"))` is true.
+    self.strict_eql_compare = false
   end
 
   def self.inherited(base)
diff --git a/lib/money/money/arithmetic.rb b/lib/money/money/arithmetic.rb
@@ -22,24 +22,32 @@ def -@
     end
 
     # Checks whether two Money objects have the same currency and the same
-    # amount. If Money objects have a different currency it will only be true
-    # if the amounts are both zero. Checks against objects that are not Money or
-    # a subclass will always return false.
+    # amount. Checks against objects that are not Money or a subclass will
+    # always return false.
     #
     # @param [Money] other_money Value to compare with.
     #
     # @return [Boolean]
     #
     # @example
-    #   Money.new(100).eql?(Money.new(101))                #=> false
-    #   Money.new(100).eql?(Money.new(100))                #=> true
-    #   Money.new(100, "USD").eql?(Money.new(100, "GBP"))  #=> false
-    #   Money.new(0, "USD").eql?(Money.new(0, "EUR"))      #=> true
-    #   Money.new(100).eql?("1.00")                        #=> false
+    #   Money.new(1_00).eql?(Money.new(1_00))               #=> true
+    #   Money.new(1_00).eql?(Money.new(1_01))               #=> false
+    #   Money.new(1_00, "USD").eql?(Money.new(1_00, "GBP")) #=> false
+    #   Money.new(0, "USD").eql?(Money.new(0, "EUR"))       #=> false
+    #   Money.new(1_00).eql?("1.00")                        #=> false
+    #
+    # @see Money.strict_eql_compare
     def eql?(other_money)
       if other_money.is_a?(Money)
-        (fractional == other_money.fractional && currency == other_money.currency) ||
-          (fractional == 0 && other_money.fractional == 0)
+        if !Money.strict_eql_compare && fractional == 0 && other_money.fractional == 0
+          warn "[DEPRECATION] Comparing 0 #{currency} with 0 " \
+                "#{other_money.currency} using `#eql?` will return false in " \
+                "future versions of Money. Opt-in to the new behavior by " \
+                "setting `Money.strict_eql_compare = true`."
+          return true
+        end
+
+        fractional == other_money.fractional && currency == other_money.currency
       else
         false
       end
diff --git a/sig/lib/money/money.rbs b/sig/lib/money/money.rbs
@@ -193,6 +193,24 @@ class Money
   #   @return [Integer]
   attr_accessor self.conversion_precision: untyped
 
+  # @!attribute [rw] strict_eql_compare
+  #    Use this to specify how +Money#eql?+ behaves. Opt-in to the new
+  #    behavior by setting this to +true+ and disable warnings when comparing
+  #    zero amounts with different currencies.
+  #
+  #    @example
+  #      Money.strict_eql_compare = false # (default)
+  #      Money.new(0, "USD").eql?(Money.new(0, "EUR")) # => true
+  #      # => [DEPRECATION] warning
+  #
+  #      Money.strict_eql_compare = true
+  #      Money.new(0, "USD").eql?(Money.new(0, "EUR")) # => false
+  #
+  #    @return [Boolean]
+  #
+  #    @see Money#eql
+  attr_accessor self.strict_eql_compare: untyped
+
   attr_reader self.use_i18n: untyped
 
   attr_reader self.locale_backend: untyped
diff --git a/sig/lib/money/money/arithmetic.rbs b/sig/lib/money/money/arithmetic.rbs
@@ -13,20 +13,21 @@ class Money
     def -@: () -> Money
 
     # Checks whether two Money objects have the same currency and the same
-    # amount. If Money objects have a different currency it will only be true
-    # if the amounts are both zero. Checks against objects that are not Money or
-    # a subclass will always return false.
+    # amount. Checks against objects that are not Money or a subclass will
+    # always return false.
     #
     # @param [Money] other_money Value to compare with.
     #
     # @return [Boolean]
     #
     # @example
-    #   Money.new(100).eql?(Money.new(101))                #=> false
-    #   Money.new(100).eql?(Money.new(100))                #=> true
-    #   Money.new(100, "USD").eql?(Money.new(100, "GBP"))  #=> false
-    #   Money.new(0, "USD").eql?(Money.new(0, "EUR"))      #=> true
-    #   Money.new(100).eql?("1.00")                        #=> false
+    #   Money.new(1_00).eql?(Money.new(1_00))               #=> true
+    #   Money.new(1_00).eql?(Money.new(1_01))               #=> false
+    #   Money.new(1_00, "USD").eql?(Money.new(1_00, "GBP")) #=> false
+    #   Money.new(0, "USD").eql?(Money.new(0, "EUR"))       #=> false
+    #   Money.new(1_00).eql?("1.00")                        #=> false
+    #
+    # @see Money.strict_eql_compare
     def eql?: (Money other_money) -> bool
 
     # Compares two Money objects. If money objects have a different currency it
@@ -194,4 +195,4 @@ class Money
     #   2 * Money.new(10) #=> #<Money @fractional=20>
     def coerce: (untyped other) -> ::Array[self | untyped]
   end
-end
+end
diff --git a/spec/money/arithmetic_spec.rb b/spec/money/arithmetic_spec.rb
@@ -49,16 +49,17 @@
     end
 
     it 'allows comparison with zero' do
-      expect(Money.new(0, :usd)).to eq 0
-      expect(Money.new(0, :usd)).to eq 0.0
-      expect(Money.new(0, :usd)).to eq BigDecimal(0)
-      expect(Money.new(1, :usd)).to_not eq 0
+      expect(Money.new(0, "USD")).to eq 0
+      expect(Money.new(0, "EUR")).to eq 0
+      expect(Money.new(0, "USD")).to eq 0.0
+      expect(Money.new(0, "USD")).to eq BigDecimal(0)
+      expect(Money.new(1, "USD")).to_not eq 0
     end
 
     it 'raises error for non-zero numerics' do
-      expect { Money.new(1_00, :usd) == 1 }.to raise_error ArgumentError
-      expect { Money.new(1_00, :usd) == -2.0 }.to raise_error ArgumentError
-      expect { Money.new(1_00, :usd) == Float::INFINITY }.to raise_error ArgumentError
+      expect { Money.new(1_00, "USD") == 1 }.to raise_error ArgumentError
+      expect { Money.new(1_00, "USD") == -2.0 }.to raise_error ArgumentError
+      expect { Money.new(1_00, "USD") == Float::INFINITY }.to raise_error ArgumentError
     end
   end
 
@@ -70,18 +71,54 @@
       expect(Money.new(1_00, "USD").eql?(Money.new(99_00, "EUR"))).to be false
     end
 
-    it "returns true when their amounts are zero and currencies differ" do
-      expect(Money.new(0, "USD").eql?(Money.new(0, "EUR"))).to be true
-      expect(Money.new(0, "USD").eql?(Money.new(0, "USD"))).to be true
-      expect(Money.new(0, "AUD").eql?(Money.new(0, "EUR"))).to be true
+    context "with strict_eql_compare set to false" do
+      before { Money.strict_eql_compare = false }
+      after { Money.setup_defaults }
+
+      it "returns true when comparing zero" do
+        expect(Money.new(0, "USD").eql?(Money.new(0, "USD"))).to be true
+        expect(Money.new(0, "USD").eql?(Money.new(0, "EUR"))).to be true
+        expect(Money.new(0, "EUR").eql?(Money.new(0, "USD"))).to be true
+      end
+
+      it "warns" do
+        amount = Money.new(0, "USD")
+        allow(amount).to receive(:warn)
+
+        expect(amount.eql?(Money.new(0, "EUR"))).to be true
+        expect(amount).to have_received(:warn)
+          .with(/\A\[DEPRECATION\] Comparing 0 USD with 0 EUR using `#eql\?`/)
+      end
+    end
+
+    context "with strict_eql_compare set to true" do
+      before { Money.strict_eql_compare = true }
+      after { Money.setup_defaults }
+
+      it "does not make an exception for zero" do
+        expect(Money.new(0, "USD").eql?(Money.new(0, "USD"))).to be true
+        expect(Money.new(0, "USD").eql?(Money.new(0, "EUR"))).to be false
+        expect(Money.new(0, "EUR").eql?(Money.new(0, "USD"))).to be false
+      end
+
+      it "does not warn" do
+        amount = Money.new(0, "USD")
+        allow(amount).to receive(:warn)
+
+        expect(amount.eql?(Money.new(0, "EUR"))).to be false
+        expect(amount).not_to have_received(:warn)
+      end
     end
 
     it "returns false if used to compare with an object that doesn't inherit from Money" do
-      expect(Money.new(1_00, "USD").eql?(Object.new)).to  be false
-      expect(Money.new(1_00, "USD").eql?(Class)).to       be false
-      expect(Money.new(1_00, "USD").eql?(Kernel)).to      be false
-      expect(Money.new(1_00, "USD").eql?(/foo/)).to       be false
-      expect(Money.new(1_00, "USD").eql?(nil)).to         be false
+      expect(Money.new(1_00, "USD").eql?(Object.new)).to be false
+      expect(Money.new(1_00, "USD").eql?(Class)).to      be false
+      expect(Money.new(1_00, "USD").eql?(Kernel)).to     be false
+      expect(Money.new(1_00, "USD").eql?(/foo/)).to      be false
+      expect(Money.new(1_00, "USD").eql?(nil)).to        be false
+      expect(Money.new(1_00, "USD").eql?(1.0)).to        be false
+      expect(Money.new(1_00, "USD").eql?(1)).to          be false
+      expect(Money.new(1_00, "USD").eql?(1_00)).to       be false
     end
 
     it "can be used to compare with an object that inherits from Money" do
