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

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
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