Skip to content

Commit

Permalink
Prefer splitting right hand side of assignments
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Dec 11, 2023
1 parent 4e461cb commit 7a3c504
Show file tree
Hide file tree
Showing 19 changed files with 1,544 additions and 396 deletions.
@@ -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
15 changes: 6 additions & 9 deletions crates/ruff_python_formatter/src/lib.rs
Expand Up @@ -181,7 +181,7 @@ mod tests {

use ruff_python_parser::{parse_ok_tokens, AsMode};

use crate::{format_module_ast, format_module_source, PyFormatOptions};
use crate::{format_module_ast, format_module_source, PreviewMode, PyFormatOptions};

/// Very basic test intentionally kept very similar to the CLI
#[test]
Expand Down Expand Up @@ -209,13 +209,9 @@ if True:
#[test]
fn quick_test() {
let source = r#"
def main() -> None:
if True:
some_very_long_variable_name_abcdefghijk = Foo()
some_very_long_variable_name_abcdefghijk = some_very_long_variable_name_abcdefghijk[
some_very_long_variable_name_abcdefghijk.some_very_long_attribute_name
== "This is a very long string abcdefghijk"
]
this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it = (
function()
)
"#;
let source_type = PySourceType::Python;
Expand All @@ -224,7 +220,8 @@ def main() -> None:
// Parse the AST.
let source_path = "code_inline.py";
let module = parse_ok_tokens(tokens, source, source_type.as_mode(), source_path).unwrap();
let options = PyFormatOptions::from_extension(Path::new(source_path));
let options = PyFormatOptions::from_extension(Path::new(source_path))
.with_preview(PreviewMode::Enabled);
let formatted = format_module_ast(&module, &comment_ranges, source, options).unwrap();

// Uncomment the `dbg` to print the IR.
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.
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

0 comments on commit 7a3c504

Please sign in to comment.