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

Call chain formatting in fluent style (#6151)

Browse source 
Implement fluent style/call chains. See the `call_chains.py` formatting
for examples.

This isn't fully like black because in `raise A from B` they allow `A`
breaking can influence the formatting of `B` even if it is already
multiline.

Similarity index:

| project      | main  | PR    |
|--------------|-------|-------|
| build        | ???   | 0.753 |
| django       | 0.991 | 0.998 |
| transformers | 0.993 | 0.994 |
| typeshed     | 0.723 | 0.723 |
| warehouse    | 0.978 | 0.994 |
| zulip        | 0.992 | 0.994 |

Call chain formatting is affected by
https://github.com/astral-sh/ruff/issues/627, but i'm cutting scope
here.

Closes #5343

**Test Plan**:
 * Added a dedicated call chains test file
 * The ecosystem checks found some bugs
 * I manually check django and zulip formatting

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
konsti 2023-08-04 15:58:01 +02:00 • committed by GitHub
parent 35bdbe43a8
commit 99baad12d8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 917 additions and 517 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/ruff_python_formatter/src/comments/placement.rs
View file

@ -1016,7 +1016,7 @@ fn handle_attribute_comment<'a>(
.contains(comment.slice().start())
);
if comment.line_position().is_end_of_line() {
// Attach to node with b
// Attach as trailing comment to a. The specific placement is only relevant for fluent style
// ```python
// x322 = (
// a
@ -1024,7 +1024,7 @@ fn handle_attribute_comment<'a>(
// b
// )
// ```
CommentPlacement::trailing(comment.enclosing_node(), comment)
CommentPlacement::trailing(attribute.value.as_ref(), comment)
} else {
CommentPlacement::dangling(attribute, comment)
}

121
crates/ruff_python_formatter/src/expression/expr_attribute.rs
View file

