From 16c2bcbd9ec98373c91990dfec8c8d8564572ef5 Mon Sep 17 00:00:00 2001 From: Grant Hutchins Date: Fri, 12 Jun 2026 17:44:19 -0500 Subject: [PATCH] Raise a descriptive ArgumentError from <=> for incompatible units Following #31, <=> returned nil for non-coercible objects, but the Numeric branch still raised IncompatibleUnitError (a TypeError) whenever two units had incompatible dimensions. Routed through Comparable that surfaced as the unhelpful "comparison of Unit with Unit failed", and sort, min, max, between? and clamp blew up with a non-standard error. Raise ArgumentError directly from <=> when the operands are dimensionally incompatible. Every Comparable path funnels through <=>, so the same descriptive message now surfaces from <, >, sort, min, max, between? and clamp. A sibling Unit is shown via #inspect (Unit("1 s")); any other numeric is shown by class (Float, Integer) rather than its coerced unit or a potentially large #inspect, matching core Ruby's own phrasing. Comparing against a genuinely foreign object (nil, String) still returns nil, matching 1 <=> "a"; only dimension mismatches between quantities raise. Arithmetic (+/-) keeps raising IncompatibleUnitError. --- lib/unit/class.rb | 20 ++++++++++++++++++-- spec/unit_spec.rb | 48 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/lib/unit/class.rb b/lib/unit/class.rb index 965a708..2266c23 100644 --- a/lib/unit/class.rb +++ b/lib/unit/class.rb @@ -131,8 +131,12 @@ def eql?(other) def <=>(other) if Numeric === other - other = coerce_numeric_compatible(other) - a, b = self.normalize, other.normalize + coerced = coerce_numeric(other) + unless compatible?(coerced) + raise ArgumentError, + "comparison of #{comparison_label(self)} with #{comparison_label(other)} failed" + end + a, b = self.normalize, coerced.normalize a.value <=> b.value elsif other.respond_to?(:coerce) apply_through_coercion(other, __method__) @@ -294,6 +298,18 @@ def apply_through_coercion(obj, oper) raise TypeError, "#{obj.class} can't be coerced into #{self.class}" end + # Label an operand for a failed-comparison message. A dimensional unit keeps + # its informative #inspect (Unit("1 m")); a dimensionless unit is just a + # number — and a coerced bare numeric arrives wrapped as one — so it is + # reported by its underlying numeric class, matching how core Ruby names the + # operands of a failed comparison (and avoiding splatting an arbitrary + # #inspect into the message). + def comparison_label(operand) + return operand.inspect if Unit === operand && !operand.unit.empty? + operand = operand.value if Unit === operand + operand.class + end + def coerce_numeric_compatible(object) object = coerce_numeric(object) raise IncompatibleUnitError, "#{inspect} and #{object.inspect} are incompatible" if !compatible?(object) diff --git a/spec/unit_spec.rb b/spec/unit_spec.rb index 5cad4bd..49d6a5d 100644 --- a/spec/unit_spec.rb +++ b/spec/unit_spec.rb @@ -86,6 +86,50 @@ expect { Unit(1) < "string" }.to raise_error(ArgumentError) end + it "should raise a descriptive ArgumentError from <=> for incompatible dimensions" do + expect { Unit(1, 'm') <=> Unit(1, 's') }.to raise_error( + ArgumentError, 'comparison of Unit("1 m") with Unit("1 s") failed' + ) + expect { Unit(1, 'g') <=> Unit(1, 'm') }.to raise_error(ArgumentError) + end + + it "should show the operand's class, not a coerced unit, for non-Unit numerics" do + expect { Unit(1, 'm') > 1.2 }.to raise_error( + ArgumentError, 'comparison of Unit("1 m") with Float failed' + ) + expect { Unit(1, 'm') > 5 }.to raise_error( + ArgumentError, 'comparison of Unit("1 m") with Integer failed' + ) + end + + it "should label operands symmetrically when the unit is on the right" do + expect { 1.2 > Unit(1, 'm') }.to raise_error( + ArgumentError, 'comparison of Float with Unit("1 m") failed' + ) + expect { 5 < Unit(1, 'm') }.to raise_error( + ArgumentError, 'comparison of Integer with Unit("1 m") failed' + ) + end + + it "should still compare units with compatible dimensions" do + expect(Unit(100, 'cm') <=> Unit(1, 'm')).to eq(0) + expect(Unit(1, 'cm') <=> Unit(1, 'm')).to eq(-1) + expect(Unit(2, 'm') <=> Unit(1, 'm')).to eq(1) + end + + it "should carry the descriptive error through every comparison path" do + expect { Unit(1, 'm') > Unit(1, 's') }.to raise_error(ArgumentError, /Unit\("1 m"\).*Unit\("1 s"\)/) + expect { [Unit(1, 'm'), Unit(2, 's')].sort }.to raise_error(ArgumentError, /incompatible|failed/) + expect { Unit(1, 'm').between?(Unit(1, 's'), Unit(2, 's')) }.to raise_error(ArgumentError) + expect { Unit(1, 'm').clamp(Unit(1, 's'), Unit(2, 's')) }.to raise_error(ArgumentError) + end + + it "should sort and aggregate compatible units" do + sorted = [Unit(3, 'm'), Unit(1, 'm'), Unit(2, 'm')].sort + expect(sorted).to eq([Unit(1, 'm'), Unit(2, 'm'), Unit(3, 'm')]) + expect([Unit(50, 'cm'), Unit(1, 'm')].max).to eq(Unit(1, 'm')) + end + it "should support eql comparison" do expect(Unit(1)).to eql(Unit(1)) expect(Unit(1.0)).not_to eql(Unit(1)) @@ -255,8 +299,8 @@ expect(Unit(100, "m")).to be > Unit(0.0001, "km") end - it "should fail comparison on differing units" do - expect { Unit(1, "second") > Unit(1, "meter") }.to raise_error(Unit::IncompatibleUnitError) + it "should fail ordered comparison on differing units" do + expect { Unit(1, "second") > Unit(1, "meter") }.to raise_error(ArgumentError) end it "should keep units when the value is zero" do