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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

UP035 --fix not working for quoted types #3508

Closed
torarvid opened this issue Mar 14, 2023 · 7 comments 路 Fixed by #3657
Closed

UP035 --fix not working for quoted types #3508

torarvid opened this issue Mar 14, 2023 · 7 comments 路 Fixed by #3657
Assignees
Labels
fixes Related to suggested fixes for violations

Comments

@torarvid
Copy link

Hello hello 馃憢馃徎

When running ruff --fix test.py (Ruff v0.0.255) for the following test file:

from typing import List

class A:
    myList: List[str]
    quotedList: "List[str]"

it will fix the first (non-quoted) variant, but still only report the second (quoted) variant. Unsure if this is a bug or expected behavior, but I thought I would report it anyways 馃檪

Keep up the awesome work 馃帀

@torarvid
Copy link
Author

This test file is even simpler (no class A: needed):

from typing import List

myList: List[str] = []
quotedList: "List[str]" = []

@JonathanPlasse
Copy link
Contributor

This is the expected behavior. pyupgrade does not fix it either.
No forward references are auto-fixable as there are tricky corner cases.

@charliermarsh charliermarsh added the fixes Related to suggested fixes for violations label Mar 14, 2023
@charliermarsh
Copy link
Member

@JonathanPlasse - I can't remember why this is hard 馃槀 Do you have an example of a tricky corner case?

@JonathanPlasse
Copy link
Contributor

After spending some time in the issues, here it is!
I think that is all the problems we encountered.

  • Removes quotes from annotations removes quotes from literal values聽#2613.

    Breaking pypa/build too: https://github.com/pypa/build/pull/576/files

    - PathType = Union[str, 'os.PathLike[str]']
    + PathType = Union[str, os.PathLike[str]]
    
    - _TProjectBuilder = TypeVar('_TProjectBuilder', bound='ProjectBuilder')
    + _TProjectBuilder = TypeVar('_TProjectBuilder', bound=ProjectBuilder)

    These are both broken. os.PathLike[str] is not valid on Python 3.7 (and it's not an annotation). TypeVar can't have the quotes removed, the thing it's bound to doesn't exist yet. And it's also not an annotation.

  • U007 Autofix on string type format聽#826
    if TYPE_CHECKING:
        from app.models.model_a import ModelA
        from app.models.model_b import ModelB
    
    ModelAOrB = Union["ModelA", "ModelB"]

    and it gets fixed into

    if TYPE_CHECKING:
        from app.models.model_a import ModelA
        from app.models.model_b import ModelB
    
    ModelAOrB = "ModelA" | "ModelB"

    which is not the same type as before

@charliermarsh
Copy link
Member

Thank you! :)

I'm wondering if we could fix (but leave quoted) instances like quotedList: "List[str]" = [], i.e., change that to quotedList: "list[str]" = [] 馃

@JonathanPlasse
Copy link
Contributor

Yes, this makes sense.

@charliermarsh
Copy link
Member

Some of these are tricky (e.g., quotedList: "Li" "st[str]" = [] is a valid annotation), but we should be able to fix the common cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants