Skip to content

fix: only raise TC101 for str literals#122

Merged
sondrelg merged 1 commit intosnok:mainfrom
shiftinv:fix/subscript-literal
Aug 4, 2022
Merged

fix: only raise TC101 for str literals#122
sondrelg merged 1 commit intosnok:mainfrom
shiftinv:fix/subscript-literal

Conversation

@shiftinv
Copy link
Copy Markdown
Contributor

@shiftinv shiftinv commented Aug 3, 2022

This PR changes the code related to the TC101 check to only show errors if there's a str literal/constant, not any other type.

There are valid use-cases for literals that don't represent types in annotation subscripts, especially with the introduction of typing.Annotated, like x: Annotated[int, 42] or generally custom types with __class_getitem__.

Adding on to this, I suppose Annotated[int, "stuff"] should technically also be allowed, but it's not possible to generally differentiate between contexts where a string literal is meant to represent a type (e.g. Dict[str, "int"]), and where it doesn't (e.g. Annotated[str, "int"]), without hardcoding specific generic types.
As such, this is by far not a perfect solution, but it should get a bit closer :)

For context, this first appeared here:
https://github.com/DisnakeDev/disnake/blob/dd1fb9e4cbfd5974bf4e2ddf001aecc7b6443478/examples/slash_commands/param.py#L76-L78

./examples/slash_commands/param.py:76:29: TC101 Annotation '1' does not need to be a string literal
./examples/slash_commands/param.py:76:32: TC101 Annotation '10' does not need to be a string literal
./examples/slash_commands/param.py:77:35: TC101 Annotation '0' does not need to be a string literal
./examples/slash_commands/param.py:78:30: TC101 Annotation '0' does not need to be a string literal
./examples/slash_commands/param.py:78:33: TC101 Annotation '1.0' does not need to be a string literal

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 3, 2022

Codecov Report

Merging #122 (12148a9) into main (7be3641) will not change coverage.
The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main    #122   +/-   ##
=====================================
  Coverage   98.7%   98.7%           
=====================================
  Files          3       3           
  Lines        466     466           
=====================================
  Hits         460     460           
  Misses         6       6           
Impacted Files Coverage Δ
flake8_type_checking/checker.py 98.5% <100.0%> (ø)

Copy link
Copy Markdown
Member

@sondrelg sondrelg left a comment

Choose a reason for hiding this comment

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

I think your example for where it doesn't might be wrong, but PR looks good! I can't think of a reason why we wouldn't limit this to string literals.

And wrt. hardcoding generic types. It seems like it could make sense to add a set of hardcoded common types at some point, for this purpose.

@sondrelg sondrelg merged commit afc4fb9 into snok:main Aug 4, 2022
@shiftinv shiftinv deleted the fix/subscript-literal branch August 4, 2022 15:14
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