False Positive of unnecessary_fallible_conversions for pyo3

[Xuanwo]

Summary

rust-clippy with 1.75 could emit false positive of unnecessary_fallible_conversions for pyo3:

warning: use of a fallible conversion when an infallible one could be used
   --> bindings/python/src/file.rs:180:27
    |
180 |     pub fn __enter__(slf: Py<Self>) -> Py<Self> {
    |                           ^^ help: use: `From::from`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions
    = note: `#[warn(clippy::unnecessary_fallible_conversions)]` on by default

warning: `opendal-python` (lib test) generated 1 warning
warning: `opendal-python` (lib) generated 1 warning (1 duplicate)

It's bit confused to raising unnecessary_fallible_conversions on API like this:

pub fn __enter__(slf: Py<Self>) -> Py<Self> {
    slf
}

Py is a transparent struct expose by pyo3: https://docs.rs/pyo3/latest/pyo3/struct.Py.html

Lint Name

unnecessary_fallible_conversions

Reproducer

I tried this code:

pub fn __enter__(slf: Py<Self>) -> Py<Self> {
    slf
}

I saw this happen:

warning: use of a fallible conversion when an infallible one could be used
   --> bindings/python/src/file.rs:180:27
    |
180 |     pub fn __enter__(slf: Py<Self>) -> Py<Self> {
    |                           ^^ help: use: `From::from`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions
    = note: `#[warn(clippy::unnecessary_fallible_conversions)]` on by default

warning: `opendal-python` (lib test) generated 1 warning
warning: `opendal-python` (lib) generated 1 warning (1 duplicate)

I expected to see this happen: no warning

Version

rustc 1.75.0 (82e1608df 2023-12-21)
binary: rustc
commit-hash: 82e1608dfa6e0b5569232559e3d385fea5a93112
commit-date: 2023-12-21
host: x86_64-unknown-linux-gnu
release: 1.75.0
LLVM version: 17.0.6

Additional Labels

No response

[djc]

I'm also seeing this. I guess this happens within the #[pymethods] expansion, and this lint isn't correcting for macro-generated code?

Also, notably, you can't just stick the allow on the function definition or even on the impl block, it apparently has to go the module level to be applied correctly.

[y21]

That's odd, all method lints already have a check if they're from a macro:

rust-clippy/clippy_lints/src/methods/mod.rs

Lines 4001 to 4005 in 174a0d7

	impl<'tcx> LateLintPass<'tcx> for Methods {
	fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
	if expr.span.from_expansion() {
	return;
	}

I guess #[pymethods] uses the span of the callsite, instead of definition site, which makes clippy think that this is something that the user wrote 🤔

[djc]

@davidhewitt is there something on the PyO3 side that could be improved here?

[davidhewitt]

We've already "fixed" this (with an allow in the generated output) on PyO3 main. I'd intended to release 0.21 with this before 1.75 landed but we got caught up in some larger work which has delayed the release and I forgot about the time-sensitive nature of this. Let me look into a patch release for 0.20 asap, as 0.21 is realistically still a couple weeks off at best.

[davidhewitt]

PyO3/pyo3#3564

[djc]

@davidhewitt awesome as always, thanks!

[davidhewitt]

Patch release is out, @Xuanwo please let me know if there's still an issue after upgrading.

[Xuanwo]

Patch release is out, @Xuanwo please let me know if there's still an issue after upgrading.

Great! The patch resolved my issue.