-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Render unsupported syntax errors in formatter tests #20777
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
Changes from 11 commits
ee3abc4
5f5059a
4917e08
f394bb3
bcddd82
aa9f30c
f396439
eea2d99
a30cdec
b6b727d
807300b
a72cb6a
2439bae
55c9826
8f88450
9781a3b
1bde294
50b2969
e26dce5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2409,3 +2409,59 @@ print( | |
| # Regression tests for https://github.com/astral-sh/ruff/issues/15536 | ||
| print(f"{ {}, 1 }") | ||
| ``` | ||
|
|
||
|
|
||
| ### Unsupported Syntax Errors | ||
| error[invalid-syntax]: Cannot use an escape sequence (backslash) in f-strings on Python 3.10 (syntax was added in Python 3.12) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these only the syntax errors that weren't suppressed with I think I'd either keep panicking in the presence of an invalid syntax error that isn't explicitly suppressed with an The risk I see with the pragma comments is that it might unintentionally suppress more syntax errors than I intended (e.g. in the If we go with rendering the errors, then I think we should add a short paragraph right below Unsupported syntax errors, saying that these need to be fixed unless they're pre-existing syntax errors already present in the source (and not introduced by the formatter).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we only render the leftover syntax errors that weren't present initially and weren't suppressed with the pragma comment. With the pragma comments, I was thinking we ideally wouldn't tolerate any snapshots, but I didn't want to fix these bugs in the same PR 😅 So I see what you mean, we should go back to panicking instead of snapshotting once these are resolved. I think we may also be able to print the fingerprint for new errors so that you could include that in the pragma comment, to make them a bit more specific. Or maybe part of the error message would be easier to use, like in mdtests. But I think I would still lean toward just rendering all of the new errors too and not having the pragma comments. That's a bit simpler and we didn't have many errors to render once we updated the fingerprint filtering anyway.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Sorry for misdirecting you on the pragma comments
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries, it was interesting to try out! I'll keep my cleanup commits from this morning and then revert them all, just in case we ever want to resurrect them.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if all those errors are now okay or if some of those are new errors introduced by the formatter. If any of them falls into the latter category, could you add a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought the errors for line 762 were related to #20774, but it turns out that they're false positives with our syntax error detection instead. I'll add the FIXME and open a separate issue. |
||
| --> fstring.py:762:19 | ||
| | | ||
| 760 | f'{1: abcd "{"aa"}" }' | ||
| 761 | f'{1=: "abcd {'aa'}}' | ||
| 762 | f"{x:a{z:hy \"user\"}} '''" | ||
| | ^ | ||
| 763 | | ||
| 764 | # Changing the outer quotes is fine because the format-spec is in a nested expression. | ||
| | | ||
|
|
||
| error[invalid-syntax]: Cannot use an escape sequence (backslash) in f-strings on Python 3.10 (syntax was added in Python 3.12) | ||
| --> fstring.py:762:13 | ||
| | | ||
| 760 | f'{1: abcd "{"aa"}" }' | ||
| 761 | f'{1=: "abcd {'aa'}}' | ||
| 762 | f"{x:a{z:hy \"user\"}} '''" | ||
| | ^ | ||
| 763 | | ||
| 764 | # Changing the outer quotes is fine because the format-spec is in a nested expression. | ||
| | | ||
|
|
||
| error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) | ||
| --> fstring.py:178:8 | ||
| | | ||
| 176 | # Here, the formatter will remove the escapes which is correct because they aren't allowed | ||
| 177 | # pre 3.12. This means we can assume that the f-string is used in the context of 3.12. | ||
| 178 | f"foo {"'bar'"}" | ||
| | ^ | ||
| 179 | f"foo {'"bar"'}" | ||
| | | ||
|
|
||
| error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) | ||
| --> fstring.py:771:14 | ||
| | | ||
| 769 | f'{1=: "abcd \'\'}' # Don't change the outer quotes, or it results in a syntax error | ||
| 770 | f"{1=: abcd \'\'}" # Changing the quotes here is fine because the inner quotes aren't the opposite quotes | ||
| 771 | f"{1=: abcd \"\"}" # Changing the quotes here is fine because the inner quotes are escaped | ||
| | ^ | ||
| 772 | # Don't change the quotes in the following cases: | ||
| 773 | f'{x=:hy "user"} \'\'\'' | ||
| | | ||
|
|
||
| error[invalid-syntax]: Cannot reuse outer quote character in f-strings on Python 3.10 (syntax was added in Python 3.12) | ||
| --> fstring.py:762:14 | ||
| | | ||
| 760 | f'{1: abcd "{"aa"}" }' | ||
| 761 | f'{1=: "abcd {'aa'}}' | ||
| 762 | f"{x:a{z:hy \"user\"}} '''" | ||
| | ^ | ||
| 763 | | ||
| 764 | # Changing the outer quotes is fine because the format-spec is in a nested expression. | ||
| | | ||
Uh oh!
There was an error while loading. Please reload this page.