Fixed rule decorator factory typing to help call-by-name call sites

[sureshjoshi]

This one was emotional...

The current problem (that was especially painful during call-by-name refactors) was that @rule in the decorator factory form (@rule(foo=..., bar=...)) was hiding types from downstream usage, and those call-by-name usages would be a mess of Anys.

This is bad, just because, but it was also particularly bad in the re-factor - because there are some manual implicitly optimizations that I can make, if I can clearly see the type and how it's being used. But, it would require manually typing every call site to do it otherwise.

I limited my re-factoring to the external @rule types and one of the kwargs, but most of the rules.py file is a pyright nightmare due to missing generics and unknown return types.

I also intentionally didn't update mypy to 1.15, since there are a lot of errors (unrelated to this PR) which show up, and that would make everything messier.

Marking this as a "plugin api change" - because the rule decorator typing changes might affect plugin authors (but as far as I can tell, it should be okay unless the plugins were crashing already).

Edit: Confirmed that pyright (default args) is showing the same or fewer errors with this update, so we haven't regressed. Also confirming that the inferred typing has improved:

[thejcannon]

Technically you can spec this and it won't affect caller typing, but it will itself be internally type-checked.

[sureshjoshi]

I thought about it, but my plan was to do that as part of the rules internals re-factor. I didn't want to obscure this PR any more than it needed to be (because the internal re-factor will touch like 150 lines).

[cognifloyd]

Can we reuse AsyncRuleT and SyncRuleT like this to make the typing even clearer? Or will the typecheckers complain about the extra level of indirection?

(sorry for the double review. I used the wrong account the first time)

[cognifloyd]

Suggested change

	def rule(_func: Callable[P, Coroutine[Any, Any, R]]) -> Callable[P, Coroutine[Any, Any, R]]:
	def rule(_func: AsyncRuleT) -> AsyncRuleT:

[sureshjoshi]

So, this won't work as-is - since those need to be given generics. Ends up being:

def rule(_func: AsyncRuleT[P, R]) -> AsyncRuleT[P, R]:

And earlier on, there is another type alias of:

RuleDecorator = Callable[[SyncRuleT | AsyncRuleT], AsyncRuleT]

# needs to be

RuleDecorator = Callable[[SyncRuleT[P, R] | AsyncRuleT[P, R]], AsyncRuleT[P, R]]

But at usage, it should be
RuleDecorator[P, R]

So, I can do all that - but I feel like we're losing the plot with the Russian dolls of generic aliases - and I'd also bet that's how some of these type errors propagated in the first place. None of the uses of SyncRuleT and AsyncRuleT are correctly typed - which is why pyright loses its mind in that file.

Take the goal_rule overloads further down, which are fully qualified with Callables/Coroutines - they don't have any type errors, except the one usage of the type alias.

Finally, at call-usage, when I want to get information about what the typings are - my options are:

# Typealiases - which hide what happens:
def rule(_func: AsyncRuleT[P@rule, R@rule]) -> AsyncRuleT[P@rule, R@rule]: ... 

# More verbose, but more precise
def rule(_func: (**P@rule) -> Coroutine[Any, Any, R@rule]) -> ((**P@rule) -> Coroutine[Any, Any, R@rule]): ...

Nine of ten times, I'd prefer more concise - but the decorator in this case, is genuinely more complex - so it's nice to actually see what's going on, versus 2-3 indirections to get to the meaning.

[cognifloyd]

Makes sense. Please disregard my suggestion then.

Btw: I've never seen @ in type hints before. Is that a thing? Or just for example?

[sureshjoshi]

That's not in the type hints, that's in the generated code from pyright - but essentially, if you use ParamSpec, you get access to stuff like P.args, and P.kwargs to use in the decorator wrapper typing, and then @location from where it came.

I read the PEP, but don't recall the terminology they use to describe that.

[cognifloyd]

Suggested change

	def rule(_func: Callable[P, R]) -> Callable[P, Coroutine[Any, Any, R]]:
	def rule(_func: SyncRuleT) -> AsyncRuleT:

[sureshjoshi]

Can we reuse AsyncRuleT and SyncRuleT like this to make the typing even clearer? Or will the typecheckers complain about the extra level of indirection?

(sorry for the double review. I used the wrong account the first time)

I put my thoughts in one of the comments. In this exact, specific usage, I think the type aliases are causing more harm than value provided.

Trying to hide away what we're doing ended up causing lots of downstream errors and type hiding - so, I think going the opposite route makes more sense in this file (especially since it's such a core file to the project).