Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prefer_splitting_right_hand_side_of_assignments preview style #8943

Merged
merged 1 commit into from Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,5 @@
[
{
"preview": "enabled"
}
]
@@ -0,0 +1,218 @@
#######
# Unsplittable target and value

# Only parenthesize the value if it makes it fit, otherwise avoid parentheses.
b = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvvee

bbbbbbbbbbbbbbbb = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvv

# Avoid parenthesizing the value even if the target exceeds the configured width
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = bbb


############
# Splittable targets

# Does not double-parenthesize tuples
(
first_item,
second_item,
) = some_looooooooong_module.some_loooooog_function_name(
first_argument, second_argument, third_argument
)


# Preserve parentheses around the first target
(
req["ticket"]["steps"]["step"][0]["tasks"]["task"]["fields"]["field"][
"access_request"
]["destinations"]["destination"][0]["ip_address"]
) = dst

# Augmented assignment
req["ticket"]["steps"]["step"][0]["tasks"]["task"]["fields"]["field"][
"access_request"
] += dst

# Always parenthesize the value if it avoids splitting the target, regardless of the value's width.
_a: a[aaaa] = (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvv
)

#####
# Avoid parenthesizing the value if the expression right before the `=` splits to avoid an unnecessary pair of parentheses

# The type annotation is guaranteed to split because it is too long.
_a: a[
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvv
] = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvv

# The target is too long
(
aaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
) = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvv

# The target splits because of a magic trailing comma
(
a,
b,
) = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvvvvv

# The targets split because of a comment
(
# leading
a
) = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvvvvv

(
a
# trailing
) = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvvvvv

(
a, # nested
b
) = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvvvvv

#######
# Multi targets

# Black always parenthesizes the right if using multiple targets regardless if the parenthesized value exceeds the
# the configured line width or not
aaaa = bbbbbbbbbbbbbbbb = (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvvee
)

# Black does parenthesize the target if the target itself exceeds the line width and only parenthesizes
# the values if it makes it fit.
# The second target is too long to ever fit into the configured line width.
aaaa = (
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbdddd
) = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvvvvvvvvee

# Does also apply for other multi target assignments, as soon as a single target exceeds the configured
# width
aaaaaa = a["aaa"] = bbbbb[aa, bbb, cccc] = dddddddddd = eeeeee = (
fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
) = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa


######################
# Call expressions:
# For unsplittable targets: Parenthesize the call expression if it makes it fit.
#
# For splittable targets:
# Only parenthesize a call expression if the parens of the call don't fit on the same line
# as the target. Don't parenthesize the call expression if the target (or annotation) right before
# splits.

