Fix parsing for single-type transitive paths (#13)

[A-Manning]

Fixes #13.

[bobozaur]

@A-Manning Thanks for doing this! I'm unfortunately a bit swamped but I promise to review this til the end of the week.

[bobozaur]

I think we can polish this a bit more before merging.

Nevertheless, the underlying problem is a bit more subtle. Not only are at least two types required in the type list, but we need, in fact, at least two distinct types.

However, let's please not tackle that in this PR.

[bobozaur]

I'd rather not use this.

There should not be many fail tests around the library but regardless of that we could make use of doc tests and compile_fail attribute to achieve the same thing. See https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html#attributes

[A-Manning]

IMO it is hacky to use doc tests for this. The tests are not documentation

[bobozaur]

I don't see any reason why such tests would not qualify as documentation since they pertain to the correct usage of the crate API.

Explicitly stating the invalid usage of the API in the crate docs with supporting examples is, at least to me, to be preferred.

Just to make it a bit more clear, I'd agree with the usage of trybuild for something like testing the macros from the sqlx crate, where the testing environment (like a live database) matters as well.

[A-Manning]

we need, in fact, at least two distinct types.

It is possible to use Rust's type system to ensure that you have at least two distinct types, however it is quite awkward to do so. See https://docs.rs/typewit/latest/typewit/struct.TypeNe.html#method.with_fn

I think it is not worth checking this, as it would require either adding typewit as a dependency, or copying several definitions

[bobozaur]

It is possible to use Rust's type system to ensure that you have at least two distinct types, however it is quite awkward to do so. See https://docs.rs/typewit/latest/typewit/struct.TypeNe.html#method.with_fn

Interesting crate, haven't heard of it before!

But yeah, I wasn't thinking of going that far with the check as much as doing the humane amount of checking against the type names. That will obviously fail for type aliases and such, but the whole point behind the check as well as this PR is providing some nicer error messages.

The outcomes do not change, either compilation succeeds or fails. Going above and beyond just for that is indeed an overkill. Perhaps down the line if TypeId::of becomes stable in const contexts some const assert! checks could be introduced. EDIT: Nvm I was looking at https://doc.rust-lang.org/beta/std/intrinsics/fn.type_id.html returning a u128 which I believe could be compared at compile time by an assert! in a const context. But mixed that up with the actual TypeId that gets returned from the stable function.

[bobozaur]

Hey @A-Manning , not meaning to rush but I was wondering if you plan to work on this more anytime soon. It seems there shouldn't be a lot more left.

If you're unable to (which is perfectly fine), would you mind if I wrap it up instead?

[A-Manning]

would you mind if I wrap it up instead?

Feel free to push fixup commits and merge.
I can make the requested changes regarding Vec vs VecDeque this weekend.

I may of may not remove the trybuild dev dependency and use doctests instead, feel free to push this if you want to merge it sooner

[bobozaur]

@A-Manning I patched some things up and will merge this. Thank you very much for taking the time to work on this and for your contribution!

I'll soon follow up with some other changes. For all it's worth, I figured out how to enforce unique types in the path list by emulating what the static_assertions crate does here. It's actually so simple and yet so genius.