Correctly handly QSelf in riker macro

[hardliner66]

Currently the riker macro does not correctly handle the qself field of the TypePath. This can lead to conflicts, if you use an associated trait type with two different structs, because they only differ by the QSelf part.

In order to correctly handle this, we need to pattern match on QSelf and handle all type enum values that are actually possible in this case (maybe all, but that needs to be evaluated).

This relates to #154 and #155.

[elenaf9]

@hardliner66 @leenozara is solving this issue necessary before a new version is released? In this case I would implement it (if you are not already working on that yourself).

[hardliner66]

I personally think it's more of an advanced feature. But we would still be shipping with incomplete generics support.

So while I don't think it is strictly necessary for a new release, I think it still would be better if we would implement it directly. I'm also interested in your opinions on the topic.

[elenaf9]

Ok, I agree that it is better to have a complete implementation right now rather than having to fix it later on.
I'll try to make a PR within the next days for you to review, but right now I can't estimate how complex it will be because I don't know the different enum types that have to be handled.

[hardliner66]

I tried to build a basic proof of concept yesterday, but you need to recursively create a string from the type. I also changed name from returning an Ident to a string, because depending on the type, we need to call get_name recursively again.

Maybe it would be easier to use the Debug impl of QSelf to generate an ident we can work with?

[elenaf9]

My first attempt was to use Debug, but from what it looks like, this only uses the default derived Debug, and parsing that into a proper name isn't easier that using directly the enum.
But looking at that Type enum, I come to think that Type is used in multiple places and not something specific for QSelf because most of these types I think aren't even possible in a Self type (e.g. BareFn). And considering that, I actually would only handle Type::Path (which can be done recursively), and ignore the rest of the types (they would still be possible to use if somebody ever needs that, but just not mentioned in the name). Please correct me if I am doing any wrong assumptions.

[hardliner66]

I just looked at the possible enums and these are the ones I think are actually possible:
TypeArray, TypeParen, TypePath, TypePtr, TypeReference, TypeSlice and TypeTuple.

Maybe TypeGroup is possible as well, but I'm not sure about that.

Also I think that TypeInfer is actually possible, but I don't think we can get the type the compiler infers. That means that we should actually throw some kind of error to tell the user to not use the underscore.

[elenaf9]

I drafted a PR for this issue that handles QSelf by generating the name for Type recursively, please have a look whether this is a acceptable solution. Please note that by considering also the QSelf property, the names get quite long, maybe we should discuss shortening them to e.g. just the first letter of the names within the path, so instead of TestModGenericMessage name it TMGMessage.
@hardliner66 I only modified one test to check QSelf with type Type::Path, it would be great if you could add some tests for the other enums types you mentioned above, since I am not sure how this would look like.

[hardliner66]

The names do get quite long and I think this is something we should work on, but just shortening them will probably lead to conflicts. We might be able to do something fancier in order to shorten them, but for now I would just leave them as is.

[elenaf9]

@hardliner66 @leenozara is there any update from your side about this PR / anything that is still missing?

[hardliner66]

There is nothing missing from my side.