From b823866155b3cec8511dd8b246ad3aceea53b642 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 16:34:01 -0500 Subject: [PATCH] revert preview and comment placement changes --- .../ruff_python_formatter/src/comments/mod.rs | 17 +-- .../src/comments/placement.rs | 62 +-------- .../src/comments/visitor.rs | 25 +--- crates/ruff_python_formatter/src/lib.rs | 16 +-- crates/ruff_python_formatter/src/range.rs | 7 +- .../src/string/docstring.rs | 7 +- .../format@expression__lambda.py.snap | 128 ++++++------------ crates/ruff_wasm/src/lib.rs | 7 +- 8 files changed, 62 insertions(+), 207 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index 9c137080e5..07fc789f34 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -101,7 +101,6 @@ use ruff_python_trivia::{CommentLinePosition, CommentRanges, SuppressionKind}; use ruff_text_size::{Ranged, TextRange}; pub(crate) use visitor::collect_comments; -use crate::PreviewMode; use crate::comments::debug::{DebugComment, DebugComments}; use crate::comments::map::{LeadingDanglingTrailing, MultiMap}; use crate::comments::node_key::NodeRefEqualityKey; @@ -249,19 +248,16 @@ impl<'a> Comments<'a> { root: impl Into>, source_code: SourceCode<'a>, comment_ranges: &'a CommentRanges, - preview: PreviewMode, ) -> Self { fn collect_comments<'a>( root: AnyNodeRef<'a>, source_code: SourceCode<'a>, comment_ranges: &'a CommentRanges, - preview: PreviewMode, ) -> Comments<'a> { let map = if comment_ranges.is_empty() { CommentsMap::new() } else { - let mut builder = - CommentsMapBuilder::new(source_code.as_str(), comment_ranges, preview); + let mut builder = CommentsMapBuilder::new(source_code.as_str(), comment_ranges); CommentsVisitor::new(source_code, comment_ranges, &mut builder).visit(root); builder.finish() }; @@ -269,7 +265,7 @@ impl<'a> Comments<'a> { Comments::new(map, comment_ranges) } - collect_comments(root.into(), source_code, comment_ranges, preview) + collect_comments(root.into(), source_code, comment_ranges) } /// Returns `true` if the given `node` has any comments. @@ -524,7 +520,7 @@ mod tests { use ruff_python_parser::{ParseOptions, Parsed, parse}; use ruff_python_trivia::CommentRanges; - use crate::{PreviewMode, comments::Comments}; + use crate::comments::Comments; struct CommentsTestCase<'a> { parsed: Parsed, @@ -548,12 +544,7 @@ mod tests { } fn to_comments(&self) -> Comments<'_> { - Comments::from_ast( - self.parsed.syntax(), - self.source_code, - &self.comment_ranges, - PreviewMode::Disabled, - ) + Comments::from_ast(self.parsed.syntax(), self.source_code, &self.comment_ranges) } } diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 8bf936f5fb..28397b6dcf 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1,8 +1,7 @@ use ast::helpers::comment_indentation_after; use ruff_python_ast::whitespace::indentation; use ruff_python_ast::{ - self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, ParameterWithDefault, - Parameters, StringLike, + self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, Parameters, StringLike, }; use ruff_python_trivia::{ BackwardsTokenizer, CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer, @@ -12,7 +11,6 @@ use ruff_source_file::LineRanges; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use std::cmp::Ordering; -use crate::PreviewMode; use crate::comments::visitor::{CommentPlacement, DecoratedComment}; use crate::expression::expr_slice::{ExprSliceCommentSection, assign_comment_in_slice}; use crate::expression::parentheses::is_expression_parenthesized; @@ -26,12 +24,11 @@ pub(super) fn place_comment<'a>( comment: DecoratedComment<'a>, comment_ranges: &CommentRanges, source: &str, - preview: PreviewMode, ) -> CommentPlacement<'a> { handle_parenthesized_comment(comment, source) .or_else(|comment| handle_end_of_line_comment_around_body(comment, source)) .or_else(|comment| handle_own_line_comment_around_body(comment, source)) - .or_else(|comment| handle_enclosed_comment(comment, comment_ranges, source, preview)) + .or_else(|comment| handle_enclosed_comment(comment, comment_ranges, source)) } /// Handle parenthesized comments. A parenthesized comment is a comment that appears within a @@ -196,7 +193,6 @@ fn handle_enclosed_comment<'a>( comment: DecoratedComment<'a>, comment_ranges: &CommentRanges, source: &str, - preview: PreviewMode, ) -> CommentPlacement<'a> { match comment.enclosing_node() { AnyNodeRef::Parameters(parameters) => { @@ -209,9 +205,6 @@ fn handle_enclosed_comment<'a>( }) } AnyNodeRef::Parameter(parameter) => handle_parameter_comment(comment, parameter, source), - AnyNodeRef::ParameterWithDefault(parameter) => { - handle_parameter_with_default_comment(comment, parameter, source) - } AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) | AnyNodeRef::PatternArguments(_) => { handle_bracketed_end_of_line_comment(comment, source) } @@ -238,7 +231,7 @@ fn handle_enclosed_comment<'a>( } AnyNodeRef::ExprUnaryOp(unary_op) => handle_unary_op_comment(comment, unary_op, source), AnyNodeRef::ExprNamed(_) => handle_named_expr_comment(comment, source), - AnyNodeRef::ExprLambda(lambda) => handle_lambda_comment(comment, lambda, source, preview), + AnyNodeRef::ExprLambda(lambda) => handle_lambda_comment(comment, lambda, source), AnyNodeRef::ExprDict(_) => handle_dict_unpacking_comment(comment, source) .or_else(|comment| handle_bracketed_end_of_line_comment(comment, source)) .or_else(|comment| handle_key_value_comment(comment, source)), @@ -871,63 +864,19 @@ fn handle_parameter_comment<'a>( assert_eq!(colon.kind(), SimpleTokenKind::Colon); - return if comment.start() < colon.start() { + if comment.start() < colon.start() { // The comment is before the colon, pull it out and make it a leading comment of the parameter. CommentPlacement::leading(parameter, comment) } else { CommentPlacement::Default(comment) - }; - } - - // If the comment falls before the parameter, and the grandparent node is a lambda (the - // parent node is always `Parameters`), make the comment a dangling lambda parameter, so - // that it can be formatted with the other dangling lambda comments in the lambda header. - // - // This is especially important for niche cases like: - // - // ```py - // ( - // lambda # comment 1 - // * # comment 2 - // x: # comment 3 - // x - // ) - // ``` - // - // where `# comment 2` otherwise becomes a leading comment on `*x` in the first format and a - // dangling comment on the lambda in the second. Instead, make it a dangling comment on the - // enclosing lambda from the start. - match comment.enclosing_grandparent() { - Some(AnyNodeRef::ExprLambda(lambda)) => return CommentPlacement::dangling(lambda, comment), - Some(AnyNodeRef::StmtExpr(ast::StmtExpr { value, .. })) if value.is_lambda_expr() => { - return CommentPlacement::dangling(&**value, comment); } - _ => {} - } - - if comment.start() < parameter.name.start() { + } else if comment.start() < parameter.name.start() { CommentPlacement::leading(parameter, comment) } else { CommentPlacement::Default(comment) } } -/// Handles parameters with defaults inside of lambda expressions by making them dangling comments -/// on the lambda itself. -fn handle_parameter_with_default_comment<'a>( - comment: DecoratedComment<'a>, - _parameter: &'a ParameterWithDefault, - _source: &str, -) -> CommentPlacement<'a> { - match comment.enclosing_grandparent() { - Some(AnyNodeRef::ExprLambda(lambda)) => CommentPlacement::dangling(lambda, comment), - Some(AnyNodeRef::StmtExpr(ast::StmtExpr { value, .. })) if value.is_lambda_expr() => { - CommentPlacement::dangling(&**value, comment) - } - _ => CommentPlacement::Default(comment), - } -} - /// Handles comments between the left side and the operator of a binary expression (trailing comments of the left), /// and trailing end-of-line comments that are on the same line as the operator. /// @@ -1865,7 +1814,6 @@ fn handle_lambda_comment<'a>( comment: DecoratedComment<'a>, lambda: &'a ast::ExprLambda, source: &str, - _preview: PreviewMode, ) -> CommentPlacement<'a> { if let Some(parameters) = lambda.parameters.as_deref() { // End-of-line comments between the `lambda` and the parameters are dangling on the lambda: diff --git a/crates/ruff_python_formatter/src/comments/visitor.rs b/crates/ruff_python_formatter/src/comments/visitor.rs index 43a4445a19..b9670a3ee8 100644 --- a/crates/ruff_python_formatter/src/comments/visitor.rs +++ b/crates/ruff_python_formatter/src/comments/visitor.rs @@ -11,7 +11,6 @@ use ruff_python_ast::visitor::source_order::*; use ruff_python_trivia::{CommentLinePosition, CommentRanges}; use ruff_text_size::{Ranged, TextRange, TextSize}; -use crate::PreviewMode; use crate::comments::node_key::NodeRefEqualityKey; use crate::comments::placement::place_comment; use crate::comments::{CommentsMap, SourceComment}; @@ -85,13 +84,11 @@ impl<'ast> SourceOrderVisitor<'ast> for CommentsVisitor<'ast, '_> { break; } - let mut parents = self.parents.iter().rev(); let comment = DecoratedComment { enclosing: enclosing_node, preceding: self.preceding_node, following: Some(node), - parent: parents.nth(1).copied(), - grandparent: parents.next().copied(), + parent: self.parents.iter().rev().nth(1).copied(), line_position: CommentLinePosition::for_range( *comment_range, self.source_code.as_str(), @@ -130,11 +127,9 @@ impl<'ast> SourceOrderVisitor<'ast> for CommentsVisitor<'ast, '_> { break; } - let mut parents = self.parents.iter().rev(); let comment = DecoratedComment { enclosing: node, - parent: parents.nth(1).copied(), - grandparent: parents.next().copied(), + parent: self.parents.last().copied(), preceding: self.preceding_node, following: None, line_position: CommentLinePosition::for_range( @@ -186,7 +181,6 @@ pub(crate) struct DecoratedComment<'a> { preceding: Option>, following: Option>, parent: Option>, - grandparent: Option>, line_position: CommentLinePosition, slice: SourceCodeSlice, } @@ -217,11 +211,6 @@ impl<'a> DecoratedComment<'a> { self.parent } - /// Returns the grandparent of the enclosing node, if any - pub(super) fn enclosing_grandparent(&self) -> Option> { - self.grandparent - } - /// Returns the comment's preceding node. /// /// The direct child node (ignoring lists) of the [`enclosing_node`](DecoratedComment::enclosing_node) that precedes this comment. @@ -546,12 +535,11 @@ pub(super) struct CommentsMapBuilder<'a> { /// We need those for backwards lexing comment_ranges: &'a CommentRanges, source: &'a str, - preview: PreviewMode, } impl<'a> PushComment<'a> for CommentsMapBuilder<'a> { fn push_comment(&mut self, placement: DecoratedComment<'a>) { - let placement = place_comment(placement, self.comment_ranges, self.source, self.preview); + let placement = place_comment(placement, self.comment_ranges, self.source); match placement { CommentPlacement::Leading { node, comment } => { self.push_leading_comment(node, comment); @@ -613,16 +601,11 @@ impl<'a> PushComment<'a> for CommentsMapBuilder<'a> { } impl<'a> CommentsMapBuilder<'a> { - pub(crate) fn new( - source: &'a str, - comment_ranges: &'a CommentRanges, - preview: PreviewMode, - ) -> Self { + pub(crate) fn new(source: &'a str, comment_ranges: &'a CommentRanges) -> Self { Self { comments: CommentsMap::default(), comment_ranges, source, - preview, } } diff --git a/crates/ruff_python_formatter/src/lib.rs b/crates/ruff_python_formatter/src/lib.rs index c797b67018..bf68598e13 100644 --- a/crates/ruff_python_formatter/src/lib.rs +++ b/crates/ruff_python_formatter/src/lib.rs @@ -177,12 +177,7 @@ where &'a N: Into>, { let source_code = SourceCode::new(source); - let comments = Comments::from_ast( - parsed.syntax(), - source_code, - comment_ranges, - options.preview(), - ); + let comments = Comments::from_ast(parsed.syntax(), source_code, comment_ranges); let formatted = format!( PyFormatContext::new(options, source, comments, parsed.tokens()), @@ -218,14 +213,9 @@ pub fn formatted_file(db: &dyn Db, file: File) -> Result, FormatM } /// Public function for generating a printable string of the debug comments. -pub fn pretty_comments( - module: &Mod, - comment_ranges: &CommentRanges, - source: &str, - preview: PreviewMode, -) -> String { +pub fn pretty_comments(module: &Mod, comment_ranges: &CommentRanges, source: &str) -> String { let source_code = SourceCode::new(source); - let comments = Comments::from_ast(module, source_code, comment_ranges, preview); + let comments = Comments::from_ast(module, source_code, comment_ranges); std::format!("{comments:#?}", comments = comments.debug(source_code)) } diff --git a/crates/ruff_python_formatter/src/range.rs b/crates/ruff_python_formatter/src/range.rs index fdbfb64e81..436c1a12c2 100644 --- a/crates/ruff_python_formatter/src/range.rs +++ b/crates/ruff_python_formatter/src/range.rs @@ -76,12 +76,7 @@ pub fn format_range( let parsed = parse(source, ParseOptions::from(options.source_type()))?; let source_code = SourceCode::new(source); let comment_ranges = CommentRanges::from(parsed.tokens()); - let comments = Comments::from_ast( - parsed.syntax(), - source_code, - &comment_ranges, - options.preview(), - ); + let comments = Comments::from_ast(parsed.syntax(), source_code, &comment_ranges); let mut context = PyFormatContext::new( options.with_source_map_generation(SourceMapGeneration::Enabled), diff --git a/crates/ruff_python_formatter/src/string/docstring.rs b/crates/ruff_python_formatter/src/string/docstring.rs index e0cb398c48..ad357fc65e 100644 --- a/crates/ruff_python_formatter/src/string/docstring.rs +++ b/crates/ruff_python_formatter/src/string/docstring.rs @@ -1584,12 +1584,7 @@ fn docstring_format_source( let parsed = ruff_python_parser::parse(source, ParseOptions::from(source_type))?; let comment_ranges = CommentRanges::from(parsed.tokens()); let source_code = ruff_formatter::SourceCode::new(source); - let comments = crate::Comments::from_ast( - parsed.syntax(), - source_code, - &comment_ranges, - options.preview(), - ); + let comments = crate::Comments::from_ast(parsed.syntax(), source_code, &comment_ranges); let ctx = PyFormatContext::new(options, source, comments, parsed.tokens()) .in_docstring(docstring_quote_style); diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap index e1fd5c4a0c..f0b948270a 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap @@ -894,16 +894,16 @@ lambda a, /, c: a ( lambda # comment 1 - *x: # comment 2 + *x: # comment 3 x ) ( lambda # comment 1 - *x: # comment 2 - # comment 3 + # comment 2 + *x: # comment 3 x ) @@ -1500,20 +1500,22 @@ transform = ( ) ( - lambda *x: # comment 2 - x + lambda + # comment 2 + *x: x ) ( lambda # comment 1 - *x: # comment 2 - x + # comment 2 + *x: x ) ( lambda # comment 1 - y, *x: # comment 2 - x + y, + # comment 2 + *x: x ) ( @@ -1523,13 +1525,15 @@ transform = ( ) ( - lambda *x, **y: # comment 2 - x + lambda + # comment 2 + *x, **y: x ) ( - lambda **x: # comment 1 - x + lambda + # comment 1 + **x: x ) ``` @@ -1626,16 +1630,14 @@ transform = ( ) ( -@@ -135,17 +119,18 @@ - ( +@@ -136,16 +120,18 @@ lambda # comment 1 + # comment 2 - *x: -- # comment 2 - # comment 3 - x + *x: ( -+ # comment 2 + # comment 3 + x + ) @@ -1643,16 +1645,16 @@ transform = ( ( lambda # comment 1 -- *x: # comment 2 -- # comment 3 + # comment 2 +- *x: # comment 3 - x -+ *x: ( # comment 2 # comment 3 ++ *x: ( # comment 3 + x + ) ) lambda *x: x -@@ -161,30 +146,34 @@ +@@ -161,30 +147,34 @@ ) ( @@ -1699,7 +1701,7 @@ transform = ( x ) ) -@@ -192,11 +181,12 @@ +@@ -192,11 +182,12 @@ ( lambda # 1 # 2 @@ -1717,7 +1719,7 @@ transform = ( ) ( -@@ -204,9 +194,10 @@ +@@ -204,9 +195,10 @@ # 2 x, # 3 # 4 @@ -1731,7 +1733,7 @@ transform = ( ) ( -@@ -218,71 +209,79 @@ +@@ -218,71 +210,79 @@ # Leading lambda x: ( @@ -1870,7 +1872,7 @@ transform = ( # Regression tests for https://github.com/astral-sh/ruff/issues/8179 -@@ -291,9 +290,9 @@ +@@ -291,9 +291,9 @@ c, d, e, @@ -1883,7 +1885,7 @@ transform = ( ) -@@ -302,15 +301,9 @@ +@@ -302,15 +302,9 @@ c, d, e, @@ -1902,7 +1904,7 @@ transform = ( g=10, ) -@@ -320,9 +313,9 @@ +@@ -320,9 +314,9 @@ c, d, e, @@ -1915,7 +1917,7 @@ transform = ( ) -@@ -338,9 +331,9 @@ +@@ -338,9 +332,9 @@ class C: function_dict: Dict[Text, Callable[[CRFToken], Any]] = { @@ -1928,7 +1930,7 @@ transform = ( } -@@ -352,42 +345,40 @@ +@@ -352,42 +346,40 @@ def foo(): if True: if True: @@ -1987,7 +1989,7 @@ transform = ( CREATE TABLE {table} AS SELECT ROW_NUMBER() OVER () AS id, {var} FROM ( -@@ -402,18 +393,19 @@ +@@ -402,18 +394,19 @@ long_assignment_target.with_attribute.and_a_slice[with_an_index] = ( # 1 # 2 @@ -2014,7 +2016,7 @@ transform = ( ) very_long_variable_name_x, very_long_variable_name_y = ( -@@ -421,8 +413,8 @@ +@@ -421,8 +414,8 @@ lambda b: b * another_very_long_expression_here, ) @@ -2025,7 +2027,7 @@ transform = ( x, more_args, additional_parameters ) ) -@@ -458,12 +450,12 @@ +@@ -458,12 +451,12 @@ [ # Not fluent param( @@ -2040,7 +2042,7 @@ transform = ( ), param( lambda left, right: ( -@@ -472,9 +464,9 @@ +@@ -472,9 +465,9 @@ ), param(lambda left, right: ibis.timestamp("2017-04-01").cast(dt.date)), param( @@ -2053,7 +2055,7 @@ transform = ( ), # This is too long on one line in the lambda body and gets wrapped # inside the body. -@@ -508,16 +500,18 @@ +@@ -508,16 +501,18 @@ ] # adds parentheses around the body @@ -2075,7 +2077,7 @@ transform = ( lambda x, y, z: ( x + y + z -@@ -528,7 +522,7 @@ +@@ -528,7 +523,7 @@ x + y + z # trailing eol body ) @@ -2084,7 +2086,7 @@ transform = ( lambda x, y, z: ( # leading body -@@ -540,21 +534,23 @@ +@@ -540,21 +535,23 @@ ) ( @@ -2118,7 +2120,7 @@ transform = ( # dangling header comment source_bucket if name == source_bucket_name -@@ -562,8 +558,7 @@ +@@ -562,8 +559,7 @@ ) ( @@ -2128,7 +2130,7 @@ transform = ( source_bucket if name == source_bucket_name else storage.Bucket(mock_service, destination_bucket_name) -@@ -571,61 +566,70 @@ +@@ -571,61 +567,70 @@ ) ( @@ -2231,7 +2233,7 @@ transform = ( ) ( -@@ -638,27 +642,31 @@ +@@ -638,27 +643,31 @@ ( lambda # comment @@ -2271,7 +2273,7 @@ transform = ( ) ( -@@ -666,19 +674,20 @@ +@@ -666,19 +675,20 @@ # 2 left, # 3 # 4 @@ -2302,7 +2304,7 @@ transform = ( ) ) ) -@@ -696,65 +705,72 @@ +@@ -696,48 +706,52 @@ foo( lambda from_ts, # but still wrap the body if it gets too long to_ts, @@ -2379,48 +2381,4 @@ transform = ( ) ( -- lambda *x: # comment 2 -- x -+ lambda *x: ( # comment 2 -+ x -+ ) - ) - - ( - lambda # comment 1 -- *x: # comment 2 -- x -+ *x: ( # comment 2 -+ x -+ ) - ) - - ( - lambda # comment 1 -- y, *x: # comment 2 -- x -+ y, *x: ( # comment 2 -+ x -+ ) - ) - - ( -@@ -764,11 +780,13 @@ - ) - - ( -- lambda *x, **y: # comment 2 -- x -+ lambda *x, **y: ( # comment 2 -+ x -+ ) - ) - - ( -- lambda **x: # comment 1 -- x -+ lambda **x: ( # comment 1 -+ x -+ ) - ) ``` diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index c23a510ab3..6dd49a15bf 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -289,12 +289,7 @@ impl Workspace { pub fn comments(&self, contents: &str) -> Result { let parsed = ParsedModule::from_source(contents)?; let comment_ranges = CommentRanges::from(parsed.parsed.tokens()); - let comments = pretty_comments( - parsed.parsed.syntax(), - &comment_ranges, - contents, - self.settings.formatter.preview, - ); + let comments = pretty_comments(parsed.parsed.syntax(), &comment_ranges, contents); Ok(comments) }