Add missing override for `Nil#same?`

[potomak]

I was trying to match a value that can be Set(T) | Nil with nil and was getting the error:

In src/spec/expectations.cr:64:20

 64 | actual_value.same? @expected_value
                   ^----
Error: no overload matches 'Nil#same?' with type Set(Int32)

Overloads are:
 - Nil#same?(other : Nil)
 - Nil#same?(other : Reference)

The test that was failing:

describe "Set" do
  describe "select!" do
    it "returns self if changes were made" do
      set = Set{1, 2, 3}
      set.select! { |n| n < 2 }.should be(set)
    end
  end
end

Where Set#select! is defined as:

def select!(& : T ->) : self | Nil
  # ...
end

I guess the issue is values of type Set(T), that is a struct, are neither a Reference nor a Nil, so I updated one of the definitions of same? to accept Object rather than the more specific Reference.

Note: initially I added a new definition of same? that works with values of type Value, but I think this approach is better because it reduces the number of same? overrides.

[HertzDevil]

But be_nil calls nil?, not same?, so the above example should compile already, whatever definition Set#select! is.

Even if we were to do this, same?'s argument must be nil or something with reference semantics. So one should define Nil#same?(Set) rather than what this PR does.

[potomak]

I was able to repro here: https://play.crystal-lang.org/#/r/dgbb

require "spec"
 
def foo(n): Set(Int32) | Nil
  if n > 1
    return nil
  else
    return Set {n}
  end
end
 
describe "foo" do
  it "returns nil" do
    foo(10).should be_nil
  end
 
  it "returns a set" do
    foo(1).should be(Set {1})
  end
end

Result:

Showing last frame. Use --error-trace for full trace.

In /usr/lib/crystal/spec/expectations.cr:64:20

 64 | actual_value.same? @expected_value
                   ^----
Error: no overload matches 'Nil#same?' with type Set(Int32)

Overloads are:
 - Nil#same?(other : Nil)
 - Nil#same?(other : Reference)

In https://play.crystal-lang.org/#/r/dgbc I just changed be(...) with eq(...) in the second expectation and the test worked:

it "returns a set" do
  foo(1).should eq(Set {1})
end

[potomak]

But be_nil calls nil?, not same?, so the above example should compile already, whatever definition Set#select! is.

@HertzDevil you're right, in fact the issue is not with the be_nil expectation, but with the be(set) one.

I will update the PR description.

[potomak]

Better example:

require "spec"
 
it "foo" do
  nil.should_not be(Set {1})
end

Playground: https://play.crystal-lang.org/#/r/dgbe

[potomak]

I guess the issue is that I'm using be rather than eq and that doesn't make sense for types such as Set, does it?

[HertzDevil]

It does, because Set has reference semantics despite being a struct. And it is natural to expect that a.same?(b) implies b.same?(a) is callable and returns the same result. But that's about it, and we shouldn't expand this to arbitrary structs.

[straight-shoota]

I suppose the first thing to consider should be adding an overload Set#same?(Nil) for the commutative inversion. Set is the special type and when we discuss enabling Nil#same?(Set), the inverse should be applicable as well and it's the easier one, because it's simply defined on the "special" type Set.

Perhaps we could facilitate operand inversion for non-reference types in Nil#same?:

def same?(other : Value)
  other.same?(self)
end

This would compile with Set (when Set#same?(Nil) is defined) but fail to compile for any other type that doesn't define reference equality with Nil.

This implicitly works with any type that defines reference equality semantics fully (i.e. considering Nil) and doesn't require reopening and spamming Nil with specific overloads.

As a downside, error messages would be reversed (if you call Nil#same?(Other), the compiler would report a missing def for Other#same?(Nil) in stdlib, instead of the location of the original call).

We could perhaps use a macro raise to improve that error message.

  def same?(other : T) forall T
    if other.responds_to?(:same?)
      other.same?(self)
    else
    {% raise "no overload matches 'Nil#same?' with type #{T}" %}
    end
  end