gh-84644: Fix singledispatch annotation parsing

[johnslavik]

A very practical but more general approach than GH-140255 to fixing annotation parsing in functools.singledispatch and functools.singledispatchmethod.

Fixes issues GH-84644, GH-130827, and GH-143886.

It can be broken if one uses a user-defined alternative implementation of staticmethod or something analogous.
Will break incorrect but working registrees. I haven't investigated stripping Annotated typeforms yet.

Consulting a test which fails with this fix at https://github.com/python/cpython/pull/130309/changes#r2663516538 -- I think that the test is wrong.

[johnslavik]

I'm thinking that this could be labeled a new feature to minimize risks, similar to #144615.

cc @picnixz @encukou, what are your thoughts?

[picnixz]

I also think it'd be better as a new feature so that it pairs well with what we already shipped. Considering it relies on annotationlib, it wouldn't be possible to backport it to 3.13 either so I'd prefer keeping it in 3.15 only.

[johnslavik]

Thanks for the careful review, @picnixz :) I've also added a "What's New" entry. Do you think this looks good now?

[johnslavik]

@picnixz @rhettinger

[johnslavik]

I'm still reflecting on whether we could somehow not touch this and instead leverage the fact that the user can easily workaround these bugs by using argument-based registration (@d.register(the_type) instead of relying on type annotation). cc @ambv

[encukou]

It can be broken if one uses a user-defined alternative implementation of staticmethod or something analogous.

Yep. And it's things like this make it impossible for Cython to compile functions transparently. (Checking Py_TPFLAGS_METHOD_DESCRIPTOR get you closer, but that's CPython-specific C API.)

To me it looks like we're in a hole, and we need to stop digging. Or as PEP 20 puts it, refuse the temptation to guess.
I think that it would be better to document the quirk in the current behaviour, positioning the implicit form of register as only a shortcut for the common case.

[johnslavik]

I have a similar hunch. This is a lot of fragile code to maintain in a code path that only supplies some simple convenience.

Curiously enough, these issues aren't inherent to the original solution.

Back when the annotation-based registration using next(get_type_hints(...)) was being added, singledispatchmethod didn't exist and positional-only arguments didn't exist. It was a correct, bug-free heuristic that rotted with time.

That said, while I do believe this PR is the correct solution for any practical use, I think I'm in favor of documenting the constraint instead (given how easy and at hand it is to work around it).

Thanks @encukou.
@serhiy-storchaka?

[JelleZijlstra]

next(get_type_hints()) was never correct as it could always have picked up the wrong annotation if the parameter we should be dispatching on was unannotated. That's the underlying issue for all the bugs listed in the PR description, and I do think it should be fixed somehow.

Perhaps we can start with emitting a DeprecationWarning when singledispatch would pick up an annotation for the wrong parameter?

[johnslavik]

Thanks @JelleZijlstra, sounds better to me!

[johnslavik]

I assumed correct use of type hint based registration as in the contract. But for all bugs we're fixing here, yes, next(get_type_hints(...)) wasn't the ideal solution. Within the contract (which can be missed!) it worked. I was specifically thinking about positional-only argument confusion bug which (at least to me) seems the most severe because it appears even if I use registration correctly.

[encukou]

First step should be documenting the status quo, and backporting that.

emitting a DeprecationWarning when singledispatch would pick up an annotation for the wrong parameter?

Yeah, that can be limited to the cases where we know which argument is correct.

[picnixz]

FWIW, here's my take:

Since it breaks alternative implementations of staticmethod, it may not really be good. If this makes Cython worse, I prefer to avoid it.

• For functools.singledispatch doesn't verify annotation is on FIRST parameter #84644 (missing annotation), I would suggest that we simply don't try to do something smarter, but just document it. Linters may be better to flag this omission. We can document it though.
• For singledispatchmethod.register fails with typing.Self annotation #130827, this is unfortunate, but I would recommend that users switch to a non-based auto-registration in this case. We can also document it.
• For functools.singledispatch registration prioritizes the types of normal arguments before positional-only ones #143886, it is a real issue that I would like to fix.

So out of the 3 issues, only fixing the priority seems legitimate. Because that's legitimately an issue IMO that doesn't even appear inside classes, so with plain @singledispatch which is more likely to happen than with @singledispatchmethod (I honestly almost never needed that form).

TL;DR: I also recommend the following:

positioning the implicit form of register as only a shortcut for the common case.