Range formatting: Fix invalid syntax after parenthesizing expression (#9751)

crates/ruff_python_formatter/src/comments/format.rs

@@ -430,31 +430,25 @@ pub(crate) struct FormatNormalizedComment<'a> {
 
 impl Format<PyFormatContext<'_>> for FormatNormalizedComment<'_> {
     fn fmt(&self, f: &mut Formatter<PyFormatContext>) -> FormatResult<()> {
+        let write_sourcemap = f.options().source_map_generation().is_enabled();
+
+        write_sourcemap
+            .then_some(source_position(self.range.start()))
+            .fmt(f)?;
+
         match self.comment {
             Cow::Borrowed(borrowed) => {
                 source_text_slice(TextRange::at(self.range.start(), borrowed.text_len())).fmt(f)?;
-
-                // Write the end position if the borrowed comment is shorter than the original comment
-                // So that we still can map back the end of a comment to the formatted code.
-                if f.options().source_map_generation().is_enabled()
-                    && self.range.len() != borrowed.text_len()
-                {
-                    source_position(self.range.end()).fmt(f)?;
-                }
-
-                Ok(())
             }
 
             Cow::Owned(ref owned) => {
-                write!(
-                    f,
-                    [
-                        text(owned, Some(self.range.start())),
-                        source_position(self.range.end())
-                    ]
-                )
+                text(owned).fmt(f)?;
             }
         }
+
+        write_sourcemap
+            .then_some(source_position(self.range.end()))
+            .fmt(f)
     }
 }
 

crates/ruff_python_formatter/src/expression/expr_number_literal.rs

@@ -20,7 +20,7 @@ impl FormatNodeRule<ExprNumberLiteral> for FormatExprNumberLiteral {
 
                 match normalized {
                     Cow::Borrowed(_) => source_text_slice(range).fmt(f),
-                    Cow::Owned(normalized) => text(&normalized, Some(range.start())).fmt(f),
+                    Cow::Owned(normalized) => text(&normalized).fmt(f),
                 }
             }
             Number::Float(_) => {
@@ -30,7 +30,7 @@ impl FormatNodeRule<ExprNumberLiteral> for FormatExprNumberLiteral {
 
                 match normalized {
                     Cow::Borrowed(_) => source_text_slice(range).fmt(f),
-                    Cow::Owned(normalized) => text(&normalized, Some(range.start())).fmt(f),
+                    Cow::Owned(normalized) => text(&normalized).fmt(f),
                 }
             }
             Number::Complex { .. } => {
@@ -43,7 +43,7 @@ impl FormatNodeRule<ExprNumberLiteral> for FormatExprNumberLiteral {
                         source_text_slice(range.sub_end(TextSize::from(1))).fmt(f)?;
                     }
                     Cow::Owned(normalized) => {
-                        text(&normalized, Some(range.start())).fmt(f)?;
+                        text(&normalized).fmt(f)?;
                     }
                 }
 

crates/ruff_python_formatter/src/expression/mod.rs

@@ -304,30 +304,25 @@ fn format_with_parentheses_comments(
     // Custom FormatNodeRule::fmt variant that only formats the inner comments
     let format_node_rule_fmt = format_with(|f| {
         // No need to handle suppression comments, those are statement only
-        leading_comments(leading_inner).fmt(f)?;
-
-        let is_source_map_enabled = f.options().source_map_generation().is_enabled();
-
-        if is_source_map_enabled {
-            source_position(expression.start()).fmt(f)?;
-        }
-
-        fmt_fields.fmt(f)?;
-
-        if is_source_map_enabled {
-            source_position(expression.end()).fmt(f)?;
-        }
-
-        trailing_comments(trailing_inner).fmt(f)
+        write!(
+            f,
+            [
+                leading_comments(leading_inner),
+                fmt_fields,
+                trailing_comments(trailing_inner)
+            ]
+        )
     });
 
     // The actual parenthesized formatting
-    parenthesized("(", &format_node_rule_fmt, ")")
-        .with_dangling_comments(parentheses_comment)
-        .fmt(f)?;
-    trailing_comments(trailing_outer).fmt(f)?;
-
-    Ok(())
+    write!(
+        f,
+        [
+            parenthesized("(", &format_node_rule_fmt, ")")
+                .with_dangling_comments(parentheses_comment),
+            trailing_comments(trailing_outer)
+        ]
+    )
 }
 
 /// Wraps an expression in an optional parentheses except if its [`NeedsParentheses::needs_parentheses`] implementation

crates/ruff_python_formatter/src/lib.rs

@@ -3,7 +3,7 @@ use tracing::Level;
 
 pub use range::format_range;
 use ruff_formatter::prelude::*;
-use ruff_formatter::{format, FormatError, Formatted, PrintError, Printed, SourceCode};
+use ruff_formatter::{format, write, FormatError, Formatted, PrintError, Printed, SourceCode};
 use ruff_python_ast::AstNode;
 use ruff_python_ast::Mod;
 use ruff_python_index::tokens_and_ranges;
@@ -19,6 +19,7 @@ pub use crate::options::{
     DocstringCode, DocstringCodeLineWidth, MagicTrailingComma, PreviewMode, PyFormatOptions,
     PythonVersion, QuoteStyle,
 };
+use crate::range::is_logical_line;
 pub use crate::shared_traits::{AsFormat, FormattedIter, FormattedIterExt, IntoFormat};
 use crate::verbatim::suppressed_node;
 
@@ -59,19 +60,27 @@ where
         } else {
             leading_comments(node_comments.leading).fmt(f)?;
 
-            let is_source_map_enabled = f.options().source_map_generation().is_enabled();
+            let node_ref = node.as_any_node_ref();
 
-            if is_source_map_enabled {
-                source_position(node.start()).fmt(f)?;
-            }
+            // Emit source map information for nodes that are valid "narrowing" targets
+            // in range formatting. Never emit source map information if they're disabled
+            // for performance reasons.
+            let emit_source_position = (is_logical_line(node_ref) || node_ref.is_mod_module())
+                && f.options().source_map_generation().is_enabled();
+
+            emit_source_position
+                .then_some(source_position(node.start()))
+                .fmt(f)?;
 
             self.fmt_fields(node, f)?;
 
-            if is_source_map_enabled {
-                source_position(node.end()).fmt(f)?;
-            }
-
-            trailing_comments(node_comments.trailing).fmt(f)
+            write!(
+                f,
+                [
+                    emit_source_position.then_some(source_position(node.end())),
+                    trailing_comments(node_comments.trailing)
+                ]
+            )
         }
     }
 
