From 9a817a2922a881b680c2aee96c3fde52119e3018 Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 15 Jul 2024 12:59:33 +0200 Subject: [PATCH] Insert empty line between suite and alternative branch after def/class (#12294) When there is a function or class definition at the end of a suite followed by the beginning of an alternative block, we have to insert a single empty line between them. In the if-else-statement example below, we insert an empty line after the `foo` in the if-block, but none after the else-block `foo`, since in the latter case the enclosing suite already adds empty lines. ```python if sys.version_info >= (3, 10): def foo(): return "new" else: def foo(): return "old" class Bar: pass ``` To do so, we track whether the current suite is the last one in the current statement with a new option on the suite kind. Fixes #12199 --------- Co-authored-by: Micha Reiser --- .../resources/test/fixtures/ruff/newlines.py | 62 ++++ .../resources/test/fixtures/ruff/newlines.pyi | 6 + .../src/other/elif_else_clause.rs | 12 +- .../other/except_handler_except_handler.rs | 20 +- .../src/other/match_case.rs | 22 +- .../src/statement/clause.rs | 11 +- .../src/statement/stmt_class_def.rs | 2 +- .../src/statement/stmt_for.rs | 9 +- .../src/statement/stmt_function_def.rs | 2 +- .../src/statement/stmt_if.rs | 20 +- .../src/statement/stmt_match.rs | 3 +- .../src/statement/stmt_try.rs | 71 +++-- .../src/statement/stmt_while.rs | 9 +- .../src/statement/stmt_with.rs | 3 +- .../src/statement/suite.rs | 266 ++++++++++++------ ...ack_compatibility@cases__comments9.py.snap | 2 - ...ack_compatibility@cases__function2.py.snap | 2 - .../black_compatibility@cases__stub.pyi.snap | 2 - .../tests/snapshots/format@newlines.py.snap | 202 ++++++++++++- .../tests/snapshots/format@newlines.pyi.snap | 26 +- .../format@statement__function.py.snap | 23 ++ .../snapshots/format@statement__if.py.snap | 20 +- .../snapshots/format@statement__try.py.snap | 26 +- ...lank_line_after_nested_stub_class.pyi.snap | 3 - .../format@stub_files__suite.pyi.snap | 22 +- 25 files changed, 698 insertions(+), 148 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py index 648c10a531f28..2afbd182294f4 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py @@ -249,6 +249,68 @@ def y(): print() +if True: + def a(): + return 1 +else: + pass + +if True: + # fmt: off + def a(): + return 1 + # fmt: on +else: + pass + +match True: + case 1: + def a(): + return 1 + case 1: + def a(): + return 1 + +try: + def a(): + return 1 +except RuntimeError: + def a(): + return 1 + +try: + def a(): + return 1 +finally: + def a(): + return 1 + +try: + def a(): + return 1 +except RuntimeError: + def a(): + return 1 +except ZeroDivisionError: + def a(): + return 1 +else: + def a(): + return 1 +finally: + def a(): + return 1 + +if raw: + def show_file(lines): + for line in lines: + pass + # Trailing comment not on function or class + +else: + pass + + # NOTE: Please keep this the last block in this file. This tests that we don't insert # empty line(s) at the end of the file due to nested function if True: diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.pyi b/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.pyi index 0d33408ecbf4d..e768e2a4abd5a 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.pyi +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.pyi @@ -154,8 +154,14 @@ def f(): pass +if True: + def a(): + return 1 +else: + pass # comment x = 1 + diff --git a/crates/ruff_python_formatter/src/other/elif_else_clause.rs b/crates/ruff_python_formatter/src/other/elif_else_clause.rs index 7568351169dc2..bff92ca82bbe1 100644 --- a/crates/ruff_python_formatter/src/other/elif_else_clause.rs +++ b/crates/ruff_python_formatter/src/other/elif_else_clause.rs @@ -2,6 +2,7 @@ use ruff_python_ast::ElifElseClause; use crate::prelude::*; use crate::statement::stmt_if::format_elif_else_clause; +use crate::statement::suite::SuiteKind; /// Note that this implementation misses the leading newlines before the leading comments because /// it does not have access to the last node of the previous branch. The `StmtIf` therefore doesn't @@ -11,6 +12,15 @@ pub struct FormatElifElseClause; impl FormatNodeRule for FormatElifElseClause { fn fmt_fields(&self, item: &ElifElseClause, f: &mut PyFormatter) -> FormatResult<()> { - format_elif_else_clause(item, f, None) + format_elif_else_clause( + item, + f, + None, + SuiteKind::Other { + // For stability, we can't insert an empty line if we don't know if the outer suite + // also does. + last_suite_in_statement: true, + }, + ) } } diff --git a/crates/ruff_python_formatter/src/other/except_handler_except_handler.rs b/crates/ruff_python_formatter/src/other/except_handler_except_handler.rs index 48198c0b09460..11fdf640ad9dd 100644 --- a/crates/ruff_python_formatter/src/other/except_handler_except_handler.rs +++ b/crates/ruff_python_formatter/src/other/except_handler_except_handler.rs @@ -6,9 +6,10 @@ use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader}; +use crate::statement::suite::SuiteKind; #[derive(Copy, Clone, Default)] -pub enum ExceptHandlerKind { +pub(crate) enum ExceptHandlerKind { #[default] Regular, Starred, @@ -16,16 +17,18 @@ pub enum ExceptHandlerKind { #[derive(Default)] pub struct FormatExceptHandlerExceptHandler { - except_handler_kind: ExceptHandlerKind, + pub(crate) except_handler_kind: ExceptHandlerKind, + pub(crate) last_suite_in_statement: bool, } impl FormatRuleWithOptions> for FormatExceptHandlerExceptHandler { - type Options = ExceptHandlerKind; + type Options = FormatExceptHandlerExceptHandler; fn with_options(mut self, options: Self::Options) -> Self { - self.except_handler_kind = options; + self.except_handler_kind = options.except_handler_kind; + self.last_suite_in_statement = options.last_suite_in_statement; self } } @@ -36,6 +39,7 @@ impl FormatNodeRule for FormatExceptHandlerExceptHan item: &ExceptHandlerExceptHandler, f: &mut PyFormatter, ) -> FormatResult<()> { + let except_handler_kind = self.except_handler_kind; let ExceptHandlerExceptHandler { range: _, type_, @@ -57,7 +61,7 @@ impl FormatNodeRule for FormatExceptHandlerExceptHan f, [ token("except"), - match self.except_handler_kind { + match except_handler_kind { ExceptHandlerKind::Regular => None, ExceptHandlerKind::Starred => Some(token("*")), } @@ -84,7 +88,11 @@ impl FormatNodeRule for FormatExceptHandlerExceptHan Ok(()) }), ), - clause_body(body, dangling_comments), + clause_body( + body, + SuiteKind::other(self.last_suite_in_statement), + dangling_comments + ), ] ) } diff --git a/crates/ruff_python_formatter/src/other/match_case.rs b/crates/ruff_python_formatter/src/other/match_case.rs index 086916dc19541..fd722a6ccf599 100644 --- a/crates/ruff_python_formatter/src/other/match_case.rs +++ b/crates/ruff_python_formatter/src/other/match_case.rs @@ -1,4 +1,4 @@ -use ruff_formatter::write; +use ruff_formatter::{write, FormatRuleWithOptions}; use ruff_python_ast::AstNode; use ruff_python_ast::MatchCase; @@ -6,9 +6,21 @@ use crate::builders::parenthesize_if_expands; use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses, Parentheses}; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader}; +use crate::statement::suite::SuiteKind; #[derive(Default)] -pub struct FormatMatchCase; +pub struct FormatMatchCase { + last_suite_in_statement: bool, +} + +impl FormatRuleWithOptions> for FormatMatchCase { + type Options = bool; + + fn with_options(mut self, options: Self::Options) -> Self { + self.last_suite_in_statement = options; + self + } +} impl FormatNodeRule for FormatMatchCase { fn fmt_fields(&self, item: &MatchCase, f: &mut PyFormatter) -> FormatResult<()> { @@ -63,7 +75,11 @@ impl FormatNodeRule for FormatMatchCase { Ok(()) }), ), - clause_body(body, dangling_item_comments), + clause_body( + body, + SuiteKind::other(self.last_suite_in_statement), + dangling_item_comments + ), ] ) } diff --git a/crates/ruff_python_formatter/src/statement/clause.rs b/crates/ruff_python_formatter/src/statement/clause.rs index 7ca1d4b7ebf64..f00729fcc8418 100644 --- a/crates/ruff_python_formatter/src/statement/clause.rs +++ b/crates/ruff_python_formatter/src/statement/clause.rs @@ -380,21 +380,14 @@ pub(crate) struct FormatClauseBody<'a> { trailing_comments: &'a [SourceComment], } -impl<'a> FormatClauseBody<'a> { - #[must_use] - pub(crate) fn with_kind(mut self, kind: SuiteKind) -> Self { - self.kind = kind; - self - } -} - pub(crate) fn clause_body<'a>( body: &'a Suite, + kind: SuiteKind, trailing_comments: &'a [SourceComment], ) -> FormatClauseBody<'a> { FormatClauseBody { body, - kind: SuiteKind::default(), + kind, trailing_comments, } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_class_def.rs b/crates/ruff_python_formatter/src/statement/stmt_class_def.rs index 063199131ecd6..6077720412371 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_class_def.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_class_def.rs @@ -132,7 +132,7 @@ impl FormatNodeRule for FormatStmtClassDef { Ok(()) }), ), - clause_body(body, trailing_definition_comments).with_kind(SuiteKind::Class), + clause_body(body, SuiteKind::Class, trailing_definition_comments), ] )?; diff --git a/crates/ruff_python_formatter/src/statement/stmt_for.rs b/crates/ruff_python_formatter/src/statement/stmt_for.rs index 378bfe54ac5b2..7d9d334d95fcf 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_for.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_for.rs @@ -7,6 +7,7 @@ use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader, ElseClause}; +use crate::statement::suite::SuiteKind; #[derive(Debug)] struct ExprTupleWithoutParentheses<'a>(&'a Expr); @@ -63,7 +64,11 @@ impl FormatNodeRule for FormatStmtFor { maybe_parenthesize_expression(iter, item, Parenthesize::IfBreaks), ], ), - clause_body(body, trailing_condition_comments), + clause_body( + body, + SuiteKind::other(orelse.is_empty()), + trailing_condition_comments + ), ] )?; @@ -85,7 +90,7 @@ impl FormatNodeRule for FormatStmtFor { &token("else"), ) .with_leading_comments(leading, body.last()), - clause_body(orelse, trailing), + clause_body(orelse, SuiteKind::other(true), trailing), ] )?; } diff --git a/crates/ruff_python_formatter/src/statement/stmt_function_def.rs b/crates/ruff_python_formatter/src/statement/stmt_function_def.rs index 24a578414fad9..6f5c735d39d81 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_function_def.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_function_def.rs @@ -66,7 +66,7 @@ impl FormatNodeRule for FormatStmtFunctionDef { trailing_definition_comments, &format_with(|f| format_function_header(f, item)), ), - clause_body(body, trailing_definition_comments).with_kind(SuiteKind::Function), + clause_body(body, SuiteKind::Function, trailing_definition_comments), ] )?; diff --git a/crates/ruff_python_formatter/src/statement/stmt_if.rs b/crates/ruff_python_formatter/src/statement/stmt_if.rs index cfc33fcdac57c..f58333dc67b46 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_if.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_if.rs @@ -1,12 +1,12 @@ use ruff_formatter::{format_args, write}; -use ruff_python_ast::AnyNodeRef; -use ruff_python_ast::{ElifElseClause, StmtIf}; +use ruff_python_ast::{AnyNodeRef, ElifElseClause, StmtIf}; use ruff_text_size::Ranged; use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader}; +use crate::statement::suite::SuiteKind; #[derive(Default)] pub struct FormatStmtIf; @@ -35,13 +35,22 @@ impl FormatNodeRule for FormatStmtIf { maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks), ], ), - clause_body(body, trailing_colon_comment), + clause_body( + body, + SuiteKind::other(elif_else_clauses.is_empty()), + trailing_colon_comment + ), ] )?; let mut last_node = body.last().unwrap().into(); for clause in elif_else_clauses { - format_elif_else_clause(clause, f, Some(last_node))?; + format_elif_else_clause( + clause, + f, + Some(last_node), + SuiteKind::other(clause == elif_else_clauses.last().unwrap()), + )?; last_node = clause.body.last().unwrap().into(); } @@ -55,6 +64,7 @@ pub(crate) fn format_elif_else_clause( item: &ElifElseClause, f: &mut PyFormatter, last_node: Option, + suite_kind: SuiteKind, ) -> FormatResult<()> { let ElifElseClause { range: _, @@ -93,7 +103,7 @@ pub(crate) fn format_elif_else_clause( }), ) .with_leading_comments(leading_comments, last_node), - clause_body(body, trailing_colon_comment), + clause_body(body, suite_kind, trailing_colon_comment), f.options() .source_map_generation() .is_enabled() diff --git a/crates/ruff_python_formatter/src/statement/stmt_match.rs b/crates/ruff_python_formatter/src/statement/stmt_match.rs index 8d7be71d75e6c..441c881c7a0f1 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_match.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_match.rs @@ -48,6 +48,7 @@ impl FormatNodeRule for FormatStmtMatch { let mut last_case = first; for case in cases_iter { + let last_suite_in_statement = Some(case) == cases.last(); write!( f, [block_indent(&format_args!( @@ -55,7 +56,7 @@ impl FormatNodeRule for FormatStmtMatch { comments.leading(case), last_case.body.last(), ), - case.format() + case.format().with_options(last_suite_in_statement) ))] )?; last_case = case; diff --git a/crates/ruff_python_formatter/src/statement/stmt_try.rs b/crates/ruff_python_formatter/src/statement/stmt_try.rs index 1ec0ea9220bcd..a24cc654ccea7 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_try.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_try.rs @@ -5,9 +5,12 @@ use ruff_text_size::Ranged; use crate::comments; use crate::comments::leading_alternate_branch_comments; use crate::comments::SourceComment; -use crate::other::except_handler_except_handler::ExceptHandlerKind; +use crate::other::except_handler_except_handler::{ + ExceptHandlerKind, FormatExceptHandlerExceptHandler, +}; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader, ElseClause}; +use crate::statement::suite::SuiteKind; use crate::statement::{FormatRefWithRule, Stmt}; #[derive(Default)] @@ -16,13 +19,15 @@ pub struct FormatStmtTry; #[derive(Copy, Clone, Default)] pub struct FormatExceptHandler { except_handler_kind: ExceptHandlerKind, + last_suite_in_statement: bool, } impl FormatRuleWithOptions> for FormatExceptHandler { - type Options = ExceptHandlerKind; + type Options = FormatExceptHandler; fn with_options(mut self, options: Self::Options) -> Self { - self.except_handler_kind = options; + self.except_handler_kind = options.except_handler_kind; + self.last_suite_in_statement = options.last_suite_in_statement; self } } @@ -32,7 +37,10 @@ impl FormatRule> for FormatExceptHandler { match item { ExceptHandler::ExceptHandler(except_handler) => except_handler .format() - .with_options(self.except_handler_kind) + .with_options(FormatExceptHandlerExceptHandler { + except_handler_kind: self.except_handler_kind, + last_suite_in_statement: self.last_suite_in_statement, + }) .fmt(f), } } @@ -56,8 +64,8 @@ impl FormatNodeRule for FormatStmtTry { let StmtTry { body, handlers, - orelse: _, - finalbody: _, + orelse, + finalbody, is_star, range: _, } = item; @@ -65,31 +73,51 @@ impl FormatNodeRule for FormatStmtTry { let comments_info = f.context().comments().clone(); let mut dangling_comments = comments_info.dangling(item); - (_, dangling_comments) = format_case(item, CaseKind::Try, None, dangling_comments, f)?; + (_, dangling_comments) = + format_case(item, CaseKind::Try, None, dangling_comments, false, f)?; let mut previous_node = body.last(); for handler in handlers { let handler_comments = comments_info.leading(handler); + let ExceptHandler::ExceptHandler(except_handler) = handler; + let except_handler_kind = if *is_star { + ExceptHandlerKind::Starred + } else { + ExceptHandlerKind::Regular + }; + let last_suite_in_statement = + handler == handlers.last().unwrap() && orelse.is_empty() && finalbody.is_empty(); + write!( f, [ leading_alternate_branch_comments(handler_comments, previous_node), - &handler.format().with_options(if *is_star { - ExceptHandlerKind::Starred - } else { - ExceptHandlerKind::Regular - }), + &handler.format().with_options(FormatExceptHandler { + except_handler_kind, + last_suite_in_statement + }) ] )?; - previous_node = match handler { - ExceptHandler::ExceptHandler(handler) => handler.body.last(), - }; + previous_node = except_handler.body.last(); } - (previous_node, dangling_comments) = - format_case(item, CaseKind::Else, previous_node, dangling_comments, f)?; + (previous_node, dangling_comments) = format_case( + item, + CaseKind::Else, + previous_node, + dangling_comments, + finalbody.is_empty(), + f, + )?; - format_case(item, CaseKind::Finally, previous_node, dangling_comments, f)?; + format_case( + item, + CaseKind::Finally, + previous_node, + dangling_comments, + true, + f, + )?; write!(f, [comments::dangling_comments(dangling_comments)]) } @@ -100,6 +128,7 @@ fn format_case<'a>( kind: CaseKind, previous_node: Option<&'a Stmt>, dangling_comments: &'a [SourceComment], + last_suite_in_statement: bool, f: &mut PyFormatter, ) -> FormatResult<(Option<&'a Stmt>, &'a [SourceComment])> { let body = match kind { @@ -129,7 +158,11 @@ fn format_case<'a>( [ clause_header(header, trailing_case_comments, &token(kind.keyword())) .with_leading_comments(leading_case_comments, previous_node), - clause_body(body, trailing_case_comments), + clause_body( + body, + SuiteKind::other(last_suite_in_statement), + trailing_case_comments + ), ] )?; (Some(last), rest) diff --git a/crates/ruff_python_formatter/src/statement/stmt_while.rs b/crates/ruff_python_formatter/src/statement/stmt_while.rs index e1a926aa4a731..19dc175998deb 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_while.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_while.rs @@ -7,6 +7,7 @@ use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader, ElseClause}; +use crate::statement::suite::SuiteKind; #[derive(Default)] pub struct FormatStmtWhile; @@ -42,7 +43,11 @@ impl FormatNodeRule for FormatStmtWhile { maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks), ] ), - clause_body(body, trailing_condition_comments), + clause_body( + body, + SuiteKind::other(orelse.is_empty()), + trailing_condition_comments + ), ] )?; @@ -62,7 +67,7 @@ impl FormatNodeRule for FormatStmtWhile { &token("else") ) .with_leading_comments(leading, body.last()), - clause_body(orelse, trailing), + clause_body(orelse, SuiteKind::other(true), trailing), ] )?; } diff --git a/crates/ruff_python_formatter/src/statement/stmt_with.rs b/crates/ruff_python_formatter/src/statement/stmt_with.rs index 15b943a08db7f..79d2cb0bfa655 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_with.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_with.rs @@ -14,6 +14,7 @@ use crate::other::commas; use crate::other::with_item::WithItemLayout; use crate::prelude::*; use crate::statement::clause::{clause_body, clause_header, ClauseHeader}; +use crate::statement::suite::SuiteKind; use crate::PythonVersion; #[derive(Default)] @@ -124,7 +125,7 @@ impl FormatNodeRule for FormatStmtWith { } }) ), - clause_body(&with_stmt.body, colon_comments) + clause_body(&with_stmt.body, SuiteKind::other(true), colon_comments) ] ) } diff --git a/crates/ruff_python_formatter/src/statement/suite.rs b/crates/ruff_python_formatter/src/statement/suite.rs index f7218d4ce8d46..d0d89839ccf73 100644 --- a/crates/ruff_python_formatter/src/statement/suite.rs +++ b/crates/ruff_python_formatter/src/statement/suite.rs @@ -20,7 +20,7 @@ use crate::verbatim::{ }; /// Level at which the [`Suite`] appears in the source code. -#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum SuiteKind { /// Statements at the module level / top level TopLevel, @@ -32,23 +32,62 @@ pub enum SuiteKind { Class, /// Statements in any other body (e.g., `if` or `while`). - #[default] - Other, + Other { + /// Whether this suite is the last suite in the current statement. + /// + /// Below, `last_suite_in_statement` is `false` for the suite containing `foo10` and `foo12` + /// and `true` for the suite containing `bar`. + /// ```python + /// if sys.version_info >= (3, 10): + /// def foo10(): + /// return "new" + /// elif sys.version_info >= (3, 12): + /// def foo12(): + /// return "new" + /// else: + /// def bar(): + /// return "old" + /// ``` + /// + /// When this value is true, we don't insert trailing empty lines since the containing suite + /// will do that. + last_suite_in_statement: bool, + }, } -#[derive(Debug)] -pub struct FormatSuite { - kind: SuiteKind, +impl Default for SuiteKind { + fn default() -> Self { + Self::Other { + // For stability, we can't insert an empty line if we don't know if the outer suite + // also does. + last_suite_in_statement: true, + } + } } -impl Default for FormatSuite { - fn default() -> Self { - FormatSuite { - kind: SuiteKind::Other, +impl SuiteKind { + /// See [`SuiteKind::Other`]. + pub fn other(last_suite_in_statement: bool) -> Self { + Self::Other { + last_suite_in_statement, + } + } + + pub fn last_suite_in_statement(self) -> bool { + match self { + Self::Other { + last_suite_in_statement, + } => last_suite_in_statement, + _ => true, } } } +#[derive(Debug, Default)] +pub struct FormatSuite { + kind: SuiteKind, +} + impl FormatRule> for FormatSuite { fn fmt(&self, statements: &Suite, f: &mut PyFormatter) -> FormatResult<()> { let mut iter = statements.iter(); @@ -64,7 +103,7 @@ impl FormatRule> for FormatSuite { TopLevelStatementPosition::Other }), ), - SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => { + SuiteKind::Function | SuiteKind::Class | SuiteKind::Other { .. } => { NodeLevel::CompoundStatement } }; @@ -78,7 +117,7 @@ impl FormatRule> for FormatSuite { // Format the first statement in the body, which often has special formatting rules. let first = match self.kind { - SuiteKind::Other => { + SuiteKind::Other { .. } => { if is_class_or_function_definition(first) && !comments.has_leading(first) && !source_type.is_stub() @@ -161,55 +200,7 @@ impl FormatRule> for FormatSuite { // Here we insert empty lines even if the preceding has a trailing own line comment true } else { - // Find nested class or function definitions that need an empty line after them. - // - // ```python - // def f(): - // if True: - // - // def double(s): - // return s + s - // - // print("below function") - // ``` - std::iter::successors( - Some(AnyNodeRef::from(preceding)), - AnyNodeRef::last_child_in_body, - ) - .take_while(|last_child| - // If there is a comment between preceding and following the empty lines were - // inserted before the comment by preceding and there are no extra empty lines - // after the comment. - // ```python - // class Test: - // def a(self): - // pass - // # trailing comment - // - // - // # two lines before, one line after - // - // c = 30 - // ```` - // This also includes nested class/function definitions, so we stop recursing - // once we see a node with a trailing own line comment: - // ```python - // def f(): - // if True: - // - // def double(s): - // return s + s - // - // # nested trailing own line comment - // print("below function with trailing own line comment") - // ``` - !comments.has_trailing_own_line(*last_child)) - .any(|last_child| { - matches!( - last_child, - AnyNodeRef::StmtFunctionDef(_) | AnyNodeRef::StmtClassDef(_) - ) - }) + trailing_function_or_class_def(Some(preceding), &comments).is_some() }; // Add empty lines before and after a function or class definition. If the preceding @@ -248,7 +239,7 @@ impl FormatRule> for FormatSuite { SuiteKind::TopLevel => { write!(f, [empty_line(), empty_line()])?; } - SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => { + SuiteKind::Function | SuiteKind::Class | SuiteKind::Other { .. } => { empty_line().fmt(f)?; } } @@ -280,7 +271,7 @@ impl FormatRule> for FormatSuite { }, } } - SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => { + SuiteKind::Function | SuiteKind::Class | SuiteKind::Other { .. } => { empty_line().fmt(f)?; } } @@ -319,7 +310,7 @@ impl FormatRule> for FormatSuite { write!(f, [empty_line(), empty_line()])?; } }, - SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => { + SuiteKind::Function | SuiteKind::Class | SuiteKind::Other { .. } => { empty_line().fmt(f)?; } }, @@ -413,10 +404,129 @@ impl FormatRule> for FormatSuite { empty_line_after_docstring = false; } + self.between_alternative_blocks_empty_line(statements, &comments, f)?; + + Ok(()) + } +} + +impl FormatSuite { + /// Add an empty line between a function or class and an alternative body. + /// + /// We only insert an empty if we're between suites in a multi-suite statement. In the + /// if-else-statement below, we insert an empty line after the `foo` in the if-block, but none + /// after the else-block `foo`, since in the latter case the enclosing suite already adds + /// empty lines. + /// + /// ```python + /// if sys.version_info >= (3, 10): + /// def foo(): + /// return "new" + /// else: + /// def foo(): + /// return "old" + /// class Bar: + /// pass + /// ``` + fn between_alternative_blocks_empty_line( + &self, + statements: &Suite, + comments: &Comments, + f: &mut PyFormatter, + ) -> FormatResult<()> { + if self.kind.last_suite_in_statement() { + // If we're at the end of the current statement, the outer suite will insert one or + // two empty lines already. + return Ok(()); + } + + let Some(last_def_or_class) = trailing_function_or_class_def(statements.last(), comments) + else { + // An empty line is only inserted for function and class definitions. + return Ok(()); + }; + + // Skip the last trailing own line comment of the suite, if any, otherwise we count + // the lines wrongly by stopping at that comment. + let node_with_last_trailing_comment = std::iter::successors( + statements.last().map(AnyNodeRef::from), + AnyNodeRef::last_child_in_body, + ) + .find(|last_child| comments.has_trailing_own_line(*last_child)); + + let end_of_def_or_class = node_with_last_trailing_comment + .and_then(|child| comments.trailing(child).last().map(Ranged::end)) + .unwrap_or(last_def_or_class.end()); + let existing_newlines = + lines_after_ignoring_end_of_line_trivia(end_of_def_or_class, f.context().source()); + if existing_newlines < 2 { + if f.context().is_preview() { + empty_line().fmt(f)?; + } else { + if last_def_or_class.is_stmt_class_def() && f.options().source_type().is_stub() { + empty_line().fmt(f)?; + } + } + } Ok(()) } } +/// Find nested class or function definitions that need an empty line after them. +/// +/// ```python +/// def f(): +/// if True: +/// +/// def double(s): +/// return s + s +/// +/// print("below function") +/// ``` +fn trailing_function_or_class_def<'a>( + preceding: Option<&'a Stmt>, + comments: &Comments, +) -> Option> { + std::iter::successors( + preceding.map(AnyNodeRef::from), + AnyNodeRef::last_child_in_body, + ) + .take_while(|last_child| + // If there is a comment between preceding and following the empty lines were + // inserted before the comment by preceding and there are no extra empty lines + // after the comment. + // ```python + // class Test: + // def a(self): + // pass + // # trailing comment + // + // + // # two lines before, one line after + // + // c = 30 + // ```` + // This also includes nested class/function definitions, so we stop recursing + // once we see a node with a trailing own line comment: + // ```python + // def f(): + // if True: + // + // def double(s): + // return s + s + // + // # nested trailing own line comment + // print("below function with trailing own line comment") + // ``` + !comments.has_trailing_own_line(*last_child)) + .find(|last_child| { + matches!( + last_child, + AnyNodeRef::StmtFunctionDef(_) | AnyNodeRef::StmtClassDef(_) + ) + }) +} + /// Stub files have bespoke rules for empty lines. /// /// These rules are ported from black (preview mode at time of writing) using the stubs test case: @@ -447,7 +557,7 @@ fn stub_file_empty_lines( hard_line_break().fmt(f) } } - SuiteKind::Class | SuiteKind::Other | SuiteKind::Function => { + SuiteKind::Class | SuiteKind::Other { .. } | SuiteKind::Function => { if (empty_line_condition && lines_after_ignoring_end_of_line_trivia(preceding.end(), source) > 1) || require_empty_line @@ -477,26 +587,14 @@ pub(crate) fn should_insert_blank_line_after_class_in_stub_file( return false; } + let Some(following) = following else { + // We handle newlines at the end of a suite in `between_alternative_blocks_empty_line`. + return false; + }; + let comments = context.comments(); match preceding.as_stmt_class_def() { Some(class) if contains_only_an_ellipsis(&class.body, comments) => { - let Some(following) = following else { - // The formatter is at the start of an alternate branch such as - // an `else` block. - // - // ```python - // if foo: - // class Nested: - // pass - // else: - // pass - // ``` - // - // In the above code, the preceding node is the `Nested` class - // which has no following node. - return true; - }; - // If the preceding class has decorators, then we need to add an empty // line even if it only contains ellipsis. // @@ -916,7 +1014,9 @@ def trailing_func(): #[test] fn nested_level() { - let formatted = format_suite(SuiteKind::Other); + let formatted = format_suite(SuiteKind::Other { + last_suite_in_statement: true, + }); assert_eq!( formatted, diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__comments9.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__comments9.py.snap index 5e25b161c919a..85d3044bacef4 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__comments9.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__comments9.py.snap @@ -505,5 +505,3 @@ def foo(): def bar(): pass ``` - - diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function2.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function2.py.snap index f85dbd5fef575..a9ffd5a3f9a17 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function2.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function2.py.snap @@ -220,5 +220,3 @@ else: with hmm_but_this_should_get_two_preceding_newlines(): pass ``` - - diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__stub.pyi.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__stub.pyi.snap index a2ebb5280a749..13083c6ed09b3 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__stub.pyi.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__stub.pyi.snap @@ -255,5 +255,3 @@ class Conditional: def l(self): ... def m(self): ... ``` - - diff --git a/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap index 08abf4bbeec2b..292918d126272 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap @@ -255,6 +255,68 @@ if True: print() +if True: + def a(): + return 1 +else: + pass + +if True: + # fmt: off + def a(): + return 1 + # fmt: on +else: + pass + +match True: + case 1: + def a(): + return 1 + case 1: + def a(): + return 1 + +try: + def a(): + return 1 +except RuntimeError: + def a(): + return 1 + +try: + def a(): + return 1 +finally: + def a(): + return 1 + +try: + def a(): + return 1 +except RuntimeError: + def a(): + return 1 +except ZeroDivisionError: + def a(): + return 1 +else: + def a(): + return 1 +finally: + def a(): + return 1 + +if raw: + def show_file(lines): + for line in lines: + pass + # Trailing comment not on function or class + +else: + pass + + # NOTE: Please keep this the last block in this file. This tests that we don't insert # empty line(s) at the end of the file due to nested function if True: @@ -558,6 +620,85 @@ if True: print() +if True: + + def a(): + return 1 +else: + pass + +if True: + # fmt: off + def a(): + return 1 + # fmt: on +else: + pass + +match True: + case 1: + + def a(): + return 1 + case 1: + + def a(): + return 1 + + +try: + + def a(): + return 1 +except RuntimeError: + + def a(): + return 1 + + +try: + + def a(): + return 1 +finally: + + def a(): + return 1 + + +try: + + def a(): + return 1 +except RuntimeError: + + def a(): + return 1 +except ZeroDivisionError: + + def a(): + return 1 +else: + + def a(): + return 1 +finally: + + def a(): + return 1 + + +if raw: + + def show_file(lines): + for line in lines: + pass + # Trailing comment not on function or class + +else: + pass + + # NOTE: Please keep this the last block in this file. This tests that we don't insert # empty line(s) at the end of the file due to nested function if True: @@ -593,4 +734,63 @@ def overload4(a: int): ... ``` - +## Preview changes +```diff +--- Stable ++++ Preview +@@ -277,6 +277,7 @@ + + def a(): + return 1 ++ + else: + pass + +@@ -293,6 +294,7 @@ + + def a(): + return 1 ++ + case 1: + + def a(): +@@ -303,6 +305,7 @@ + + def a(): + return 1 ++ + except RuntimeError: + + def a(): +@@ -313,6 +316,7 @@ + + def a(): + return 1 ++ + finally: + + def a(): +@@ -323,18 +327,22 @@ + + def a(): + return 1 ++ + except RuntimeError: + + def a(): + return 1 ++ + except ZeroDivisionError: + + def a(): + return 1 ++ + else: + + def a(): + return 1 ++ + finally: + + def a(): +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@newlines.pyi.snap b/crates/ruff_python_formatter/tests/snapshots/format@newlines.pyi.snap index 98a8c15e96d48..121c3bb85e2d7 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@newlines.pyi.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@newlines.pyi.snap @@ -160,11 +160,17 @@ def f(): pass +if True: + def a(): + return 1 +else: + pass # comment x = 1 + ``` ## Output @@ -302,10 +308,28 @@ x = 1 def f(): pass +if True: + def a(): + return 1 +else: + pass + # comment x = 1 ``` - +## Preview changes +```diff +--- Stable ++++ Preview +@@ -134,6 +134,7 @@ + if True: + def a(): + return 1 ++ + else: + pass + +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap index 58d8cd64f3c22..9d108fc757291 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap @@ -1042,3 +1042,26 @@ def func[T]( lotsoflongargs5: T, ) -> T: ... ``` + + +## Preview changes +```diff +--- Stable ++++ Preview +@@ -161,6 +161,7 @@ + + def f1(): + pass # a ++ + else: + pass + +@@ -170,6 +171,7 @@ + def f2(): + pass + # a ++ + else: + pass + +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap index 7ebc5fab3641b..9bc4576203c3f 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap @@ -609,4 +609,22 @@ if parent_body: ``` - +## Preview changes +```diff +--- Stable ++++ Preview +@@ -93,11 +93,13 @@ + def f(): + pass + # 1 ++ + elif True: + + def f(): + pass + # 2 ++ + else: + + def f(): +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__try.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__try.py.snap index 12409cfd84084..fad6dd510e4ca 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__try.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__try.py.snap @@ -366,4 +366,28 @@ finally: ``` - +## Preview changes +```diff +--- Stable ++++ Preview +@@ -117,16 +117,19 @@ + def f(): + pass + # a ++ + except: + + def f(): + pass + # b ++ + else: + + def f(): + pass + # c ++ + finally: + + def f(): +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@stub_files__blank_line_after_nested_stub_class.pyi.snap b/crates/ruff_python_formatter/tests/snapshots/format@stub_files__blank_line_after_nested_stub_class.pyi.snap index 4f570bba9ffa1..b1b154f8dc74a 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@stub_files__blank_line_after_nested_stub_class.pyi.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@stub_files__blank_line_after_nested_stub_class.pyi.snap @@ -414,6 +414,3 @@ class Eof: class Nested: pass ``` - - - diff --git a/crates/ruff_python_formatter/tests/snapshots/format@stub_files__suite.pyi.snap b/crates/ruff_python_formatter/tests/snapshots/format@stub_files__suite.pyi.snap index 214efa79175ba..7c2b8c94b05a6 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@stub_files__suite.pyi.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@stub_files__suite.pyi.snap @@ -311,4 +311,24 @@ class ComplexStatements: ``` - +## Preview changes +```diff +--- Stable ++++ Preview +@@ -132,6 +132,7 @@ + if sys.version_info >= (3, 12): + @classmethod + def fromtimestamp(cls, timestamp: float, tz: float | None = ...) -> Self: ... ++ + else: + @classmethod + def fromtimestamp(cls, __timestamp: float, tz: float | None = ...) -> Self: ... +@@ -141,6 +142,7 @@ + if sys.version_info >= (3, 8): + @classmethod + def now(cls, tz: float | None = None) -> Self: ... ++ + else: + @classmethod + def now(cls, tz: None = None) -> Self: ... +```