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.

Sign up for GitHub

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

Jump to bottom

Lint when | annotations contain string segments #9139

Closed
tylerlaprade opened this issue Dec 14, 2023 · 5 comments · Fixed by #9143
Closed

Lint when | annotations contain string segments #9139

tylerlaprade opened this issue Dec 14, 2023 · 5 comments · Fixed by #9143
Assignees
@charliermarsh
Labels
rule Implementing or modifying a lint rule

Comments

@tylerlaprade
Copy link
Contributor

tylerlaprade commented Dec 14, 2023 •
edited
Loading

image

This is one of the inputs to my function, after quotes were automatically added (and I manually turned QuerySet['ContractVersion'] into 'QuerySet[ContractVersion]' due to #9135 and #9136:

contract_versions_list: list['ContractVersion']
    | 'QuerySet[ContractVersion]'
    | None = None,

When I manually changed it to the following, the tests ran fine again:

contract_versions_list: 'list[ContractVersion] | QuerySet[ContractVersion]| None' = None,

I'm on Ruff v0.1.8 in a Django 4.2.7 project, running tests with PyTest 7.4.3
@charliermarsh
Copy link
Member

What was the original annotation?

@charliermarsh
Copy link
Member

The issue here is that you changed QuerySet['ContractVersion'] to 'QuerySet[ContractVersion]' which isn't valid -- you need to quote the entire binary operation. But I'm just trying to understand, because you applied that change yourself?

@charliermarsh charliermarsh added the question Asking for support or clarification label Dec 14, 2023
@tylerlaprade
Copy link
Contributor Author

Yes, it looks like I introduced this error when trying to manually resolve the warning from Ruff, since I didn't realize it was invalid to quote just part of it. I'm surprised Pyright doesn't complain about an invalid annotation. Is it possible to make Ruff complain about it?

My original annotation had no quotes in it:

list[ContractVersion]
    | QuerySet[ContractVersion]
    | None = None,

@charliermarsh
Copy link
Member

We could probably lint against that, yeah.

@charliermarsh charliermarsh changed the title TCH quote-annotations fixes introduced runtime errors into my tests Lint when | annotations contain string segments Dec 15, 2023
@charliermarsh charliermarsh added rule Implementing or modifying a lint rule and removed question Asking for support or clarification labels Dec 15, 2023
@charliermarsh
Copy link
Member

Repurposing the issue to lint against "int" | str which is invalid.

@charliermarsh charliermarsh mentioned this issue Dec 15, 2023
Merged
@charliermarsh charliermarsh self-assigned this Dec 15, 2023
charliermarsh added a commit that referenced this issue Dec 16, 2023
@charliermarsh
Add a rule to detect string members in runtime-evaluated unions (#9143)
a336c1b 
## Summary

A common mistake is to add quotes around one member in an `X | Y`-style
type union, as in:

```python
contract_versions_list: list[ContractVersion] | 'QuerySet[ContractVersion]' | None = None
```

However, doing so will lead to a runtime error if the annotation is
runtime-evaluated. This PR lints against such patterns.

Closes #9139.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@charliermarsh charliermarsh

Labels
rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add a rule to detect string members in runtime-evaluated unions astral-sh/ruff
2 participants
@charliermarsh @tylerlaprade