@@ -249,15 +258,36 @@ def main() -> None:
     #[ignore]
     #[test]
     fn range_formatting_quick_test() {
-        let source = r#"def  test2(  a):    print("body"  )
-    "#;
+        let source = r#"def convert_str(value: str) -> str:  # Trailing comment
+    """Return a string as-is."""
 
-        let start = TextSize::new(20);
-        let end = TextSize::new(35);
+<RANGE_START>
+
+    return value  # Trailing comment
+<RANGE_END>"#;
+
+        let mut source = source.to_string();
+
+        let start = TextSize::try_from(
+            source
+                .find("<RANGE_START>")
+                .expect("Start marker not found"),
+        )
+        .unwrap();
+
+        source.replace_range(
+            start.to_usize()..start.to_usize() + "<RANGE_START>".len(),
+            "",
+        );
+
+        let end =
+            TextSize::try_from(source.find("<RANGE_END>").expect("End marker not found")).unwrap();
+
+        source.replace_range(end.to_usize()..end.to_usize() + "<RANGE_END>".len(), "");
 
         let source_type = PySourceType::Python;
         let options = PyFormatOptions::from_source_type(source_type);
-        let printed = format_range(source, TextRange::new(start, end), options).unwrap();
+        let printed = format_range(&source, TextRange::new(start, end), options).unwrap();
 
         let mut formatted = source.to_string();
         formatted.replace_range(
@@ -267,9 +297,11 @@ def main() -> None:
 
         assert_eq!(
             formatted,
-            r#"def test2(a):
-    print("body")
-    "#
+            r#"print ( "format me" )
+print("format me")
+print("format me")
+print ( "format me" )
+print ( "format me" )"#
         );
     }
 
@@ -300,7 +332,7 @@ def main() -> None:
                     while let Some(word) = words.next() {
                         let is_last = words.peek().is_none();
                         let format_word = format_with(|f| {
-                            write!(f, [text(word, None)])?;
+                            write!(f, [text(word)])?;
 
                             if is_last {
                                 write!(f, [token("\"")])?;

crates/ruff_python_formatter/src/options.rs

@@ -230,7 +230,6 @@ impl FormatOptions for PyFormatOptions {
             line_width: self.line_width,
             line_ending: self.line_ending,
             indent_style: self.indent_style,
-            source_map_generation: self.source_map_generation,
         }
     }
 }

crates/ruff_python_formatter/src/other/identifier.rs

@@ -74,7 +74,7 @@ impl Format<PyFormatContext<'_>> for DotDelimitedIdentifier<'_> {
                 .chars()
                 .filter(|c| !is_python_whitespace(*c) && !matches!(c, '\n' | '\r' | '\\'))
                 .collect();
-            text(&no_whitespace, Some(self.0.start())).fmt(f)
+            text(&no_whitespace).fmt(f)
         } else {
             source_text_slice(self.0.range()).fmt(f)
         }

crates/ruff_python_formatter/src/range.rs

@@ -118,7 +118,7 @@ pub fn format_range(
     )?;
 
     let printed = formatted.print_with_indent(base_indent)?;
-    Ok(printed.slice_range(narrowed_range))
+    Ok(printed.slice_range(narrowed_range, source))
 }
 
 /// Finds the node with the minimum covering range of `range`.
@@ -420,8 +420,13 @@ impl PreorderVisitor<'_> for NarrowRange<'_> {
         // Find the end offset of the closest node to the end offset of the formatting range.
         // We do this by iterating over end positions that we know generate source map entries end pick the end
         // that ends closest or after the searched range's end.
-        let trailing_comments = self.context.comments().trailing(node);
-        self.narrow(trailing_comments);
+        self.narrow(
+            self.context
+                .comments()
+                .trailing(node)
+                .iter()
+                .filter(|comment| comment.line_position().is_own_line()),
+        );
     }
 
     fn visit_body(&mut self, body: &'_ [Stmt]) {
@@ -552,7 +557,7 @@ impl NarrowRange<'_> {
     }
 }
 
-const fn is_logical_line(node: AnyNodeRef) -> bool {
+pub(crate) const fn is_logical_line(node: AnyNodeRef) -> bool {
     // Make sure to update [`FormatEnclosingLine`] when changing this.
     node.is_statement()
         || node.is_decorator()

crates/ruff_python_formatter/src/statement/suite.rs

@@ -772,9 +772,17 @@ impl Format<PyFormatContext<'_>> for DocstringStmt<'_> {
                 f,
                 [
                     leading_comments(node_comments.leading),
+                    f.options()
+                        .source_map_generation()
+                        .is_enabled()
+                        .then_some(source_position(self.docstring.start())),
                     string_literal
                         .format()
                         .with_options(ExprStringLiteralKind::Docstring),
+                    f.options()
+                        .source_map_generation()
+                        .is_enabled()
+                        .then_some(source_position(self.docstring.end())),
                 ]
             )?;
 

crates/ruff_python_formatter/src/string/docstring.rs

@@ -4,6 +4,7 @@
 
 use std::{borrow::Cow, collections::VecDeque};
 
+use ruff_formatter::printer::SourceMapGeneration;
 use ruff_python_parser::ParseError;
 use {once_cell::sync::Lazy, regex::Regex};
 use {
@@ -116,14 +117,7 @@ pub(crate) fn format(normalized: &NormalizedString, f: &mut PyFormatter) -> Form
     let mut lines = docstring.lines().peekable();
 
     // Start the string
-    write!(
-        f,
-        [
-            normalized.prefix,
-            normalized.quotes,
-            source_position(normalized.start()),
-        ]
-    )?;
+    write!(f, [normalized.prefix, normalized.quotes])?;
     // We track where in the source docstring we are (in source code byte offsets)
     let mut offset = normalized.start();
 
@@ -152,7 +146,7 @@ pub(crate) fn format(normalized: &NormalizedString, f: &mut PyFormatter) -> Form
         if already_normalized {
             source_text_slice(trimmed_line_range).fmt(f)?;
         } else {
-            text(trim_both, Some(trimmed_line_range.start())).fmt(f)?;
+            text(trim_both).fmt(f)?;
         }
     }
     offset += first.text_len();
@@ -205,7 +199,7 @@ pub(crate) fn format(normalized: &NormalizedString, f: &mut PyFormatter) -> Form
         space().fmt(f)?;
     }
 
-    write!(f, [source_position(normalized.end()), normalized.quotes])
+    write!(f, [normalized.quotes])
 }
 
 fn contains_unescaped_newline(haystack: &str) -> bool {
@@ -404,7 +398,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
             // prepend the in-docstring indentation to the string.
             let indent_len = indentation_length(trim_end) - self.stripped_indentation_length;
             let in_docstring_indent = " ".repeat(usize::from(indent_len)) + trim_end.trim_start();
-            text(&in_docstring_indent, Some(line.offset)).fmt(self.f)?;
+            text(&in_docstring_indent).fmt(self.f)?;
         } else {
             // Take the string with the trailing whitespace removed, then also
             // skip the leading whitespace.
@@ -414,11 +408,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
                 source_text_slice(trimmed_line_range).fmt(self.f)?;
             } else {
                 // All indents are ascii spaces, so the slicing is correct.
-                text(
-                    &trim_end[usize::from(self.stripped_indentation_length)..],
-                    Some(trimmed_line_range.start()),
-                )
-                .fmt(self.f)?;
+                text(&trim_end[usize::from(self.stripped_indentation_length)..]).fmt(self.f)?;
             }
         }
 
