Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# 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

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
a # three
# four
, # five
# six
) # seven

from x import ( # alpha
# bravo
a # charlie
# delta
as # echo
# foxtrot
b # golf
# hotel
, # india
# juliet
) # kilo
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# 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

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
a # three
# four
, # five
# six
) # seven

from x import ( # alpha
# bravo
a # charlie
# delta
as # echo
# foxtrot
b # golf
# hotel
, # india
# juliet
) # kilo
96 changes: 93 additions & 3 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -1922,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
Expand All @@ -1933,9 +1936,37 @@ 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,
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:
Expand Down Expand Up @@ -1963,10 +1994,69 @@ 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()
{
Comment on lines +1997 to +2003
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The place_comments logic tends to be complicated and its often non obvious what cases each branch is handling. That's why we tend to add inline comments that show examples of the patterns they match on (see the comments above).

Could you add such comments for this branch and to handle_alias_comment too?

// this should be a dangling comment but only if it comes before the `,`
// ```python
// from foo import (
//      baz # comment     
//      , 
//      bar 
// )
// ```

// 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)
}
}
}

/// 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,
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:
Expand Down
84 changes: 82 additions & 2 deletions crates/ruff_python_formatter/src/other/alias.rs
Original file line number Diff line number Diff line change
@@ -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::*;

Expand All @@ -15,10 +16,89 @@ impl FormatNodeRule<Alias> for FormatAlias {
name,
asname,
} = item;
DotDelimitedIdentifier::new(name).fmt(f)?;
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,
[
trailing_comments(comments.trailing(name)),
hard_line_break()
]
)?;
} else if asname.is_some() {
write!(f, [space()])?;
}

if let Some(asname) = asname {
write!(f, [space(), token("as"), space(), asname.format()])?;
write!(f, [token("as")])?;

// ```python
// from foo import (
// bar as # comment
// baz,
// )
// ```
if comments.has_leading(asname) {
write!(
f,
[
trailing_comments(comments.leading(asname)),
hard_line_break()
]
)?;
} else {
write!(f, [space()])?;
}

write!(f, [asname.format()])?;
}

// 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(())
}
}