Throw error when objects are passed instead of traits (#2649)

[bendk]

The corner case here is when an object implements a Rust-only trait. In the updated coverall fixture we have the following Python classes:

• StringUtilProtocol - interface for the trait
• StringUtil - object that implements the trait via rust calls
• StringUtilObject - exported Rust object that imprements the trait

Before StringUtilObject would inherit from StringUtil, which isn't correct conceptually and in practice it means that the Python code will try to lower the handle into a function that expects Arc<dyn StructUtil> when the handle was actually for Arc<StringUtilObject>, once Rust tries to use the arc a segfault will happen soon after.

Now StringUtilObject inherits from StringUtilProtocol and we don't have the segfault.

I tried to add Kotlin and Swift tests here, but it doesn't work because they throw a type error as soon as you try making that call.

[joe-p]

Just to be clear, when the trait is with_foreign StringUtilObject inherits from StringUtil which seems like correct behavior to me. Or is that not intended?

If that is intended it might be worthwhile to a add a fixture testing that it does indeed work. Also might be worth an explanation about the extra boundary crosses in the docs. I can create a separate PR for that if desired.

If not intended, I think that would be a useful feature to support. The primary use case I have in mind is for testing the generated code from Rust. Not sure what this would look like in Swift, Kotlin, or other 3rd party languages though.

[bendk]

Just to be clear, when the trait is with_foreign StringUtilObject inherits from StringUtil which seems like correct behavior to me. Or is that not intended?

That's working as intended.

If that is intended it might be worthwhile to a add a fixture testing that it does indeed work. Also might be worth an explanation about the extra boundary crosses in the docs. I can create a separate PR for that if desired.

The more I think about it, the more I think the real fix is to swap things and make trait interfaces without with_foreign have the same class structure. StringUtil would be the interface/protocol and StringUtilImpl would be the would be the implementation of that protocol via Rust. That makes everything more consistent and makes mistakes less likely -- it's fairly clear that classes should inherit from StringUtil and not StringUtilImpl.

Then regular, non-trait, objects would be the odd one out. There, MyObject is a concrete object that implements MyObjectProtocol via Rust calls. I think that makes total sense though, since MyObject is also a concrete type in Rust and MyObjectProtocol is a something extra we generate for niche use-cases like mocking.

(I thought there was already an issue for this, but I can't find it).

[bendk]

Since, we're pretty close to a release, I'm thinking I'll put up a PR once the next cycle starts.

[mhammond]

I ended up kinda accidentally adding these kinds of tests here too. I might have a go at consolidating that and this coveralls traits stuff into a dedicated traits fixture?

[mhammond]

The more I think about it, the more I think the real fix is to swap things and make trait interfaces without with_foreign have the same class structure. StringUtil would be the interface/protocol and StringUtilImpl would be the would be the implementation of that protocol via Rust. That makes everything more consistent and makes mistakes less likely -- it's fairly clear that classes should inherit from StringUtil and not StringUtilImpl.

I think I agree with that - but it's difficult to visualize exactly - it'd be great to see concrete class mockups.

[bendk]

Made #2689 to continue the discussion/work on the trait names. Let's merge this one.