Skip to content

Commit 2b28d56

Browse files
MichaReiserMichaReiser
authored
Associate a trailing end-of-line comment in a parenthesized implicit concatenated string with the last literal (#15378)
1 parent adca7bd commit 2b28d56

6 files changed

Lines changed: 378 additions & 8 deletions

File tree

‎crates/ruff_python_ast/src/expression.rs‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,13 +407,13 @@ pub enum StringLike<'a> {
407407
FString(&'a ast::ExprFString),
408408
}
409409

410-
impl StringLike<'_> {
410+
impl<'a> StringLike<'a> {
411411
pub const fn is_fstring(self) -> bool {
412412
matches!(self, Self::FString(_))
413413
}
414414

415415
/// Returns an iterator over the [`StringLikePart`] contained in this string-like expression.
416-
pub fn parts(&self) -> StringLikePartIter<'_> {
416+
pub fn parts(&self) -> StringLikePartIter<'a> {
417417
match self {
418418
StringLike::String(expr) => StringLikePartIter::String(expr.value.iter()),
419419
StringLike::Bytes(expr) => StringLikePartIter::Bytes(expr.value.iter()),
@@ -429,6 +429,14 @@ impl StringLike<'_> {
429429
Self::FString(ExprFString { value, .. }) => value.is_implicit_concatenated(),
430430
}
431431
}
432+
433+
pub const fn as_expression_ref(self) -> ExpressionRef<'a> {
434+
match self {
435+
StringLike::String(expr) => ExpressionRef::StringLiteral(expr),
436+
StringLike::Bytes(expr) => ExpressionRef::BytesLiteral(expr),
437+
StringLike::FString(expr) => ExpressionRef::FString(expr),
438+
}
439+
}
432440
}
433441

434442
impl<'a> From<&'a ast::ExprStringLiteral> for StringLike<'a> {
@@ -488,6 +496,19 @@ impl<'a> TryFrom<&'a Expr> for StringLike<'a> {
488496
}
489497
}
490498