@ -1,15 +1,28 @@
use ruff_formatter::{write, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::{Constant, Expr, ExprAttribute, ExprConstant};
use ruff_formatter::write;
use ruff_python_ast::node::AnyNodeRef;
use crate::comments::{leading_comments, trailing_comments};
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses, Parentheses};
use crate::expression::parentheses::{
is_expression_parenthesized, NeedsParentheses, OptionalParentheses, Parentheses,
};
use crate::expression::CallChainLayout;
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatExprAttribute;
pub struct FormatExprAttribute {
call_chain_layout: CallChainLayout,
}
impl FormatRuleWithOptions<ExprAttribute, PyFormatContext<'_>> for FormatExprAttribute {
type Options = CallChainLayout;
fn with_options(mut self, options: Self::Options) -> Self {
self.call_chain_layout = options;
self
}
}
impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
fn fmt_fields(&self, item: &ExprAttribute, f: &mut PyFormatter) -> FormatResult<()> {
@ -20,6 +33,8 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
ctx: _,
} = item;
let call_chain_layout = self.call_chain_layout.apply_in_node(item, f);
let needs_parentheses = matches!(
value.as_ref(),
Expr::Constant(ExprConstant {
@ -37,11 +52,36 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
if needs_parentheses {
value.format().with_options(Parentheses::Always).fmt(f)?;
} else if let Expr::Attribute(expr_attribute) = value.as_ref() {
// We're in a attribute chain (`a.b.c`). The outermost node adds parentheses if
// required, the inner ones don't need them so we skip the `Expr` formatting that
// normally adds the parentheses.
expr_attribute.format().fmt(f)?;
} else if call_chain_layout == CallChainLayout::Fluent {
match value.as_ref() {
Expr::Attribute(expr) => {
expr.format().with_options(call_chain_layout).fmt(f)?;
}
Expr::Call(expr) => {
expr.format().with_options(call_chain_layout).fmt(f)?;
if call_chain_layout == CallChainLayout::Fluent {
// Format the dot on its own line
soft_line_break().fmt(f)?;
}
}
Expr::Subscript(expr) => {
expr.format().with_options(call_chain_layout).fmt(f)?;
if call_chain_layout == CallChainLayout::Fluent {
// Format the dot on its own line
soft_line_break().fmt(f)?;
}
}
_ => {
// This matches [`CallChainLayout::from_expression`]
if is_expression_parenthesized(value.as_ref().into(), f.context().source()) {
value.format().with_options(Parentheses::Always).fmt(f)?;
// Format the dot on its own line
soft_line_break().fmt(f)?;
} else {
value.format().fmt(f)?;
}
}
}
} else {
value.format().fmt(f)?;
}
@ -50,16 +90,51 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
hard_line_break().fmt(f)?;
}
write!(
f,
[
text("."),
trailing_comments(trailing_dot_comments),
(!leading_attribute_comments.is_empty()).then_some(hard_line_break()),
leading_comments(leading_attribute_comments),
attr.format()
]
)
if call_chain_layout == CallChainLayout::Fluent {
// Fluent style has line breaks before the dot
// ```python
// blogs3 = (
// Blog.objects.filter(
// entry__headline__contains="Lennon",
// )
// .filter(
// entry__pub_date__year=2008,
// )
// .filter(
// entry__pub_date__year=2008,
// )
// )
// ```
write!(
f,
[
(!leading_attribute_comments.is_empty()).then_some(hard_line_break()),
leading_comments(leading_attribute_comments),
text("."),
trailing_comments(trailing_dot_comments),
attr.format()
]
)
} else {
// Regular style
// ```python
// blogs2 = Blog.objects.filter(
// entry__headline__contains="Lennon",
// ).filter(
// entry__pub_date__year=2008,
// )
// ```
write!(
f,
[
text("."),
trailing_comments(trailing_dot_comments),
(!leading_attribute_comments.is_empty()).then_some(hard_line_break()),
leading_comments(leading_attribute_comments),
attr.format()
]
)
}
}
fn fmt_dangling_comments(
@ -79,7 +154,11 @@ impl NeedsParentheses for ExprAttribute {
context: &PyFormatContext,
) -> OptionalParentheses {
// Checks if there are any own line comments in an attribute chain (a.b.c).
if context
if CallChainLayout::from_expression(self.into(), context.source())
== CallChainLayout::Fluent
{
OptionalParentheses::Multiline
} else if context
.comments()
.dangling_comments(self)
.iter()

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

@ -5,9 +5,7 @@ use ruff_python_ast::{
};
use smallvec::SmallVec;
use ruff_formatter::{
format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions,
};
use ruff_formatter::{format_args, write, FormatOwnedWithRule, FormatRefWithRule};
use ruff_python_ast::node::{AnyNodeRef, AstNode};
use ruff_python_ast::str::is_implicit_concatenation;
@ -19,23 +17,11 @@ use crate::expression::parentheses::{
NeedsParentheses, OptionalParentheses,
};
use crate::expression::string::StringLayout;
use crate::expression::Parentheses;
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatExprBinOp {
parentheses: Option<Parentheses>,
}
impl FormatRuleWithOptions<ExprBinOp, PyFormatContext<'_>> for FormatExprBinOp {
type Options = Option<Parentheses>;
fn with_options(mut self, options: Self::Options) -> Self {
self.parentheses = options;
self
}
}
pub struct FormatExprBinOp;
impl FormatNodeRule<ExprBinOp> for FormatExprBinOp {
fn fmt_fields(&self, item: &ExprBinOp, f: &mut PyFormatter) -> FormatResult<()> {

53
crates/ruff_python_formatter/src/expression/expr_call.rs
View file

@ -1,13 +1,25 @@
use ruff_formatter::write;
use crate::expression::CallChainLayout;
use ruff_formatter::FormatRuleWithOptions;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprCall;
use ruff_python_ast::{Expr, ExprCall};
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatExprCall;
pub struct FormatExprCall {
call_chain_layout: CallChainLayout,
}
impl FormatRuleWithOptions<ExprCall, PyFormatContext<'_>> for FormatExprCall {
type Options = CallChainLayout;
fn with_options(mut self, options: Self::Options) -> Self {
self.call_chain_layout = options;
self
}
}
impl FormatNodeRule<ExprCall> for FormatExprCall {
fn fmt_fields(&self, item: &ExprCall, f: &mut PyFormatter) -> FormatResult<()> {
@ -17,7 +29,32 @@ impl FormatNodeRule<ExprCall> for FormatExprCall {
arguments,
} = item;
write!(f, [func.format(), arguments.format()])
let call_chain_layout = self.call_chain_layout.apply_in_node(item, f);
let fmt_inner = format_with(|f| {
match func.as_ref() {
Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f)?,
Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f)?,
Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f)?,
_ => func.format().fmt(f)?,
}
arguments.format().fmt(f)
});
// Allow to indent the parentheses while
// ```python
// g1 = (
// queryset.distinct().order_by(field.name).values_list(field_name_flat_long_long=True)
// )
// ```
if call_chain_layout == CallChainLayout::Fluent
&& self.call_chain_layout == CallChainLayout::Default
{
group(&fmt_inner).fmt(f)
} else {
fmt_inner.fmt(f)
}
}
}
@ -27,6 +64,12 @@ impl NeedsParentheses for ExprCall {
_parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
self.func.needs_parentheses(self.into(), context)
if CallChainLayout::from_expression(self.into(), context.source())
== CallChainLayout::Fluent
{
OptionalParentheses::Multiline
} else {
self.func.needs_parentheses(self.into(), context)
}
}
}

42
crates/ruff_python_formatter/src/expression/expr_subscript.rs
View file

@ -1,18 +1,29 @@
use ruff_python_ast::{Expr, ExprSubscript};
use ruff_formatter::{format_args, write};
use ruff_formatter::{format_args, write, FormatRuleWithOptions};
use ruff_python_ast::node::{AnyNodeRef, AstNode};
use ruff_python_ast::{Expr, ExprSubscript};
use crate::comments::trailing_comments;
use crate::context::PyFormatContext;
use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::expr_tuple::TupleParentheses;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::expression::CallChainLayout;
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatExprSubscript;
pub struct FormatExprSubscript {
call_chain_layout: CallChainLayout,
}
impl FormatRuleWithOptions<ExprSubscript, PyFormatContext<'_>> for FormatExprSubscript {
type Options = CallChainLayout;
fn with_options(mut self, options: Self::Options) -> Self {
self.call_chain_layout = options;
self
}
}
impl FormatNodeRule<ExprSubscript> for FormatExprSubscript {
fn fmt_fields(&self, item: &ExprSubscript, f: &mut PyFormatter) -> FormatResult<()> {
@ -23,6 +34,8 @@ impl FormatNodeRule<ExprSubscript> for FormatExprSubscript {
ctx: _,
} = item;
let call_chain_layout = self.call_chain_layout.apply_in_node(item, f);
let comments = f.context().comments().clone();
let dangling_comments = comments.dangling_comments(item.as_any_node_ref());
debug_assert!(
@ -30,12 +43,19 @@ impl FormatNodeRule<ExprSubscript> for FormatExprSubscript {
"A subscript expression can only have a single dangling comment, the one after the bracket"
);
let format_value = format_with(|f| match value.as_ref() {
Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f),
Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f),
Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f),
_ => value.format().fmt(f),
});
if let NodeLevel::Expression(Some(_)) = f.context().node_level() {
// Enforce the optional parentheses for parenthesized values.
let mut f = WithNodeLevel::new(NodeLevel::Expression(None), f);
write!(f, [value.format()])?;
write!(f, [format_value])?;
} else {
value.format().fmt(f)?;
format_value.fmt(f)?;
}
let format_slice = format_with(|f: &mut PyFormatter| {
@ -73,10 +93,16 @@ impl NeedsParentheses for ExprSubscript {
fn needs_parentheses(
&self,
_parent: AnyNodeRef,
_context: &PyFormatContext,
context: &PyFormatContext,
) -> OptionalParentheses {
{
OptionalParentheses::Never
if CallChainLayout::from_expression(self.into(), context.source())
== CallChainLayout::Fluent
{
OptionalParentheses::Multiline
} else {
self.value.needs_parentheses(self.into(), context)
}
}
}
}

178
crates/ruff_python_formatter/src/expression/mod.rs
View file

@ -1,13 +1,12 @@
use std::cmp::Ordering;
use ruff_python_ast as ast;
use ruff_python_ast::{Expr, Operator};
use ruff_formatter::{
write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions,
};
use ruff_python_ast as ast;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::visitor::preorder::{walk_expr, PreorderVisitor};
use ruff_python_ast::{Expr, Operator};
use crate::builders::parenthesize_if_expands;
use crate::context::{NodeLevel, WithNodeLevel};
@ -70,7 +69,7 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
let format_expr = format_with(|f| match expression {
Expr::BoolOp(expr) => expr.format().with_options(Some(parentheses)).fmt(f),
Expr::NamedExpr(expr) => expr.format().fmt(f),
Expr::BinOp(expr) => expr.format().with_options(Some(parentheses)).fmt(f),
Expr::BinOp(expr) => expr.format().fmt(f),
Expr::UnaryOp(expr) => expr.format().fmt(f),
Expr::Lambda(expr) => expr.format().fmt(f),
Expr::IfExp(expr) => expr.format().fmt(f),
@ -100,9 +99,10 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
let parenthesize = match parentheses {
Parentheses::Preserve => {
is_expression_parenthesized(AnyNodeRef::from(expression), f.context().source())
is_expression_parenthesized(expression.into(), f.context().source())
}
Parentheses::Always => true,
// Fluent style means we already have parentheses
Parentheses::Never => false,
};
@ -157,7 +157,7 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
let comments = f.context().comments();
let preserve_parentheses = parenthesize.is_optional()
&& is_expression_parenthesized(AnyNodeRef::from(*expression), f.context().source());
&& is_expression_parenthesized((*expression).into(), f.context().source());
let has_comments = comments.has_leading_comments(*expression)
|| comments.has_trailing_own_line_comments(*expression);
@ -178,9 +178,10 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
Parenthesize::Optional | Parenthesize::IfBreaks => needs_parentheses,
};
let can_omit_optional_parentheses = can_omit_optional_parentheses(expression, f.context());
match needs_parentheses {
OptionalParentheses::Multiline if *parenthesize != Parenthesize::IfRequired => {
if can_omit_optional_parentheses(expression, f.context()) {
if can_omit_optional_parentheses {
optional_parentheses(&expression.format().with_options(Parentheses::Never))
.fmt(f)
} else {
@ -265,7 +266,24 @@ impl<'ast> IntoFormat<PyFormatContext<'ast>> for Expr {
fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool {
let mut visitor = CanOmitOptionalParenthesesVisitor::new(context.source());
visitor.visit_subexpression(expr);
visitor.can_omit()
if visitor.max_priority_count > 1 {
false
} else if visitor.max_priority == OperatorPriority::Attribute {
true
} else if !visitor.any_parenthesized_expressions {
// Only use the more complex IR when there is any expression that we can possibly split by
false
} else {
// Only use the layout if the first or last expression has parentheses of some sort.
let first_parenthesized = visitor
.first
.is_some_and(|first| has_parentheses(first, visitor.source));
let last_parenthesized = visitor
.last
.is_some_and(|last| has_parentheses(last, visitor.source));
first_parenthesized || last_parenthesized
}
}
#[derive(Clone, Debug)]
@ -365,8 +383,10 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
self.last = Some(expr);
return;
}
Expr::Subscript(_) => {
// Don't walk the value. Splitting before the value looks weird.
Expr::Subscript(ast::ExprSubscript { value, .. }) => {
self.any_parenthesized_expressions = true;
// Only walk the function, the subscript is always parenthesized
self.visit_expr(value);
// Don't walk the slice, because the slice is always parenthesized.
return;
}
@ -409,26 +429,6 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
walk_expr(self, expr);
}
fn can_omit(self) -> bool {
if self.max_priority_count > 1 {
false
} else if self.max_priority == OperatorPriority::Attribute {
true
} else if !self.any_parenthesized_expressions {
// Only use the more complex IR when there is any expression that we can possibly split by
false
} else {
// Only use the layout if the first or last expression has parentheses of some sort.
let first_parenthesized = self
.first
.is_some_and(|first| has_parentheses(first, self.source));
let last_parenthesized = self
.last
.is_some_and(|last| has_parentheses(last, self.source));
first_parenthesized || last_parenthesized
}
}
}
impl<'input> PreorderVisitor<'input> for CanOmitOptionalParenthesesVisitor<'input> {
@ -448,6 +448,124 @@ impl<'input> PreorderVisitor<'input> for CanOmitOptionalParenthesesVisitor<'inpu
}
}
/// A call chain consists only of attribute access (`.` operator), function/method calls and
/// subscripts. We use fluent style for the call chain if there are at least two attribute dots
/// after call parentheses or subscript brackets. In case of fluent style the parentheses/bracket
/// will close on the previous line and the dot gets its own line, otherwise the line will start
/// with the closing parentheses/bracket and the dot follows immediately after.
///
/// Below, the left hand side of the addition has only a single attribute access after a call, the
/// second `.filter`. The first `.filter` is a call, but it doesn't follow a call. The right hand
/// side has two, the `.limit_results` after the call and the `.filter` after the subscript, so it
/// gets formatted in fluent style. The outer expression we assign to `blogs` has zero since the
/// `.all` follows attribute parentheses and not call parentheses.
///
/// ```python
/// blogs = (
/// Blog.objects.filter(
/// entry__headline__contains="Lennon",
/// ).filter(
/// entry__pub_date__year=2008,
/// )
/// + Blog.objects.filter(
/// entry__headline__contains="McCartney",
/// )
/// .limit_results[:10]
/// .filter(
/// entry__pub_date__year=2010,
/// )
/// ).all()
/// ```
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
pub enum CallChainLayout {
/// The root of a call chain
#[default]
Default,
/// A nested call chain element that uses fluent style.
Fluent,
/// A nested call chain element not using fluent style.
NonFluent,
}
impl CallChainLayout {
pub(crate) fn from_expression(mut expr: AnyNodeRef, source: &str) -> Self {
let mut attributes_after_parentheses = 0;
loop {
match expr {
AnyNodeRef::ExprAttribute(ast::ExprAttribute { value, .. }) => {
expr = AnyNodeRef::from(value.as_ref());
// ```
// f().g
// ^^^ value
// data[:100].T`
// ^^^^^^^^^^ value
// ```
if matches!(value.as_ref(), Expr::Call(_) | Expr::Subscript(_)) {
attributes_after_parentheses += 1;
} else if is_expression_parenthesized(expr, source) {
// `(a).b`. We preserve these parentheses so don't recurse
attributes_after_parentheses += 1;
break;
}
}
// ```
// f()
// ^^^ expr
// ^ func
// data[:100]
// ^^^^^^^^^^ expr
// ^^^^ value
// ```
AnyNodeRef::ExprCall(ast::ExprCall { func: inner, .. })
| AnyNodeRef::ExprSubscript(ast::ExprSubscript { value: inner, .. }) => {
expr = AnyNodeRef::from(inner.as_ref());
}
_ => {
// We to format the following in fluent style:
// ```
// f2 = (a).w().t(1,)
// ^ expr
// ```
if is_expression_parenthesized(expr, source) {
attributes_after_parentheses += 1;
}
break;
}
}
// We preserve these parentheses so don't recurse
if is_expression_parenthesized(expr, source) {
break;
}
}
if attributes_after_parentheses < 2 {
CallChainLayout::NonFluent
} else {
CallChainLayout::Fluent
}
}
/// Determine whether to actually apply fluent layout in attribute, call and subscript
/// formatting
pub(crate) fn apply_in_node<'a>(
self,
item: impl Into<AnyNodeRef<'a>>,
f: &mut PyFormatter,
) -> CallChainLayout {
match self {
CallChainLayout::Default => {
if f.context().node_level().is_parenthesized() {
CallChainLayout::from_expression(item.into(), f.context().source())
} else {
CallChainLayout::NonFluent
}
}
layout @ (CallChainLayout::Fluent | CallChainLayout::NonFluent) => layout,
}
}
}
fn has_parentheses(expr: &Expr, source: &str) -> bool {
has_own_parentheses(expr) || is_expression_parenthesized(AnyNodeRef::from(expr), source)
}

17
crates/ruff_python_formatter/src/expression/parentheses.rs
View file

@ -316,3 +316,20 @@ impl<'ast> Format<PyFormatContext<'ast>> for FormatInParenthesesOnlyGroup<'_, 'a
}
}
}
#[cfg(test)]
mod tests {
use crate::expression::parentheses::is_expression_parenthesized;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_parser::parse_expression;
#[test]
fn test_has_parentheses() {
let expression = r#"(b().c("")).d()"#;
let expr = parse_expression(expression, "<filename>").unwrap();
assert!(!is_expression_parenthesized(
AnyNodeRef::from(&expr),
expression
));
}
}