mirror of
https://github.com/astral-sh/ruff.git
synced 2025-07-24 05:25:17 +00:00
Preserve dangling f-string comments
<!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary This PR fixes the issue where the FString formatting dropped dangling comments between the string parts. ```python result_f = ( f' File "{__file__}", line {lineno_f+1}, in f\n' ' f()\n' # XXX: The following line changes depending on whether the tests # are run through the interactive interpreter or with -m # It also varies depending on the platform (stack size) # Fortunately, we don't care about exactness here, so we use regex r' \[Previous line repeated (\d+) more times\]' '\n' 'RecursionError: maximum recursion depth exceeded\n' ) ``` The solution here isn't ideal because it re-introduces the `enclosing_parent` on `DecoratedComment` but it is the easiest fix that I could come up. I didn't spend more time finding another solution becaues I think we have to re-write most of the fstring formatting with the upcoming Python 3.12 support (because lexing the individual parts as we do now will no longer work). closes #6440 <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan `cargo test` The child PR testing that all comments are formatted should now pass
This commit is contained in:
parent
ac5c8bb3b6
commit
e2f7862404
6 changed files with 105 additions and 32 deletions
26
crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py
vendored
Normal file
26
crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py
vendored
Normal file
|
@ -0,0 +1,26 @@
|
||||||
|
(
|
||||||
|
f'{one}'
|
||||||
|
f'{two}'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
rf"Not-so-tricky \"quote"
|
||||||
|
|
||||||
|
# Regression test for fstrings dropping comments
|
||||||
|
result_f = (
|
||||||
|
'Traceback (most recent call last):\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+5}, in _check_recursive_traceback_display\n'
|
||||||
|
' f()\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
' f()\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
' f()\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
' f()\n'
|
||||||
|
# XXX: The following line changes depending on whether the tests
|
||||||
|
# are run through the interactive interpreter or with -m
|
||||||
|
# It also varies depending on the platform (stack size)
|
||||||
|
# Fortunately, we don't care about exactness here, so we use regex
|
||||||
|
r' \[Previous line repeated (\d+) more times\]' '\n'
|
||||||
|
'RecursionError: maximum recursion depth exceeded\n'
|
||||||
|
)
|
|
@ -1,7 +0,0 @@
|
||||||
(
|
|
||||||
f'{one}'
|
|
||||||
f'{two}'
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
rf"Not-so-tricky \"quote"
|
|
|
@ -73,6 +73,13 @@ pub(super) fn place_comment<'a>(
|
||||||
handle_leading_class_with_decorators_comment(comment, class_def)
|
handle_leading_class_with_decorators_comment(comment, class_def)
|
||||||
}
|
}
|
||||||
AnyNodeRef::StmtImportFrom(import_from) => handle_import_from_comment(comment, import_from),
|
AnyNodeRef::StmtImportFrom(import_from) => handle_import_from_comment(comment, import_from),
|
||||||
|
AnyNodeRef::ExprConstant(_) => {
|
||||||
|
if let Some(AnyNodeRef::ExprFString(fstring)) = comment.enclosing_parent() {
|
||||||
|
CommentPlacement::dangling(fstring, comment)
|
||||||
|
} else {
|
||||||
|
CommentPlacement::Default(comment)
|
||||||
|
}
|
||||||
|
}
|
||||||
AnyNodeRef::ExprList(_)
|
AnyNodeRef::ExprList(_)
|
||||||
| AnyNodeRef::ExprSet(_)
|
| AnyNodeRef::ExprSet(_)
|
||||||
| AnyNodeRef::ExprGeneratorExp(_)
|
| AnyNodeRef::ExprGeneratorExp(_)
|
||||||
|
|
|
@ -77,6 +77,7 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> {
|
||||||
enclosing: enclosing_node,
|
enclosing: enclosing_node,
|
||||||
preceding: self.preceding_node,
|
preceding: self.preceding_node,
|
||||||
following: Some(node),
|
following: Some(node),
|
||||||
|
parent: self.parents.iter().rev().nth(1).copied(),
|
||||||
line_position: text_position(*comment_range, self.source_code),
|
line_position: text_position(*comment_range, self.source_code),
|
||||||
slice: self.source_code.slice(*comment_range),
|
slice: self.source_code.slice(*comment_range),
|
||||||
};
|
};
|
||||||
|
@ -117,6 +118,7 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast> {
|
||||||
|
|
||||||
let comment = DecoratedComment {
|
let comment = DecoratedComment {
|
||||||
enclosing: node,
|
enclosing: node,
|
||||||
|
parent: self.parents.last().copied(),
|
||||||
preceding: self.preceding_node,
|
preceding: self.preceding_node,
|
||||||
following: None,
|
following: None,
|
||||||
line_position: text_position(*comment_range, self.source_code),
|
line_position: text_position(*comment_range, self.source_code),
|
||||||
|
@ -179,6 +181,7 @@ pub(super) struct DecoratedComment<'a> {
|
||||||
enclosing: AnyNodeRef<'a>,
|
enclosing: AnyNodeRef<'a>,
|
||||||
preceding: Option<AnyNodeRef<'a>>,
|
preceding: Option<AnyNodeRef<'a>>,
|
||||||
following: Option<AnyNodeRef<'a>>,
|
following: Option<AnyNodeRef<'a>>,
|
||||||
|
parent: Option<AnyNodeRef<'a>>,
|
||||||
line_position: CommentLinePosition,
|
line_position: CommentLinePosition,
|
||||||
slice: SourceCodeSlice,
|
slice: SourceCodeSlice,
|
||||||
}
|
}
|
||||||
|
@ -204,6 +207,11 @@ impl<'a> DecoratedComment<'a> {
|
||||||
self.enclosing
|
self.enclosing
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns the parent of the enclosing node, if any
|
||||||
|
pub(super) fn enclosing_parent(&self) -> Option<AnyNodeRef<'a>> {
|
||||||
|
self.parent
|
||||||
|
}
|
||||||
|
|
||||||
/// Returns the slice into the source code.
|
/// Returns the slice into the source code.
|
||||||
pub(super) fn slice(&self) -> &SourceCodeSlice {
|
pub(super) fn slice(&self) -> &SourceCodeSlice {
|
||||||
&self.slice
|
&self.slice
|
||||||
|
|
|
@ -0,0 +1,64 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_python_formatter/tests/fixtures.rs
|
||||||
|
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py
|
||||||
|
---
|
||||||
|
## Input
|
||||||
|
```py
|
||||||
|
(
|
||||||
|
f'{one}'
|
||||||
|
f'{two}'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
rf"Not-so-tricky \"quote"
|
||||||
|
|
||||||
|
# Regression test for fstrings dropping comments
|
||||||
|
result_f = (
|
||||||
|
'Traceback (most recent call last):\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+5}, in _check_recursive_traceback_display\n'
|
||||||
|
' f()\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
' f()\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
' f()\n'
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
' f()\n'
|
||||||
|
# XXX: The following line changes depending on whether the tests
|
||||||
|
# are run through the interactive interpreter or with -m
|
||||||
|
# It also varies depending on the platform (stack size)
|
||||||
|
# Fortunately, we don't care about exactness here, so we use regex
|
||||||
|
r' \[Previous line repeated (\d+) more times\]' '\n'
|
||||||
|
'RecursionError: maximum recursion depth exceeded\n'
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
## Output
|
||||||
|
```py
|
||||||
|
(f"{one}" f"{two}")
|
||||||
|
|
||||||
|
|
||||||
|
rf'Not-so-tricky "quote'
|
||||||
|
|
||||||
|
# Regression test for fstrings dropping comments
|
||||||
|
result_f = (
|
||||||
|
"Traceback (most recent call last):\n"
|
||||||
|
f' File "{__file__}", line {lineno_f+5}, in _check_recursive_traceback_display\n'
|
||||||
|
" f()\n"
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
" f()\n"
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
" f()\n"
|
||||||
|
f' File "{__file__}", line {lineno_f+1}, in f\n'
|
||||||
|
" f()\n"
|
||||||
|
# XXX: The following line changes depending on whether the tests
|
||||||
|
# are run through the interactive interpreter or with -m
|
||||||
|
# It also varies depending on the platform (stack size)
|
||||||
|
# Fortunately, we don't care about exactness here, so we use regex
|
||||||
|
r" \[Previous line repeated (\d+) more times\]"
|
||||||
|
"\n"
|
||||||
|
"RecursionError: maximum recursion depth exceeded\n"
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -1,25 +0,0 @@
|
||||||
---
|
|
||||||
source: crates/ruff_python_formatter/tests/fixtures.rs
|
|
||||||
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/joined_string.py
|
|
||||||
---
|
|
||||||
## Input
|
|
||||||
```py
|
|
||||||
(
|
|
||||||
f'{one}'
|
|
||||||
f'{two}'
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
rf"Not-so-tricky \"quote"
|
|
||||||
```
|
|
||||||
|
|
||||||
## Output
|
|
||||||
```py
|
|
||||||
(f"{one}" f"{two}")
|
|
||||||
|
|
||||||
|
|
||||||
rf'Not-so-tricky "quote'
|
|
||||||
```
|
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue