beIdenticalTo / ===: disallow comparing non-objects

[NachoSoto]

Equality comparisons don't make sense for anything other than objects, and they will always fail.
Using === with structs for example is likely a programmer error, so to avoid it, a compile-time error is better.

This is potentially a breaking change.

[NachoSoto]

This is illegal now. The NSArray equivalent is in the next line.

[NachoSoto]

Any thoughts on this @ikesyo? I was just bitten by it again comparing 2 structs with === instead of ==:

[ikesyo]

Completely makes sense 👍

[acecilia]

Hi 👋 After this change I am experiencing that below code that previously worked fine, now it does not compile:

protocol MyProtocol { }

final class MyClass: MyProtocol {
    init() { }
}

final class Spec: QuickSpec {
    override func spec() {
        it("success") {
            let object1 = MyClass()
            let object2: MyProtocol = object1
            expect(object2).to(be(object1))
        }

        it("fails") {
            let object1 = MyClass()
            let object2: MyProtocol = MyClass()
            expect(object2).to(be(object1))
        }
    }
}

I understand that there are multiple ways to make these tests compile:

• Make MyProtocol conform to AnyObject
• Cast the object with type MyProtocol to the concrete class MyClass: expect(object2 as? MyClass).to(be(object1))

But I am not sure any of above solutions are correct:

• Make MyProtocol conform to AnyObject: this may not be possible, as there can be usecases where structs conform to the protocol
• Cast the object with type MyProtocol to the concrete class MyClass: in this case I would prefer if is Nimble itself the one doing the casting and failing the test. Casting to a type that is already in the right part of the expectation seems unnecessary and repetitive

I would consider above code snippet correct: it should build and run as expected without further modifications. Wdyt?

I believe the issue in this case boils down to this: the compiler is not able to tell at compile time that a type (in this case a protocol) is a class

[NachoSoto]

I believe the issue in this case boils down to this: the compiler is not able to tell at compile time that a type (in this case a protocol) is a class

That's exactly right. And without that, === can't really work.

[acecilia]

I believe the issue in this case boils down to this: the compiler is not able to tell at compile time that a type (in this case a protocol) is a class

That's exactly right. And without that, === can't really work.

=== used to work by casting and performing the check at runtime.

The change in this PR is based on an assumption that is false, no? The change modified the code so instead of making a run-time check, it performs a compile-time check, based on the assumption that all valid comparisons can be checked at compile-time

so to avoid it, a compile-time error is better

But we are saying that for some valid cases (the one above) the compiler does not know at compile time if a type is a class or not, so the current code does not allow valid comparisons that previously (with the run-time check) were allowed

[acecilia]

I would consider above code snippet correct: it should build and run as expected without further modifications. Wdyt?

@NachoSoto could you please reconsider this. I am considering opening a PR reverting the change here, but would like to hear if you have any alternative for the stated problem before doing so

[NachoSoto]

If you prefer to do this at runtime you could always add an overload for Any that casts it to AnyObject.

[acecilia]

If you prefer to do this at runtime

I do not prefer it, but protocol comparisons I believe cannot happen at compile time, as I shown above

you could always add an overload for Any that casts it to AnyObject.

@NachoSoto I cannot imagine how. Can you elaborate?

[NachoSoto]

extension Expectation where T == Any {
    public static func === (lhs: Expectation, rhs: Any?) {
        lhs.to(beIdenticalTo(rhs as? AnyObject))
    }
}

[acecilia]

Thanks for that. But above code does not build, throws the error Cannot convert value of type 'Predicate<AnyObject>' to expected argument type 'Predicate<Any>'