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

Show closest candidates for misspellings : keyword argument case #7888

Merged

Conversation

@Wuisch
Copy link
Contributor

Wuisch commented Nov 5, 2019

This fixes the following case outlined in #824

def f(other: A) -> None: pass
f(otter: A())

Unexpected keyword argument "otter" for "f"
is now
Unexpected keyword argument "otter" for "f"; did you mean "other"?

Copy link
Collaborator

ilevkivskyi left a comment

Thanks, looks very good. I have just couple minor comments.

mypy/messages.py Outdated Show resolved Hide resolved
[case testKeywordMisspellingFloatInt]
def f(atter: float, btter: int) -> None: pass # N: "f" defined here
x: int = 5
f(otter=x) # E: Unexpected keyword argument "otter" for "f"; did you mean "btter" or "atter"?

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Nov 6, 2019

Collaborator

Could you please add a test or two involving *args to be sure they don't interfere with the logic?

This comment has been minimized.

Copy link
@Wuisch

Wuisch Nov 6, 2019

Author Contributor

Good catch!
Added some tests.
And the *args did interfere with the logic. --> improved logic

Martijn Wuis Martijn Wuis
Add tests for *args
Fix bugs with *args
Remove redundant type check
Copy link
Collaborator

ilevkivskyi left a comment

Thanks for updates, this looks almost ready, just two more style comments.

mypy/messages.py Outdated Show resolved Hide resolved
mypy/messages.py Outdated Show resolved Hide resolved
Martijn Wuis Martijn Wuis
style improvements
Copy link
Collaborator

ilevkivskyi left a comment

Thanks for updates, LGTM!

@ilevkivskyi ilevkivskyi merged commit 8252a0b into python:master Nov 7, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.