From 5f720150fb91cc4cf8b91b0ce0d8d5a88540e56c Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 24 Sep 2025 13:48:05 -0700 Subject: [PATCH 1/4] [ruff] fix crash with dangling comments in importfrom alias Resolves the crash when attempting to format code like: ``` from x import (a as # whatever b) ``` And chooses to format it as: ``` from x import ( a as b, # whatever ) ``` Fixes issue #19138 --- crates/ruff_python_formatter/src/other/alias.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/ruff_python_formatter/src/other/alias.rs b/crates/ruff_python_formatter/src/other/alias.rs index bce6485e12492..4ad9f5679873b 100644 --- a/crates/ruff_python_formatter/src/other/alias.rs +++ b/crates/ruff_python_formatter/src/other/alias.rs @@ -1,6 +1,7 @@ use ruff_formatter::write; use ruff_python_ast::Alias; +use crate::comments::trailing_comments; use crate::other::identifier::DotDelimitedIdentifier; use crate::prelude::*; @@ -17,7 +18,18 @@ impl FormatNodeRule for FormatAlias { } = item; DotDelimitedIdentifier::new(name).fmt(f)?; if let Some(asname) = asname { - write!(f, [space(), token("as"), space(), asname.format()])?; + let comments = f.context().comments().clone(); + let dangling = comments.dangling(item); + write!( + f, + [ + space(), + token("as"), + space(), + asname.format(), + trailing_comments(dangling), + ] + )?; } Ok(()) } From a7f95498915c84ddce365e973a658d5f2f8b6b62 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 30 Sep 2025 12:00:23 -0700 Subject: [PATCH 2/4] Basic test cases for comments in import statements --- .../fixtures/black/cases/import_comments.py | 21 +++++++++++++++++++ .../black/cases/import_comments.py.expect | 20 ++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py new file mode 100644 index 0000000000000..3244e9c47539c --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py @@ -0,0 +1,21 @@ +# ensure trailing comments are preserved +import x # comment +from x import a # comment +from x import a, b # comment +from x import a as b # comment +from x import a as b, b as c # comment + +# ensure intermixed end- and own-line comments are all preserved +# and at least kept in their original order, if not their original +# positions within the import statement +from x import ( # alpha + # bravo + a # charlie + # delta + as # echo + # foxtrot + b # golf + # hotel + , # india + # juliet +) # kilo diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect new file mode 100644 index 0000000000000..d850daf71550f --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect @@ -0,0 +1,20 @@ +# ensure trailing comments are preserved +import x # comment +from x import a # comment +from x import a, b # comment +from x import a as b # comment +from x import a as b, b as c # comment + +# ensure intermixed end- and own-line comments are all preserved +# and at least kept in their original order, if not their original +# positions within the import statement +from x import ( # alpha + # bravo + a as b, # charlie + # delta + # echo + # foxtrot # golf + # hotel + # india + # juliet +) # kilo From 2636902f57fda1c69d32ce0d684f2b80c888e148 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Fri, 3 Oct 2025 16:52:54 -0700 Subject: [PATCH 3/4] Revisit comment placement in ast and formatting --- .../fixtures/black/cases/import_comments.py | 12 +++-- .../black/cases/import_comments.py.expect | 21 ++++++--- .../src/comments/placement.rs | 46 ++++++++++++++++++- .../ruff_python_formatter/src/other/alias.rs | 43 +++++++++++++---- 4 files changed, 102 insertions(+), 20 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py index 3244e9c47539c..a36864dc29284 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py @@ -5,9 +5,15 @@ from x import a as b # comment from x import a as b, b as c # comment -# ensure intermixed end- and own-line comments are all preserved -# and at least kept in their original order, if not their original -# positions within the import statement +# ensure intermixed comments are all preserved +from x import ( # one + # two + a # three + # four + , # five + # six +) # seven + from x import ( # alpha # bravo a # charlie diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect index d850daf71550f..a36864dc29284 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect @@ -5,16 +5,23 @@ from x import a, b # comment from x import a as b # comment from x import a as b, b as c # comment -# ensure intermixed end- and own-line comments are all preserved -# and at least kept in their original order, if not their original -# positions within the import statement +# ensure intermixed comments are all preserved +from x import ( # one + # two + a # three + # four + , # five + # six +) # seven + from x import ( # alpha # bravo - a as b, # charlie + a # charlie # delta - # echo - # foxtrot # golf + as # echo + # foxtrot + b # golf # hotel - # india + , # india # juliet ) # kilo diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 0ec2aa6d2fbe3..083218f92c755 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -311,7 +311,10 @@ fn handle_enclosed_comment<'a>( AnyNodeRef::StmtClassDef(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, source) + } + AnyNodeRef::Alias(alias) => handle_alias_comment(comment, alias, source), AnyNodeRef::StmtWith(with_) => handle_with_comment(comment, with_), AnyNodeRef::ExprCall(_) => handle_call_comment(comment), AnyNodeRef::ExprStringLiteral(_) => match comment.enclosing_parent() { @@ -1936,6 +1939,7 @@ fn handle_bracketed_end_of_line_comment<'a>( fn handle_import_from_comment<'a>( comment: DecoratedComment<'a>, import_from: &'a ast::StmtImportFrom, + source: &str, ) -> CommentPlacement<'a> { // The comment needs to be on the same line, but before the first member. For example, we want // to treat this as a dangling comment: @@ -1963,10 +1967,48 @@ fn handle_import_from_comment<'a>( { CommentPlacement::dangling(comment.enclosing_node(), comment) } else { - CommentPlacement::Default(comment) + if let Some(SimpleToken { + kind: SimpleTokenKind::Comma, + .. + }) = SimpleTokenizer::starts_at(comment.start(), source) + .skip_trivia() + .next() + { + // treat comments before the comma as dangling, after as trailing (default) + if let Some(AnyNodeRef::Alias(alias)) = comment.preceding_node() { + CommentPlacement::dangling(alias, comment) + } else { + CommentPlacement::Default(comment) + } + } else { + CommentPlacement::Default(comment) + } } } +fn handle_alias_comment<'a>( + comment: DecoratedComment<'a>, + alias: &'a ruff_python_ast::Alias, + source: &str, +) -> CommentPlacement<'a> { + if let Some(asname) = &alias.asname { + if let Some(SimpleToken { + kind: SimpleTokenKind::As, + range: as_range, + }) = SimpleTokenizer::starts_at(alias.name.end(), source) + .skip_trivia() + .next() + { + return if comment.start() < as_range.start() { + CommentPlacement::trailing(&alias.name, comment) + } else { + CommentPlacement::leading(asname, comment) + }; + } + } + CommentPlacement::Default(comment) +} + /// Attach an enclosed end-of-line comment to a [`ast::StmtWith`]. /// /// For example, given: diff --git a/crates/ruff_python_formatter/src/other/alias.rs b/crates/ruff_python_formatter/src/other/alias.rs index 4ad9f5679873b..118194ce0157e 100644 --- a/crates/ruff_python_formatter/src/other/alias.rs +++ b/crates/ruff_python_formatter/src/other/alias.rs @@ -16,18 +16,45 @@ impl FormatNodeRule for FormatAlias { name, asname, } = item; - DotDelimitedIdentifier::new(name).fmt(f)?; + write!(f, [DotDelimitedIdentifier::new(name)])?; + + let comments = f.context().comments().clone(); + if comments.has_trailing(name) { + write!( + f, + [ + trailing_comments(comments.trailing(name)), + hard_line_break() + ] + )?; + } else if asname.is_some() { + write!(f, [space()])?; + } + if let Some(asname) = asname { - let comments = f.context().comments().clone(); - let dangling = comments.dangling(item); + write!(f, [token("as"),])?; + + if comments.has_leading(asname) { + write!( + f, + [ + trailing_comments(comments.leading(asname)), + hard_line_break() + ] + )?; + } else { + write!(f, [space()])?; + } + + write!(f, [asname.format()])?; + } + + if comments.has_dangling(item) { write!( f, [ - space(), - token("as"), - space(), - asname.format(), - trailing_comments(dangling), + trailing_comments(comments.dangling(item)), + hard_line_break() ] )?; } From 1936389425745ce37d9ca3f68e0666df2ca60cb1 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 6 Oct 2025 12:30:50 -0700 Subject: [PATCH 4/4] More documentation, more test cases, more black --- .../fixtures/black/cases/import_comments.py | 20 +++++++ .../black/cases/import_comments.py.expect | 19 ++++++ .../src/comments/placement.rs | 52 +++++++++++++++- .../ruff_python_formatter/src/other/alias.rs | 59 ++++++++++++++++--- 4 files changed, 139 insertions(+), 11 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py index a36864dc29284..d65ea7272088b 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py @@ -5,6 +5,26 @@ from x import a as b # comment from x import a as b, b as c # comment +from x import ( + a, # comment +) +from x import ( + a, # comment + b, +) + +# ensure comma is added +from x import ( + a # comment +) + +# follow black style by merging cases without own-line comments +from x import ( + a # alpha + , # beta + b, +) + # ensure intermixed comments are all preserved from x import ( # one # two diff --git a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect index a36864dc29284..a493a13d91624 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect +++ b/crates/ruff_python_formatter/resources/test/fixtures/black/cases/import_comments.py.expect @@ -5,6 +5,25 @@ from x import a, b # comment from x import a as b # comment from x import a as b, b as c # comment +from x import ( + a, # comment +) +from x import ( + a, # comment + b, +) + +# ensure comma is added +from x import ( + a, # comment +) + +# follow black style by merging cases without own-line comments +from x import ( + a, # alpha # beta + b, +) + # ensure intermixed comments are all preserved from x import ( # one # two diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 083218f92c755..234752ba5319b 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1925,7 +1925,7 @@ fn handle_bracketed_end_of_line_comment<'a>( CommentPlacement::Default(comment) } -/// Attach an enclosed end-of-line comment to a [`ast::StmtImportFrom`]. +/// Attach an enclosed comment to a [`ast::StmtImportFrom`]. /// /// For example, given: /// ```python @@ -1936,6 +1936,33 @@ fn handle_bracketed_end_of_line_comment<'a>( /// /// The comment will be attached to the [`ast::StmtImportFrom`] node as a dangling comment, to /// ensure that it remains on the same line as the [`ast::StmtImportFrom`] itself. +/// +/// If the comment's preceding node is an alias, and the comment is *before* a comma: +/// ```python +/// from foo import ( +/// bar as baz # comment +/// , +/// ) +/// ``` +/// +/// The comment will then be attached to the [`ast::Alias`] node as a dangling comment instead, +/// to ensure that it retains its position before the comma. +/// +/// Otherwise, if the comment is *after* the comma or before a following alias: +/// ```python +/// from foo import ( +/// bar as baz, # comment +/// ) +/// +/// from foo import ( +/// bar, +/// # comment +/// baz, +/// ) +/// ``` +/// +/// Then it will retain the default behavior of being attached to the relevant [`ast::Alias`] node +/// as either a leading or trailing comment. fn handle_import_from_comment<'a>( comment: DecoratedComment<'a>, import_from: &'a ast::StmtImportFrom, @@ -1974,7 +2001,7 @@ fn handle_import_from_comment<'a>( .skip_trivia() .next() { - // treat comments before the comma as dangling, after as trailing (default) + // Treat comments before the comma as dangling, after as trailing (default) if let Some(AnyNodeRef::Alias(alias)) = comment.preceding_node() { CommentPlacement::dangling(alias, comment) } else { @@ -1986,6 +2013,27 @@ fn handle_import_from_comment<'a>( } } +/// Attach an enclosed comment to the appropriate [`ast::Identifier`] within an [`ast::Alias`]. +/// +/// For example: +/// ```python +/// from foo import ( +/// bar # comment +/// as baz, +/// ) +/// ``` +/// +/// Will attach the comment as a trailing comment on the first name [`ast::Identifier`]. +/// +/// Whereas: +/// ```python +/// from foo import ( +/// bar as # comment +/// baz, +/// ) +/// ``` +/// +/// Will attach the comment as a leading comment on the second name [`ast::Identifier`]. fn handle_alias_comment<'a>( comment: DecoratedComment<'a>, alias: &'a ruff_python_ast::Alias, diff --git a/crates/ruff_python_formatter/src/other/alias.rs b/crates/ruff_python_formatter/src/other/alias.rs index 118194ce0157e..9b997565fa9a2 100644 --- a/crates/ruff_python_formatter/src/other/alias.rs +++ b/crates/ruff_python_formatter/src/other/alias.rs @@ -19,6 +19,13 @@ impl FormatNodeRule for FormatAlias { write!(f, [DotDelimitedIdentifier::new(name)])?; let comments = f.context().comments().clone(); + + // ```python + // from foo import ( + // bar # comment + // as baz, + // ) + // ``` if comments.has_trailing(name) { write!( f, @@ -32,8 +39,14 @@ impl FormatNodeRule for FormatAlias { } if let Some(asname) = asname { - write!(f, [token("as"),])?; + write!(f, [token("as")])?; + // ```python + // from foo import ( + // bar as # comment + // baz, + // ) + // ``` if comments.has_leading(asname) { write!( f, @@ -49,15 +62,43 @@ impl FormatNodeRule for FormatAlias { write!(f, [asname.format()])?; } - if comments.has_dangling(item) { - write!( - f, - [ - trailing_comments(comments.dangling(item)), - hard_line_break() - ] - )?; + // Dangling comment between alias and comma on a following line + // ```python + // from foo import ( + // bar # comment + // , + // ) + // ``` + let dangling = comments.dangling(item); + if !dangling.is_empty() { + write!(f, [trailing_comments(comments.dangling(item))])?; + + // Black will move the comma and merge comments if there is no own-line comment between + // the alias and the comma. + // + // Eg: + // ```python + // from foo import ( + // bar # one + // , # two + // ) + // ``` + // + // Will become: + // ```python + // from foo import ( + // bar, # one # two) + // ``` + // + // Only force a hard line break if an own-line dangling comment is present. + if dangling + .iter() + .any(|comment| comment.line_position().is_own_line()) + { + write!(f, [hard_line_break()])?; + } } + Ok(()) } }