Fix curly brace escape handling in f-strings #7331
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
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
This was referenced Sep 13, 2023
MichaReiser
approved these changes
Sep 13, 2023
| // valid here because it's a substring of a f-string. Even though | ||
| // curly braces cannot be escaped, it's a valid syntax. | ||
| // | ||
| // Refer to point 3: https://peps.python.org/pep-0701/#rejected-ideas |
There was a problem hiding this comment.
This section doesn't explain why it is valid. It only explains why it isn't an escape sequence. I assume it is valid because allows invalid escape sequences in general (Do we have a lint rule for that, does it need updating?)
There was a problem hiding this comment.
I actually noticed this while I was working on https://github.com/astral-sh/ruff/pull/7327/files#diff-739ef20efc60adce8f2acfaeda1005a932c58536433cbb895b323d335fffdd63 where at the end of the test file, the following was mentioned:
# To be fixed
# Error: f-string: single '}' is not allowed at line 41 column 8
# f"\{{x}}"So, then I started looking into it and realize that we don't parse this as a valid syntax. Python parses it as a valid syntax pre 3.12 as well although with 3.12 it produces a syntax warning:
$ python3.12-dev fstring.py
/Users/dhruv/playground/ruff/fstring.py:2: SyntaxWarning: invalid escape sequence '\{'
f"\{foo}"(Do we have a lint rule for that, does it need updating?)
We do have W605 (tracking issue) but as we didn't parse the syntax before, we didn't raise any warning. Now, we have the ability to raise that warning and I think we should do it.
dhruvmanila
force-pushed
the
dhruv/fstring-parser-4
branch
from
d184850
to
6c4f8c2
Compare
This will be used to extract the `leading` and `trailing` text for f-string debug expressions.
dhruvmanila
force-pushed
the
dhruv/fstring-parser-3
branch
from
25fad0c
to
06ba358
Compare
dhruvmanila
force-pushed
the
dhruv/fstring-parser-4
branch
from
6c4f8c2
to
44f3155
Compare
CodSpeed Performance ReportMerging #7331 will degrade performances by 9.55%Falling back to comparing Summary
Benchmarks breakdown
|
This was
linked to
issues
Sep 15, 2023
dhruvmanila
added a commit
that referenced
this pull request
Sep 18, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 19, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 20, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 22, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 26, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 27, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 28, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 29, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
dhruvmanila
added a commit
that referenced
this pull request
Sep 29, 2023
## Summary
This PR fixes the escape handling of curly braces inside a f-string.
There are 2 main changes:
### Lexer
The lexer change was actually a bug. Instead of breaking as soon as we
find a curly brace after the `\` character, we'll continue and let the next
iteration handle it in the curly brace branch. This fixes the following case:
```python
f"\{{foo}}"
# ^ use the curly brace branch to handle this character instead of breaking
```
### Parser
We can encounter a `\` as the last character in a `FStringMiddle` token
which is valid in this context[^1]. For example,
```python
f"\{foo} \{bar:\}"
# ^ ^^ ^
# The marked characters are part of 3 different `FStringMiddle` token
```
Here, the `FStringMiddle` token content will be `"\"` and `" \"` which
is invalid in a regular string literal. However, it's valid here because it's a
substring of a f-string. Even though curly braces cannot be escaped, it's a valid
syntax.
[^1]: Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas
## Test Plan
Verified that existing test cases are passing and add new test cases for
the lexer and parser.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 fixes the escape handling of curly braces inside a f-string. There are 2
main changes:
Lexer
The lexer change was actually a bug. Instead of breaking as soon as we find a
curly brace after the
\character, we'll continue and let the next iterationhandle it in the curly brace branch. This fixes the following case:
Parser
We can encounter a
\as the last character in aFStringMiddletoken which isvalid in this context1. For example,
Here, the
FStringMiddletoken content will be"\"and" \"which is invalid ina regular string literal. However, it's valid here because it's a substring of a
f-string. Even though curly braces cannot be escaped, it's a valid syntax.
Test Plan
Verified that existing test cases are passing and add new test cases for the lexer and parser.
Footnotes
Refer to point 3 in https://peps.python.org/pep-0701/#rejected-ideas ↩