diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py index 572518635e..45b6e40732 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py @@ -222,3 +222,94 @@ x = ( x = ( () - () # ) + + +# Avoid unnecessary parentheses around multiline strings. +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" % ( + self.base_url, + date.today(), +) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + # Needs parentheses + % ( + self.base_url, + date.today(), + ) +) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + # Needs parentheses + ( + self.base_url, + date.today(), + ) +) + + +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" + a.call.expression( + self.base_url, + date.today(), +) + +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" + sssssssssssssssssssssssssssssssssssssssssooooo * looooooooooooooooooooooooooooooongggggggggggg + +call(arg1, arg2, """ +short +""", arg3=True) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + ( + self.base_url + ) +) + + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + ( + # Needs parentheses + self.base_url + ) +) diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index 57ded7ba08..91ff7e2b8b 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -330,6 +330,17 @@ impl<'a> Comments<'a> { Self::new(map) } + /// Returns `true` if the given `node` has any comments. + #[inline] + pub(crate) fn has(&self, node: T) -> bool + where + T: Into>, + { + self.data + .comments + .has(&NodeRefEqualityKey::from_ref(node.into())) + } + /// Returns `true` if the given `node` has any [leading comments](self#leading-comments). #[inline] pub(crate) fn has_leading(&self, node: T) -> bool diff --git a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs index 8c1f823f43..e25aaeaaa4 100644 --- a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs @@ -10,7 +10,8 @@ use ruff_python_ast::{ }; use crate::comments::{trailing_comments, trailing_node_comments, SourceComment}; -use crate::expression::expr_constant::ExprConstantLayout; +use crate::expression::expr_constant::{is_multiline_string, ExprConstantLayout}; +use crate::expression::has_parentheses; use crate::expression::parentheses::{ in_parentheses_only_group, in_parentheses_only_soft_line_break, in_parentheses_only_soft_line_break_or_space, is_expression_parenthesized, parenthesized, @@ -263,10 +264,23 @@ impl NeedsParentheses for ExprBinOp { fn needs_parentheses( &self, parent: AnyNodeRef, - _context: &PyFormatContext, + context: &PyFormatContext, ) -> OptionalParentheses { if parent.is_expr_await() && !self.op.is_pow() { OptionalParentheses::Always + } else if let Expr::Constant(constant) = self.left.as_ref() { + // Multiline strings are guaranteed to never fit, avoid adding unnecessary parentheses + if !constant.value.is_implicit_concatenated() + && is_multiline_string(constant, context.source()) + && has_parentheses(&self.right, context).is_some() + && !context.comments().has_dangling(self) + && !context.comments().has(self.left.as_ref()) + && !context.comments().has(self.right.as_ref()) + { + OptionalParentheses::Never + } else { + OptionalParentheses::Multiline + } } else { OptionalParentheses::Multiline } diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition.py.snap index 25780932c6..fbda66e07b 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition.py.snap @@ -227,27 +227,17 @@ class C: assert expected( value, is_going_to_be="too long to fit in a single line", srsly=True -@@ -153,7 +154,8 @@ - " because it's too long" - ) - -- dis_c_instance_method = """\ -+ dis_c_instance_method = ( -+ """\ - %3d 0 LOAD_FAST 1 (x) - 2 LOAD_CONST 1 (1) - 4 COMPARE_OP 2 (==) -@@ -161,8 +163,8 @@ +@@ -161,9 +162,7 @@ 8 STORE_ATTR 0 (x) 10 LOAD_CONST 0 (None) 12 RETURN_VALUE - """ % ( - _C.__init__.__code__.co_firstlineno + 1, -+ """ -+ % (_C.__init__.__code__.co_firstlineno + 1,) - ) +- ) ++ """ % (_C.__init__.__code__.co_firstlineno + 1,) assert ( + expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect ``` ## Ruff Output @@ -409,8 +399,7 @@ class C: " because it's too long" ) - dis_c_instance_method = ( - """\ + dis_c_instance_method = """\ %3d 0 LOAD_FAST 1 (x) 2 LOAD_CONST 1 (1) 4 COMPARE_OP 2 (==) @@ -418,9 +407,7 @@ class C: 8 STORE_ATTR 0 (x) 10 LOAD_CONST 0 (None) 12 RETURN_VALUE - """ - % (_C.__init__.__code__.co_firstlineno + 1,) - ) + """ % (_C.__init__.__code__.co_firstlineno + 1,) assert ( expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition_no_trailing_comma.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition_no_trailing_comma.py.snap index 48a59144c5..06bcb6b4e7 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition_no_trailing_comma.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition_no_trailing_comma.py.snap @@ -227,27 +227,17 @@ class C: assert expected( value, is_going_to_be="too long to fit in a single line", srsly=True -@@ -153,7 +154,8 @@ - " because it's too long" - ) - -- dis_c_instance_method = """\ -+ dis_c_instance_method = ( -+ """\ - %3d 0 LOAD_FAST 1 (x) - 2 LOAD_CONST 1 (1) - 4 COMPARE_OP 2 (==) -@@ -161,8 +163,8 @@ +@@ -161,9 +162,7 @@ 8 STORE_ATTR 0 (x) 10 LOAD_CONST 0 (None) 12 RETURN_VALUE - """ % ( - _C.__init__.__code__.co_firstlineno + 1, -+ """ -+ % (_C.__init__.__code__.co_firstlineno + 1,) - ) +- ) ++ """ % (_C.__init__.__code__.co_firstlineno + 1,) assert ( + expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect ``` ## Ruff Output @@ -409,8 +399,7 @@ class C: " because it's too long" ) - dis_c_instance_method = ( - """\ + dis_c_instance_method = """\ %3d 0 LOAD_FAST 1 (x) 2 LOAD_CONST 1 (1) 4 COMPARE_OP 2 (==) @@ -418,9 +407,7 @@ class C: 8 STORE_ATTR 0 (x) 10 LOAD_CONST 0 (None) 12 RETURN_VALUE - """ - % (_C.__init__.__code__.co_firstlineno + 1,) - ) + """ % (_C.__init__.__code__.co_firstlineno + 1,) assert ( expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap index 172b318812..1e0a59d33e 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap @@ -228,6 +228,97 @@ x = ( x = ( () - () # ) + + +# Avoid unnecessary parentheses around multiline strings. +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" % ( + self.base_url, + date.today(), +) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + # Needs parentheses + % ( + self.base_url, + date.today(), + ) +) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + # Needs parentheses + ( + self.base_url, + date.today(), + ) +) + + +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" + a.call.expression( + self.base_url, + date.today(), +) + +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" + sssssssssssssssssssssssssssssssssssssssssooooo * looooooooooooooooooooooooooooooongggggggggggg + +call(arg1, arg2, """ +short +""", arg3=True) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + ( + self.base_url + ) +) + + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + ( + # Needs parentheses + self.base_url + ) +) ``` ## Output @@ -512,6 +603,100 @@ x = ( x = ( () - () # ) + + +# Avoid unnecessary parentheses around multiline strings. +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" % ( + self.base_url, + date.today(), +) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + # Needs parentheses + % ( + self.base_url, + date.today(), + ) +) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + # Needs parentheses + ( + self.base_url, + date.today(), + ) +) + + +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" + a.call.expression( + self.base_url, + date.today(), +) + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + + sssssssssssssssssssssssssssssssssssssssssooooo + * looooooooooooooooooooooooooooooongggggggggggg +) + +call( + arg1, + arg2, + """ +short +""", + arg3=True, +) + +expected_content = """ + +%s/simple/sitemap-simple.xml%s + + +""" % (self.base_url) + + +expected_content = ( + """ + +%s/simple/sitemap-simple.xml%s + + +""" + % + ( + # Needs parentheses + self.base_url + ) +) ```