Fix trait bounds for associated types

[dvdplm]

When a type has a type parameter that is bound to a trait with an associated type and that associated type has the same name as the type we're deriving TypeInfo for, the trait bounds are wrong.

See #73 for more gory details.

Thanks to @thiolliere for the fix.

Closes #73

[Robbepop]

I won't stop you from merging this but will definitely tell you (again) that fixes like these are not implementation details in a proc. macro but will from now on be part of its API. So people are going to depend on these semantics that are based on syntactic analysis of the code. This is always pretty fragile and it is not really possible to catch all the cutting edges. What about type alises? What about nested associated types? What about macro types? What about GATs? What about upcoming specialization? All of these will very likely break this feature in some way or another. Etc.

So this is at best a best-effort approach and not an ideal solution. As it I would rather see more momentum in making it possible for users to define custom trait bounds. This would equally solve the same problems (and more since it is a more general solution) at the obvious cost of demanding users to provide the trait bounds instead of inferring them for some common cases ... what happens on the other cases though? 🤔

[gui1117]

This special case is just a best effort to avoid having wrong bounds in recursive types.

But you are right, and now I realize struct Foo ( Option<Box<Self>> ); or other alias of Foo is probably failing too.

This special shouldn't harm too much because any recursive types (which is what we try to detect here, with lot of misses in case of alias, but now there shouldn't be false positive anymore) will have wrong bounds if we derive them normally.

But I actually agree best is to allow user to write bounds. Unfortunately the error message for failing default bounds in recursive types will not be very good, but maybe more understandable than potentially failing best effort.

[dvdplm]

So this is at best a best-effort approach and not an ideal solution.

This is crystal clear to me and this bug here is the perfect illustration of your point (and also, imo, validates the dogfood-work @ascjones started and I am pushing over the finish line).

I think the short/mid-term end-goal of scale-info remains the same though: be "good enough" for substrate and polkadot while we think about what a better approach could be.

As it I would rather see more momentum in making it possible for users to define custom trait bounds.

Adding support for custom bounds is the highest priority. :)
If we see that approach work out well maybe we can scale back on the syntax hacks but I suspect that asking regular users to type out the bounds manually is going to be a high barrier to adoption.

[Robbepop]

Adding support for custom bounds is the highest priority. :)

So maybe we should wait for this work before we merge this PR because merging this PR and later replacing it with custom trait bounds is both a breaking change due to both PRs being API changes to the proc. macro.

[..] but I suspect that asking regular users to type out the bounds manually is going to be a high barrier to adoption.

I am more optimistic since it was not an adoption barrier for serde as well.

[ascjones]

LGTM

[Robbepop]

In a meeting it was decided that we put forward with scale-info in the following way:

• We will go with the "hacks" that have already been implemented for it and for parity-scale-codec crates to cover the most common use cases with 80/20 rule.
• For the other 20% of cases that are not covered by the scale-info and parity-scale-codec derive macros we will provide a feature for users to define custom trait bounds similar to the design serde chose. Those custom trait bounds will be replacing the generated ones instead of being additive.
• We might consider in the future to remove the "cruft" of "hacks" for common cases and consider removing them altogether with a major release for both derives.