mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-07-24 13:33:50 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Correctly associate own-line comments in bodies (#4671)

Browse source
This commit is contained in:
Micha Reiser 2023-06-01 08:12:53 +02:00 • committed by GitHub
parent 46c3b3af94
commit be31d71849
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 1747 additions and 144 deletions
Download patch file Download diff file Expand all files Collapse all files

13
crates/ruff_python_formatter/src/comments/debug.rs
View file

@ -1,5 +1,6 @@
use crate::comments::node_key::NodeRefEqualityKey;
use crate::comments::{CommentsMap, SourceComment};
use itertools::Itertools;
use ruff_formatter::SourceCode;
use ruff_python_ast::prelude::*;
use std::fmt::{Debug, Formatter, Write};
@ -53,7 +54,7 @@ impl Debug for DebugComments<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
let mut map = f.debug_map();
for node in self.comments.keys() {
for node in self.comments.keys().sorted_by_key(|key| key.node().start()) {
map.entry(
&NodeKindWithSource {
key: *node,
@ -176,19 +177,13 @@ impl Debug for DebugNodeCommentSlice<'_> {
#[cfg(test)]
mod tests {
use crate::comments::map::MultiMap;
use crate::comments::node_key::NodeRefEqualityKey;
use crate::comments::{
CommentTextPosition, Comments, CommentsData, CommentsMap, SourceComment,
};
use insta::_macro_support::assert_snapshot;
use insta::{assert_debug_snapshot, assert_snapshot};
use crate::comments::{CommentTextPosition, Comments, CommentsMap, SourceComment};
use insta::assert_debug_snapshot;
use ruff_formatter::SourceCode;
use ruff_python_ast::node::AnyNode;
use ruff_python_ast::source_code;
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{StmtBreak, StmtContinue};
use std::cell::Cell;
use std::rc::Rc;
#[test]
fn debug() {

24
crates/ruff_python_formatter/src/comments/map.rs
View file

@ -60,7 +60,7 @@ pub(super) struct MultiMap<K, V> {
}
impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
pub fn new() -> Self {
pub(super) fn new() -> Self {
Self {
index: FxHashMap::default(),
parts: Vec::new(),
@ -69,7 +69,7 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
}
/// Pushes a *leading* part for `key`.
pub fn push_leading(&mut self, key: K, part: V)
pub(super) fn push_leading(&mut self, key: K, part: V)
where
V: Clone,
{
@ -106,7 +106,7 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
}
/// Pushes a *dangling* part for `key`
pub fn push_dangling(&mut self, key: K, part: V)
pub(super) fn push_dangling(&mut self, key: K, part: V)
where
V: Clone,
{
@ -143,7 +143,7 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
}
/// Pushes a *trailing* part for `key`.
pub fn push_trailing(&mut self, key: K, part: V)
pub(super) fn push_trailing(&mut self, key: K, part: V)
where
V: Clone,
{
@ -210,14 +210,14 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
}
}
pub fn keys(&self) -> Keys<'_, K> {
pub(super) fn keys(&self) -> Keys<'_, K> {
Keys {
inner: self.index.keys(),
}
}
/// Returns the *leading* parts of `key` in insertion-order.
pub fn leading(&self, key: &K) -> &[V] {
pub(super) fn leading(&self, key: &K) -> &[V] {
match self.index.get(key) {
None => &[],
Some(Entry::InOrder(in_order)) => &self.parts[in_order.leading_range()],
@ -226,7 +226,7 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
}
/// Returns the *dangling* parts of `key` in insertion-order.
pub fn dangling(&self, key: &K) -> &[V] {
pub(super) fn dangling(&self, key: &K) -> &[V] {
match self.index.get(key) {
None => &[],
Some(Entry::InOrder(in_order)) => &self.parts[in_order.dangling_range()],
@ -235,7 +235,7 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
}
/// Returns the *trailing* parts of `key` in insertion order.
pub fn trailing(&self, key: &K) -> &[V] {
pub(super) fn trailing(&self, key: &K) -> &[V] {
match self.index.get(key) {
None => &[],
Some(Entry::InOrder(in_order)) => &self.parts[in_order.trailing_range()],
@ -244,12 +244,12 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
}
/// Returns `true` if `key` has any *leading*, *dangling*, or *trailing* parts.
pub fn has(&self, key: &K) -> bool {
pub(super) fn has(&self, key: &K) -> bool {
self.index.get(key).is_some()
}
/// Returns an iterator over the *leading*, *dangling*, and *trailing* parts of `key`.
pub fn parts(&self, key: &K) -> PartsIterator<V> {
pub(super) fn parts(&self, key: &K) -> PartsIterator<V> {
match self.index.get(key) {
None => PartsIterator::Slice([].iter()),
Some(entry) => PartsIterator::from_entry(entry, self),
@ -258,7 +258,7 @@ impl<K: std::hash::Hash + Eq, V> MultiMap<K, V> {
/// Returns an iterator over the parts of all keys.
#[allow(unused)]
pub fn all_parts(&self) -> impl Iterator<Item = &V> {
pub(super) fn all_parts(&self) -> impl Iterator<Item = &V> {
self.index
.values()
.flat_map(|entry| PartsIterator::from_entry(entry, self))
@ -767,7 +767,7 @@ impl PartIndex {
}
/// Iterator over the keys of a comments multi map
pub struct Keys<'a, K> {
pub(super) struct Keys<'a, K> {
inner: std::collections::hash_map::Keys<'a, K, Entry>,
}

162
crates/ruff_python_formatter/src/comments/mod.rs
View file

@ -1,5 +1,3 @@
#![allow(unused, unreachable_pub)] // TODO(micha): Remove after using the new comments infrastructure in the formatter.
//! Types for extracting and representing comments of a syntax tree.
//!
//! Most programming languages support comments allowing programmers to document their programs.
@ -91,7 +89,7 @@
use rustpython_parser::ast::Mod;
use std::cell::Cell;
use std::fmt::{Debug, Formatter};
use std::fmt::Debug;
use std::rc::Rc;
mod debug;
@ -121,6 +119,8 @@ pub(crate) struct SourceComment {
position: CommentTextPosition,
}
#[allow(unused)]
// TODO(micha): Remove after using the new comments infrastructure in the formatter.
impl SourceComment {
/// Returns the location of the comment in the original source code.
/// Allows retrieving the text of the comment.
@ -182,6 +182,8 @@ pub(crate) enum CommentTextPosition {
OwnLine,
}
#[allow(unused)]
// TODO(micha): Remove after using the new comments infrastructure in the formatter.
impl CommentTextPosition {
pub(crate) const fn is_own_line(self) -> bool {
matches!(self, CommentTextPosition::OwnLine)
@ -224,6 +226,8 @@ pub(crate) struct Comments<'a> {
data: Rc<CommentsData<'a>>,
}
#[allow(unused)]
// TODO(micha): Remove after using the new comments infrastructure in the formatter.
impl<'a> Comments<'a> {
fn new(comments: CommentsMap<'a>) -> Self {
Self {
@ -496,7 +500,15 @@ def test(x, y):
else:
print("Greater")
# Trailing comment
# trailing `else` comment
# Trailing `if` statement comment
def other(y, z):
if y == z:
pass
# Trailing `if` comment
# Trailing `other` function comment
test(10, 20)
"#;
@ -507,6 +519,148 @@ test(10, 20)
assert_debug_snapshot!(comments.debug(test_case.source_code));
}
#[test]
fn trailing_comment_after_single_statement_body() {
let source = r#"
if x == y: pass
# Test
print("test")
"#;
let test_case = CommentsTestCase::from_code(source);
let comments = test_case.to_comments();
assert_debug_snapshot!(comments.debug(test_case.source_code));
}
#[test]
fn if_elif_else_comments() {
let source = r#"
if x == y:
pass # trailing `pass` comment
# Root `if` trailing comment
# Leading elif comment
elif x < y:
pass
# `elif` trailing comment
# Leading else comment
else:
pass
# `else` trailing comment
"#;
let test_case = CommentsTestCase::from_code(source);
let comments = test_case.to_comments();
assert_debug_snapshot!(comments.debug(test_case.source_code));
}
#[test]
fn if_elif_if_else_comments() {
let source = r#"
if x == y:
pass
elif x < y:
if x < 10:
pass
# `elif` trailing comment
# Leading else comment
else:
pass
"#;
let test_case = CommentsTestCase::from_code(source);
let comments = test_case.to_comments();
assert_debug_snapshot!(comments.debug(test_case.source_code));
}
#[test]
fn try_except_finally_else() {
let source = r#"
try:
pass
# trailing try comment
# leading handler comment
except Exception as ex:
pass
# Trailing except comment
# leading else comment
else:
pass
# trailing else comment
# leading finally comment
finally:
print("Finally!")
# Trailing finally comment
"#;
let test_case = CommentsTestCase::from_code(source);
let comments = test_case.to_comments();
assert_debug_snapshot!(comments.debug(test_case.source_code));
}
#[test]
fn try_except() {
let source = r#"
def test():
try:
pass
# trailing try comment
# leading handler comment
except Exception as ex:
pass
# Trailing except comment
# Trailing function comment
print("Next statement");
"#;
let test_case = CommentsTestCase::from_code(source);
let comments = test_case.to_comments();
assert_debug_snapshot!(comments.debug(test_case.source_code));
}
// Issue: Match cases
#[test]
fn match_cases() {
let source = r#"def make_point_3d(pt):
match pt:
# Leading `case(x, y)` comment
case (x, y):
return Point3d(x, y, 0)
# Trailing `case(x, y) comment
# Leading `case (x, y, z)` comment
case (x, y, z):
if x < y:
print("if")
else:
print("else")
# Trailing else comment
# trailing case comment
case Point2d(x, y):
return Point3d(x, y, 0)
case _:
raise TypeError("not a point we support")
# Trailing last case comment
# Trailing match comment
# After match comment
print("other")
"#;
let test_case = CommentsTestCase::from_code(source);
let comments = test_case.to_comments();
assert_debug_snapshot!(comments.debug(test_case.source_code));
}
#[test]
fn leading_most_outer() {
let source = r#"

85
crates/ruff_python_formatter/src/comments/node_key.rs
View file

@ -1,7 +1,6 @@
use ruff_python_ast::node::AnyNodeRef;
use std::fmt::{Debug, Formatter};
use std::hash::{Hash, Hasher};
use std::ptr::NonNull;
/// Used as key into the [`MultiMap`] storing the comments per node by [`Comments`].
///
@ -22,86 +21,6 @@ impl<'a> NodeRefEqualityKey<'a> {
pub(super) fn node(&self) -> AnyNodeRef {
self.node
}
fn ptr(self) -> NonNull<()> {
match self.node {
AnyNodeRef::ModModule(node) => NonNull::from(node).cast(),
AnyNodeRef::ModInteractive(node) => NonNull::from(node).cast(),
AnyNodeRef::ModExpression(node) => NonNull::from(node).cast(),
AnyNodeRef::ModFunctionType(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtFunctionDef(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtAsyncFunctionDef(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtClassDef(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtReturn(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtDelete(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtAssign(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtAugAssign(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtAnnAssign(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtFor(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtAsyncFor(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtWhile(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtIf(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtWith(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtAsyncWith(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtMatch(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtRaise(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtTry(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtTryStar(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtAssert(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtImport(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtImportFrom(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtGlobal(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtNonlocal(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtExpr(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtPass(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtBreak(node) => NonNull::from(node).cast(),
AnyNodeRef::StmtContinue(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprBoolOp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprNamedExpr(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprBinOp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprUnaryOp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprLambda(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprIfExp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprDict(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprSet(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprListComp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprSetComp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprDictComp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprGeneratorExp(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprAwait(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprYield(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprYieldFrom(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprCompare(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprCall(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprFormattedValue(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprJoinedStr(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprConstant(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprAttribute(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprSubscript(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprStarred(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprName(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprList(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprTuple(node) => NonNull::from(node).cast(),
AnyNodeRef::ExprSlice(node) => NonNull::from(node).cast(),
AnyNodeRef::ExcepthandlerExceptHandler(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchValue(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchSingleton(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchSequence(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchMapping(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchClass(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchStar(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchAs(node) => NonNull::from(node).cast(),
AnyNodeRef::PatternMatchOr(node) => NonNull::from(node).cast(),
AnyNodeRef::TypeIgnoreTypeIgnore(node) => NonNull::from(node).cast(),
AnyNodeRef::Comprehension(node) => NonNull::from(node).cast(),
AnyNodeRef::Arguments(node) => NonNull::from(node).cast(),
AnyNodeRef::Arg(node) => NonNull::from(node).cast(),
AnyNodeRef::Keyword(node) => NonNull::from(node).cast(),
AnyNodeRef::Alias(node) => NonNull::from(node).cast(),
AnyNodeRef::Withitem(node) => NonNull::from(node).cast(),
AnyNodeRef::MatchCase(node) => NonNull::from(node).cast(),
}
}
}
impl Debug for NodeRefEqualityKey<'_> {
@ -112,7 +31,7 @@ impl Debug for NodeRefEqualityKey<'_> {
impl PartialEq for NodeRefEqualityKey<'_> {
fn eq(&self, other: &Self) -> bool {
self.ptr().eq(&other.ptr())
self.node.ptr_eq(other.node)
}
}
@ -120,7 +39,7 @@ impl Eq for NodeRefEqualityKey<'_> {}
impl Hash for NodeRefEqualityKey<'_> {
fn hash<H: Hasher>(&self, state: &mut H) {
self.ptr().hash(state);
self.node.as_ptr().hash(state);
}
}

459
crates/ruff_python_formatter/src/comments/placement.rs
View file

@ -1,13 +1,464 @@
use crate::comments::visitor::{CommentPlacement, DecoratedComment};
use crate::comments::{CommentTextPosition, SourceComment};
use ruff_formatter::{SourceCode, SourceCodeSlice};
use ruff_python_ast::node::AnyNodeRef;
use std::cell::Cell;
use ruff_python_ast::source_code::Locator;
use ruff_python_ast::whitespace;
use std::cmp::Ordering;
/// Implements the custom comment placement logic.
pub(super) fn place_comment<'a>(
comment: DecoratedComment<'a>,
_source_code: SourceCode,
locator: &Locator,
) -> CommentPlacement<'a> {
handle_in_between_excepthandlers_or_except_handler_and_else_or_finally_comment(comment, locator)
.or_else(|comment| handle_match_comment(comment, locator))
.or_else(|comment| handle_in_between_bodies_comment(comment, locator))
.or_else(|comment| handle_trailing_body_comment(comment, locator))
}
/// Handles leading comments in front of a match case or a trailing comment of the `match` statement.
/// ```python
/// match pt:
/// # Leading `case(x, y)` comment
/// case (x, y):
/// return Point3d(x, y, 0)
/// # Leading `case (x, y, z)` comment
/// case _:
/// ```
fn handle_match_comment<'a>(
comment: DecoratedComment<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
// Must be an own line comment after the last statement in a match case
if comment.text_position().is_end_of_line() || comment.following_node().is_some() {
return CommentPlacement::Default(comment);
}
// Get the enclosing match case
let Some(match_case) = comment.enclosing_node().match_case() else {
return CommentPlacement::Default(comment)
};
// And its parent match statement.
let Some(match_stmt) = comment
.enclosing_parent()
.and_then(AnyNodeRef::stmt_match) else {
return CommentPlacement::Default(comment)
};
// Get the next sibling (sibling traversal would be really nice)
let current_case_index = match_stmt
.cases
.iter()
.position(|case| case == match_case)
.expect("Expected case to belong to parent match statement.");
let next_case = match_stmt.cases.get(current_case_index + 1);
let comment_indentation =
whitespace::indentation_at_offset(locator, comment.slice().range().start())
.map(str::len)
.unwrap_or_default();
let match_case_indentation = whitespace::indentation(locator, match_case).unwrap().len();
if let Some(next_case) = next_case {
// The comment's indentation is less or equal to the `case` indention and there's a following
// `case` arm.
// ```python
// match pt:
// case (x, y):
// return Point3d(x, y, 0)
// # Leading `case (x, y, z)` comment
// case _:
// pass
// ```
// Attach the `comment` as leading comment to the next case.
if comment_indentation <= match_case_indentation {
CommentPlacement::leading(next_case.into(), comment)
} else {
// Otherwise, delegate to `handle_trailing_body_comment`
// ```python
// match pt:
// case (x, y):
// return Point3d(x, y, 0)
// # Trailing case body comment
// case _:
// pass
// ```
CommentPlacement::Default(comment)
}
} else {
// Comment after the last statement in a match case...
let match_stmt_indentation = whitespace::indentation(locator, match_stmt)
.unwrap_or_default()
.len();
if comment_indentation <= match_case_indentation
&& comment_indentation > match_stmt_indentation
{
// The comment's indent matches the `case` indent (or is larger than the `match`'s indent).
// ```python
// match pt:
// case (x, y):
// return Point3d(x, y, 0)
// case _:
// pass
// # Trailing match comment
// ```
// This is a trailing comment of the last case.
CommentPlacement::trailing(match_case.into(), comment)
} else {
// Delegate to `handle_trailing_body_comment` because it's either a trailing indent
// for the last statement in the `case` body or a comment for the parent of the `match`
//
// ```python
// match pt:
// case (x, y):
// return Point3d(x, y, 0)
// case _:
// pass
// # trailing case comment
// ```
CommentPlacement::Default(comment)
}
}
}
/// Handles comments between excepthandlers and between the last except handler and any following `else` or `finally` block.
fn handle_in_between_excepthandlers_or_except_handler_and_else_or_finally_comment<'a>(
comment: DecoratedComment<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
if comment.text_position().is_end_of_line() || comment.following_node().is_none() {
return CommentPlacement::Default(comment);
}
if let Some(AnyNodeRef::ExcepthandlerExceptHandler(except_handler)) = comment.preceding_node() {
// it now depends on the indentation level of the comment if it is a leading comment for e.g.
// the following `elif` or indeed a trailing comment of the previous body's last statement.
let comment_indentation =
whitespace::indentation_at_offset(locator, comment.slice().range().start())
.map(str::len)
.unwrap_or_default();
if let Some(except_indentation) =
whitespace::indentation(locator, except_handler).map(str::len)
{
return if comment_indentation <= except_indentation {
// It has equal, or less indent than the `except` handler. It must be a comment
// of the following `finally` or `else` block
//
// ```python
// try:
// pass
// except Exception:
// print("noop")
// # leading
// finally:
// pass
// ```
// Attach it to the `try` statement.
CommentPlacement::dangling(comment.enclosing_node(), comment)
} else {
// Delegate to `handle_trailing_body_comment`
CommentPlacement::Default(comment)
};
}
}
CommentPlacement::Default(comment)
}
/// Handles comments between the last statement and the first statement of two bodies.
///
/// ```python
/// if x == y:
/// pass
/// # This should be a trailing comment of `pass` and not a leading comment of the `print`
/// # in the `else` branch
/// else:
/// print("I have no comments")
/// ```
fn handle_in_between_bodies_comment<'a>(
comment: DecoratedComment<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
use ruff_python_ast::prelude::*;
// The rule only applies to own line comments. The default logic associates end of line comments
// correctly.
if comment.text_position().is_end_of_line() {
return CommentPlacement::Default(comment);
}
// The comment must be between two statements...
if let (Some(preceding), Some(following)) = (comment.preceding_node(), comment.following_node())
{
// ...and the following statement must be the first statement in an alternate body of the parent...
let is_following_the_first_statement_in_a_parents_alternate_body =
match comment.enclosing_node() {
AnyNodeRef::StmtIf(StmtIf { orelse, .. })
| AnyNodeRef::StmtFor(StmtFor { orelse, .. })
| AnyNodeRef::StmtAsyncFor(StmtAsyncFor { orelse, .. })
| AnyNodeRef::StmtWhile(StmtWhile { orelse, .. }) => {
are_same_optional(following, orelse.first())
}
AnyNodeRef::StmtTry(StmtTry {
handlers,
orelse,
finalbody,
..
})
| AnyNodeRef::StmtTryStar(StmtTryStar {
handlers,
orelse,
finalbody,
..
}) => {
are_same_optional(following, handlers.first())
// Comments between the handlers and the `else`, or comments between the `handlers` and the `finally`
// are already handled by `handle_in_between_excepthandlers_or_except_handler_and_else_or_finally_comment`
|| handlers.is_empty() && are_same_optional(following, orelse.first())
|| (handlers.is_empty() || !orelse.is_empty())
&& are_same_optional(following, finalbody.first())
}
_ => false,
};
if !is_following_the_first_statement_in_a_parents_alternate_body {
// ```python
// if test:
// a
// # comment
// b
// ```
return CommentPlacement::Default(comment);
}
// it now depends on the indentation level of the comment if it is a leading comment for e.g.
// the following `elif` or indeed a trailing comment of the previous body's last statement.
let comment_indentation =
whitespace::indentation_at_offset(locator, comment.slice().range().start())
.map(str::len)
.unwrap_or_default();
if let Some(preceding_indentation) =
whitespace::indentation(locator, &preceding).map(str::len)
{
return if comment_indentation >= preceding_indentation {
// `# comment` has the same or a larger indent than the `pass` statement.
// It likely is a trailing comment of the `pass` statement.
// ```python
// if x == y:
// pass
// # comment
// else:
// print("noop")
// ```
CommentPlacement::trailing(preceding, comment)
} else {
// Otherwise it has less indent than the previous statement. Meaning that it is a leading comment
// of the following block.
//
// ```python
// if x == y:
// pass
// # I'm a leading comment of the `elif` statement.
// elif:
// print("nooop")
// ```
if following.is_stmt_if() || following.is_except_handler() {
// The `elif` or except handlers have their own body to which we can attach the leading comment
CommentPlacement::leading(following, comment)
} else {
// There are no bodies for the "else" branch and other bodies that are represented as a `Vec<Stmt>`.
// This means, there's no good place to attach the comments to.
// That's why we make these dangling comments and format them manually
// in the enclosing node's formatting logic. For `try`, it's the formatters responsibility
// to correctly identify the comments for the `finally` and `orelse` block by looking
// at the comment's range.
//
// ```python
// if x == y:
// pass
// # I'm a leading comment of the `else` branch but there's no `else` node.
// else:
// print("nooop")
// ```
CommentPlacement::dangling(comment.enclosing_node(), comment)
}
};
}
}
CommentPlacement::Default(comment)
}
/// Handles trailing comments at the end of a body block (or any other block that is indented).
/// ```python
/// def test():
/// pass
/// # This is a trailing comment that belongs to the function `test`
/// # and not to the next statement.
///
/// print("I have no comments")
/// ```
fn handle_trailing_body_comment<'a>(
comment: DecoratedComment<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
if comment.text_position().is_end_of_line() {
return CommentPlacement::Default(comment);
}
// Only do something if the preceding node has a body (has indented statements).
let Some(last_child) = comment.preceding_node().and_then(last_child_in_body) else {
return CommentPlacement::Default(comment);
};
let Some(comment_indentation) = whitespace::indentation_at_offset(locator, comment.slice().range().start()) else {
// The comment can't be a comment for the previous block if it isn't indented..
return CommentPlacement::Default(comment);
};
// We only care about the length because indentations with mixed spaces and tabs are only valid if
// the indent-level doesn't depend on the tab width (the indent level must be the same if the tab width is 1 or 8).
let comment_indentation_len = comment_indentation.len();
let mut current_child = last_child;
let mut parent_body = comment.preceding_node();
let mut grand_parent_body = None;
loop {
let child_indentation = whitespace::indentation(locator, &current_child)
.map(str::len)
.unwrap_or_default();
match comment_indentation_len.cmp(&child_indentation) {
Ordering::Less => {
break if let Some(parent_block) = grand_parent_body {
// Comment belongs to the parent block.
// ```python
// if test:
// if other:
// pass
// # comment
// ```
CommentPlacement::trailing(parent_block, comment)
} else {
// The comment does not belong to this block.
// ```python
// if test:
// pass
// # comment
// ```
CommentPlacement::Default(comment)
};
}
Ordering::Equal => {
// The comment belongs to this block.
// ```python
// if test:
// pass
// # comment
// ```
break CommentPlacement::trailing(current_child, comment);
}
Ordering::Greater => {
if let Some(nested_child) = last_child_in_body(current_child) {
// The comment belongs to the inner block.
// ```python
// def a():
// if test:
// pass
// # comment
// ```
// Comment belongs to the `if`'s inner body
grand_parent_body = parent_body;
parent_body = Some(current_child);
current_child = nested_child;
} else {
// The comment belongs to this block.
// ```python
// if test:
// pass
// # comment
// ```
break CommentPlacement::trailing(current_child, comment);
}
}
}
}
}
fn are_same_optional<'a, T>(left: AnyNodeRef, right: Option<T>) -> bool
where
T: Into<AnyNodeRef<'a>>,
{
right.map_or(false, |right| left.ptr_eq(right.into()))
}
fn last_child_in_body(node: AnyNodeRef) -> Option<AnyNodeRef> {
use ruff_python_ast::prelude::*;
let body = match node {
AnyNodeRef::StmtFunctionDef(StmtFunctionDef { body, .. })
| AnyNodeRef::StmtAsyncFunctionDef(StmtAsyncFunctionDef { body, .. })
| AnyNodeRef::StmtClassDef(StmtClassDef { body, .. })
| AnyNodeRef::StmtWith(StmtWith { body, .. })
| AnyNodeRef::StmtAsyncWith(StmtAsyncWith { body, .. })
| AnyNodeRef::MatchCase(MatchCase { body, .. })
| AnyNodeRef::ExcepthandlerExceptHandler(ExcepthandlerExceptHandler { body, .. }) => body,
AnyNodeRef::StmtIf(StmtIf { body, orelse, .. })
| AnyNodeRef::StmtFor(StmtFor { body, orelse, .. })
| AnyNodeRef::StmtAsyncFor(StmtAsyncFor { body, orelse, .. })
| AnyNodeRef::StmtWhile(StmtWhile { body, orelse, .. }) => {
if orelse.is_empty() {
body
} else {
orelse
}
}
AnyNodeRef::StmtMatch(StmtMatch { cases, .. }) => {
return cases.last().map(AnyNodeRef::from)
}
AnyNodeRef::StmtTry(StmtTry {
body,
handlers,
orelse,
finalbody,
..
})
| AnyNodeRef::StmtTryStar(StmtTryStar {
body,
handlers,
orelse,
finalbody,
..
}) => {
if finalbody.is_empty() {
if orelse.is_empty() {
if handlers.is_empty() {
body
} else {
return handlers.last().map(AnyNodeRef::from);
}
} else {
orelse
}
} else {
finalbody
}
}
// Not a node that contains an indented child node.
_ => return None,
};
body.last().map(AnyNodeRef::from)
}

77
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap Normal file
View file

@ -0,0 +1,77 @@
---
source: crates/ruff_python_formatter/src/comments/mod.rs
expression: comments.debug(test_case.source_code)
---
{
Node {
kind: StmtPass,
range: 16..20,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# trailing `pass` comment",
position: EndOfLine,
formatted: false,
},
SourceComment {
text: "# Root `if` trailing comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtIf,
range: 104..192,
source: `elif x < y:⏎`,
}: {
"leading": [
SourceComment {
text: "# Leading elif comment",
position: OwnLine,
formatted: false,
},
],
"dangling": [
SourceComment {
text: "# Leading else comment",
position: OwnLine,
formatted: false,
},
],
"trailing": [],
},
Node {
kind: StmtPass,
range: 120..124,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# `elif` trailing comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtPass,
range: 188..192,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# `else` trailing comment",
position: OwnLine,
formatted: false,
},
],
},
}

36
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap Normal file
View file

@ -0,0 +1,36 @@
---
source: crates/ruff_python_formatter/src/comments/mod.rs
expression: comments.debug(test_case.source_code)
---
{
Node {
kind: StmtIf,
range: 21..128,
source: `elif x < y:⏎`,
}: {
"leading": [],
"dangling": [
SourceComment {
text: "# Leading else comment",
position: OwnLine,
formatted: false,
},
],
"trailing": [],
},
Node {
kind: StmtIf,
range: 37..60,
source: `if x < 10:⏎`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# `elif` trailing comment",
position: OwnLine,
formatted: false,
},
],
},
}

126
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__match_cases.snap Normal file
View file

@ -0,0 +1,126 @@
---
source: crates/ruff_python_formatter/src/comments/mod.rs
expression: comments.debug(test_case.source_code)
---
{
Node {
kind: MatchCase,
range: 84..132,
source: `case (x, y):⏎`,
}: {
"leading": [
SourceComment {
text: "# Leading `case(x, y)` comment",
position: OwnLine,
formatted: false,
},
],
"dangling": [],
"trailing": [],
},
Node {
kind: StmtReturn,
range: 109..132,
source: `return Point3d(x, y, 0)`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing `case(x, y) comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: MatchCase,
range: 227..340,
source: `case (x, y, z):⏎`,
}: {
"leading": [
SourceComment {
text: "# Leading `case (x, y, z)` comment",
position: OwnLine,
formatted: false,
},
],
"dangling": [],
"trailing": [],
},
Node {
kind: StmtIf,
range: 255..340,
source: `if x < y:⏎`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# trailing case comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtExpr,
range: 327..340,
source: `print("else")`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing else comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: MatchCase,
range: 489..550,
source: `case _:⏎`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing match comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtRaise,
range: 509..550,
source: `raise TypeError("not a poin... support")`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing last case comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtExpr,
range: 656..670,
source: `print("other")`,
}: {
"leading": [
SourceComment {
text: "# After match comment",
position: OwnLine,
formatted: false,
},
],
"dangling": [],
"trailing": [],
},
}

21
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_comment_after_single_statement_body.snap Normal file
View file

@ -0,0 +1,21 @@
---
source: crates/ruff_python_formatter/src/comments/mod.rs
expression: comments.debug(test_case.source_code)
---
{
Node {
kind: StmtPass,
range: 12..16,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Test",
position: OwnLine,
formatted: false,
},
],
},
}

57
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap
View file

@ -4,18 +4,63 @@ expression: comments.debug(test_case.source_code)
---
{
Node {
kind: StmtExpr,
range: 143..155,
source: `test(10, 20)`,
kind: StmtIf,
range: 21..117,
source: `if x == y:⏎`,
}: {
"leading": [
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing comment",
text: "# Trailing `if` statement comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtExpr,
range: 101..117,
source: `print("Greater")`,
}: {
"leading": [],
"dangling": [],
"trailing": [],
"trailing": [
SourceComment {
text: "# trailing `else` comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtFunctionDef,
range: 193..237,
source: `def other(y, z):⏎`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing `other` function comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtPass,
range: 233..237,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing `if` comment",
position: OwnLine,
formatted: false,
},
],
},
}

30
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_most_outer_nested.snap
View file

@ -3,21 +3,6 @@ source: crates/ruff_python_formatter/src/comments/mod.rs
expression: comments.debug(test_case.source_code)
---
{
Node {
kind: ExprConstant,
range: 11..12,
source: `3`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# trailing comment",
position: EndOfLine,
formatted: false,
},
],
},
Node {
kind: StmtExpr,
range: 1..33,
@ -33,4 +18,19 @@ expression: comments.debug(test_case.source_code)
},
],
},
Node {
kind: ExprConstant,
range: 11..12,
source: `3`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# trailing comment",
position: EndOfLine,
formatted: false,
},
],
},
}

66
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except.snap Normal file
View file

@ -0,0 +1,66 @@
---
source: crates/ruff_python_formatter/src/comments/mod.rs
expression: comments.debug(test_case.source_code)
---
{
Node {
kind: StmtTry,
range: 17..136,
source: `try:⏎`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing function comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtPass,
range: 30..34,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# trailing try comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: ExcepthandlerExceptHandler,
range: 100..136,
source: `except Exception as ex:⏎`,
}: {
"leading": [
SourceComment {
text: "# leading handler comment",
position: OwnLine,
formatted: false,
},
],
"dangling": [],
"trailing": [],
},
Node {
kind: StmtPass,
range: 132..136,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing except comment",
position: OwnLine,
formatted: false,
},
],
},
}

101
crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except_finally_else.snap Normal file
View file

@ -0,0 +1,101 @@
---
source: crates/ruff_python_formatter/src/comments/mod.rs
expression: comments.debug(test_case.source_code)
---
{
Node {
kind: StmtTry,
range: 1..253,
source: `try:⏎`,
}: {
"leading": [],
"dangling": [
SourceComment {
text: "# leading else comment",
position: OwnLine,
formatted: false,
},
SourceComment {
text: "# leading finally comment",
position: OwnLine,
formatted: false,
},
],
"trailing": [],
},
Node {
kind: StmtPass,
range: 10..14,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# trailing try comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: ExcepthandlerExceptHandler,
range: 68..100,
source: `except Exception as ex:⏎`,
}: {
"leading": [
SourceComment {
text: "# leading handler comment",
position: OwnLine,
formatted: false,
},
],
"dangling": [],
"trailing": [],
},
Node {
kind: StmtPass,
range: 96..100,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing except comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtPass,
range: 164..168,
source: `pass`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# trailing else comment",
position: OwnLine,
formatted: false,
},
],
},
Node {
kind: StmtExpr,
range: 236..253,
source: `print("Finally!")`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# Trailing finally comment",
position: OwnLine,
formatted: false,
},
],
},
}

26
crates/ruff_python_formatter/src/comments/visitor.rs
View file

@ -1,11 +1,10 @@
use crate::comments::map::MultiMap;
use crate::comments::node_key::NodeRefEqualityKey;
use crate::comments::placement::place_comment;
use crate::comments::{CommentTextPosition, CommentsMap, SourceComment};
use ruff_formatter::{SourceCode, SourceCodeSlice};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::prelude::*;
use ruff_python_ast::source_code::CommentRanges;
use ruff_python_ast::source_code::{CommentRanges, Locator};
use std::cell::Cell;
// The interface is designed to only export the members relevant for iterating nodes in
// pre-order.
@ -13,7 +12,6 @@ use std::cell::Cell;
use ruff_python_ast::visitor::preorder::*;
use ruff_python_ast::whitespace::is_python_whitespace;
use ruff_text_size::TextRange;
use std::cmp::Ordering;
use std::iter::Peekable;
/// Visitor extracting the comments from an AST.
@ -68,12 +66,15 @@ impl<'a> CommentsVisitor<'a> {
enclosing: enclosing_node,
preceding: self.preceding_node,
following: Some(node),
parent: self.parents.iter().rev().nth(1).copied(),
text_position: text_position(*comment_range, self.source_code),
slice: self.source_code.slice(*comment_range),
};
self.builder
.add_comment(place_comment(comment, self.source_code));
self.builder.add_comment(place_comment(
comment,
&Locator::new(self.source_code.as_str()),
));
self.comment_ranges.next();
}
@ -121,13 +122,16 @@ impl<'a> CommentsVisitor<'a> {
let comment = DecoratedComment {
enclosing: node,
preceding: self.preceding_node,
parent: self.parents.last().copied(),
following: None,
text_position: text_position(*comment_range, self.source_code),
slice: self.source_code.slice(*comment_range),
};
self.builder
.add_comment(place_comment(comment, self.source_code));
self.builder.add_comment(place_comment(
comment,
&Locator::new(self.source_code.as_str()),
));
self.comment_ranges.next();
}
@ -135,7 +139,7 @@ impl<'a> CommentsVisitor<'a> {
self.preceding_node = Some(node);
}
fn finish(mut self) -> CommentsMap<'a> {
fn finish(self) -> CommentsMap<'a> {
self.builder.finish()
}
}
@ -264,6 +268,7 @@ pub(super) struct DecoratedComment<'a> {
enclosing: AnyNodeRef<'a>,
preceding: Option<AnyNodeRef<'a>>,
following: Option<AnyNodeRef<'a>>,
parent: Option<AnyNodeRef<'a>>,
text_position: CommentTextPosition,
slice: SourceCodeSlice,
}
@ -289,6 +294,11 @@ impl<'a> DecoratedComment<'a> {
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.
pub(super) fn slice(&self) -> &SourceCodeSlice {
&self.slice