499+
impl<'a> TryFrom<AnyNodeRef<'a>> for StringLike<'a> {
500+
type Error = ();
501+
502+
fn try_from(value: AnyNodeRef<'a>) -> Result<Self, Self::Error> {
503+
match value {
504+
AnyNodeRef::ExprStringLiteral(value) => Ok(Self::String(value)),
505+
AnyNodeRef::ExprBytesLiteral(value) => Ok(Self::Bytes(value)),
506+
AnyNodeRef::ExprFString(value) => Ok(Self::FString(value)),
507+
_ => Err(()),
508+
}
509+
}
510+
}
511+
491512
impl Ranged for StringLike<'_> {
492513
fn range(&self) -> TextRange {
493514
match self {
@@ -561,6 +582,12 @@ impl<'a> From<&'a ast::FString> for StringLikePart<'a> {
561582

562583
impl<'a> From<&StringLikePart<'a>> for AnyNodeRef<'a> {
563584
fn from(value: &StringLikePart<'a>) -> Self {
585+
AnyNodeRef::from(*value)
586+
}
587+
}
588+
589+
impl<'a> From<StringLikePart<'a>> for AnyNodeRef<'a> {
590+
fn from(value: StringLikePart<'a>) -> Self {
564591
match value {
565592
StringLikePart::String(part) => AnyNodeRef::StringLiteral(part),
566593
StringLikePart::Bytes(part) => AnyNodeRef::BytesLiteral(part),

‎crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/join_implicit_concatenated_string_assignment.py‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,3 +291,87 @@
291291
f"testeeeeeeeeeeeeeeeeeeeeeeeee{a
292292
=}" "moreeeeeeeeeeeeeeeeeetest" # comment
293293
)
294+
295+
296+
# Trailing last-part comments
297+
298+
a = (
299+
"a"
300+
"b" # belongs to `b`
301+
)
302+
303+
a: Literal[str] = (
304+
"a"
305+
"b" # belongs to `b`
306+
)
307+
308+
a += (
309+
"a"
310+
"b" # belongs to `b`
311+
)
312+
313+
a = (
314+
r"a"
315+
r"b" # belongs to `b`
316+
)
317+
318+
a = (
319+
"a"
320+
"b"
321+
) # belongs to the assignment
322+
323+
a = (((
324+
"a"
325+
"b" # belongs to `b`
326+
)))
327+
328+
a = (((
329+
"a"
330+
"b"
331+
) # belongs to the f-string expression
332+
))
333+
334+
a = (
335+
"a" "b" # belongs to the f-string expression
336+
)
337+
338+
a = (
339+
"a" "b"
340+
# belongs to the f-string expression
341+
)
342+
343+
# There's no "right" answer if some parts are on the same line while others are on separate lines.
344+
# This is likely a comment for one of the last two parts but could also just be a comment for the entire f-string expression.
345+
# Because there's no right answer, follow what we do elsewhere and associate the comment with the outer-most node which
346+
# is the f-string expression.
347+
a = (
348+
"a"
349+
"b" "ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" # belongs to the f-string expression
350+
)
351+
352+
logger.error(
353+
f"Failed to run task {task} for job"
354+
f"with id {str(job.id)}" # type: ignore[union-attr]
355+
)
356+
357+
a = (10 +
358+
"Exception in {call_back_name} "
359+
f"'{msg}'" # belongs to binary operation
360+
)
361+
362+
a = 10 + (
363+
"Exception in {call_back_name} "
364+
f"'{msg}'" # belongs to f-string
365+
)
366+
367+
self._attr_unique_id = (
368+
f"{self._device.temperature.group_address_state}_"
369+
f"{self._device.target_temperature.group_address_state}_"
370+
f"{self._device.target_temperature.group_address}_"
371+
f"{self._device._setpoint_shift.group_address}" # noqa: SLF001
372+
)
373+
374+
return (
375+
f"Exception in {call_back_name} when handling msg on "
376+
f"'{msg.topic}': '{msg.payload}'" # type: ignore[str-bytes-safe]
377+
)

‎crates/ruff_python_formatter/src/comments/placement.rs‎

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
1-
use std::cmp::Ordering;
2-
31
use ast::helpers::comment_indentation_after;
42
use ruff_python_ast::whitespace::indentation;
53
use ruff_python_ast::{
6-
self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, Parameters,
4+
self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, Parameters, StringLike,
75
};
86
use ruff_python_trivia::{
97
find_only_token_in_range, first_non_trivia_token, indentation_at_offset, BackwardsTokenizer,
108
CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer,
119
};
1210
use ruff_source_file::LineRanges;
1311
use ruff_text_size::{Ranged, TextLen, TextRange};
12+
use std::cmp::Ordering;
1413

1514
use crate::comments::visitor::{CommentPlacement, DecoratedComment};
1615
use crate::expression::expr_slice::{assign_comment_in_slice, ExprSliceCommentSection};
16+
use crate::expression::parentheses::is_expression_parenthesized;
1717
use crate::other::parameters::{
1818
assign_argument_separator_comment_placement, find_parameter_separators,
1919
};
@@ -355,6 +355,41 @@ fn handle_enclosed_comment<'a>(
355355
AnyNodeRef::ExprGenerator(generator) if generator.parenthesized => {
356356
handle_bracketed_end_of_line_comment(comment, source)
357357
}
358+
AnyNodeRef::StmtReturn(_) => {
359+
handle_trailing_implicit_concatenated_string_comment(comment, comment_ranges, source)
360+
}
361+
AnyNodeRef::StmtAssign(assignment)
362+
if comment.preceding_node().is_some_and(|preceding| {
363+
preceding.ptr_eq(AnyNodeRef::from(&*assignment.value))
364+
}) =>
365+
{
366+
handle_trailing_implicit_concatenated_string_comment(comment, comment_ranges, source)
367+
}
368+
AnyNodeRef::StmtAnnAssign(assignment)
369+
if comment.preceding_node().is_some_and(|preceding| {
370+
assignment
371+
.value
372+
.as_deref()
373+
.is_some_and(|value| preceding.ptr_eq(value.into()))
374+
}) =>
375+
{
376+
handle_trailing_implicit_concatenated_string_comment(comment, comment_ranges, source)
377+
}
378+
AnyNodeRef::StmtAugAssign(assignment)
379+
if comment.preceding_node().is_some_and(|preceding| {
380+
preceding.ptr_eq(AnyNodeRef::from(&*assignment.value))
381+
}) =>
382+
{
383+
handle_trailing_implicit_concatenated_string_comment(comment, comment_ranges, source)
384+
}
385+
AnyNodeRef::StmtTypeAlias(assignment)
386+
if comment.preceding_node().is_some_and(|preceding| {
387+
preceding.ptr_eq(AnyNodeRef::from(&*assignment.value))
388+
}) =>
389+
{
390+
handle_trailing_implicit_concatenated_string_comment(comment, comment_ranges, source)
391+
}
392+
358393
_ => CommentPlacement::Default(comment),
359394
}
360395
}
@@ -2086,6 +2121,75 @@ fn handle_comprehension_comment<'a>(
20862121
CommentPlacement::Default(comment)
20872122
}
20882123

