Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let function signature hook have context of caller #14845

Closed
ikonst opened this issue Mar 6, 2023 · 2 comments · Fixed by #15369
Closed

Let function signature hook have context of caller #14845

ikonst opened this issue Mar 6, 2023 · 2 comments · Fixed by #15369
Labels
feature topic-plugins The plugin API and ideas for new plugins

Comments

@ikonst
Copy link
Contributor

ikonst commented Mar 6, 2023

When executing (plugin) function signature hooks, actual argument types are not available yet (and, in fact, there's no caller context).

In #14526, the signature of attr.evolve(inst, ...) depended on the inst argument, and we had no choice but to do

inst_arg = ctx.args[0][0]

assert isinstance(ctx.api, TypeChecker)
inst_type = ctx.api.expr_checker.accept(inst_arg)

Actual argument types are passed to the function hook, but then it's too late since arguments were already checked, and the hook can only affect the callee's return type.

I'm not sure what's the best approach:

  1. Add equivalent of CheckerPluginInterface.accept for ad-hoc type resolution (i.e. current solution but w/o private API). The assumption is that this will not normally be used by hook authors except in specific cases for specific args (since doing it generally creates a chicken-and-egg problem, as infer_arg_types_in_context gets the callee after the hook's mutation).
  2. Pass actual arg types to function sig hook. Not sure it'll be doable, since as I mentioned infer_arg_types_in_context depends on callee (after hook's mutation).
  3. Change function hook to allow returning not only a ret_type but also arg_types/kinds/names. This means that in those cases checks will be done twice: it first has to pass old formal sig, then (if plugin returns one) it has to pass the new formal sig.

Please advise :)

P.S. Same applies to method signature hook.

@ikonst ikonst added the feature label Mar 6, 2023
@AlexWaygood AlexWaygood added the topic-plugins The plugin API and ideas for new plugins label Mar 6, 2023
@ikonst
Copy link
Contributor Author

ikonst commented Mar 7, 2023

Notably, for method signature hooks there's MethodSigContext.type that is a piece of caller context.

@ikonst
Copy link
Contributor Author

ikonst commented Mar 7, 2023

Hmpf, I forgot there was an issue: #10216

ilevkivskyi pushed a commit that referenced this issue Jun 11, 2023
Fixes #14845.

p.s. In the issue above, I was concerned that adding this method would
create an avenue for infinite recursions (if called carelessly), but in
fact I haven't managed to induce it, e.g. FunctionSigContext has `args`
but not the call expression itself.
ilevkivskyi pushed a commit that referenced this issue Jun 17, 2023
Validate `dataclassses.replace` actual arguments to match the fields:

- Unlike `__init__`, the arguments are always named.
- All arguments are optional except for `InitVar`s without a default
value.

The tricks:
- We're looking up type of the first positional argument ("obj") through
private API. See #10216, #14845.
- We're preparing the signature of "replace" (for that specific
dataclass) during the dataclass transformation and storing it in a
"private" class attribute `__mypy-replace` (obviously not part of
PEP-557 but contains a hyphen so should not conflict with any future
valid identifier). Stashing the signature into the symbol table allows
it to be passed across phases and cached across invocations. The stashed
signature lacks the first argument, which we prepend at function
signature hook time, since it depends on the type that `replace` is
called on.

Based on #14526 but actually simpler.
Partially addresses #5152.

# Remaining tasks

- [x] handle generic dataclasses
- [x] avoid data class transforms
- [x] fine-grained mode tests

---------

Co-authored-by: Alex Waygood <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-plugins The plugin API and ideas for new plugins
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants