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

Stubgen: guess return types based on returned values #18116

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yangdanny97
Copy link
Contributor

This PR extends stubgen to guess return types for some functions based on the returned expression. This is inspired by autotyping's scalar-return flag, with the addition of being able to handle literal lists, tuples, sets, and dicts.

There are some limitations:

This only works on concrete functions at least one return statement & no yield/yieldfroms. It only works on literals, tuples of literals, or homogeneous container literals. Additionally, it only works if all the return statements return the same type or None.

To verify this behavior, I added a test case with some examples and updated all the existing stubtest cases.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this seems like a useful feature! Is there a reason we can't use the general purpose ExpressionChecker? (If we can use it, let me know if you have questions or issues getting it to work!)

@yangdanny97
Copy link
Contributor Author

As far as I can tell we don't use the typechecker for inference anywhere else in stub generation. Those results would be dependent on context and I imagine there's a lot of edge cases to handle which would expand the scope of the change significantly.

This PR is much simpler, it's not context-sensitive and is entirely syntax-based. I think I'd prefer investigating ExpressionChecker as a separate/future issue, since it seems like a much bigger task.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 16, 2024

I'd prefer if we used ExpressionChecker. Having a second visitor that does some subset of expression inference is just confusing and is a maintenance burden (since it will accrete functionality that already exists and is well trodden). I'm not sure what you mean by context sensitive here / I'm not sure that that's a bad thing. If you're concerned about inference. results, I'd be happy with a PR that used ExpressionChecker but made use of inference optional via flag / filtered out complex types (e.g. just filtered to non-builtins.object Instance or two element UnionType with None)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants