mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-07-23 04:55:09 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Prefer breaking the implicit string concatenation over breaking before % (#5947)

Browse source
This commit is contained in:
Micha Reiser 2023-07-24 18:30:42 +02:00 • committed by GitHub
parent 42d969f19f
commit fdb3c8852f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 637 additions and 80 deletions
Download patch file Download diff file Expand all files Collapse all files

243
crates/ruff_python_formatter/src/expression/expr_bin_op.rs
View file

@ -1,19 +1,27 @@
use crate::comments::{trailing_comments, trailing_node_comments};
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, NeedsParentheses,
OptionalParentheses,
};
use crate::expression::Parentheses;
use crate::prelude::*;
use crate::FormatNodeRule;
use ruff_formatter::{write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions};
use ruff_python_ast::node::{AnyNodeRef, AstNode};
use std::iter;
use rustpython_parser::ast::{
Constant, Expr, ExprAttribute, ExprBinOp, ExprConstant, ExprUnaryOp, Operator, UnaryOp,
};
use smallvec::SmallVec;
use std::iter;
use ruff_formatter::{
format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions,
};
use ruff_python_ast::node::{AnyNodeRef, AstNode};
use ruff_python_ast::str::is_implicit_concatenation;
use crate::comments::{trailing_comments, trailing_node_comments};
use crate::expression::expr_constant::ExprConstantLayout;
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,
NeedsParentheses, OptionalParentheses,
};
use crate::expression::string::StringLayout;
use crate::expression::Parentheses;
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatExprBinOp {
@ -33,75 +41,124 @@ impl FormatNodeRule<ExprBinOp> for FormatExprBinOp {
fn fmt_fields(&self, item: &ExprBinOp, f: &mut PyFormatter) -> FormatResult<()> {
let comments = f.context().comments().clone();
let format_inner = format_with(|f: &mut PyFormatter| {
let source = f.context().source();
let binary_chain: SmallVec<[&ExprBinOp; 4]> = iter::successors(Some(item), |parent| {
parent.left.as_bin_op_expr().and_then(|bin_expression| {
if is_expression_parenthesized(bin_expression.as_any_node_ref(), source) {
None
match Self::layout(item, f.context()) {
BinOpLayout::LeftString(expression) => {
let right_has_leading_comment = f
.context()
.comments()
.has_leading_comments(item.right.as_ref());
let format_right_and_op = format_with(|f| {
if right_has_leading_comment {
space().fmt(f)?;
} else {
Some(bin_expression)
soft_line_break_or_space().fmt(f)?;
}
})
})
.collect();
// SAFETY: `binary_chain` is guaranteed not to be empty because it always contains the current expression.
let left_most = binary_chain.last().unwrap();
item.op.format().fmt(f)?;
// Format the left most expression
in_parentheses_only_group(&left_most.left.format()).fmt(f)?;
if right_has_leading_comment {
hard_line_break().fmt(f)?;
} else {
space().fmt(f)?;
}
// Iterate upwards in the binary expression tree and, for each level, format the operator
// and the right expression.
for current in binary_chain.into_iter().rev() {
let ExprBinOp {
range: _,
left: _,
op,
right,
} = current;
group(&item.right.format()).fmt(f)
});
let operator_comments = comments.dangling_comments(current);
let needs_space = !is_simple_power_expression(current);
let format_left = format_with(|f: &mut PyFormatter| {
let format_string =
expression.format().with_options(ExprConstantLayout::String(
StringLayout::ImplicitConcatenatedBinaryLeftSide,
));
let before_operator_space = if needs_space {
in_parentheses_only_soft_line_break_or_space()
} else {
in_parentheses_only_soft_line_break()
};
if is_expression_parenthesized(expression.into(), f.context().source()) {
parenthesized("(", &format_string, ")").fmt(f)
} else {
format_string.fmt(f)
}
});
write!(
f,
[
before_operator_space,
op.format(),
trailing_comments(operator_comments),
]
)?;
// Format the operator on its own line if the right side has any leading comments.
if comments.has_leading_comments(right.as_ref()) || !operator_comments.is_empty() {
hard_line_break().fmt(f)?;
} else if needs_space {
space().fmt(f)?;
}
in_parentheses_only_group(&right.format()).fmt(f)?;
// It's necessary to format the trailing comments because the code bypasses
// `FormatNodeRule::fmt` for the nested binary expressions.
// Don't call the formatting function for the most outer binary expression because
// these comments have already been formatted.
if current != item {
trailing_node_comments(current).fmt(f)?;
}
group(&format_args![format_left, group(&format_right_and_op)]).fmt(f)
}
BinOpLayout::Default => {
let format_inner = format_with(|f: &mut PyFormatter| {
let source = f.context().source();
let binary_chain: SmallVec<[&ExprBinOp; 4]> =
iter::successors(Some(item), |parent| {
parent.left.as_bin_op_expr().and_then(|bin_expression| {
if is_expression_parenthesized(
bin_expression.as_any_node_ref(),
source,
) {
None
} else {
Some(bin_expression)
}
})
})
.collect();
Ok(())
});
// SAFETY: `binary_chain` is guaranteed not to be empty because it always contains the current expression.
let left_most = binary_chain.last().unwrap();
in_parentheses_only_group(&format_inner).fmt(f)
// Format the left most expression
in_parentheses_only_group(&left_most.left.format()).fmt(f)?;
// Iterate upwards in the binary expression tree and, for each level, format the operator
// and the right expression.
for current in binary_chain.into_iter().rev() {
let ExprBinOp {
range: _,
left: _,
op,
right,
} = current;
let operator_comments = comments.dangling_comments(current);
let needs_space = !is_simple_power_expression(current);
let before_operator_space = if needs_space {
in_parentheses_only_soft_line_break_or_space()
} else {
in_parentheses_only_soft_line_break()
};
write!(
f,
[
before_operator_space,
op.format(),
trailing_comments(operator_comments),
]
)?;
// Format the operator on its own line if the right side has any leading comments.
if comments.has_leading_comments(right.as_ref())
|| !operator_comments.is_empty()
{
hard_line_break().fmt(f)?;
} else if needs_space {
space().fmt(f)?;
}
in_parentheses_only_group(&right.format()).fmt(f)?;
// It's necessary to format the trailing comments because the code bypasses
// `FormatNodeRule::fmt` for the nested binary expressions.
// Don't call the formatting function for the most outer binary expression because
// these comments have already been formatted.
if current != item {
trailing_node_comments(current).fmt(f)?;
}
}
Ok(())
});
in_parentheses_only_group(&format_inner).fmt(f)
}
}
}
fn fmt_dangling_comments(&self, _node: &ExprBinOp, _f: &mut PyFormatter) -> FormatResult<()> {
@ -110,6 +167,34 @@ impl FormatNodeRule<ExprBinOp> for FormatExprBinOp {
}
}
impl FormatExprBinOp {
fn layout<'a>(bin_op: &'a ExprBinOp, context: &PyFormatContext) -> BinOpLayout<'a> {
if let Some(
constant @ ExprConstant {
value: Constant::Str(_),
range,
..
},
) = bin_op.left.as_constant_expr()
{
let comments = context.comments();
if bin_op.op == Operator::Mod
&& context.node_level().is_parenthesized()
&& !comments.has_dangling_comments(constant)
&& !comments.has_dangling_comments(bin_op)
&& is_implicit_concatenation(&context.source()[*range])
{
BinOpLayout::LeftString(constant)
} else {
BinOpLayout::Default
}
} else {
BinOpLayout::Default
}
}
}
const fn is_simple_power_expression(expr: &ExprBinOp) -> bool {
expr.op.is_pow() && is_simple_power_operand(&expr.left) && is_simple_power_operand(&expr.right)
}
@ -132,6 +217,24 @@ const fn is_simple_power_operand(expr: &Expr) -> bool {
}
}
#[derive(Copy, Clone, Debug)]
enum BinOpLayout<'a> {
Default,
/// Specific layout for an implicit concatenated string using the "old" c-style formatting.
///
/// ```python
/// (
/// "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa %s"
/// "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb %s" % (a, b)
/// )
/// ```
///
/// Prefers breaking the string parts over breaking in front of the `%` because it looks better if it
/// is kept on the same line.
LeftString(&'a ExprConstant),
}
#[derive(Copy, Clone)]
pub struct FormatOperator;

32
crates/ruff_python_formatter/src/expression/expr_constant.rs
View file

@ -1,17 +1,37 @@
use ruff_text_size::{TextLen, TextRange};
use rustpython_parser::ast::{Constant, ExprConstant, Ranged};
use ruff_formatter::FormatRuleWithOptions;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::str::is_implicit_concatenation;
use crate::expression::number::{FormatComplex, FormatFloat, FormatInt};
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::expression::string::{FormatString, StringPrefix, StringQuotes};
use crate::expression::string::{FormatString, StringLayout, StringPrefix, StringQuotes};
use crate::prelude::*;
use crate::{not_yet_implemented_custom_text, FormatNodeRule};
#[derive(Default)]
pub struct FormatExprConstant;
pub struct FormatExprConstant {
layout: ExprConstantLayout,
}
#[derive(Copy, Clone, Debug, Default)]
pub enum ExprConstantLayout {
#[default]
Default,
String(StringLayout),
}
impl FormatRuleWithOptions<ExprConstant, PyFormatContext<'_>> for FormatExprConstant {
type Options = ExprConstantLayout;
fn with_options(mut self, options: Self::Options) -> Self {
self.layout = options;
self
}
}
impl FormatNodeRule<ExprConstant> for FormatExprConstant {
fn fmt_fields(&self, item: &ExprConstant, f: &mut PyFormatter) -> FormatResult<()> {
@ -31,7 +51,13 @@ impl FormatNodeRule<ExprConstant> for FormatExprConstant {
Constant::Int(_) => FormatInt::new(item).fmt(f),
Constant::Float(_) => FormatFloat::new(item).fmt(f),
Constant::Complex { .. } => FormatComplex::new(item).fmt(f),
Constant::Str(_) => FormatString::new(item).fmt(f),
Constant::Str(_) => {
let string_layout = match self.layout {
ExprConstantLayout::Default => StringLayout::Default,
ExprConstantLayout::String(layout) => layout,
};
FormatString::new(item).with_layout(string_layout).fmt(f)
}
Constant::Bytes(_) => {
not_yet_implemented_custom_text(r#"b"NOT_YET_IMPLEMENTED_BYTE_STRING""#).fmt(f)
}

38
crates/ruff_python_formatter/src/expression/string.rs
View file

@ -18,24 +18,48 @@ use crate::QuoteStyle;
pub(super) struct FormatString<'a> {
constant: &'a ExprConstant,
layout: StringLayout,
}
#[derive(Default, Copy, Clone, Debug)]
pub enum StringLayout {
#[default]
Default,
ImplicitConcatenatedBinaryLeftSide,
}
impl<'a> FormatString<'a> {
pub(super) fn new(constant: &'a ExprConstant) -> Self {
debug_assert!(constant.value.is_str());
Self { constant }
Self {
constant,
layout: StringLayout::Default,
}
}
pub(super) fn with_layout(mut self, layout: StringLayout) -> Self {
self.layout = layout;
self
}
}
impl<'a> Format<PyFormatContext<'_>> for FormatString<'a> {
fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
let string_range = self.constant.range();
let string_content = f.context().locator().slice(string_range);
match self.layout {
StringLayout::Default => {
let string_range = self.constant.range();
let string_content = f.context().locator().slice(string_range);
if is_implicit_concatenation(string_content) {
in_parentheses_only_group(&FormatStringContinuation::new(self.constant)).fmt(f)
} else {
FormatStringPart::new(string_range).fmt(f)
if is_implicit_concatenation(string_content) {
in_parentheses_only_group(&FormatStringContinuation::new(self.constant)).fmt(f)
} else {
FormatStringPart::new(string_range).fmt(f)
}
}
StringLayout::ImplicitConcatenatedBinaryLeftSide => {
FormatStringContinuation::new(self.constant).fmt(f)
}
}
}
}