-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Disallow newlines in format specifiers of single quoted f- or t-strings #18708
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
Conversation
3e2dd95 to
d00b986
Compare
|
738f998 to
6114339
Compare
968b01a to
39f14ca
Compare
dylwil3
left a comment
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.
Looks good to me!
| Self::NewlineInFormatSpec => { | ||
| write!( | ||
| f, | ||
| "newlines are not allowed in specifiers when using single quotes" |
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.
I think this might be a bit misleading since it's not the entire format spec where it isn't allowed, it's mainly the literal part of the format spec where it isn't allowed. So, the following is valid
# Newline is inside an expression element of the format spec
f"hello {x:.{y
}f}"And, running it on Python 3.13.5:
$ uv run --python 3.13 --no-project python -m ast parser/_.py
Module(
body=[
Expr(
value=JoinedStr(
values=[
Constant(value='hello '),
FormattedValue(
value=Name(id='x', ctx=Load()),
conversion=-1,
format_spec=JoinedStr(
values=[
Constant(value='.'),
FormattedValue(
value=Name(id='y', ctx=Load()),
conversion=-1),
Constant(value='f')]))]))])While, the following is invalid:
# Newline is inside the literal element of the format spec
f"hello {x:.{y}f
}"Running it on Python 3.13.5:
$ uv run --python 3.13 --no-project python -m ast parser/_.py
Traceback (most recent call last):
File "<frozen runpy>", line 198, in _run_module_as_main
File "<frozen runpy>", line 88, in _run_code
File "/Users/dhruv/.local/share/uv/python/cpython-3.13.5-macos-aarch64-none/lib/python3.13/ast.py", line 1865, in <module>
main()
~~~~^^
File "/Users/dhruv/.local/share/uv/python/cpython-3.13.5-macos-aarch64-none/lib/python3.13/ast.py", line 1861, in main
tree = parse(source, name, args.mode, type_comments=args.no_type_comments)
File "/Users/dhruv/.local/share/uv/python/cpython-3.13.5-macos-aarch64-none/lib/python3.13/ast.py", line 50, in parse
return compile(source, filename, mode, flags,
_feature_version=feature_version, optimize=optimize)
File "parser/_.py", line 1
f"hello {x:.{y}f
^
SyntaxError: unterminated f-string literal (detected at line 1)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.
I think this is actually just an unterminated f-string literal error and it's not really that the newline isn't allowed.
I don't think we would raise a similar (newline specific) error for something like:
f"hello
world"which is the same situation except it's the outer literal element.
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.
I'm not sure I follow. The error message is the same as CPython uses. We could clarify it to format spec literal but I'm not sure if that's not more confusing for users (I doubt many know that they even can use interpolations in format specs)
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.
Oh, I didn't realize that CPython raised a similar error message.
Feel free to ignore my message, it seems I shouldn't be doing late night reviews!
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.
But your comment is 100% correct and matches the implementation. I'm just not sure if using the more precise language here helps users or would be more confusing.
dhruvmanila
left a comment
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.
Looks good!
I'd probably keep the new error as UnterminatedString, refer to my inline comment.
39f14ca to
b9ba5fe
Compare
* main: (68 commits) Unify `OldDiagnostic` and `Message` (#18391) [`pylint`] Detect more exotic NaN literals in `PLW0177` (#18630) [`flake8-async`] Mark autofix for `ASYNC115` as unsafe if the call expression contains comments (#18753) [`flake8-bugbear`] Mark autofix for `B004` as unsafe if the `hasattr` call expr contains comments (#18755) Enforce `pytest` import for decorators (#18779) [`flake8-comprehension`] Mark autofix for `C420` as unsafe if there's comments inside the dict comprehension (#18768) [flake8-async] fix detection for large integer sleep durations in `ASYNC116` rule (#18767) Update dependency ruff to v0.12.0 (#18790) Update taiki-e/install-action action to v2.53.2 (#18789) Add lint rule for calling chmod with non-octal integers (#18541) Mark `RET501` fix unsafe if comments are inside (#18780) Use `LintContext::report_diagnostic_if_enabled` in `check_tokens` (#18769) [UP008]: use `super()`, not `__super__` in error messages (#18743) Use Depot Windows runners for `cargo test` (#18754) Run ty benchmarks when `ruff_benchmark` changes (#18758) Disallow newlines in format specifiers of single quoted f- or t-strings (#18708) [ty] Add more benchmarks (#18714) [ty] Anchor all exclude patterns (#18685) Include changelog reference for other major versions (#18745) Use updated pre-commit id (#18718) ...
Summary
This PR disallows newlines in format specifiers of single-quoted f- and t-strings. This aligns ruff's behavior with CPython's upstream behavior.
I think we should wait with merging this PR until after the next release so that the formatter can fix-up invalid format specifiers.
Part of #18632
Fixes #18730