From e6584c9f042fd111d870a03a2924d87b8cbcfcb7 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 18 Dec 2022 11:35:43 -0700 Subject: [PATCH] Prevent comparing range arguments with non-compatible types in stubs Addresses https://github.com/icy-arctic-fox/spectator/issues/48 --- CHANGELOG.md | 1 + spec/issues/github_issue_48_spec.cr | 10 ++++++ src/spectator/mocks/abstract_arguments.cr | 38 ++++++++++++++--------- src/spectator/mocks/arguments.cr | 12 ++----- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46800cf..73ffadb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix macro logic to support free variables, 'self', and variants on stubbed methods. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) - Fix method stubs used on methods that capture blocks. - Fix type name resolution for when using custom types in a mocked typed. +- Prevent comparing range arguments with non-compatible types in stubs. [#48](https://github.com/icy-arctic-fox/spectator/issues/48) ### Changed - Simplify string representation of mock-related types. diff --git a/spec/issues/github_issue_48_spec.cr b/spec/issues/github_issue_48_spec.cr index 21e4664..b958c1b 100644 --- a/spec/issues/github_issue_48_spec.cr +++ b/spec/issues/github_issue_48_spec.cr @@ -38,6 +38,10 @@ Spectator.describe "GitHub Issue #48" do block.call(thing) block end + + def range(r : Range) + r + end end mock Test, make_nilable: nil @@ -122,4 +126,10 @@ Spectator.describe "GitHub Issue #48" do allow(fake).to receive(:capture).and_return(proc) expect(fake.capture(5) { 5 }).to be(proc) end + + it "handles range comparisons against non-comparable types" do + range = 1..10 + allow(fake).to receive(:range).and_return(range) + expect(fake.range(1..3)).to eq(range) + end end diff --git a/src/spectator/mocks/abstract_arguments.cr b/src/spectator/mocks/abstract_arguments.cr index b06934c..442c1d2 100644 --- a/src/spectator/mocks/abstract_arguments.cr +++ b/src/spectator/mocks/abstract_arguments.cr @@ -11,13 +11,7 @@ module Spectator return false if a.size != b.size a.zip(b) do |a_value, b_value| - if a_value.is_a?(Proc) - # Using procs as argument matchers isn't supported currently. - # Compare directly instead. - return false unless a_value == b_value - else - return false unless a_value === b_value - end + return false unless compare_values(a_value, b_value) end true end @@ -34,15 +28,31 @@ module Spectator private def compare_named_tuples(a : NamedTuple, b : NamedTuple) a.each do |k, v1| v2 = b.fetch(k) { return false } - if v1.is_a?(Proc) - # Using procs as argument matchers isn't supported currently. - # Compare directly instead. - return false unless v1 == v2 - else - return false unless v1 === v2 - end + return false unless compare_values(v1, v2) end true end + + # Utility method for comparing two arguments considering special types. + # Some types used for case-equality don't work well with unexpected right-hand types. + # This can happen when the right side is a massive union of types. + private def compare_values(a, b) + case a + when Proc + # Using procs as argument matchers isn't supported currently. + # Compare directly instead. + a == b + when Range + # Ranges can only be matched against if their right side is comparable. + # Ensure the right side is comparable, otherwise compare directly. + if b.is_a?(Comparable(typeof(b))) + a === b + else + a == b + end + else + a === b + end + end end end diff --git a/src/spectator/mocks/arguments.cr b/src/spectator/mocks/arguments.cr index 2638e77..f15a0ba 100644 --- a/src/spectator/mocks/arguments.cr +++ b/src/spectator/mocks/arguments.cr @@ -95,21 +95,13 @@ module Spectator v1 = positional[i] i += 1 - if v1.is_a?(Proc) - return false unless v1 == v2 - else - return false unless v1 === v2 - end + return false unless compare_values(v1, v2) end other.splat.try &.each do |v2| v1 = positional.fetch(i) { return false } i += 1 - if v1.is_a?(Proc) - return false unless v1 == v2 - else - return false unless v1 === v2 - end + return false unless compare_values(v1, v2) end i == positional.size