Skip to content

Commit

Permalink
Format UnaryExpr
Browse files Browse the repository at this point in the history
<!--
Thank you for contributing to Ruff! To help us out with reviewing, please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

This PR adds basic formatting for unary expressions.

<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan

I added a new `unary.py` with custom test cases
  • Loading branch information
MichaReiser authored Jun 21, 2023
1 parent 3973836 commit 1336ca6
Show file tree
Hide file tree
Showing 13 changed files with 621 additions and 113 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
if not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:
pass

a = True
not a

b = 10
-b
+b

## Leading operand comments

if not (
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass


if ~(
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb):
pass

if -(
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb):
pass


if +(
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb):
pass

if (
not
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass


if (
~
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb):
pass

if (
-
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb):
pass


if (
+
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb):
pass

## Parentheses

if (
# unary comment
not
# operand comment
(
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
)
):
pass

if (not (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
)
):
pass

if aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & (not (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
)
):
pass

if (
not (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
)
& aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
):
pass


## Trailing operator comments

if (
not # comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass


if (
~ # comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass

if (
- # comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass


if (
+ # comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass


## Varia

if not \
a:
pass
4 changes: 4 additions & 0 deletions crates/ruff_python_formatter/src/expression/expr_bin_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ fn can_break(expr: &Expr) -> bool {
}) => !expressions.is_empty(),
Expr::Call(ExprCall { args, keywords, .. }) => !(args.is_empty() && keywords.is_empty()),
Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) | Expr::GeneratorExp(_) => true,
Expr::UnaryOp(ExprUnaryOp { operand, .. }) => match operand.as_ref() {
Expr::BinOp(_) => true,
_ => can_break(operand.as_ref()),
},
_ => false,
}
}
94 changes: 88 additions & 6 deletions crates/ruff_python_formatter/src/expression/expr_unary_op.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,68 @@
use crate::comments::Comments;
use crate::comments::{trailing_comments, Comments};
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::ExprUnaryOp;
use crate::prelude::*;
use crate::trivia::{SimpleTokenizer, TokenKind};
use crate::FormatNodeRule;
use ruff_formatter::FormatContext;
use ruff_python_ast::prelude::UnaryOp;
use ruff_text_size::{TextLen, TextRange};
use rustpython_parser::ast::{ExprUnaryOp, Ranged};

#[derive(Default)]
pub struct FormatExprUnaryOp;

impl FormatNodeRule<ExprUnaryOp> for FormatExprUnaryOp {
fn fmt_fields(&self, item: &ExprUnaryOp, f: &mut PyFormatter) -> FormatResult<()> {
write!(f, [not_yet_implemented(item)])
let ExprUnaryOp {
range: _,
op,
operand,
} = item;

let operator = match op {
UnaryOp::Invert => "~",
UnaryOp::Not => "not",
UnaryOp::UAdd => "+",
UnaryOp::USub => "-",
};

text(operator).fmt(f)?;

let comments = f.context().comments().clone();

// Split off the comments that follow after the operator and format them as trailing comments.
// ```python
// (not # comment
// a)
// ```
let leading_operand_comments = comments.leading_comments(operand.as_ref());
let trailing_operator_comments_end =
leading_operand_comments.partition_point(|p| p.position().is_end_of_line());
let (trailing_operator_comments, leading_operand_comments) =
leading_operand_comments.split_at(trailing_operator_comments_end);

if !trailing_operator_comments.is_empty() {
trailing_comments(trailing_operator_comments).fmt(f)?;
}

// Insert a line break if the operand has comments but itself is not parenthesized.
// ```python
// if (
// not
// # comment
// a)
// ```
if !leading_operand_comments.is_empty()
&& !is_operand_parenthesized(item, f.context().source_code().as_str())
{
hard_line_break().fmt(f)?;
} else if op.is_not() {
space().fmt(f)?;
}

operand.format().fmt(f)
}
}

Expand All @@ -22,6 +73,37 @@ impl NeedsParentheses for ExprUnaryOp {
source: &str,
comments: &Comments,
) -> Parentheses {
default_expression_needs_parentheses(self.into(), parenthesize, source, comments)
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
Parentheses::Optional => {
// We preserve the parentheses of the operand. It should not be necessary to break this expression.
if is_operand_parenthesized(self, source) {
Parentheses::Never
} else {
Parentheses::Optional
}
}
parentheses => parentheses,
}
}
}

fn is_operand_parenthesized(unary: &ExprUnaryOp, source: &str) -> bool {
let operator_len = match unary.op {
UnaryOp::Invert => '~'.text_len(),
UnaryOp::Not => "not".text_len(),
UnaryOp::UAdd => '+'.text_len(),
UnaryOp::USub => '-'.text_len(),
};

let trivia_range = TextRange::new(unary.range.start() + operator_len, unary.operand.start());

if let Some(token) = SimpleTokenizer::new(source, trivia_range)
.skip_trivia()
.next()
{
debug_assert_eq!(token.kind(), TokenKind::LParen);
true
} else {
false
}
}
6 changes: 3 additions & 3 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(super) fn default_expression_needs_parentheses(
}

