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

Enable Ruff PLC (Pylint Convention) #13306

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Dec 26, 2024

Ref #13295
https://docs.astral.sh/ruff/rules/#convention-c

The only stub changes are flagged by type-name-incorrect-variance (PLC0105), which enforces the _co/_contra suffix. Which we didn't consistently do. Even sometimes incorrectly suffixed.

import-private-name (PLC2701) is still in preview, so we're not affected yet, but typeshed uses many private imports in its stubs to reference types accurately. I don't think this rule applies to our stubs.
It's unclear whether Ruff should consider disabling it for stubs entirely, but it's mentioned here: astral-sh/ruff#14535

@@ -14,7 +14,7 @@
from packaging.requirements import Requirement

try:
from termcolor import colored as colored # pyright: ignore[reportAssignmentType]
from termcolor import colored as colored # pyright: ignore[reportAssignmentType] # noqa: PLC0414
Copy link
Member

Choose a reason for hiding this comment

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

We should probably just disable this rule, I don't think it's that helpful in typed code where "import X as X" is meaningful.

Copy link
Collaborator Author

@Avasam Avasam Dec 26, 2024

Choose a reason for hiding this comment

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

Some context: This rule is disabled in *.pyi files (astral-sh/ruff#3761) and there's talks of disabling it out of the box for __init__.py (or it can be easily configured to do so astral-sh/ruff#6294 (comment)).
So it only flags possible accidental re-exports from non-index/init runtime modules.

That being said, we're not a (public) library either (technically ts_utils this is a local lib), and I believe typeshed maintainers should be able to quickly spot an involuntary reexports in non-stubs files.

This comment has been minimized.

This comment has been minimized.

stdlib/typing.pyi Outdated Show resolved Hide resolved

This comment has been minimized.

Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

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