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

Rule UP040 incorrect fix when TypeVar is used multiple times #9904

Closed
ostr00000 opened this issue Feb 9, 2024 · 2 comments · Fixed by #9905
Closed

Rule UP040 incorrect fix when TypeVar is used multiple times #9904

ostr00000 opened this issue Feb 9, 2024 · 2 comments · Fixed by #9905
Assignees
Labels
bug Something isn't working

Comments

@ostr00000
Copy link

Initial code

This is code in stub file decorators.pyi:

import logging
from collections.abc import Callable
from typing import TypeAlias, TypeVar

_BaseFun = TypeVar('_BaseFun', bound=Callable)
_Decorator: TypeAlias = Callable[[_BaseFun], _BaseFun]

def logCurrentTask(
    *, logger: logging.Logger | None = None, msg: str = ''
) -> _Decorator: ...

Ruff command

After running ruff with command:

ruff check ./decorators.pyi --isolated --select UP040 --target-version py312 --fix

TypeVar is replaced to generic type alias.

Code after fix

The code after change is:

import logging
from collections.abc import Callable
from typing import TypeAlias, TypeVar

_BaseFun = TypeVar('_BaseFun', bound=Callable)
type _Decorator[_BaseFun: Callable, _BaseFun: Callable] = Callable[[_BaseFun], _BaseFun]

def logCurrentTask(
    *, logger: logging.Logger | None = None, msg: str = ''
) -> _Decorator: ...

What is wrong

Type alias _Decorator has two declarations with same name _BaseFun which is incorrect syntax.
The PEP 695 states:

Type parameter names within a generic class, function, or type alias must be unique within that same class, function, or type alias

Possible fix

Need to check for duplicates.

Ruff version

ruff --version

ruff 0.2.1

Additional thinking

Not required to this issue, but it may be worth to notify user about remaining, old TypeVar or even remove it when rule UP040 is applying.
Although the variable is not used anymore, it remains in code, possibly unnoticed (I assume that currently only stubs are processed for this rule, but in future it may change).
The old object remains in global scope, which is not detected by most linters/type checkers.

@charliermarsh charliermarsh added the bug Something isn't working label Feb 9, 2024
@charliermarsh
Copy link
Member

Interesting, thank you for the clear issue! We can fix this.

@charliermarsh
Copy link
Member

Put up a PR to fix here: #9905

charliermarsh added a commit that referenced this issue Feb 9, 2024
## Summary

If a generic appears multiple times on the right-hand side, we should
only include it once on the left-hand side when rewriting.

Closes #9904.
nkxxll pushed a commit to nkxxll/ruff that referenced this issue Mar 10, 2024
## Summary

If a generic appears multiple times on the right-hand side, we should
only include it once on the left-hand side when rewriting.

Closes astral-sh#9904.
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 a pull request may close this issue.

2 participants