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

[pylint] Fix false positives, add missing methods, and support positional-only parameters (PLE0302) #16263

Merged
merged 4 commits into from
Feb 21, 2025

Conversation

dcarrier
Copy link
Contributor

@dcarrier dcarrier commented Feb 19, 2025

Summary

Resolves 3/4 requests in #16217:

  • ✅ Remove not special methods: __cmp__, __div__, __nonzero__, and __unicode__.
  • ✅ Add special methods: __next__, __buffer__, __class_getitem__, __mro_entries__, __release_buffer__, and __subclasshook__.
  • ✅ Support positional-only arguments.
  • ❌ Add support for module functions __dir__ and __getattr__. As mentioned in the issue the check is scoped for methods rather than module functions. I am hesitant to expand the scope of this check without a discussion.

Test Plan

  • Manually confirmed each example file from the issue functioned as expected.
  • Ran cargo nextest to ensure unexpected_special_method_signature test still passed.

Fixes #16217.

Copy link
Contributor

github-actions bot commented Feb 19, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre added the bug Something isn't working label Feb 20, 2025
@MichaReiser
Copy link
Member

@ntBre do you want to take this one?

@ntBre
Copy link
Contributor

ntBre commented Feb 20, 2025

Sure!

@ntBre ntBre self-assigned this Feb 20, 2025
Copy link
Contributor

@ntBre ntBre 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 working on this! The code changes look right to me, but I think it would be great to paste in all of the examples from the issue and add their snapshot results just to make sure.

I definitely agree that the module functions are good to leave for a separate rule, so this will be perfect with a few more tests.

@dcarrier
Copy link
Contributor Author

dcarrier commented Feb 20, 2025

Thank you for the review! I have added the test cases along with the snapshots as requested.

Side note, we are asserting __cmp__, __div__, __nonzero__, and __unicode__ are skipped by including failure cases which do not generate a snapshot.

@dcarrier dcarrier requested a review from ntBre February 20, 2025 19:31
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Nice, this looks great. Thanks again!

@ntBre ntBre changed the title [pylint] fix false positives, add missing methods and support posonlyargs (PLE0302) [pylint] Fix false positives, add missing methods, and support positional-only parameters (PLE0302) Feb 20, 2025
@ntBre ntBre merged commit b9b0948 into astral-sh:main Feb 21, 2025
21 checks passed
@dcarrier dcarrier deleted the issue-16217 branch February 25, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PLE0302 has false positives and false negatives
3 participants