Avoid clippy::useless_conversion lint in macros
Using #[allow(clippy::useless_conversion)] still causes failures if the PyO3 user uses #[forbid(clippy::useless_conversion)]. Instead we can ensure the into call is not within a quote_spanned! block, allowing clippy to know this is macro code.
https://github.com/rust-lang/rust-clippy/issues/14272#issuecomment-2686661392
I tried adding a UI test for this, but I couldn't get it to fail (before implementing the fix). I can confirm that this patch resolves the error in my project.
I believe the reason for the original implementation is that the Into::into call can output diagnostics if the type does implement it. If it does not have a Span attached these diagnostics will be emitted on the macro call site, which is not so nice for UX and can be pretty confusing at times. All of the additions to the UI tests are not great IMO. (I believe this will also cause rust-analyzer to mark the entire body, which is also not nice, but I might be wrong there)
I'm inclined to say the the trade off is not worth it, but others might have a different opinion.
Potential alternatives I can think of:
- for the
BoundRefchanges, we could maybe add a generic method on it, which does the conversion instead of callingintodirectly - for the
PyErrchange, we could add a function insideimpl_to do that, but that does sound a bit stupid, maybe there is something smarter.
But I'm still not sure if it would be worth the extra complexity.
Another thought: We can't just change the span of the attribute, can we?
Yeah, I'm also not sure the tradeoffs are worth it here - probably no-one is setting #[forbid(clippy::useless_conversion). Thanks for explaining the prior thinking though, that's interesting context. This PR was pretty much showing where I'd got to and then seeing if anyone had any better ideas. I'll have a think about those potential alternatives.
Will set this to draft in the meanwhile.