Skip to content

Commit

Permalink
Parenthesize with statements
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Jul 14, 2023
1 parent 58a79c1 commit 8f98884
Show file tree
Hide file tree
Showing 10 changed files with 392 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@
with (a): # should remove brackets
...

# TODO: black doesn't wrap this, but maybe we want to anyway?
# if we do want to wrap, do we prefer to wrap the entire WithItem or to let the
# WithItem allow the `aa + bb` content expression to be wrapped
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c:
...

Expand All @@ -52,3 +49,62 @@
pass
with (a, *b):
pass

with (
# leading comment
a) as b: ...

with (
# leading comment
a as b
): ...


with (a # trailing same line comment
# trailing own line comment
) as b: ...

with (
a # trailing same line comment
# trailing own line comment
as b
): ...


with (a # trailing same line comment
# trailing own line comment
) as b: ...

with (
(a
# trailing own line comment
)
as # trailing as same line comment
b # trailing b same line comment
): ...

with (
[
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1,
aaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+ cccccccccccccccccccccccccccc
+ ddddddddddddddddd as example2,
CtxManager2() as example2,
CtxManager2() as example2,
CtxManager2() as example2,
):
...


with [
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1, aaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccccccc + ddddddddddddddddd as example2, CtxManager222222222222222() as example2:
...
15 changes: 14 additions & 1 deletion crates/ruff_python_formatter/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,23 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
) -> &mut Self
where
T: Ranged,
{
self.entry_with_line_separator(node, content, soft_line_break_or_space())
}

pub(crate) fn entry_with_line_separator<N, Separator>(
&mut self,
node: &N,
content: &dyn Format<PyFormatContext<'ast>>,
separator: Separator,
) -> &mut Self
where
N: Ranged,
Separator: Format<PyFormatContext<'ast>>,
{
self.result = self.result.and_then(|_| {
if self.end_of_last_entry.is_some() {
write!(self.fmt, [text(","), soft_line_break_or_space()])?;
write!(self.fmt, [text(","), separator])?;
}

self.end_of_last_entry = Some(node.end());
Expand Down
45 changes: 45 additions & 0 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub(super) fn place_comment<'a>(
handle_expr_if_comment,
handle_comprehension_comment,
handle_trailing_expression_starred_star_end_of_line_comment,
handle_with_item_comment,
];
for handler in HANDLERS {
comment = match handler(comment, locator) {
Expand Down Expand Up @@ -1232,6 +1233,50 @@ fn handle_trailing_expression_starred_star_end_of_line_comment<'a>(
CommentPlacement::leading(starred.as_any_node_ref(), comment)
}

/// Handles trailing own line comments before the `as` keyword of a with item and
/// end of line comments that are on the same line as the `as` keyword:
///
/// ```python
/// with (
/// a
/// # trailing a own line comment
/// as # trailing as same line comment
/// b
// ): ...
/// ```
fn handle_with_item_comment<'a>(
comment: DecoratedComment<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
if !comment.enclosing_node().is_with_item() {
return CommentPlacement::Default(comment);
}

// Needs to be a with item with an `as` expression.
let (Some(context_expr), Some(optional_vars)) =
(comment.preceding_node(), comment.following_node())
else {
return CommentPlacement::Default(comment);
};

let as_token = find_only_token_in_range(
TextRange::new(context_expr.end(), optional_vars.start()),
locator,
TokenKind::As,
);

// If before the `as` keyword, then it must be a trailing comment of the context expression.
if comment.end() < as_token.start() {
CommentPlacement::trailing(context_expr, comment)
}
// Trailing end of line comment coming after the `as` keyword`.
else if comment.line_position().is_end_of_line() {
CommentPlacement::dangling(comment.enclosing_node(), comment)
} else {
CommentPlacement::Default(comment)
}
}

/// Looks for a token in the range that contains no other tokens except for parentheses outside
/// the expression ranges
fn find_only_token_in_range(range: TextRange, locator: &Locator, token_kind: TokenKind) -> Token {
Expand Down
29 changes: 18 additions & 11 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,22 +159,29 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
parenthesize,
} = self;

let parenthesize = match parenthesize {
let preserve_parentheses = match parenthesize {
Parenthesize::Optional => {
is_expression_parenthesized(AnyNodeRef::from(*expression), f.context().source())
}
Parenthesize::IfBreaks => false,
Parenthesize::IfBreaks | Parenthesize::IfRequired => false,
};

let parentheses =
if parenthesize || f.context().comments().has_leading_comments(*expression) {
OptionalParentheses::Always
} else {
expression.needs_parentheses(*parent, f.context())
};
let comments = f.context().comments();
let needs_parentheses = if preserve_parentheses
|| comments.has_leading_comments(*expression)
|| comments.has_trailing_own_line_comments(*expression)
{
OptionalParentheses::Always
} else if *parenthesize == Parenthesize::IfRequired
&& f.context().node_level().is_parenthesized()
{
OptionalParentheses::Never
} else {
expression.needs_parentheses(*parent, f.context())
};

match parentheses {
OptionalParentheses::Multiline => {
match needs_parentheses {
OptionalParentheses::Multiline if *parenthesize != Parenthesize::IfRequired => {
if can_omit_optional_parentheses(expression, f.context()) {
optional_parentheses(&expression.format().with_options(Parentheses::Never))
.fmt(f)
Expand All @@ -186,7 +193,7 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
OptionalParentheses::Always => {
expression.format().with_options(Parentheses::Always).fmt(f)
}
OptionalParentheses::Never => {
OptionalParentheses::Never | OptionalParentheses::Multiline => {
expression.format().with_options(Parentheses::Never).fmt(f)
}
}
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ pub(crate) enum Parenthesize {

/// Parenthesizes the expression only if it doesn't fit on a line.
IfBreaks,

/// Only adds parentheses if absolutely necessary:
/// * The expression is not enclosed by another parenthesized expression and it expands over multiple lines
/// * The expression has leading or trailing comments. Adding parentheses is desired to prevent the comments from wandering.
IfRequired,
}

/// Whether it is necessary to add parentheses around an expression.
Expand Down
33 changes: 24 additions & 9 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,30 @@ if True:
#[test]
fn quick_test() {
let src = r#"
if a * [
bbbbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccccccccccccdddddddddddddddddddddddddd,
] + a * e * [
ffff,
gggg,
hhhhhhhhhhhhhh,
] * c:
pass
with (
[
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1,
aaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+ cccccccccccccccccccccccccccc
+ ddddddddddddddddd as example2,
CtxManager2() as example2,
CtxManager2() as example2,
CtxManager2() as example2,
):
...
with [
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbb",
"cccccccccccccccccccccccccccccccccccccccccc",
dddddddddddddddddddddddddddddddd,
] as example1, aaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccccccc + ddddddddddddddddd as example2, CtxManager222222222222222() as example2:
...
"#;
// Tokenize once
Expand Down
38 changes: 24 additions & 14 deletions crates/ruff_python_formatter/src/other/with_item.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use rustpython_parser::ast::WithItem;

use ruff_formatter::{write, Buffer, FormatResult};

use crate::comments::trailing_comments;
use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::Parenthesize;
use crate::prelude::*;
use crate::{FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::WithItem;

#[derive(Default)]
pub struct FormatWithItem;
Expand All @@ -16,20 +19,27 @@ impl FormatNodeRule<WithItem> for FormatWithItem {
optional_vars,
} = item;

let inner = format_with(|f| {
let comments = f.context().comments().clone();
let trailing_as_comments = comments.dangling_comments(item);

maybe_parenthesize_expression(context_expr, item, Parenthesize::IfRequired).fmt(f)?;

if let Some(optional_vars) = optional_vars {
write!(
f,
[maybe_parenthesize_expression(
context_expr,
item,
Parenthesize::IfBreaks
)]
[
space(),
text("as"),
trailing_comments(trailing_as_comments),
space(),
optional_vars.format(),
]
)?;
if let Some(optional_vars) = optional_vars {
write!(f, [space(), text("as"), space(), optional_vars.format()])?;
}
Ok(())
});
write!(f, [group(&inner)])
}
Ok(())
}

fn fmt_dangling_comments(&self, _node: &WithItem, _f: &mut PyFormatter) -> FormatResult<()> {
Ok(())
}
}
Loading

0 comments on commit 8f98884

Please sign in to comment.