-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Detect noqa
directives inside f-strings
#7291
Comments
Continuing the discussion started in #7326 (comment)
NoQA for f-strings:There are two options:
For (ii), things gets a bit tricky. Take for example the following code snippet: f"""thiiiiiiiiiiiiiiisssssssssss iiiiiiiiiisssssssssss aaaaaaaaaaaaa verrrrrrrrrrrrrryyyyyyyyyyyy
{
x
*
y
} looooooonnnnnggg string""" Here, the rule f"""thiiiiiiiiiiiiiiisssssssssss iiiiiiiiiisssssssssss aaaaaaaaaaaaa verrrrrrrrrrrrrryyyyyyyyyyyy
{
x # noqa: F821
*
y # noqa: F821
} looooooonnnnnggg string""" # noqa: E501 The f"""thiiiiiiiiiiiiiiisssssssssss iiiiiiiiiisssssssssss aaaaaaaaaaaaa verrrrrrrrrrrrrryyyyyyyyyyyy {
x
*
y
} looooooonnnnnggg string""" Now, we can add the f"""thiiiiiiiiiiiiiiisssssssssss iiiiiiiiiisssssssssss aaaaaaaaaaaaa verrrrrrrrrrrrrryyyyyyyyyyyy { # noqa: E501
x # noqa: F821
*
y # noqa: F821
} looooooonnnnnggg string""" For single-quoted f-strings this problem doesn't come as the expression should start on the same line as the opening quote i.e., the opening curly brace should be on the same line as the opening quote: # valid
f"first line {
x * y
}"
# invalid
f"first line
{
x * y
}" ImplementationHere, it get's a bit tricky. To give some context, we build the For f-strings, we could use the For example, # 1
f"""first line
{
expr
}"""
# 2
f"""first line
and another {
expr
}"""
# 3
f"""first line {
expr
}""" Here, the content of When I was thinking about the possible implementation, I found another problem w.r.t. (2) in the above code snippet. If the first line is too long, the NoQA directive has to go after the end of f-string but if the second line is too long, it could be added to the same line or at the end of the f-string. This is a confusing behavior: f"""assume this is a long line
and this is a long line too with an opening curly brace { # noqa: E501 # should this even be here?
expr
}""" # noqa: E501 # for the first line Then, if there's no NoQA comment on the second line but it is present at the end of the f-string, should the second line trigger f"""assume this is a long line
and this is a long line too with an opening curly brace { # should this trigger E501?
expr
}""" # noqa: E501 # for the first line \cc @charliermarsh @MichaReiser I would love your thoughts on this but I believe we should be consistent in the NoQA handling and should add it at the end of the f-string |
I'm okay moving forward with our current design but I see inline-noqa comments as a potential improvement because it avoids suppressing a too large range. |
I feel similarly. For now, I'm fine with only respecting In the future, I suppose both of these should work: f"""thiiiiiiiiiiiiiiisssssssssss iiiiiiiiiisssssssssss aaaaaaaaaaaaa verrrrrrrrrrrrrryyyyyyyyyyyy { # noqa: E501
x
*
y
} looooooonnnnnggg string""" # noqa: E501 We have precedence for this with, e.g.: from foo import ( # noqa: F401
Bar, # noqa: F401
) In which both of those |
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
Thanks for the feedback! I've opened #7561 to track that. I'll close this now. |
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
## Summary This PR updates the NoQA directive detection to consider the new f-string tokens. The reason being that now there can be multi-line f-strings without triple-quotes: ```python f"{ x * y }" ``` Here, the `noqa` directive should go at the end of the last line. ## Test Plan * Add new test cases for f-strings * Tested with `--add-noqa` using the following command with the above code snippet: ```console $ cargo run --bin ruff -- check --select=F821 --no-cache --isolated ~/playground/ruff/fstring.py --add-noqa Added 1 noqa directive. ``` Output: ```python f"{ x * y }" # noqa: F821 ``` Running the same command again doesn't add `noqa` directive and without the `--add-noqa` flag, the violation isn't reported. fixes: #7291
noqa
comments for strings goes at the end of it. Similarly, for multi-line strings (triple-quoted), thenoqa
directive is expected on the last line of the string.Now, with the new f-string specification, multi-line expressions are allowed in a single-quoted f-string. For example, the following code is valid in Python 3.12 but invalid before 3.12:
We need to make sure to add the
noqa
directives at the end of f-string regardless if it's single or triple-quoted. For the above example, the outcome would be:The
extract_noqa_line_for
function needs to be updated to fix this. Currently, it only checks for tripled-quoted strings using theString
token. Now, we will need to use theFStringStart
token to (1) check if it's a tripled-quoted f-string, if it is then (2) get the end range of the correspondingFStringEnd
token. This could be done by either using a stack or using theIndexer
to get the f-string range.The text was updated successfully, but these errors were encountered: