Skip to content

Commit 8fb29ea

Browse files
authored
[ruff] improve handling of intermixed comments inside from-imports (#20561)
Resolves a crash when attempting to format code like: ``` from x import (a as # whatever b) ``` Reworks the way comments are associated with nodes when parsing modules, so that all possible comment positions can be retained and reproduced during formatting. Overall follows Black's formatting style for multi-line import statements. Fixes issue #19138
1 parent 23ebfe7 commit 8fb29ea

File tree

4 files changed

+268
-5
lines changed

4 files changed

+268
-5
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# ensure trailing comments are preserved
2+
import x # comment
3+
from x import a # comment
4+
from x import a, b # comment
5+
from x import a as b # comment
6+
from x import a as b, b as c # comment
7+
8+
from x import (
9+
a, # comment
10+
)
11+
from x import (
12+
a, # comment
13+
b,
14+
)
15+
16+
# ensure comma is added
17+
from x import (
18+
a # comment
19+
)
20+
21+
# follow black style by merging cases without own-line comments
22+
from x import (
23+
a # alpha
24+
, # beta
25+
b,
26+
)
27+
28+
# ensure intermixed comments are all preserved
29+
from x import ( # one
30+
# two
31+
a # three
32+
# four
33+
, # five
34+
# six
35+
) # seven
36+
37+
from x import ( # alpha
38+
# bravo
39+
a # charlie
40+
# delta
41+
as # echo
42+
# foxtrot
43+
b # golf
44+
# hotel
45+
, # india
46+
# juliet
47+
) # kilo
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# ensure trailing comments are preserved
2+
import x # comment
3+
from x import a # comment
4+
from x import a, b # comment
5+
from x import a as b # comment
6+
from x import a as b, b as c # comment
7+
8+
from x import (
9+
a, # comment
10+
)
11+
from x import (
12+
a, # comment
13+
b,
14+
)
15+
16+
# ensure comma is added
17+
from x import (
18+
a, # comment
19+
)
20+
21+
# follow black style by merging cases without own-line comments
22+
from x import (
23+
a, # alpha # beta
24+
b,
25+
)
26+
27+
# ensure intermixed comments are all preserved
28+
from x import ( # one
29+
# two
30+
a # three
31+
# four
32+
, # five
33+
# six
34+
) # seven
35+
36+
from x import ( # alpha
37+
# bravo
38+
a # charlie
39+
# delta
40+
as # echo
41+
# foxtrot
42+
b # golf
43+
# hotel
44+
, # india
45+
# juliet
46+
) # kilo

crates/ruff_python_formatter/src/comments/placement.rs

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,10 @@ fn handle_enclosed_comment<'a>(
311311
AnyNodeRef::StmtClassDef(class_def) => {
312312
handle_leading_class_with_decorators_comment(comment, class_def)
313313
}
314-
AnyNodeRef::StmtImportFrom(import_from) => handle_import_from_comment(comment, import_from),
314+
AnyNodeRef::StmtImportFrom(import_from) => {
315+
handle_import_from_comment(comment, import_from, source)
316+
}
317+
AnyNodeRef::Alias(alias) => handle_alias_comment(comment, alias, source),
315318
AnyNodeRef::StmtWith(with_) => handle_with_comment(comment, with_),
316319
AnyNodeRef::ExprCall(_) => handle_call_comment(comment),
317320
AnyNodeRef::ExprStringLiteral(_) => match comment.enclosing_parent() {
@@ -1922,7 +1925,7 @@ fn handle_bracketed_end_of_line_comment<'a>(
19221925
CommentPlacement::Default(comment)
19231926
}
19241927

1925-
/// Attach an enclosed end-of-line comment to a [`ast::StmtImportFrom`].
1928+
/// Attach an enclosed comment to a [`ast::StmtImportFrom`].
19261929
///
19271930
/// For example, given:
19281931
/// ```python
@@ -1933,9 +1936,37 @@ fn handle_bracketed_end_of_line_comment<'a>(
19331936
///
19341937
/// The comment will be attached to the [`ast::StmtImportFrom`] node as a dangling comment, to
19351938
/// ensure that it remains on the same line as the [`ast::StmtImportFrom`] itself.
1939+
///
1940+
/// If the comment's preceding node is an alias, and the comment is *before* a comma:
1941+
/// ```python
1942+
/// from foo import (
1943+
/// bar as baz # comment
1944+
/// ,
1945+
/// )
1946+
/// ```
1947+
///
1948+
/// The comment will then be attached to the [`ast::Alias`] node as a dangling comment instead,
1949+
/// to ensure that it retains its position before the comma.
1950+
///
1951+
/// Otherwise, if the comment is *after* the comma or before a following alias:
1952+
/// ```python
1953+
/// from foo import (
1954+
/// bar as baz, # comment
1955+
/// )
1956+
///
1957+
/// from foo import (
1958+
/// bar,
1959+
/// # comment
1960+
/// baz,
1961+
/// )
1962+
/// ```
1963+
///
1964+
/// Then it will retain the default behavior of being attached to the relevant [`ast::Alias`] node
1965+
/// as either a leading or trailing comment.
19361966
fn handle_import_from_comment<'a>(
19371967
comment: DecoratedComment<'a>,
19381968
import_from: &'a ast::StmtImportFrom,
1969+
source: &str,
19391970
) -> CommentPlacement<'a> {
19401971
// The comment needs to be on the same line, but before the first member. For example, we want
19411972
// to treat this as a dangling comment:
@@ -1963,10 +1994,69 @@ fn handle_import_from_comment<'a>(
19631994
{
19641995
CommentPlacement::dangling(comment.enclosing_node(), comment)
19651996
} else {
1966-
CommentPlacement::Default(comment)
1997+
if let Some(SimpleToken {
1998+
kind: SimpleTokenKind::Comma,
1999+
..
2000+
}) = SimpleTokenizer::starts_at(comment.start(), source)
2001+
.skip_trivia()
2002+
.next()
2003+
{
2004+
// Treat comments before the comma as dangling, after as trailing (default)
2005+
if let Some(AnyNodeRef::Alias(alias)) = comment.preceding_node() {
2006+
CommentPlacement::dangling(alias, comment)
2007+
} else {
2008+
CommentPlacement::Default(comment)
2009+
}
2010+
} else {
2011+
CommentPlacement::Default(comment)
2012+
}
19672013
}
19682014
}
19692015

