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

string-or-bytes-too-long (PYI053) flags and removes long strings in Literal, producing invalid type #12995

Open
Avasam opened this issue Aug 19, 2024 · 9 comments · Fixed by #13002 · May be fixed by #13020
Open

string-or-bytes-too-long (PYI053) flags and removes long strings in Literal, producing invalid type #12995

Avasam opened this issue Aug 19, 2024 · 9 comments · Fixed by #13002 · May be fixed by #13020
Labels
bug Something isn't working

Comments

@Avasam
Copy link

Avasam commented Aug 19, 2024

In the example reproduction below,

  • List of keywords you searched for before creating this issue. Write them down here so that others can find this issue more easily and help provide feedback.
    string-or-bytes-too-long PYI053

  • A minimal code snippet that reproduces the bug.

def foo(bar: typing.Literal["a", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"]):...

becomes

import typing

def foo(bar: typing.Literal["a", ...]):...

Not only is this an invalid type form (https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportInvalidTypeForm) but even if autofixed to Literal["a"] that would fundamentally change the parameter's type.

I assume this is an oversight as it's meant to replace long default values and is not flagged by flake8-pyi .

  • The command you invoked (e.g., ruff /path/to/file.py --fix), ideally including the --isolated flag.
    ruff check --isolated --select=PYI053

  • The current Ruff settings (any relevant sections from your pyproject.toml).
    not relevant

  • The current Ruff version (ruff --version).
    ruff 0.6.1

@Avasam Avasam changed the title string-or-bytes-too-long (PYI053) flags and removes long strings in Literal, changing parameter type and producing invalid type string-or-bytes-too-long (PYI053) flags and removes long strings in Literal, producing invalid type Aug 19, 2024
@AlexWaygood AlexWaygood added the bug Something isn't working label Aug 19, 2024
@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 20, 2024

is not flagged by flake8-pyi

I can actually repro this bug with flake8-pyi as well on this snippet in a .pyi file:

from typing import Literal

x: Literal["fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooO"]

We disable the flake8-pyi rule at typeshed for our generated stubs; maybe that's what was hiding the bug? https://github.com/python/typeshed/blob/a7ff1be4394cef089dd62090a2e2ad10cc77cfe6/.flake8#L16

@AlexWaygood
Copy link
Member

I merged #13002, but then immediately afterwards realised that it doesn't deal properly with type aliases. We still get two false positives on this snippet, even after #13002:

from typing import TypeAlias, Literal, Annotated

x: TypeAlias = Literal["fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooO"]
y: TypeAlias = Annotated[int, "metadataaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]

@dylwil3, don't suppose you'd be interested in a followup PR, would you? :-) The alternative approach I suggested in #13002 (comment) might work better here after all:

  • we'd want to add tracking in the semantic model for when we're entering Annotated slices, the same as we do when we're entering Literal slices
  • we'd want to skip PYI053 when we're inside any Annotated or Literal slice, not just when we're inside annotations.

@dylwil3
Copy link
Collaborator

dylwil3 commented Aug 20, 2024

Sounds good I'll give it a try!

@dylwil3
Copy link
Collaborator

dylwil3 commented Aug 20, 2024

@AlexWaygood Can you not do

def f(x: int) --> "AnnotationsForClassesWithVeryLongNamesInQuotesAsReturnTypes"

?

@AlexWaygood
Copy link
Member

@AlexWaygood Can you not do

def f(x: int) --> "AnnotationsForClassesWithVeryLongNamesInQuotesAsReturnTypes"

?

Well, there's no reason to ever use quoted annotations in stubs, since forward references can be used even with __future__ annotations enabled. We have another rule (PYI020) specifically for flagging any quoted annotation in a stub... but I suppose if you are going to use quoted annotations in stubs for some reason, and have PYI020 switched off for some reason, then maybe it makes sense for this rule to ignore veryyyyyyyyyyyyyyyyyyyy long quoted annotations.

In case it's not obvious, I changed my mind about this while writing this response out 😆 So it's definitely worth adding a comment about this to the code for the rule!

@dylwil3
Copy link
Collaborator

dylwil3 commented Aug 20, 2024

Hmm... for some reason in_typing_literal has no effect - it doesn't trip for either the argument annotation or typevar. Maybe I've done something wrong. I'll try to investigate later today.

@AlexWaygood
Copy link
Member

Hmm... for some reason in_typing_literal has no effect - it doesn't trip for either the argument annotation or typevar. Maybe I've done something wrong. I'll try to investigate later today.

That is odd. I can't immediately see why that would be... don't worry too much about fixing it if you can't figure it out; feel free to ping me if you're stuck!

@Avasam
Copy link
Author

Avasam commented Aug 20, 2024

We disable the flake8-pyi rule at typeshed for our generated stubs; maybe that's what was hiding the bug?

Yep, you're right ! That's exactly what happened.

@dylwil3
Copy link
Collaborator

dylwil3 commented Aug 21, 2024

Sorry for the delay - I was derailed by my baby learning to crawl all of a sudden 🚀 🚀 🚀

Hmm... for some reason in_typing_literal has no effect - it doesn't trip for either the argument annotation or typevar. Maybe I've done something wrong. I'll try to investigate later today.

That is odd. I can't immediately see why that would be... don't worry too much about fixing it if you can't figure it out; feel free to ping me if you're stuck!

Mystery solved - I had an invalid .pyi file since I did not actually import Literal or Annotated 🤦

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