[red-knot] Add __init__ arguments check when doing try_call on a class literal

[mishamsk]

Summary

Addresses #16511 for simple cases where only __init__ method is bound on class or doesn't exist at all.

No handling of __new__ or modified __call__ on metaclass.

Test Plan

• A couple new cases in mdtests
• cargo nextest run -p red_knot_python_semantic --no-fail-fast

[mishamsk]

This is still a draft, as I wanted to discuss a couple of things first.

First of all, @sharkdp did all the hard work and it seems almost too easy now. However, there are a couple of caveats.

• If I follow the current CallOutcome setup and how other dunder calls are done - incorrect call signature causes the resulting type to become Unknown. I think this is not every helpful as it would cause all kinds of missed downstream diagnostics. You can see this happening in this test - I haven't fixed it intentionally.
  • I would suggest that for __init__ call, the diagnostics are added to CallOutcome but an instance type is returned even if error occurs. This is not possible now, because official API that creates CallOutcome is bind_call which will emit argument mismatch second time (following whatever try_call_dunder returns). The only other way to create CallOutcome - from_return_type is marked as deprecated. @carljm any objections to creating a CallOutcome ::new method for this case?
• I have two concerns with the current diagnostics like [too-many-positional-arguments] "Too many positional arguments to bound method ``__init__``: expected 1, got 2"
  • Maybe it is worth special casing __init__ and avoiding the bound method portion?
  • Not strictly related to this PR, but for bound methods expected/actual number of arguments should not count the first one. So if there are no arguments to __init__ the diagnostic should be expected 0, got 1 instead of the current expected 1, got 2

[carljm]

• Maybe it is worth special casing __init__ and avoiding the bound method portion?

I'm not convinced. __init__ is a bound method. What's the harm in saying so?

• if there are no arguments to __init__ the diagnostic should be expected 0, got 1 instead of the current expected 1, got 2

Agreed, should be a pretty easy change I think?

[carljm]

I'm not sure I understand your first point. It's both possible and common to have a CallOutcome that both includes binding errors and has a return type -- in fact that's what we generally do for function calls, as this test will show:

def f() -> int:
    return 1

x = f(1)  # error: [too-many-positional-arguments]

reveal_type(x)  # revealed: int

What's unusual about __init__ is that it returns None, so our usual return-type handling won't work here. This gets into what I mentioned in Discord about modeling both __new__ and __init__. The proper modeling here is that we call __new__, which returns an instance of the type (in order to do this properly based on the typeshed definition of object.__new__, we would need to support typing.Self -- but it may not be hard to add a hacky version of support for Self), and then we call __init__ on that instance, and we don't care about the return type of __init__. (We don't have to model things totally correctly on the first PR, but we'll want to have TODO comments for the correct modeling, and we shouldn't put too much work into workarounds to model things wrongly, if we can avoid it.)

Those are just some general comments, haven't looked at the code in the PR yet, will do that tomorrow!

[sharkdp]

and then we call __init__ on that instance, and we don't care about the return type of __init__

Maybe this is what you meant by "We don't have to model things totally correctly", but I guess we should (eventually) emit a diagnostic if __init__ does not return None, since the runtime throws a TypeError in case it doesn't.

[mishamsk]

I'm not sure I understand your first point

First, I realized I confused everyone by saying CallOutcome doesn't have the right API's, but that was supposed to be CallBinding of course. Let me break it down step by step.

• I must call try_call_dunder to trigger descriptor protocol and see what __init__ call yields
• The call returns Result<CallOutcome<'db>, CallDunderError<'db>>. In case of errors I'll get CallDunderError
• I then map CallDunderError to CallError. So far so good
• Now, I need to return something like Ok(CallOutcome::Single(CallBinding { ..., errors: vec![error_from_dunder_call] })). However, there is only one public API that creates it bind_call which does the full type checking logic and inserts its own errors. There is no API to create a CallBinding by supplying types & errors directly

Hence, I suggest adding a method of a function in bind.rs for this case.

Any objections?

Agreed, should be a pretty easy change I think?

I haven't looked yet, I'll apply the change since everyone is fine. But this means that more tests will change in this PR than just those related to __init__. Or I can make it a separate PR too...

[carljm]

(Didn't get to this today, sorry; will come back to it tomorrow.)