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

F821 Undefined name code position incorrect in f-strings #145

Closed
nikolaik opened this issue Sep 10, 2022 · 5 comments · Fixed by #244
Closed

F821 Undefined name code position incorrect in f-strings #145

nikolaik opened this issue Sep 10, 2022 · 5 comments · Fixed by #244
Labels
bug Something isn't working

Comments

@nikolaik
Copy link

nikolaik commented Sep 10, 2022

When trying to ignore F821 in some cases, like importing variables using star imports, I can't when the variable is used in an f-string. The line and column position seems to be off. Is that used to match the noqa comments?

# a.py:
some_string = 'to increase the line count'

A = f'{B}'  # noqa: F821
A = B  # noqa: F821
$ ruff a.py
a.py:1:2: F821 Undefined name `B`

Found 1 error(s).
@charliermarsh charliermarsh added the bug Something isn't working label Sep 11, 2022
@charliermarsh
Copy link
Member

Yeah this looks like an issue in the parser -- the positions on the sub-expressions seem to be relative to the parent f-string. Thanks.

@charliermarsh charliermarsh added this to the Release 0.1.0 milestone Sep 15, 2022
@charliermarsh
Copy link
Member

(This is a blocker for 0.1.0.)

@charliermarsh
Copy link
Member

charliermarsh commented Sep 21, 2022

#244 modifies the reporter to use the location of the start of the f-string. It's probably the best we can do without fixing this in the parse itself (which is a bit more involved). With this change, it'll at least be clear where you should put the noqa, and the location is a lot more reasonable (at the line on which the f-string starts, instead of at the top of the file).

@nikolaik
Copy link
Author

Thank you @charliermarsh ! This works and is the last piece missing to allow us to use ruff one of our projects ❤️

@charliermarsh
Copy link
Member

Np! If you hit any issues, don't hesitate to reach out!

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