Fix property docstrings - #6252

All the CI failures seem to be ubuntu network errors

Both failures are unrelated build issues

    ERROR: Could not find a version that satisfies the requirement numpy (from versions: none)
    ERROR: No matching distribution found for numpy

Here's a fairly quick AI review. Our CI is pretty overloaded at the moment, so I'll probably come back to this tonight or tomorrow.

🤖 AI text below 🤖

The review of PR #6252 found 7 issues. All of them are about test coverage, API design, missing docs or style. None is a definite crash or miscompile.

Design (include/pybind11/pybind11.h)

1. Lines 1625 and 1590–1684: the root cause is still there. The signature is generated when the cpp_function is built. That happens before def_property_static adds is_method(*this). The PR adds is_method(*this) by hand at three call sites, so any new helper that leaves it out gets the old arg0: Cls docstring again. A private class_ helper, for example make_property_fn(f, extra...), could always add is_method and handle nullptr. That would fix all call sites in one place.
2. Line 149: the new nullptr constructors are too narrow. The two new public cpp_function(std::nullptr_t, ...) overloads accept the extras only in one fixed order. cpp_function(nullptr, is_method(...), is_setter()) does not compile, but the reverse order does. One variadic template <typename... Extra> cpp_function(std::nullptr_t, const Extra &...) would work, and so would the helper from item 1.
3. Line 1684: a comment is now misleading. It says late self args come from is_method + pos_only/kw_only in def_property_static. That is now true only for cpp_function objects that users pass in. This code is sensitive to ownership ([BUG]: Crash due to invalid free during cleanup, if any enum is registerd, due to attempting to free a string literal #5976), so a maintainer who trusts the comment can make wrong changes.

User-visible change

4. No changelog note. For properties bound with lambdas or member-function pointers, fget/fset now show self where they showed arg0. The setter's value parameter changes from arg1 to arg0. type(prop.fget) changes from builtin_function_or_method to instancemethod. This can affect stub generators and doctests, so the PR description should have a "Suggested changelog entry" for it.

Tests (tests/test_methods_and_attributes.py)

5. Line 171: the case the PR is named for is not tested. The PR title says "lambda properties", but the test does not check the docstring of any lambda getter. The only lambda in the test is the def_writeonly setter.
6. Line 172: some assertions add no coverage. The def_readonly/def_readwrite checks already passed before the fix, because property_cpp_function already added is_method. Only the def_property*/def_writeonly checks fail without the fix.
7. Line 171: the test repeats itself. The same two long signature strings appear eight times. @pytest.mark.parametrize or two module-level constants would mean one edit if the int type hint changes.

I recommend that the author fix items 2, 4 and 5 before merge. Items 1 and 3 are worth suggesting, and 6 and 7 are style. I can draft a review comment for the PR, or make these changes on the branch.

After trying to understand point 1 I understand the issue a little better.
The function signature string is built when cpp_function is constructed.
This function tries to set is_method on the fget and fset cpp_functions which sets the bool flag but it does not rebuild the signature.
This also explains why docstrings are broken for properties constructed with cpp_function without is_method

Patching in is_method at cpp_function construction works for some cases but the true fix would be to rebuild the function signature but that would need more work

Those commits should fix issues 2, 5 and 7
I don't understand issue 3.
There was already a changelog note so 4 is invalid.

I have added a test case for a cpp_function property which should fail.
The question is do we want to accept a partial solution that covers function pointers and lambdas in which case I can uncomment the cpp_function test or will you only accept this if it fixes all issues.
The full solution will need more work to rebuild the signature string.

You don't need to fix every issue from an AI review, that's one of the reasons it's marked as an AI review. I noticed that 4 was technically wrong before pasting it, though the current one is a bit short and unclear; that's fine though, I often modify them after merging. "Property docstrings use self more often." might be more accurate.

I think this is fine for now; a solution for cpp_function would be a large change.

I'll push a fix for older compilers.

Your AI edit removed one of the failing tests