Make joins of callables respect positional parameter names #4920
Conversation
This commit fixes python#2777 -- specifically, it enhances joining callables to erase the names of positional arguments as necessary. For example, consider the following program: def f(x: int) -> int: ... def g(y: int) -> int: ... lst = [f, g] Previously mypy would treat the final line as an error since 'f' and 'g' have different types due to the different parameter names. Now, mypy infers that `lst` has type `list[def (int) -> int]`, effectively erasing the parameter name from the inferred type. This commit does not attempt to handle keyword-only arguments.
mypy/join.py
Outdated
| ret_type=join_types(t.ret_type, s.ret_type), | ||
| fallback=fallback, | ||
| name=None) | ||
|
|
||
|
|
||
| def combine_arg_names(t: CallableType, s: CallableType) -> List[Optional[str]]: | ||
| # Note: assumes is_similar_type(t, s) is true |
There was a problem hiding this comment.
Add a docstring explaining what is going on and why it is necessary.
| s = {1, 2, *(3, 4)} | ||
| t = {1, 2, *s} | ||
| reveal_type(s) # E: Revealed type is 'builtins.set[builtins.int*]' | ||
| reveal_type(t) # E: Revealed type is 'builtins.set[builtins.int*]' | ||
| [builtins fixtures/set.pyi] | ||
|
|
||
| [case testListLiteralWithFunctionsErasesNames] |
There was a problem hiding this comment.
Maybe add a test for the join_similar_callables path by joining functions whose argument types differ but have a nontrivial meet. (For example, if the arguments are Union[bytes, float] and Union[str, int], the argument for the joined function should be int)
|
@msullivan -- sorry for the delay! I made the changes you suggested. |
There was a problem hiding this comment.
A couple small doc nits but I think this looks good.
I'm being kind of picky about the docs partially because this kind of code (meets and joins in particular) always takes me a while to remember enough context for, which is a shame because they are underdocumented in every language implementation I've ever worked on ;)
(I also can never remember which is meet and which is join without looking it up. It just never sticks. I don't know why.)
mypy/join.py
Outdated
| and 's' had the signature (a: int, b: str, z: str) -> None, the | ||
| join of their argument names would be ["a", "b", None]. | ||
|
|
||
| This information is then used to compute more accurate joins for |
There was a problem hiding this comment.
Meets and joins, right?
There was a problem hiding this comment.
I think it's just joins -- this pull request doesn't touch meet.py.
(iirc, the meet of two types 't' and 's' would be some type that's basically a valid subtype of both. I couldn't think of a way of creating a callable that would be a valid subtype of two callables with different arg names without doing something messy with keyword arguments, so didn't attempt to modify that file.)
mypy/join.py
Outdated
| @@ -391,7 +391,17 @@ def combine_similar_callables(t: CallableType, s: CallableType) -> CallableType: | |||
|
|
|||
|
|
|||
| def combine_arg_names(t: CallableType, s: CallableType) -> List[Optional[str]]: | |||
| # Note: assumes is_similar_type(t, s) is true | |||
| """Takes two callables and produces the meet of their argument names. | |||
There was a problem hiding this comment.
Extreme nit: this calls this operation a meet in one place and a join in another, and should use the same term twice. I think it is more of a meet than a join (since it is like an intersection, which are meets?). Just saying "combination" or something fine also.
There was a problem hiding this comment.
Oh whoops, that's embarrassing -- I didn't mean to use the word 'meet' here...
But yeah, I agree it's confusing, especially since this function isn't even manipulating types. I reworded the comment completely and just used the phrase "compatible with" which is hopefully better.
This commit fixes #2777 -- specifically, it enhances joining callables to erase the names of positional arguments as necessary.
For example, consider the following program:
Previously mypy would treat the final line as an error since 'f' and 'g' have different types due to the different parameter names.
Now, mypy infers that
lsthas typelist[def (int) -> int], effectively erasing the parameter name from the inferred type.This commit does not attempt change how handle keyword-only arguments are currently handled.