# Don't parenthesize the function call if the left is unsplittable.
this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = a.b.function(
arg1, arg2, arg3
)
this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = function(
[1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3
)
this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = function(
[1, 2, 3],
arg1,
[1, 2, 3],
arg2,
[1, 2, 3],
arg3,
dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd,
eeeeeeeeeeeeee,
)

this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = (
function()
)
this_is_a_ridiculously_long_name_and_nobodyddddddddddddddddddddddddddddddd = (
a.b.function(arg1, arg2, arg3)
)
this_is_a_ridiculously_long_name_and_nobodyddddddddddddddddddddddddddddddd = function()
this_is_a_ridiculously_long_name_and_nobodyddddddddddddddddddddddddddddddd = function(
[1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3
)
this_is_a_ridiculously_long_name_and_nobodyddddddddddddddddddddddddddddddd = function(
[1, 2, 3],
arg1,
[1, 2, 3],
arg2,
[1, 2, 3],
arg3,
dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd,
eeeeeeeeeeeeee,
)

####### Fluent call expressions
# Uses the regular `Multiline` layout where the entire `value` gets parenthesized
# if it doesn't fit on the line.
this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use = (
function().b().c([1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3)
)


#######
# Test comment inlining
value.__dict__[key] = (
"test" # set some Thrift field to non-None in the struct aa bb cc dd ee
)
value.__dict__.keye = (
"test" # set some Thrift field to non-None in the struct aa bb cc dd ee
)
value.__dict__.keye = (
"test" # set some Thrift field to non-None in the struct aa bb cc dd ee
)


# Don't parenthesize the value because the target's trailing comma forces it to split.
a[
aaaaaaa,
b,
] = cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc # comment

# Parenthesize the value, but don't duplicate the comment.
a[aaaaaaa, b] = (
cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc # comment
)

# Format both as flat, but don't loos the comment.
a[aaaaaaa, b] = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # comment

#######################################################
# Test the case where a parenthesized value now fits:
a[
aaaaaaa,
b
] = (
cccccccc # comment
)

# Splits the target but not the value because of the magic trailing comma.
a[
aaaaaaa,
b,
] = (
cccccccc # comment
)

# Splits the second target because of the comment and the first target because of the trailing comma.
a[
aaaaaaa,
b,
] = (
# leading comment
b
) = (
cccccccc # comment
)


########
# Type Alias Statement
type A[str, int, number] = VeryLongTypeNameThatShouldBreakFirstToTheRightBeforeSplitngtin

type A[VeryLongTypeNameThatShouldBreakFirstToTheRightBeforeSplitngtinthatExceedsTheWidth] = str

2 changes: 1 addition & 1 deletion crates/ruff_python_formatter/src/expression/mod.rs
Expand Up @@ -952,7 +952,7 @@ impl OwnParentheses {
/// Differs from [`has_own_parentheses`] in that it returns [`OwnParentheses::NonEmpty`] for
/// parenthesized expressions, like `(1)` or `([1])`, regardless of whether those expression have
/// their _own_ parentheses.
fn has_parentheses(expr: &Expr, context: &PyFormatContext) -> Option<OwnParentheses> {
pub(crate) fn has_parentheses(expr: &Expr, context: &PyFormatContext) -> Option<OwnParentheses> {
let own_parentheses = has_own_parentheses(expr, context);

// If the node has its own non-empty parentheses, we don't need to check for surrounding
Expand Down
7 changes: 7 additions & 0 deletions crates/ruff_python_formatter/src/preview.rs
Expand Up @@ -17,3 +17,10 @@ pub(crate) const fn is_hug_parens_with_braces_and_square_brackets_enabled(
) -> bool {
context.is_preview()
}

/// Returns `true` if the [`prefer_splitting_right_hand_side_of_assignments`](https://github.com/astral-sh/ruff/issues/6975) preview style is enabled.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that previously, but do we also want to do an enum here, and then have is_enabled(PreviewStyles::PreferSplittingRhsOfAssignments)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an opinion. We could use f.context().is_enabled(PreviewStyle::LongName), which may be easier to document and add. For me, any approach that allows identifying the call sites easily works for me.

pub(crate) const fn is_prefer_splitting_right_hand_side_of_assignments_enabled(
context: &PyFormatContext,
) -> bool {
context.is_preview()
}
44 changes: 30 additions & 14 deletions crates/ruff_python_formatter/src/statement/stmt_ann_assign.rs
Expand Up @@ -2,8 +2,12 @@ use ruff_formatter::write;
use ruff_python_ast::StmtAnnAssign;

use crate::comments::{SourceComment, SuppressionKind};
use crate::expression::has_parentheses;
use crate::prelude::*;
use crate::statement::stmt_assign::FormatStatementsLastExpression;
use crate::preview::is_prefer_splitting_right_hand_side_of_assignments_enabled;
use crate::statement::stmt_assign::{
AnyAssignmentOperator, AnyBeforeOperator, FormatStatementsLastExpression,
};
use crate::statement::trailing_semicolon;

#[derive(Default)]
Expand All @@ -19,21 +23,33 @@ impl FormatNodeRule<StmtAnnAssign> for FormatStmtAnnAssign {
simple: _,
} = item;

write!(
f,
[target.format(), token(":"), space(), annotation.format(),]
)?;
write!(f, [target.format(), token(":"), space()])?;

if let Some(value) = value {
write!(
f,
[
space(),
token("="),
space(),
FormatStatementsLastExpression::new(value, item)
]
)?;
if is_prefer_splitting_right_hand_side_of_assignments_enabled(f.context())
&& has_parentheses(annotation, f.context()).is_some()
{
FormatStatementsLastExpression::RightToLeft {
before_operator: AnyBeforeOperator::Expression(annotation),
operator: AnyAssignmentOperator::Assign,
value,
statement: item.into(),
}
.fmt(f)?;
} else {
write!(
f,
[
annotation.format(),
space(),
token("="),
space(),
FormatStatementsLastExpression::left_to_right(value, item)
]
)?;
}
} else {
annotation.format().fmt(f)?;
}

if f.options().source_type().is_ipynb()
Expand Down