@@ -495,7 +485,8 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
             // tabs will get erased anyway, we just clobber them here
             // instead of later, and as a result, get more consistent
             // results.
-            .with_indent_style(IndentStyle::Space);
+            .with_indent_style(IndentStyle::Space)
+            .with_source_map_generation(SourceMapGeneration::Disabled);
         let printed = match docstring_format_source(options, self.quote_char, &codeblob) {
             Ok(printed) => printed,
             Err(FormatModuleError::FormatError(err)) => return Err(err),

crates/ruff_python_formatter/src/string/mod.rs

@@ -417,7 +417,7 @@ impl Format<PyFormatContext<'_>> for NormalizedString<'_> {
                 source_text_slice(self.range()).fmt(f)?;
             }
             Cow::Owned(normalized) => {
-                text(normalized, Some(self.start())).fmt(f)?;
+                text(normalized).fmt(f)?;
             }
         }
         self.quotes.fmt(f)

crates/ruff_python_formatter/src/verbatim.rs

@@ -752,7 +752,14 @@ impl Format<PyFormatContext<'_>> for FormatVerbatimStatementRange {
                 }
             } else {
                 // Non empty line, write the text of the line
-                verbatim_text(trimmed_line_range).fmt(f)?;
+                write!(
+                    f,
+                    [
+                        source_position(trimmed_line_range.start()),
+                        verbatim_text(trimmed_line_range),
+                        source_position(trimmed_line_range.end())
+                    ]
+                )?;
 
                 // Write the line separator that terminates the line, except if it is the last line (that isn't separated by a hard line break).
                 if logical_line.has_trailing_newline {
@@ -892,13 +899,7 @@ impl Format<PyFormatContext<'_>> for VerbatimText {
                 write!(f, [source_text_slice(self.verbatim_range)])?;
             }
             Cow::Owned(cleaned) => {
-                write!(
-                    f,
-                    [
-                        text(&cleaned, Some(self.verbatim_range.start())),
-                        source_position(self.verbatim_range.end())
-                    ]
-                )?;
+                text(&cleaned).fmt(f)?;
             }
         }
 
@@ -957,7 +958,9 @@ impl Format<PyFormatContext<'_>> for FormatSuppressedNode<'_> {
             f,
             [
                 leading_comments(node_comments.leading),
+                source_position(verbatim_range.start()),
                 verbatim_text(verbatim_range),
+                source_position(verbatim_range.end()),
                 trailing_comments(node_comments.trailing)
             ]
         )
@@ -969,8 +972,17 @@ pub(crate) fn write_suppressed_clause_header(
     header: ClauseHeader,
     f: &mut PyFormatter,
 ) -> FormatResult<()> {
+    let range = header.range(f.context().source())?;
+
     // Write the outer comments and format the node as verbatim
-    write!(f, [verbatim_text(header.range(f.context().source())?)])?;
+    write!(
+        f,
+        [
+            source_position(range.start()),
+            verbatim_text(range),
+            source_position(range.end())
+        ]
+    )?;
 
     let comments = f.context().comments();
     header.visit(&mut |child| {