I agree with @edvilme: self is only a naming convention. What matters is that the first parameter receives the instance. Consider something like:

". Did you forget the instance parameter (e.g. self) in the function definition?"

This avoids misleading beginners into adding a parameter named self in the wrong position.

This avoids misleading beginners into adding a parameter named self in the wrong position.

I'm not so sure that "instance parameter" means anything more to a beginner than a "self" parameter. "Self" has the advantage of being a very, very common pattern in Python, and I think that's more easily searchable than "instance parameter". For reference, our docs use "self" hundreds of times, whereas "instance parameter" does not appear once.

I would lean more towards this:

Did you forget the 'self' parameter in the function definition?

"self parameter python" on Google will yield millions of results describing exactly what the user wants, whereas "instance parameter python" returns some unrelated results about class methods vs. instance methods and similar.

I must agree with
'"self parameter python"' on Google will yield millions of results'.

I've updated the error message to include the self parameter:

        self_hint = ". Did you forget the 'self' parameter "
                    "in the function definition?";

If PyUnicode_FromString fails, the original TypeError is never raised and the user gets no error message. Better to skip the hint and still emit the normal _PyErr_Format.

We should actually propagate the exception in this case, and not suggest anything at all. This is already the case when creating kwonly_sig

I've switched it to a char*. Does that suffice?

Good setup for the happy path. If possible, add a def method_with_self(self, x) on the same class A to cover the false positive case.

Can you take another look: https://github.com/python/cpython/pull/152326/changes#diff-f27c08f13ff54ee9a69325f9291a3a2510a05b82ff8d813a8ab2e0072af632bb

I've added coverage for common cases.

The heuristic of matching tp_name against the class segment in qualname is solid. A negative test for a @classmethod missing cls would be useful, it probably shouldnt trigger the hint, and that would document the expected behavior.

I've switched it to checking function name matching the self dictionary. Please take a fresh look at the PR and lmk if it suffices.

Hmm, this fires whenever argcount > co_argcount and the first argument's type matches the class in qualname. That's broader than the beginner mistake this PR targets.

Ex:

This should hint

def lol(a): ...
A().lol(1)   

but his should not:

def method(self, x): ...
A().method(1, 2, 3) 

I'd suggest combining argcount == co_argcount + 1 with a second check, like, the first declared parameter is not named "self", or verifying via _PyType_Lookup that this code object is the function bound on the instance's type.

ok, I've included your runtime arg count == defined arg count + 1 check. It also does both of your suggested checks on the first parameter. Please review it again

Added it in the HEAD

This feels like a nasty hack. Relying on the __qualname__ feels very icky. I need to look into this more, but it's probably possible to determine whether this is an instance method earlier in the eval loop. That would be much cleaner.

I asked AI on other options. Please review this approach

There's no real need for this test. We test method calling in many other test cases in CPython. It does not add any real value.

This was my response to #152326 (comment) . I think I interpreted the happy path incorrectly

Removed it.

Please, add a test case for def method_with_self_arg(x, self): ...

and then call it like inst.method_with_self().

Also: you can add def zero_args_self(self): ... and call it like: A.zero_args_self() and check that error message is also correct.

added bark_with_self_arg and test_unbound_method_with_self_keeps_missing_argument_error for this.

Your self_hint is non NULL (before assignment). While empty strings are immortal, you still need to decref it if this ever changes. For that use Py_SETREF.

I don't feel super strongly about this, but I disagree with refcounting immortal objects. I've expressed before that too many users rely on certain things being immortal (such as empty strings), so changing that would be very difficult anyway. We might as well enjoy the performance boost rather than trying to maintain forward compatibility with a seemingly impossible case.

I'm ok with not changing this path though I would prefer to have it initialized to NULL instead in this case and before constructing the message we would put it to something. But I won't be strongly opposed to keep things as is.

I've switched it to char*. Does that suffice?

We should actually propagate the exception in this case, and not suggest anything at all. This is already the case when creating kwonly_sig

Would this suggestion be also there if we are using classmethods? Or what about methodws on metaclasses? they can have cls as the first parameter name.

I've added exemption for those cases.