Skip to content

Don't use an arg type corresponding to ** arg kind as inference context for the argument #1361

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

Merged
merged 6 commits into from
Apr 11, 2016

Conversation

gvanrossum
Copy link
Member

Fix #1360.

@gvanrossum
Copy link
Member Author

I also want to fix the confusing error messages mentioned in #1360 (comment)

Guido van Rossum added 2 commits April 11, 2016 09:36
…hether * or ** was involved.

This fixes the issue reported in #1360 (comment) .

Specifically, suppose you have these three calls (regardless of how f() is defined!)
```
f(a)
f(*a)
f(**a)
```
If the type checker determines that there's something wrong with the
argument type it always gives the same error message. That error
message is appropriate for the first example, `f(a)`, but for the
second and third it should at least indicate that the mismatch is due
to the star(s) in the call site.

I am addressing this by adding one or two stars (as appropriate) in
front of the actual type in the error message, so that e.g. instead of
```
Argument 1 to "f" has incompatible type Dict[str, int]; expected "str"
```
you would see
```
Argument 1 to "f" has incompatible type **Dict[str, int]; expected "str"
```
Hopefully this will inform the user that (1) the type of a in `f(**a)`
is `Dict[str, int]` and (2) the actual type that doesn't match the
expected type "str" is the value slot, in this case `int`.

Similar for `*args`.
@ddfisher
Copy link
Collaborator

We should really consider refactoring arg_name, arg_type, and arg_kind from parallel lists into an object at some point.

@ddfisher ddfisher merged commit ac193cf into master Apr 11, 2016
@ddfisher ddfisher deleted the star2infer branch April 11, 2016 20:45
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