2016+
/// Attach an enclosed comment to the appropriate [`ast::Identifier`] within an [`ast::Alias`].
2017+
///
2018+
/// For example:
2019+
/// ```python
2020+
/// from foo import (
2021+
/// bar # comment
2022+
/// as baz,
2023+
/// )
2024+
/// ```
2025+
///
2026+
/// Will attach the comment as a trailing comment on the first name [`ast::Identifier`].
2027+
///
2028+
/// Whereas:
2029+
/// ```python
2030+
/// from foo import (
2031+
/// bar as # comment
2032+
/// baz,
2033+
/// )
2034+
/// ```
2035+
///
2036+
/// Will attach the comment as a leading comment on the second name [`ast::Identifier`].
2037+
fn handle_alias_comment<'a>(
2038+
comment: DecoratedComment<'a>,
2039+
alias: &'a ruff_python_ast::Alias,
2040+
source: &str,
2041+
) -> CommentPlacement<'a> {
2042+
if let Some(asname) = &alias.asname {
2043+
if let Some(SimpleToken {
2044+
kind: SimpleTokenKind::As,
2045+
range: as_range,
2046+
}) = SimpleTokenizer::starts_at(alias.name.end(), source)
2047+
.skip_trivia()
2048+
.next()
2049+
{
2050+
return if comment.start() < as_range.start() {
2051+
CommentPlacement::trailing(&alias.name, comment)
2052+
} else {
2053+
CommentPlacement::leading(asname, comment)
2054+
};
2055+
}
2056+
}
2057+
CommentPlacement::Default(comment)
2058+
}
2059+
19702060
/// Attach an enclosed end-of-line comment to a [`ast::StmtWith`].
19712061
///
19722062
/// For example, given:

crates/ruff_python_formatter/src/other/alias.rs

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use ruff_formatter::write;
22
use ruff_python_ast::Alias;
33

4+
use crate::comments::trailing_comments;
45
use crate::other::identifier::DotDelimitedIdentifier;
56
use crate::prelude::*;
67

@@ -15,10 +16,89 @@ impl FormatNodeRule<Alias> for FormatAlias {
1516
name,
1617
asname,
1718
} = item;
18-
DotDelimitedIdentifier::new(name).fmt(f)?;
19+
write!(f, [DotDelimitedIdentifier::new(name)])?;
20+
21+
let comments = f.context().comments().clone();
22+
23+
// ```python
24+
// from foo import (
25+
// bar # comment
26+
// as baz,
27+
// )
28+
// ```
29+
if comments.has_trailing(name) {
30+
write!(
31+
f,
32+
[
33+
trailing_comments(comments.trailing(name)),
34+
hard_line_break()
35+
]
36+
)?;
37+
} else if asname.is_some() {
38+
write!(f, [space()])?;
39+
}
40+
1941
if let Some(asname) = asname {
20-
write!(f, [space(), token("as"), space(), asname.format()])?;
42+
write!(f, [token("as")])?;
43+
44+
// ```python
45+
// from foo import (
46+
// bar as # comment
47+
// baz,
48+
// )
49+
// ```
50+
if comments.has_leading(asname) {
51+
write!(
52+
f,
53+
[
54+
trailing_comments(comments.leading(asname)),
55+
hard_line_break()
56+
]
57+
)?;
58+
} else {
59+
write!(f, [space()])?;
60+
}
61+
62+
write!(f, [asname.format()])?;
2163
}
64+
65+
// Dangling comment between alias and comma on a following line
66+
// ```python
67+
// from foo import (
68+
// bar # comment
69+
// ,
70+
// )
71+
// ```
72+
let dangling = comments.dangling(item);
73+
if !dangling.is_empty() {
74+
write!(f, [trailing_comments(comments.dangling(item))])?;
75+
76+
// Black will move the comma and merge comments if there is no own-line comment between
77+
// the alias and the comma.
78+
//
79+
// Eg:
80+
// ```python
81+
// from foo import (
82+
// bar # one
83+
// , # two
84+
// )
85+
// ```
86+
//
87+
// Will become:
88+
// ```python
89+
// from foo import (
90+
// bar, # one # two)
91+
// ```
92+
//
93+
// Only force a hard line break if an own-line dangling comment is present.
94+
if dangling
95+
.iter()
96+
.any(|comment| comment.line_position().is_own_line())
97+
{
98+
write!(f, [hard_line_break()])?;
99+
}
100+
}
101+
22102
Ok(())
23103
}
24104
}

0 commit comments

Comments
 (0)