/// Configures if the expression should be parenthesized.
#[derive(Copy, Clone, Debug, Default)]
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
pub enum Parenthesize {
/// Parenthesize the expression if it has parenthesis in the source.
#[default]
Expand All @@ -56,11 +56,11 @@ pub enum Parenthesize {
}

impl Parenthesize {
const fn is_if_breaks(self) -> bool {
pub(crate) const fn is_if_breaks(self) -> bool {
matches!(self, Parenthesize::IfBreaks)
}

const fn is_preserve(self) -> bool {
pub(crate) const fn is_preserve(self) -> bool {
matches!(self, Parenthesize::Preserve)
}
}
Expand Down
9 changes: 4 additions & 5 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,11 +411,10 @@ Formatted twice:
#[test]
fn quick_test() {
let src = r#"
def foo(
b=3
+ 2 # comment
):
if [
aaaaaa,
BBBB,ccccccccc,ddddddd,eeeeeeeeee,ffffff
] & bbbbbbbbbbbbbbbbbbddddddddddddddddddddddddddddbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:
...
"#;
// Tokenize once
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ y = 100(no)
+x = NOT_IMPLEMENTED_call()
+x = 0O777 .NOT_IMPLEMENTED_attr
+x = NOT_IMPLEMENTED_call()
+x = NOT_YET_IMPLEMENTED_ExprUnaryOp
+x = -100.0000J
-if (10).real:
+if 10 .NOT_IMPLEMENTED_attr:
Expand Down Expand Up @@ -97,7 +97,7 @@ x = NOT_IMPLEMENTED_call()
x = NOT_IMPLEMENTED_call()
x = 0O777 .NOT_IMPLEMENTED_attr
x = NOT_IMPLEMENTED_call()
x = NOT_YET_IMPLEMENTED_ExprUnaryOp
x = -100.0000J
if 10 .NOT_IMPLEMENTED_attr:
...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def g():
```diff
--- Black
+++ Ruff
@@ -1,89 +1,70 @@
@@ -1,47 +1,35 @@
-"""Docstring."""
+"NOT_YET_IMPLEMENTED_STRING"
Expand Down Expand Up @@ -137,11 +137,10 @@ def g():
+ NOT_YET_IMPLEMENTED_StmtAssert
- prev = leaf.prev_sibling
- if not prev:
+ prev = leaf.NOT_IMPLEMENTED_attr
if not prev:
- prevp = preceding_leaf(p)
- if not prevp or prevp.type in OPENING_BRACKETS:
+ prev = leaf.NOT_IMPLEMENTED_attr
+ if NOT_YET_IMPLEMENTED_ExprUnaryOp:
+ prevp = NOT_IMPLEMENTED_call()
+ if NOT_IMPLEMENTED_bool_op1 and NOT_IMPLEMENTED_bool_op2:
return NO
Expand Down Expand Up @@ -171,11 +170,11 @@ def g():
return NO
###############################################################################
@@ -49,41 +37,34 @@
# SECTION BECAUSE SECTIONS
###############################################################################
-
-
def g():
- NO = ""
- SPACE = " "
Expand Down Expand Up @@ -205,10 +204,9 @@ def g():
+ NOT_YET_IMPLEMENTED_StmtAssert
- prev = leaf.prev_sibling
- if not prev:
- prevp = preceding_leaf(p)
+ prev = leaf.NOT_IMPLEMENTED_attr
+ if NOT_YET_IMPLEMENTED_ExprUnaryOp:
if not prev:
- prevp = preceding_leaf(p)
+ prevp = NOT_IMPLEMENTED_call()
- if not prevp or prevp.type in OPENING_BRACKETS:
Expand Down Expand Up @@ -254,7 +252,7 @@ def f():
NOT_YET_IMPLEMENTED_StmtAssert
prev = leaf.NOT_IMPLEMENTED_attr
if NOT_YET_IMPLEMENTED_ExprUnaryOp:
if not prev:
prevp = NOT_IMPLEMENTED_call()
if NOT_IMPLEMENTED_bool_op1 and NOT_IMPLEMENTED_bool_op2:
return NO
Expand Down Expand Up @@ -292,7 +290,7 @@ def g():
NOT_YET_IMPLEMENTED_StmtAssert
prev = leaf.NOT_IMPLEMENTED_attr
if NOT_YET_IMPLEMENTED_ExprUnaryOp:
if not prev:
prevp = NOT_IMPLEMENTED_call()
if NOT_IMPLEMENTED_bool_op1 and NOT_IMPLEMENTED_bool_op2:
Expand Down
Loading

0 comments on commit 1336ca6

Please sign in to comment.