From be31d71849a97a56c7d903e3a5f349e74d7f8e3b Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 1 Jun 2023 08:12:53 +0200 Subject: [PATCH] Correctly associate own-line comments in bodies (#4671) --- crates/ruff_python_ast/src/node.rs | 596 ++++++++++++++++++ crates/ruff_python_ast/src/whitespace.rs | 12 +- .../src/comments/debug.rs | 13 +- .../ruff_python_formatter/src/comments/map.rs | 24 +- .../ruff_python_formatter/src/comments/mod.rs | 162 ++++- .../src/comments/node_key.rs | 85 +-- .../src/comments/placement.rs | 459 +++++++++++++- ...omments__tests__if_elif_else_comments.snap | 77 +++ ...ents__tests__if_elif_if_else_comments.snap | 36 ++ ...rmatter__comments__tests__match_cases.snap | 126 ++++ ...g_comment_after_single_statement_body.snap | 21 + ...nts__tests__trailing_function_comment.snap | 57 +- ...ts__tests__trailing_most_outer_nested.snap | 30 +- ...ormatter__comments__tests__try_except.snap | 66 ++ ...ments__tests__try_except_finally_else.snap | 101 +++ .../src/comments/visitor.rs | 26 +- 16 files changed, 1747 insertions(+), 144 deletions(-) create mode 100644 crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap create mode 100644 crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap create mode 100644 crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__match_cases.snap create mode 100644 crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_comment_after_single_statement_body.snap create mode 100644 crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except.snap create mode 100644 crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except_finally_else.snap diff --git a/crates/ruff_python_ast/src/node.rs b/crates/ruff_python_ast/src/node.rs index 7996b2b7cd..b1fd7516bc 100644 --- a/crates/ruff_python_ast/src/node.rs +++ b/crates/ruff_python_ast/src/node.rs @@ -1,5 +1,6 @@ use crate::prelude::*; use ruff_text_size::TextRange; +use std::ptr::NonNull; pub trait AstNode: Ranged { fn cast(kind: AnyNode) -> Option @@ -574,6 +575,30 @@ impl AnyNode { } } + pub const fn is_statement(&self) -> bool { + self.as_ref().is_statement() + } + + pub const fn is_expression(&self) -> bool { + self.as_ref().is_expression() + } + + pub const fn is_module(&self) -> bool { + self.as_ref().is_module() + } + + pub const fn is_pattern(&self) -> bool { + self.as_ref().is_pattern() + } + + pub const fn is_except_handler(&self) -> bool { + self.as_ref().is_except_handler() + } + + pub const fn is_type_ignore(&self) -> bool { + self.as_ref().is_type_ignore() + } + pub const fn as_ref(&self) -> AnyNodeRef { match self { Self::ModModule(node) => AnyNodeRef::ModModule(node), @@ -2878,6 +2903,91 @@ pub enum AnyNodeRef<'a> { } impl AnyNodeRef<'_> { + pub fn as_ptr(&self) -> NonNull<()> { + match self { + 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(), + } + } + + /// Compares two any node refs by their pointers (referential equality). + pub fn ptr_eq(self, other: AnyNodeRef) -> bool { + self.as_ptr().eq(&other.as_ptr()) + } + /// Returns the node's [`kind`](NodeKind) that has no data associated and is [`Copy`]. pub const fn kind(self) -> NodeKind { match self { @@ -2958,6 +3068,492 @@ impl AnyNodeRef<'_> { AnyNodeRef::MatchCase(_) => NodeKind::MatchCase, } } + + pub const fn is_statement(self) -> bool { + match self { + AnyNodeRef::StmtFunctionDef(_) + | AnyNodeRef::StmtAsyncFunctionDef(_) + | AnyNodeRef::StmtClassDef(_) + | AnyNodeRef::StmtReturn(_) + | AnyNodeRef::StmtDelete(_) + | AnyNodeRef::StmtAssign(_) + | AnyNodeRef::StmtAugAssign(_) + | AnyNodeRef::StmtAnnAssign(_) + | AnyNodeRef::StmtFor(_) + | AnyNodeRef::StmtAsyncFor(_) + | AnyNodeRef::StmtWhile(_) + | AnyNodeRef::StmtIf(_) + | AnyNodeRef::StmtWith(_) + | AnyNodeRef::StmtAsyncWith(_) + | AnyNodeRef::StmtMatch(_) + | AnyNodeRef::StmtRaise(_) + | AnyNodeRef::StmtTry(_) + | AnyNodeRef::StmtTryStar(_) + | AnyNodeRef::StmtAssert(_) + | AnyNodeRef::StmtImport(_) + | AnyNodeRef::StmtImportFrom(_) + | AnyNodeRef::StmtGlobal(_) + | AnyNodeRef::StmtNonlocal(_) + | AnyNodeRef::StmtExpr(_) + | AnyNodeRef::StmtPass(_) + | AnyNodeRef::StmtBreak(_) + | AnyNodeRef::StmtContinue(_) => true, + + AnyNodeRef::ModModule(_) + | AnyNodeRef::ModInteractive(_) + | AnyNodeRef::ModExpression(_) + | AnyNodeRef::ModFunctionType(_) + | AnyNodeRef::ExprBoolOp(_) + | AnyNodeRef::ExprNamedExpr(_) + | AnyNodeRef::ExprBinOp(_) + | AnyNodeRef::ExprUnaryOp(_) + | AnyNodeRef::ExprLambda(_) + | AnyNodeRef::ExprIfExp(_) + | AnyNodeRef::ExprDict(_) + | AnyNodeRef::ExprSet(_) + | AnyNodeRef::ExprListComp(_) + | AnyNodeRef::ExprSetComp(_) + | AnyNodeRef::ExprDictComp(_) + | AnyNodeRef::ExprGeneratorExp(_) + | AnyNodeRef::ExprAwait(_) + | AnyNodeRef::ExprYield(_) + | AnyNodeRef::ExprYieldFrom(_) + | AnyNodeRef::ExprCompare(_) + | AnyNodeRef::ExprCall(_) + | AnyNodeRef::ExprFormattedValue(_) + | AnyNodeRef::ExprJoinedStr(_) + | AnyNodeRef::ExprConstant(_) + | AnyNodeRef::ExprAttribute(_) + | AnyNodeRef::ExprSubscript(_) + | AnyNodeRef::ExprStarred(_) + | AnyNodeRef::ExprName(_) + | AnyNodeRef::ExprList(_) + | AnyNodeRef::ExprTuple(_) + | AnyNodeRef::ExprSlice(_) + | AnyNodeRef::ExcepthandlerExceptHandler(_) + | AnyNodeRef::PatternMatchValue(_) + | AnyNodeRef::PatternMatchSingleton(_) + | AnyNodeRef::PatternMatchSequence(_) + | AnyNodeRef::PatternMatchMapping(_) + | AnyNodeRef::PatternMatchClass(_) + | AnyNodeRef::PatternMatchStar(_) + | AnyNodeRef::PatternMatchAs(_) + | AnyNodeRef::PatternMatchOr(_) + | AnyNodeRef::TypeIgnoreTypeIgnore(_) + | AnyNodeRef::Comprehension(_) + | AnyNodeRef::Arguments(_) + | AnyNodeRef::Arg(_) + | AnyNodeRef::Keyword(_) + | AnyNodeRef::Alias(_) + | AnyNodeRef::Withitem(_) + | AnyNodeRef::MatchCase(_) => false, + } + } + + pub const fn is_expression(self) -> bool { + match self { + AnyNodeRef::ExprBoolOp(_) + | AnyNodeRef::ExprNamedExpr(_) + | AnyNodeRef::ExprBinOp(_) + | AnyNodeRef::ExprUnaryOp(_) + | AnyNodeRef::ExprLambda(_) + | AnyNodeRef::ExprIfExp(_) + | AnyNodeRef::ExprDict(_) + | AnyNodeRef::ExprSet(_) + | AnyNodeRef::ExprListComp(_) + | AnyNodeRef::ExprSetComp(_) + | AnyNodeRef::ExprDictComp(_) + | AnyNodeRef::ExprGeneratorExp(_) + | AnyNodeRef::ExprAwait(_) + | AnyNodeRef::ExprYield(_) + | AnyNodeRef::ExprYieldFrom(_) + | AnyNodeRef::ExprCompare(_) + | AnyNodeRef::ExprCall(_) + | AnyNodeRef::ExprFormattedValue(_) + | AnyNodeRef::ExprJoinedStr(_) + | AnyNodeRef::ExprConstant(_) + | AnyNodeRef::ExprAttribute(_) + | AnyNodeRef::ExprSubscript(_) + | AnyNodeRef::ExprStarred(_) + | AnyNodeRef::ExprName(_) + | AnyNodeRef::ExprList(_) + | AnyNodeRef::ExprTuple(_) + | AnyNodeRef::ExprSlice(_) => true, + + AnyNodeRef::ModModule(_) + | AnyNodeRef::ModInteractive(_) + | AnyNodeRef::ModExpression(_) + | AnyNodeRef::ModFunctionType(_) + | AnyNodeRef::StmtFunctionDef(_) + | AnyNodeRef::StmtAsyncFunctionDef(_) + | AnyNodeRef::StmtClassDef(_) + | AnyNodeRef::StmtReturn(_) + | AnyNodeRef::StmtDelete(_) + | AnyNodeRef::StmtAssign(_) + | AnyNodeRef::StmtAugAssign(_) + | AnyNodeRef::StmtAnnAssign(_) + | AnyNodeRef::StmtFor(_) + | AnyNodeRef::StmtAsyncFor(_) + | AnyNodeRef::StmtWhile(_) + | AnyNodeRef::StmtIf(_) + | AnyNodeRef::StmtWith(_) + | AnyNodeRef::StmtAsyncWith(_) + | AnyNodeRef::StmtMatch(_) + | AnyNodeRef::StmtRaise(_) + | AnyNodeRef::StmtTry(_) + | AnyNodeRef::StmtTryStar(_) + | AnyNodeRef::StmtAssert(_) + | AnyNodeRef::StmtImport(_) + | AnyNodeRef::StmtImportFrom(_) + | AnyNodeRef::StmtGlobal(_) + | AnyNodeRef::StmtNonlocal(_) + | AnyNodeRef::StmtExpr(_) + | AnyNodeRef::StmtPass(_) + | AnyNodeRef::StmtBreak(_) + | AnyNodeRef::StmtContinue(_) + | AnyNodeRef::ExcepthandlerExceptHandler(_) + | AnyNodeRef::PatternMatchValue(_) + | AnyNodeRef::PatternMatchSingleton(_) + | AnyNodeRef::PatternMatchSequence(_) + | AnyNodeRef::PatternMatchMapping(_) + | AnyNodeRef::PatternMatchClass(_) + | AnyNodeRef::PatternMatchStar(_) + | AnyNodeRef::PatternMatchAs(_) + | AnyNodeRef::PatternMatchOr(_) + | AnyNodeRef::TypeIgnoreTypeIgnore(_) + | AnyNodeRef::Comprehension(_) + | AnyNodeRef::Arguments(_) + | AnyNodeRef::Arg(_) + | AnyNodeRef::Keyword(_) + | AnyNodeRef::Alias(_) + | AnyNodeRef::Withitem(_) + | AnyNodeRef::MatchCase(_) => false, + } + } + + pub const fn is_module(self) -> bool { + match self { + AnyNodeRef::ModModule(_) + | AnyNodeRef::ModInteractive(_) + | AnyNodeRef::ModExpression(_) + | AnyNodeRef::ModFunctionType(_) => true, + + AnyNodeRef::StmtFunctionDef(_) + | AnyNodeRef::StmtAsyncFunctionDef(_) + | AnyNodeRef::StmtClassDef(_) + | AnyNodeRef::StmtReturn(_) + | AnyNodeRef::StmtDelete(_) + | AnyNodeRef::StmtAssign(_) + | AnyNodeRef::StmtAugAssign(_) + | AnyNodeRef::StmtAnnAssign(_) + | AnyNodeRef::StmtFor(_) + | AnyNodeRef::StmtAsyncFor(_) + | AnyNodeRef::StmtWhile(_) + | AnyNodeRef::StmtIf(_) + | AnyNodeRef::StmtWith(_) + | AnyNodeRef::StmtAsyncWith(_) + | AnyNodeRef::StmtMatch(_) + | AnyNodeRef::StmtRaise(_) + | AnyNodeRef::StmtTry(_) + | AnyNodeRef::StmtTryStar(_) + | AnyNodeRef::StmtAssert(_) + | AnyNodeRef::StmtImport(_) + | AnyNodeRef::StmtImportFrom(_) + | AnyNodeRef::StmtGlobal(_) + | AnyNodeRef::StmtNonlocal(_) + | AnyNodeRef::StmtExpr(_) + | AnyNodeRef::StmtPass(_) + | AnyNodeRef::StmtBreak(_) + | AnyNodeRef::StmtContinue(_) + | AnyNodeRef::ExprBoolOp(_) + | AnyNodeRef::ExprNamedExpr(_) + | AnyNodeRef::ExprBinOp(_) + | AnyNodeRef::ExprUnaryOp(_) + | AnyNodeRef::ExprLambda(_) + | AnyNodeRef::ExprIfExp(_) + | AnyNodeRef::ExprDict(_) + | AnyNodeRef::ExprSet(_) + | AnyNodeRef::ExprListComp(_) + | AnyNodeRef::ExprSetComp(_) + | AnyNodeRef::ExprDictComp(_) + | AnyNodeRef::ExprGeneratorExp(_) + | AnyNodeRef::ExprAwait(_) + | AnyNodeRef::ExprYield(_) + | AnyNodeRef::ExprYieldFrom(_) + | AnyNodeRef::ExprCompare(_) + | AnyNodeRef::ExprCall(_) + | AnyNodeRef::ExprFormattedValue(_) + | AnyNodeRef::ExprJoinedStr(_) + | AnyNodeRef::ExprConstant(_) + | AnyNodeRef::ExprAttribute(_) + | AnyNodeRef::ExprSubscript(_) + | AnyNodeRef::ExprStarred(_) + | AnyNodeRef::ExprName(_) + | AnyNodeRef::ExprList(_) + | AnyNodeRef::ExprTuple(_) + | AnyNodeRef::ExprSlice(_) + | AnyNodeRef::ExcepthandlerExceptHandler(_) + | AnyNodeRef::PatternMatchValue(_) + | AnyNodeRef::PatternMatchSingleton(_) + | AnyNodeRef::PatternMatchSequence(_) + | AnyNodeRef::PatternMatchMapping(_) + | AnyNodeRef::PatternMatchClass(_) + | AnyNodeRef::PatternMatchStar(_) + | AnyNodeRef::PatternMatchAs(_) + | AnyNodeRef::PatternMatchOr(_) + | AnyNodeRef::TypeIgnoreTypeIgnore(_) + | AnyNodeRef::Comprehension(_) + | AnyNodeRef::Arguments(_) + | AnyNodeRef::Arg(_) + | AnyNodeRef::Keyword(_) + | AnyNodeRef::Alias(_) + | AnyNodeRef::Withitem(_) + | AnyNodeRef::MatchCase(_) => false, + } + } + + pub const fn is_pattern(self) -> bool { + match self { + AnyNodeRef::PatternMatchValue(_) + | AnyNodeRef::PatternMatchSingleton(_) + | AnyNodeRef::PatternMatchSequence(_) + | AnyNodeRef::PatternMatchMapping(_) + | AnyNodeRef::PatternMatchClass(_) + | AnyNodeRef::PatternMatchStar(_) + | AnyNodeRef::PatternMatchAs(_) + | AnyNodeRef::PatternMatchOr(_) => true, + + AnyNodeRef::ModModule(_) + | AnyNodeRef::ModInteractive(_) + | AnyNodeRef::ModExpression(_) + | AnyNodeRef::ModFunctionType(_) + | AnyNodeRef::StmtFunctionDef(_) + | AnyNodeRef::StmtAsyncFunctionDef(_) + | AnyNodeRef::StmtClassDef(_) + | AnyNodeRef::StmtReturn(_) + | AnyNodeRef::StmtDelete(_) + | AnyNodeRef::StmtAssign(_) + | AnyNodeRef::StmtAugAssign(_) + | AnyNodeRef::StmtAnnAssign(_) + | AnyNodeRef::StmtFor(_) + | AnyNodeRef::StmtAsyncFor(_) + | AnyNodeRef::StmtWhile(_) + | AnyNodeRef::StmtIf(_) + | AnyNodeRef::StmtWith(_) + | AnyNodeRef::StmtAsyncWith(_) + | AnyNodeRef::StmtMatch(_) + | AnyNodeRef::StmtRaise(_) + | AnyNodeRef::StmtTry(_) + | AnyNodeRef::StmtTryStar(_) + | AnyNodeRef::StmtAssert(_) + | AnyNodeRef::StmtImport(_) + | AnyNodeRef::StmtImportFrom(_) + | AnyNodeRef::StmtGlobal(_) + | AnyNodeRef::StmtNonlocal(_) + | AnyNodeRef::StmtExpr(_) + | AnyNodeRef::StmtPass(_) + | AnyNodeRef::StmtBreak(_) + | AnyNodeRef::StmtContinue(_) + | AnyNodeRef::ExprBoolOp(_) + | AnyNodeRef::ExprNamedExpr(_) + | AnyNodeRef::ExprBinOp(_) + | AnyNodeRef::ExprUnaryOp(_) + | AnyNodeRef::ExprLambda(_) + | AnyNodeRef::ExprIfExp(_) + | AnyNodeRef::ExprDict(_) + | AnyNodeRef::ExprSet(_) + | AnyNodeRef::ExprListComp(_) + | AnyNodeRef::ExprSetComp(_) + | AnyNodeRef::ExprDictComp(_) + | AnyNodeRef::ExprGeneratorExp(_) + | AnyNodeRef::ExprAwait(_) + | AnyNodeRef::ExprYield(_) + | AnyNodeRef::ExprYieldFrom(_) + | AnyNodeRef::ExprCompare(_) + | AnyNodeRef::ExprCall(_) + | AnyNodeRef::ExprFormattedValue(_) + | AnyNodeRef::ExprJoinedStr(_) + | AnyNodeRef::ExprConstant(_) + | AnyNodeRef::ExprAttribute(_) + | AnyNodeRef::ExprSubscript(_) + | AnyNodeRef::ExprStarred(_) + | AnyNodeRef::ExprName(_) + | AnyNodeRef::ExprList(_) + | AnyNodeRef::ExprTuple(_) + | AnyNodeRef::ExprSlice(_) + | AnyNodeRef::ExcepthandlerExceptHandler(_) + | AnyNodeRef::TypeIgnoreTypeIgnore(_) + | AnyNodeRef::Comprehension(_) + | AnyNodeRef::Arguments(_) + | AnyNodeRef::Arg(_) + | AnyNodeRef::Keyword(_) + | AnyNodeRef::Alias(_) + | AnyNodeRef::Withitem(_) + | AnyNodeRef::MatchCase(_) => false, + } + } + + pub const fn is_except_handler(self) -> bool { + match self { + AnyNodeRef::ExcepthandlerExceptHandler(_) => true, + + AnyNodeRef::ModModule(_) + | AnyNodeRef::ModInteractive(_) + | AnyNodeRef::ModExpression(_) + | AnyNodeRef::ModFunctionType(_) + | AnyNodeRef::StmtFunctionDef(_) + | AnyNodeRef::StmtAsyncFunctionDef(_) + | AnyNodeRef::StmtClassDef(_) + | AnyNodeRef::StmtReturn(_) + | AnyNodeRef::StmtDelete(_) + | AnyNodeRef::StmtAssign(_) + | AnyNodeRef::StmtAugAssign(_) + | AnyNodeRef::StmtAnnAssign(_) + | AnyNodeRef::StmtFor(_) + | AnyNodeRef::StmtAsyncFor(_) + | AnyNodeRef::StmtWhile(_) + | AnyNodeRef::StmtIf(_) + | AnyNodeRef::StmtWith(_) + | AnyNodeRef::StmtAsyncWith(_) + | AnyNodeRef::StmtMatch(_) + | AnyNodeRef::StmtRaise(_) + | AnyNodeRef::StmtTry(_) + | AnyNodeRef::StmtTryStar(_) + | AnyNodeRef::StmtAssert(_) + | AnyNodeRef::StmtImport(_) + | AnyNodeRef::StmtImportFrom(_) + | AnyNodeRef::StmtGlobal(_) + | AnyNodeRef::StmtNonlocal(_) + | AnyNodeRef::StmtExpr(_) + | AnyNodeRef::StmtPass(_) + | AnyNodeRef::StmtBreak(_) + | AnyNodeRef::StmtContinue(_) + | AnyNodeRef::ExprBoolOp(_) + | AnyNodeRef::ExprNamedExpr(_) + | AnyNodeRef::ExprBinOp(_) + | AnyNodeRef::ExprUnaryOp(_) + | AnyNodeRef::ExprLambda(_) + | AnyNodeRef::ExprIfExp(_) + | AnyNodeRef::ExprDict(_) + | AnyNodeRef::ExprSet(_) + | AnyNodeRef::ExprListComp(_) + | AnyNodeRef::ExprSetComp(_) + | AnyNodeRef::ExprDictComp(_) + | AnyNodeRef::ExprGeneratorExp(_) + | AnyNodeRef::ExprAwait(_) + | AnyNodeRef::ExprYield(_) + | AnyNodeRef::ExprYieldFrom(_) + | AnyNodeRef::ExprCompare(_) + | AnyNodeRef::ExprCall(_) + | AnyNodeRef::ExprFormattedValue(_) + | AnyNodeRef::ExprJoinedStr(_) + | AnyNodeRef::ExprConstant(_) + | AnyNodeRef::ExprAttribute(_) + | AnyNodeRef::ExprSubscript(_) + | AnyNodeRef::ExprStarred(_) + | AnyNodeRef::ExprName(_) + | AnyNodeRef::ExprList(_) + | AnyNodeRef::ExprTuple(_) + | AnyNodeRef::ExprSlice(_) + | AnyNodeRef::PatternMatchValue(_) + | AnyNodeRef::PatternMatchSingleton(_) + | AnyNodeRef::PatternMatchSequence(_) + | AnyNodeRef::PatternMatchMapping(_) + | AnyNodeRef::PatternMatchClass(_) + | AnyNodeRef::PatternMatchStar(_) + | AnyNodeRef::PatternMatchAs(_) + | AnyNodeRef::PatternMatchOr(_) + | AnyNodeRef::TypeIgnoreTypeIgnore(_) + | AnyNodeRef::Comprehension(_) + | AnyNodeRef::Arguments(_) + | AnyNodeRef::Arg(_) + | AnyNodeRef::Keyword(_) + | AnyNodeRef::Alias(_) + | AnyNodeRef::Withitem(_) + | AnyNodeRef::MatchCase(_) => false, + } + } + + pub const fn is_type_ignore(self) -> bool { + match self { + AnyNodeRef::TypeIgnoreTypeIgnore(_) => true, + + AnyNodeRef::ModModule(_) + | AnyNodeRef::ModInteractive(_) + | AnyNodeRef::ModExpression(_) + | AnyNodeRef::ModFunctionType(_) + | AnyNodeRef::StmtFunctionDef(_) + | AnyNodeRef::StmtAsyncFunctionDef(_) + | AnyNodeRef::StmtClassDef(_) + | AnyNodeRef::StmtReturn(_) + | AnyNodeRef::StmtDelete(_) + | AnyNodeRef::StmtAssign(_) + | AnyNodeRef::StmtAugAssign(_) + | AnyNodeRef::StmtAnnAssign(_) + | AnyNodeRef::StmtFor(_) + | AnyNodeRef::StmtAsyncFor(_) + | AnyNodeRef::StmtWhile(_) + | AnyNodeRef::StmtIf(_) + | AnyNodeRef::StmtWith(_) + | AnyNodeRef::StmtAsyncWith(_) + | AnyNodeRef::StmtMatch(_) + | AnyNodeRef::StmtRaise(_) + | AnyNodeRef::StmtTry(_) + | AnyNodeRef::StmtTryStar(_) + | AnyNodeRef::StmtAssert(_) + | AnyNodeRef::StmtImport(_) + | AnyNodeRef::StmtImportFrom(_) + | AnyNodeRef::StmtGlobal(_) + | AnyNodeRef::StmtNonlocal(_) + | AnyNodeRef::StmtExpr(_) + | AnyNodeRef::StmtPass(_) + | AnyNodeRef::StmtBreak(_) + | AnyNodeRef::StmtContinue(_) + | AnyNodeRef::ExprBoolOp(_) + | AnyNodeRef::ExprNamedExpr(_) + | AnyNodeRef::ExprBinOp(_) + | AnyNodeRef::ExprUnaryOp(_) + | AnyNodeRef::ExprLambda(_) + | AnyNodeRef::ExprIfExp(_) + | AnyNodeRef::ExprDict(_) + | AnyNodeRef::ExprSet(_) + | AnyNodeRef::ExprListComp(_) + | AnyNodeRef::ExprSetComp(_) + | AnyNodeRef::ExprDictComp(_) + | AnyNodeRef::ExprGeneratorExp(_) + | AnyNodeRef::ExprAwait(_) + | AnyNodeRef::ExprYield(_) + | AnyNodeRef::ExprYieldFrom(_) + | AnyNodeRef::ExprCompare(_) + | AnyNodeRef::ExprCall(_) + | AnyNodeRef::ExprFormattedValue(_) + | AnyNodeRef::ExprJoinedStr(_) + | AnyNodeRef::ExprConstant(_) + | AnyNodeRef::ExprAttribute(_) + | AnyNodeRef::ExprSubscript(_) + | AnyNodeRef::ExprStarred(_) + | AnyNodeRef::ExprName(_) + | AnyNodeRef::ExprList(_) + | AnyNodeRef::ExprTuple(_) + | AnyNodeRef::ExprSlice(_) + | AnyNodeRef::PatternMatchValue(_) + | AnyNodeRef::PatternMatchSingleton(_) + | AnyNodeRef::PatternMatchSequence(_) + | AnyNodeRef::PatternMatchMapping(_) + | AnyNodeRef::PatternMatchClass(_) + | AnyNodeRef::PatternMatchStar(_) + | AnyNodeRef::PatternMatchAs(_) + | AnyNodeRef::PatternMatchOr(_) + | AnyNodeRef::ExcepthandlerExceptHandler(_) + | AnyNodeRef::Comprehension(_) + | AnyNodeRef::Arguments(_) + | AnyNodeRef::Arg(_) + | AnyNodeRef::Keyword(_) + | AnyNodeRef::Alias(_) + | AnyNodeRef::Withitem(_) + | AnyNodeRef::MatchCase(_) => false, + } + } } impl<'a> From<&'a ModModule> for AnyNodeRef<'a> { diff --git a/crates/ruff_python_ast/src/whitespace.rs b/crates/ruff_python_ast/src/whitespace.rs index cf4de111f1..0fd47276e2 100644 --- a/crates/ruff_python_ast/src/whitespace.rs +++ b/crates/ruff_python_ast/src/whitespace.rs @@ -1,15 +1,21 @@ -use ruff_text_size::TextRange; +use ruff_text_size::{TextRange, TextSize}; use rustpython_parser::ast::Ranged; use crate::source_code::Locator; /// Extract the leading indentation from a line. +#[inline] pub fn indentation<'a, T>(locator: &'a Locator, located: &T) -> Option<&'a str> where T: Ranged, { - let line_start = locator.line_start(located.start()); - let indentation = &locator.contents()[TextRange::new(line_start, located.start())]; + indentation_at_offset(locator, located.start()) +} + +/// Extract the leading indentation from a line. +pub fn indentation_at_offset<'a>(locator: &'a Locator, offset: TextSize) -> Option<&'a str> { + let line_start = locator.line_start(offset); + let indentation = &locator.contents()[TextRange::new(line_start, offset)]; if indentation.chars().all(char::is_whitespace) { Some(indentation) diff --git a/crates/ruff_python_formatter/src/comments/debug.rs b/crates/ruff_python_formatter/src/comments/debug.rs index ff8b3e7adc..8583461a44 100644 --- a/crates/ruff_python_formatter/src/comments/debug.rs +++ b/crates/ruff_python_formatter/src/comments/debug.rs @@ -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() { diff --git a/crates/ruff_python_formatter/src/comments/map.rs b/crates/ruff_python_formatter/src/comments/map.rs index eea4bb50a7..9bef831903 100644 --- a/crates/ruff_python_formatter/src/comments/map.rs +++ b/crates/ruff_python_formatter/src/comments/map.rs @@ -60,7 +60,7 @@ pub(super) struct MultiMap { } impl MultiMap { - pub fn new() -> Self { + pub(super) fn new() -> Self { Self { index: FxHashMap::default(), parts: Vec::new(), @@ -69,7 +69,7 @@ impl MultiMap { } /// 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 MultiMap { } /// 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 MultiMap { } /// 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 MultiMap { } } - 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 MultiMap { } /// 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 MultiMap { } /// 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 MultiMap { } /// 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 { + pub(super) fn parts(&self, key: &K) -> PartsIterator { match self.index.get(key) { None => PartsIterator::Slice([].iter()), Some(entry) => PartsIterator::from_entry(entry, self), @@ -258,7 +258,7 @@ impl MultiMap { /// Returns an iterator over the parts of all keys. #[allow(unused)] - pub fn all_parts(&self) -> impl Iterator { + pub(super) fn all_parts(&self) -> impl Iterator { 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>, } diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index f0713c58eb..9791040bea 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -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>, } +#[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#" diff --git a/crates/ruff_python_formatter/src/comments/node_key.rs b/crates/ruff_python_formatter/src/comments/node_key.rs index 0a6ae15972..38e8f026c7 100644 --- a/crates/ruff_python_formatter/src/comments/node_key.rs +++ b/crates/ruff_python_formatter/src/comments/node_key.rs @@ -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(&self, state: &mut H) { - self.ptr().hash(state); + self.node.as_ptr().hash(state); } } diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index ee23283161..2ed5c5e8c6 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -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`. + // 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, ¤t_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) -> bool +where + T: Into>, +{ + right.map_or(false, |right| left.ptr_eq(right.into())) +} + +fn last_child_in_body(node: AnyNodeRef) -> Option { + 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) +} diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap new file mode 100644 index 0000000000..af56bdb4b9 --- /dev/null +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_else_comments.snap @@ -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, + }, + ], + }, +} diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap new file mode 100644 index 0000000000..1aa79f4f4e --- /dev/null +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__if_elif_if_else_comments.snap @@ -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, + }, + ], + }, +} diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__match_cases.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__match_cases.snap new file mode 100644 index 0000000000..a7056aad31 --- /dev/null +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__match_cases.snap @@ -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": [], + }, +} diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_comment_after_single_statement_body.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_comment_after_single_statement_body.snap new file mode 100644 index 0000000000..62b0b6652b --- /dev/null +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_comment_after_single_statement_body.snap @@ -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, + }, + ], + }, +} diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap index 4ca22d68ee..005e4fcf8d 100644 --- a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap @@ -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, + }, + ], }, } diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_most_outer_nested.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_most_outer_nested.snap index bc1b444b3d..278320eb58 100644 --- a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_most_outer_nested.snap +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_most_outer_nested.snap @@ -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, + }, + ], + }, } diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except.snap new file mode 100644 index 0000000000..1bcf35e95c --- /dev/null +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except.snap @@ -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, + }, + ], + }, +} diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except_finally_else.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except_finally_else.snap new file mode 100644 index 0000000000..d5b6bbcb7f --- /dev/null +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__try_except_finally_else.snap @@ -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, + }, + ], + }, +} diff --git a/crates/ruff_python_formatter/src/comments/visitor.rs b/crates/ruff_python_formatter/src/comments/visitor.rs index d107c8366f..4e2c75ffc6 100644 --- a/crates/ruff_python_formatter/src/comments/visitor.rs +++ b/crates/ruff_python_formatter/src/comments/visitor.rs @@ -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>, following: Option>, + parent: Option>, 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> { + self.parent + } + /// Returns the slice into the source code. pub(super) fn slice(&self) -> &SourceCodeSlice { &self.slice