KEMBAR78
Remove "builtins." from output for overloads and builtins.None by JelleZijlstra · Pull Request #5073 · python/mypy · GitHub
Skip to content

Conversation

@JelleZijlstra
Copy link
Member

Part of #5072.

Overloads are probably the most common user-facing feature that still outputs "builtins" in error messages; this diff fixes them to use standard type formatting.

I also change builtins.None to None, since builtins.None is a syntax error. A comment claimed that we use builtins.None to distinguish the type from the value, but I don't think there are a lot of contexts where that confusion is likely.

@ilevkivskyi
Copy link
Member

Thanks for cleaning this up! I would propose to change the error format to something like this:

  • No overload variant of "f" of "A" matches argument type "B" if there is a single argument
  • No overload variant of "f" of "A" matches argument types "A", "B", "C" otherwise.

(Note that quotes are used by some editor plugins to better highlight types/methods in error messages.)

@JelleZijlstra
Copy link
Member Author

Yes, that would be better. What do you suggest for the case where the list is empty? (All overloads take at least one argument, but the user called f().) I suggest All overloads of "f" require at least one argument.

@ilevkivskyi
Copy link
Member

Yes this makes sense.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

LGTM! Unless there are some objections, I will merge this soon.

@ilevkivskyi
Copy link
Member

(also error message in tests needs to be updated)

@JelleZijlstra
Copy link
Member Author

Oops, just pushed a commit fixing the two tests I messed up.

@ilevkivskyi ilevkivskyi merged commit d6566be into python:master May 18, 2018
@JelleZijlstra JelleZijlstra deleted the nobuiltins branch May 18, 2018 06:09
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