2124+
/// Handle end-of-line comments for parenthesized implicitly concatenated strings when used in
2125+
/// a `FormatStatementLastExpression` context:
2126+
///
2127+
/// ```python
2128+
/// a = (
2129+
/// "a"
2130+
/// "b"
2131+
/// "c" # comment
2132+
/// )
2133+
/// ```
2134+
///
2135+
/// `# comment` is a trailing comment of the last part and not a trailing comment of the entire f-string.
2136+
/// Associating the comment with the last part is important or the assignment formatting might move
2137+
/// the comment at the end of the assignment, making it impossible to suppress an error for the last part.
2138+
///
2139+
/// On the other hand, `# comment` is a trailing end-of-line f-string comment for:
2140+
///
2141+
/// ```python
2142+
/// a = (
2143+
/// "a" "b" "c" # comment
2144+
/// )
2145+
///
2146+
/// a = (
2147+
/// "a"
2148+
/// "b"
2149+
/// "c"
2150+
/// ) # comment
2151+
/// ```
2152+
///
2153+
/// Associating the comment with the f-string is desired in those cases because it allows
2154+
/// joining the string literals into a single string literal if it fits on the line.
2155+
fn handle_trailing_implicit_concatenated_string_comment<'a>(
2156+
comment: DecoratedComment<'a>,
2157+
comment_ranges: &CommentRanges,
2158+
source: &str,
2159+
) -> CommentPlacement<'a> {
2160+
if !comment.line_position().is_end_of_line() {
2161+
return CommentPlacement::Default(comment);
2162+
}
2163+
2164+
let Some(string_like) = comment
2165+
.preceding_node()
2166+
.and_then(|preceding| StringLike::try_from(preceding).ok())
2167+
else {
2168+
return CommentPlacement::Default(comment);
2169+
};
2170+
2171+
let mut parts = string_like.parts();
2172+
2173+
let (Some(last), Some(second_last)) = (parts.next_back(), parts.next_back()) else {
2174+
return CommentPlacement::Default(comment);
2175+
};
2176+
2177+
if source.contains_line_break(TextRange::new(second_last.end(), last.start()))
2178+
&& is_expression_parenthesized(string_like.as_expression_ref(), comment_ranges, source)
2179+
{
2180+
let range = TextRange::new(last.end(), comment.start());
2181+
2182+
if !SimpleTokenizer::new(source, range)
2183+
.skip_trivia()
2184+
.any(|token| token.kind() == SimpleTokenKind::RParen)
2185+
{
2186+
return CommentPlacement::trailing(AnyNodeRef::from(last), comment);
2187+
}
2188+
}
2189+
2190+
CommentPlacement::Default(comment)
2191+
}
2192+
20892193
/// Returns `true` if the parameters are parenthesized (as in a function definition), or `false` if
20902194
/// not (as in a lambda).
20912195
fn are_parameters_parenthesized(parameters: &Parameters, contents: &str) -> bool {

‎crates/ruff_python_formatter/src/string/implicit.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ impl Format<PyFormatContext<'_>> for FormatImplicitConcatenatedStringExpanded<'_
9999
StringLikePart::FString(part) => part.format().fmt(f),
100100
});
101101

102-
let part_comments = comments.leading_dangling_trailing(&part);
102+
let part_comments = comments.leading_dangling_trailing(part);
103103
joiner.entry(&format_args![
104104
leading_comments(part_comments.leading),
105105
format_part,

‎crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
source: crates/ruff_python_formatter/tests/fixtures.rs
33
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py
4-
snapshot_kind: text
54
---
65
## Input
76
```python

0 commit comments

Comments
 (0)