Fix invalid syntax for binary expression in unary op (#5370)

This commit is contained in:
Micha Reiser 2023-06-29 08:09:26 +02:00 committed by GitHub
parent 38189ed913
commit 955e9ef821
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 200 additions and 116 deletions

1
Cargo.lock generated
View file

@ -2088,6 +2088,7 @@ dependencies = [
"serde",
"serde_json",
"similar",
"smallvec",
]
[[package]]

View file

@ -26,6 +26,7 @@ once_cell = { workspace = true }
rustc-hash = { workspace = true }
rustpython-parser = { workspace = true }
serde = { workspace = true, optional = true }
smallvec = { workspace = true }
[dev-dependencies]
ruff_formatter = { path = "../ruff_formatter", features = ["serde"]}

View file

@ -10,6 +10,19 @@
b
)
(
# leading left most comment
aaaaaaaa
+ # trailing operator comment
# leading b comment
b # trailing b comment
# trailing b ownline comment
+ # trailing second operator comment
# leading c comment
c # trailing c comment
# trailing own line comment
)
# Black breaks the right side first for the following expressions:
aaaaaaaaaaaaaa + caaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaal(argument1, argument2, argument3)

View file

@ -136,3 +136,7 @@ if (
if not \
a:
pass
# Regression: https://github.com/astral-sh/ruff/issues/5338
if a and not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

View file

@ -32,5 +32,8 @@ class Test((Aaaa)):
class Test(aaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccccc + dddddddddddddddddddddd + eeeeeeeee, ffffffffffffffffff, gggggggggggggggggg):
pass
class Test(aaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccc + dddddddddddddddddddddd + eeeeeeeee, ffffffffffffffffff, gggggggggggggggggg):
pass
class Test(Aaaa): # trailing comment
pass

View file

@ -5,7 +5,7 @@ use rustpython_parser::ast::{self, Expr};
use ruff_formatter::{format_args, write};
use crate::expression::parentheses::Parentheses;
use crate::expression::parentheses::{is_expression_parenthesized, Parentheses};
use crate::prelude::*;
/// Trait to implement a binary like syntax that has a left operand, an operator, and a right operand.
@ -24,7 +24,7 @@ pub(super) trait FormatBinaryLike<'ast> {
let right = self.right()?;
let layout = if parentheses == Some(Parentheses::Custom) {
self.binary_layout()
self.binary_layout(f.context().contents())
} else {
BinaryLayout::Default
};
@ -113,9 +113,9 @@ pub(super) trait FormatBinaryLike<'ast> {
}
/// Determines which binary layout to use.
fn binary_layout(&self) -> BinaryLayout {
fn binary_layout(&self, source: &str) -> BinaryLayout {
if let (Ok(left), Ok(right)) = (self.left(), self.right()) {
BinaryLayout::from_left_right(left, right)
BinaryLayout::from_left_right(left, right, source)
} else {
BinaryLayout::Default
}
@ -134,8 +134,8 @@ pub(super) trait FormatBinaryLike<'ast> {
fn operator(&self) -> Self::FormatOperator;
}
fn can_break_expr(expr: &Expr) -> bool {
match expr {
fn can_break_expr(expr: &Expr, source: &str) -> bool {
let can_break = match expr {
Expr::Tuple(ast::ExprTuple {
elts: expressions, ..
})
@ -153,12 +153,11 @@ fn can_break_expr(expr: &Expr) -> bool {
!(args.is_empty() && keywords.is_empty())
}
Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) | Expr::GeneratorExp(_) => true,
Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => match operand.as_ref() {
Expr::BinOp(_) => true,
_ => can_break_expr(operand.as_ref()),
},
Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => can_break_expr(operand.as_ref(), source),
_ => false,
}
};
can_break || is_expression_parenthesized(expr.into(), source)
}
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
@ -206,8 +205,8 @@ pub(super) enum BinaryLayout {
}
impl BinaryLayout {
pub(super) fn from_left_right(left: &Expr, right: &Expr) -> BinaryLayout {
match (can_break_expr(left), can_break_expr(right)) {
pub(super) fn from_left_right(left: &Expr, right: &Expr, source: &str) -> BinaryLayout {
match (can_break_expr(left, source), can_break_expr(right, source)) {
(false, false) => BinaryLayout::Default,
(true, false) => BinaryLayout::ExpandLeft,
(false, true) => BinaryLayout::ExpandRight,

View file

@ -1,4 +1,4 @@
use crate::comments::{trailing_comments, Comments};
use crate::comments::{trailing_comments, trailing_node_comments, Comments};
use crate::expression::binary_like::{BinaryLayout, FormatBinaryLike};
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parenthesize,
@ -10,6 +10,8 @@ use ruff_formatter::{write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWi
use rustpython_parser::ast::{
Constant, Expr, ExprAttribute, ExprBinOp, ExprConstant, ExprUnaryOp, Operator, UnaryOp,
};
use smallvec::SmallVec;
use std::iter;
#[derive(Default)]
pub struct FormatExprBinOp {
@ -40,16 +42,30 @@ impl<'ast> FormatBinaryLike<'ast> for ExprBinOp {
type FormatOperator = FormatOwnedWithRule<Operator, FormatOperator, PyFormatContext<'ast>>;
fn fmt_default(&self, f: &mut PyFormatter<'ast, '_>) -> FormatResult<()> {
let comments = f.context().comments().clone();
let format_inner = format_with(|f| {
let binary_chain: SmallVec<[&ExprBinOp; 4]> =
iter::successors(Some(self), |parent| parent.left.as_bin_op_expr()).collect();
// SAFETY: `binary_chain` is guaranteed not to be empty because it always contains the current expression.
let left_most = binary_chain.last().unwrap();
// Format the left most expression
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,
left: _,
op,
right,
} = self;
} = current;
let comments = f.context().comments().clone();
let operator_comments = comments.dangling_comments(self);
let needs_space = !is_simple_power_expression(self);
let operator_comments = comments.dangling_comments(current);
let needs_space = !is_simple_power_expression(current);
let before_operator_space = if needs_space {
soft_line_break_or_space()
@ -60,7 +76,6 @@ impl<'ast> FormatBinaryLike<'ast> for ExprBinOp {
write!(
f,
[
left.format(),
before_operator_space,
op.format(),
trailing_comments(operator_comments),
@ -68,13 +83,27 @@ impl<'ast> FormatBinaryLike<'ast> for ExprBinOp {
)?;
// Format the operator on its own line if the right side has any leading comments.
if comments.has_leading_comments(right.as_ref()) {
write!(f, [hard_line_break()])?;
if comments.has_leading_comments(right.as_ref()) || !operator_comments.is_empty() {
hard_line_break().fmt(f)?;
} else if needs_space {
write!(f, [space()])?;
space().fmt(f)?;
}
write!(f, [group(&right.format())])
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 != self {
trailing_node_comments(current).fmt(f)?;
}
}
Ok(())
});
group(&format_inner).fmt(f)
}
fn left(&self) -> FormatResult<&Expr> {
@ -162,7 +191,7 @@ impl NeedsParentheses for ExprBinOp {
) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
Parentheses::Optional => {
if self.binary_layout() == BinaryLayout::Default
if self.binary_layout(source) == BinaryLayout::Default
|| comments.has_leading_comments(self.right.as_ref())
|| comments.has_dangling_comments(self)
{

View file

@ -31,9 +31,9 @@ impl FormatNodeRule<ExprBoolOp> for FormatExprBoolOp {
impl<'ast> FormatBinaryLike<'ast> for ExprBoolOp {
type FormatOperator = FormatOwnedWithRule<BoolOp, FormatBoolOp, PyFormatContext<'ast>>;
fn binary_layout(&self) -> BinaryLayout {
fn binary_layout(&self, source: &str) -> BinaryLayout {
match self.values.as_slice() {
[left, right] => BinaryLayout::from_left_right(left, right),
[left, right] => BinaryLayout::from_left_right(left, right, source),
[..] => BinaryLayout::Default,
}
}
@ -93,7 +93,7 @@ impl NeedsParentheses for ExprBoolOp {
comments: &Comments,
) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
Parentheses::Optional => match self.binary_layout() {
Parentheses::Optional => match self.binary_layout(source) {
BinaryLayout::Default => Parentheses::Optional,
BinaryLayout::ExpandRight

View file

@ -34,10 +34,10 @@ impl FormatNodeRule<ExprCompare> for FormatExprCompare {
impl<'ast> FormatBinaryLike<'ast> for ExprCompare {
type FormatOperator = FormatOwnedWithRule<CmpOp, FormatCmpOp, PyFormatContext<'ast>>;
fn binary_layout(&self) -> BinaryLayout {
fn binary_layout(&self, source: &str) -> BinaryLayout {
if self.ops.len() == 1 {
match self.comparators.as_slice() {
[right] => BinaryLayout::from_left_right(&self.left, right),
[right] => BinaryLayout::from_left_right(&self.left, right, source),
[..] => BinaryLayout::Default,
}
} else {
@ -102,7 +102,7 @@ impl NeedsParentheses for ExprCompare {
comments: &Comments,
) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
parentheses @ Parentheses::Optional => match self.binary_layout() {
parentheses @ Parentheses::Optional => match self.binary_layout(source) {
BinaryLayout::Default => parentheses,
BinaryLayout::ExpandRight

View file

@ -10,7 +10,7 @@ use ruff_formatter::{
};
use rustpython_parser::ast::Expr;
mod binary_like;
pub(crate) mod binary_like;
pub(crate) mod expr_attribute;
pub(crate) mod expr_await;
pub(crate) mod expr_bin_op;

View file

@ -147,7 +147,7 @@ aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*ite
an_element_with_a_long_value = calls() or more_calls() and more() # type: bool
tup = (
@@ -100,19 +98,35 @@
@@ -100,19 +98,32 @@
)
c = call(
@ -169,10 +169,7 @@ aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*ite
-AAAAAAAAAAAAA = [AAAAAAAAAAAAA] + SHARED_AAAAAAAAAAAAA + USER_AAAAAAAAAAAAA + AAAAAAAAAAAAA # type: ignore
+AAAAAAAAAAAAA = (
+ [AAAAAAAAAAAAA]
+ + SHARED_AAAAAAAAAAAAA
+ + USER_AAAAAAAAAAAAA
+ + AAAAAAAAAAAAA
+ [AAAAAAAAAAAAA] + SHARED_AAAAAAAAAAAAA + USER_AAAAAAAAAAAAA + AAAAAAAAAAAAA
+) # type: ignore
call_to_some_function_asdf(
@ -308,10 +305,7 @@ def func(
result = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" # aaa
AAAAAAAAAAAAA = (
[AAAAAAAAAAAAA]
+ SHARED_AAAAAAAAAAAAA
+ USER_AAAAAAAAAAAAA
+ AAAAAAAAAAAAA
[AAAAAAAAAAAAA] + SHARED_AAAAAAAAAAAAA + USER_AAAAAAAAAAAAA + AAAAAAAAAAAAA
) # type: ignore
call_to_some_function_asdf(

View file

@ -274,11 +274,20 @@ last_call()
Name
None
True
@@ -30,33 +31,39 @@
@@ -23,40 +24,46 @@
1 >> v2
1 % finished
1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8
-((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8)
+(1 + v2 - (v3 * 4)) ^ (5**v6 / 7 // 8)
not great
~great
+value
-1
~int and not v1 ^ 123 + v2 | True
(~int) and (not ((v1 ^ (123 + v2)) | True))
-(~int) and (not ((v1 ^ (123 + v2)) | True))
-+(really ** -(confusing ** ~(operator**-precedence)))
+(~int) and (not (v1 ^ (123 + v2) | True))
++really ** -confusing ** ~operator**-precedence
flags & ~select.EPOLLIN and waiters.write_task is not None
-lambda arg: None
@ -638,13 +647,13 @@ v1 << 2
1 >> v2
1 % finished
1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8
((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8)
(1 + v2 - (v3 * 4)) ^ (5**v6 / 7 // 8)
not great
~great
+value
-1
~int and not v1 ^ 123 + v2 | True
(~int) and (not ((v1 ^ (123 + v2)) | True))
(~int) and (not (v1 ^ (123 + v2) | True))
+really ** -confusing ** ~operator**-precedence
flags & ~select.EPOLLIN and waiters.write_task is not None
lambda x: True

View file

@ -202,11 +202,11 @@ d={'a':1,
#!/usr/bin/env python3
-import asyncio
-import sys
-
-from third_party import X, Y, Z
+NOT_YET_IMPLEMENTED_StmtImport
+NOT_YET_IMPLEMENTED_StmtImport
-from third_party import X, Y, Z
-
-from library import some_connection, some_decorator
+NOT_YET_IMPLEMENTED_StmtImportFrom
@ -306,7 +306,7 @@ d={'a':1,
h: str = "",
i: str = r"",
):
@@ -64,55 +86,55 @@
@@ -64,55 +86,54 @@
something = {
# fmt: off
@ -327,8 +327,7 @@ d={'a':1,
+ "some big and",
+ "complex subscript",
+ # fmt: on
+ goes
+ + here,
+ goes + here,
+ andhere,
+ )
]
@ -382,7 +381,7 @@ d={'a':1,
# fmt: on
@@ -133,10 +155,10 @@
@@ -133,10 +154,10 @@
"""Another known limitation."""
# fmt: on
# fmt: off
@ -397,7 +396,7 @@ d={'a':1,
# fmt: on
# fmt: off
# ...but comments still get reformatted even though they should not be
@@ -151,12 +173,10 @@
@@ -151,12 +172,10 @@
ast_args.kw_defaults,
parameters,
implicit_default=True,
@ -412,17 +411,16 @@ d={'a':1,
# fmt: on
_type_comment_re = re.compile(
r"""
@@ -179,7 +199,8 @@
@@ -179,7 +198,7 @@
$
""",
# fmt: off
- re.MULTILINE|re.VERBOSE
+ re.MULTILINE
+ | re.VERBOSE,
+ re.MULTILINE | re.VERBOSE,
# fmt: on
)
@@ -217,8 +238,7 @@
@@ -217,8 +236,7 @@
xxxxxxxxxx_xxxxxxxxxxx_xxxxxxx_xxxxxxxxx=5,
)
# fmt: off
@ -538,8 +536,7 @@ def subscriptlist():
"some big and",
"complex subscript",
# fmt: on
goes
+ here,
goes + here,
andhere,
)
]
@ -640,8 +637,7 @@ def long_lines():
$
""",
# fmt: off
re.MULTILINE
| re.VERBOSE,
re.MULTILINE | re.VERBOSE,
# fmt: on
)

View file

@ -111,14 +111,14 @@ def __await__(): return (yield)
#!/usr/bin/env python3
-import asyncio
-import sys
-
-from third_party import X, Y, Z
+NOT_YET_IMPLEMENTED_StmtImport
+NOT_YET_IMPLEMENTED_StmtImport
-from library import some_connection, some_decorator
-from third_party import X, Y, Z
+NOT_YET_IMPLEMENTED_StmtImportFrom
-from library import some_connection, some_decorator
-
-f"trigger 3.6 mode"
+NOT_YET_IMPLEMENTED_StmtImportFrom
+NOT_YET_IMPLEMENTED_ExprJoinedStr
@ -215,17 +215,7 @@ def __await__(): return (yield)
)
_type_comment_re = re.compile(
r"""
@@ -118,7 +124,8 @@
)
$
""",
- re.MULTILINE | re.VERBOSE,
+ re.MULTILINE
+ | re.VERBOSE,
)
@@ -135,14 +142,8 @@
@@ -135,14 +141,8 @@
a,
**kwargs,
) -> A:
@ -373,8 +363,7 @@ def long_lines():
)
$
""",
re.MULTILINE
| re.VERBOSE,
re.MULTILINE | re.VERBOSE,
)

View file

@ -115,12 +115,13 @@ x[
ham[lower + offset : upper + offset]
slice[::, ::]
@@ -50,10 +50,14 @@
@@ -49,11 +49,14 @@
slice[
# A
1
- 1
- + 2 :
+ + 2 :
+ 1 + 2 :
# B
- 3 :
+ 3 :
@ -189,8 +190,7 @@ slice[
slice[
# A
1
+ 2 :
1 + 2 :
# B
3 :
# C

View file

@ -16,6 +16,19 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression
b
)
(
# leading left most comment
aaaaaaaa
+ # trailing operator comment
# leading b comment
b # trailing b comment
# trailing b ownline comment
+ # trailing second operator comment
# leading c comment
c # trailing c comment
# trailing own line comment
)
# Black breaks the right side first for the following expressions:
aaaaaaaaaaaaaa + caaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaal(argument1, argument2, argument3)
@ -204,7 +217,8 @@ for user_id in set(target_user_ids) - {u.user_id for u in updates}:
```py
(
aaaaaaaa
+ b # trailing operator comment # trailing right comment
+ # trailing operator comment
b # trailing right comment
)
@ -215,6 +229,19 @@ for user_id in set(target_user_ids) - {u.user_id for u in updates}:
b
)
(
# leading left most comment
aaaaaaaa
+ # trailing operator comment
# leading b comment
b # trailing b comment
# trailing b ownline comment
+ # trailing second operator comment
# leading c comment
c # trailing c comment
# trailing own line comment
)
# Black breaks the right side first for the following expressions:
aaaaaaaaaaaaaa + caaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaal(
@ -269,8 +296,7 @@ not (aaaaaaaaaaaaaa + {NOT_IMPLEMENTED_set_value for value in NOT_IMPLEMENTED_se
# leading comment
(
# comment
content
+ b
content + b
)
@ -417,8 +443,7 @@ if (
&
(
# comment
a
+ b
a + b
)
):
...
@ -433,8 +458,7 @@ if (
]
&
# comment
a
+ b
a + b
):
...

View file

@ -142,6 +142,10 @@ if (
if not \
a:
pass
# Regression: https://github.com/astral-sh/ruff/issues/5338
if a and not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...
```
## Output
@ -253,10 +257,8 @@ if aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & (
pass
if (
not (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
)
& aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
):
pass
@ -295,6 +297,14 @@ if (
if not a:
pass
# Regression: https://github.com/astral-sh/ruff/issues/5338
if (
a
and not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
& aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
):
...
```

View file

@ -125,8 +125,7 @@ f(
f(
this_is_a_very_long_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd,
these_arguments_have_values_that_need_to_break_because_they_are_too_long1=(
100000
- 100000000000
100000 - 100000000000
),
these_arguments_have_values_that_need_to_break_because_they_are_too_long2="akshfdlakjsdfad"
+ "asdfasdfa",

View file

@ -38,6 +38,9 @@ class Test((Aaaa)):
class Test(aaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccccc + dddddddddddddddddddddd + eeeeeeeee, ffffffffffffffffff, gggggggggggggggggg):
pass
class Test(aaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccc + dddddddddddddddddddddd + eeeeeeeee, ffffffffffffffffff, gggggggggggggggggg):
pass
class Test(Aaaa): # trailing comment
pass
```
@ -88,6 +91,17 @@ class Test(
pass
class Test(
aaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccc
+ dddddddddddddddddddddd
+ eeeeeeeee,
ffffffffffffffffff,
gggggggggggggggggg,
):
pass
class Test(Aaaa): # trailing comment
pass
```

View file

@ -114,8 +114,7 @@ with a: # should remove brackets
# WithItem allow the `aa + bb` content expression to be wrapped
with (
(
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
) as c,
):
...