-
Notifications
You must be signed in to change notification settings - Fork 1.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 for multi-line f-strings
#7326
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 13, 2023
This was referenced Sep 13, 2023
dhruvmanila
added
python312
Related to Python 3.12
suppression
Related to supression of violations e.g. noqa
labels
Sep 13, 2023
dhruvmanila
force-pushed
the
dhruv/issue-7290
branch
from
September 13, 2023 09:46
d7e004e
to
d018b5b
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7291
branch
from
September 13, 2023 09:46
eeadcac
to
f1e192f
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7290
branch
from
September 14, 2023 01:37
d018b5b
to
79c5a1a
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7291
branch
2 times, most recently
from
September 14, 2023 02:29
fbcedf2
to
9c4d048
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7291
branch
from
September 14, 2023 02:40
9c4d048
to
0a404ec
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7291
branch
from
September 14, 2023 02:48
0a404ec
to
cc91933
Compare
dhruvmanila
commented
Sep 14, 2023
crates/ruff/src/directives.rs
Outdated
Comment on lines
110
to
112
// SAFETY: If we're at the end of a f-string, the indexer must have a | ||
// corresponding f-string range for it. | ||
let f_string_start = indexer.f_string_range(range.start()).unwrap().start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a slight performance penalty here as not many single quoted f-strings are going to be multi-line but we can't really know that until we get the entire f-string range.
MichaReiser
reviewed
Sep 14, 2023
dhruvmanila
force-pushed
the
dhruv/pep-701
branch
from
September 19, 2023 13:19
db4992a
to
101f9be
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7291
branch
2 times, most recently
from
September 20, 2023 03:48
7f349c5
to
e01be7c
Compare
dhruvmanila
force-pushed
the
dhruv/pep-701
branch
from
September 20, 2023 12:22
b2cbb1a
to
4d96b57
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7291
branch
from
September 20, 2023 12:23
e01be7c
to
08eebaa
Compare
dhruvmanila
force-pushed
the
dhruv/pep-701
branch
from
September 21, 2023 02:11
4d96b57
to
1aa2390
Compare
dhruvmanila
force-pushed
the
dhruv/issue-7291
branch
from
September 21, 2023 02:12
08eebaa
to
cd374e8
Compare
dhruvmanila
added a commit
that referenced
this pull request
Sep 21, 2023
## 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
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
## 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
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
## 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
This was referenced Sep 22, 2023
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
## 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
dhruvmanila
added a commit
that referenced
this pull request
Sep 26, 2023
## 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
dhruvmanila
added a commit
that referenced
this pull request
Sep 27, 2023
## 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
dhruvmanila
added a commit
that referenced
this pull request
Sep 28, 2023
## 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
dhruvmanila
added a commit
that referenced
this pull request
Sep 29, 2023
## 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
dhruvmanila
added a commit
that referenced
this pull request
Sep 29, 2023
## 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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:Output:
Running the same command again doesn't add
noqa
directive and without the--add-noqa
flag, the violation isn't reported.fixes: #7291