Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Ensure LSP compatibility on arg names in mypy #18356

Closed
wants to merge 2 commits into from

Conversation

hauntsaninja
Copy link
Collaborator

Pairs well with #18355

@hauntsaninja hauntsaninja changed the title Ensure LSP on arg names Ensure LSP compatibility on arg names in mypy Dec 29, 2024
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Collaborator

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

Thanks for starting the work on it! I think it would make sense to extract the changes to the visit_*** methods into a separate PR as these are fairly strait forward and unlikely to cause issues. That way we could focus on the relevant changes.

Edit: Opened #18361 and #18362 for it.

if info is None:
if info is None or any(has_placeholder(tv) for tv in tvar_defs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like the change from #18351

hauntsaninja pushed a commit that referenced this pull request Dec 29, 2024
Extracted from #18356. Make `visit_*` method arguments positional only
to ensure better LSP compatibility. Also update some visitors which
don't have violations yet but are base classes for other ones, like
`TypeTranslator` and `TypeQuery`.
hauntsaninja pushed a commit that referenced this pull request Dec 29, 2024
@hauntsaninja hauntsaninja deleted the strict-arg-names branch December 29, 2024 22:07
hauntsaninja added a commit that referenced this pull request Dec 30, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants