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.

Sign up for GitHub

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

Format delete statement #5169

Merged
merged 21 commits into from
Jul 11, 2023
+328 −3
Merged

Format delete statement #5169

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
faf23c3
Format delete statement
cnpryer Jun 17, 2023
1ef57eb
Use black fixture
cnpryer Jun 18, 2023
b5df670
Move new delete fixture into ruff fixtures
cnpryer Jun 18, 2023
022e8ee
Add delete statement test case
cnpryer Jun 18, 2023
3b12221
Update delete statement snapshot
cnpryer Jun 21, 2023
a98d721
Write with just JoinBuilder
cnpryer Jun 21, 2023
3466a0b
Merge branch 'main' into stmt-delete
cnpryer Jun 24, 2023
3d7e86c
Update snapshot
cnpryer Jun 24, 2023
dfc1122
Merge branch 'main' into stmt-delete
cnpryer Jul 8, 2023
2e676f3
Update tests
cnpryer Jul 9, 2023
f57bc9e
Export default_expression_needs_parentheses for crate
cnpryer Jul 9, 2023
a1d9855
Use ExprSequence
cnpryer Jul 9, 2023
14b80b6
Update snapshot for trailing comment test case
cnpryer Jul 9, 2023
7ebd63c
Add test case for expecting no parentheses
cnpryer Jul 9, 2023
695df60
Update snapshot
cnpryer Jul 9, 2023
ce249f5
Don't expect trailing comments to trigger format
cnpryer Jul 9, 2023
b5fe6d5
Move ExprSequence to sequence.rs
cnpryer Jul 9, 2023
dee077b
Override fmt_dangling_comments
cnpryer Jul 10, 2023
da5799d
Small chores
cnpryer Jul 10, 2023
71352fc
Use join_comma_separated instead of ExprSequence
cnpryer Jul 10, 2023
f16ecb1
Small chore
cnpryer Jul 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/delete.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
x = 1
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = 1
a, b, c, d = 1, 2, 3, 4

del a, b, c, d
del a, b, c, d # Trailing

del a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a
del a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a # Trailing

del (
a,
a
)

del (
# Dangling comment
)

# Delete something
del x # Deleted something
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x, # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
# Dangling comment
) # Completed
# Done deleting

# Delete something
del x, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, b, c, d # Delete these
# Ready to delete

# Delete something
del (
x,
# Deleting this
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
b,
c,
d,
# Deleted
) # Completed
# Done
2 changes: 1 addition & 1 deletion crates/ruff_python_formatter/src/expression/parentheses.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(crate) trait NeedsParentheses {
) -> Parentheses;
}

pub(super) fn default_expression_needs_parentheses(
pub(crate) fn default_expression_needs_parentheses(
node: AnyNodeRef,
parenthesize: Parenthesize,
source: &str,
Expand Down
74 changes: 70 additions & 4 deletions crates/ruff_python_formatter/src/statement/stmt_delete.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,78 @@
use crate::{not_yet_implemented, FormatNodeRule, PyFormatter};
use ruff_formatter::{write, Buffer, FormatResult};
use rustpython_parser::ast::StmtDelete;
use crate::builders::{optional_parentheses, PyFormatterExtensions};
use crate::comments::{dangling_node_comments, Comments};
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::prelude::PyFormatContext;
use crate::{AsFormat, FormatNodeRule, PyFormatter};
use ruff_formatter::prelude::{block_indent, space, text, Formatter};
use ruff_formatter::{write, Buffer, Format, FormatResult};
use rustpython_parser::ast::{Expr, StmtDelete};

#[derive(Default)]
pub struct FormatStmtDelete;

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

write!(f, [text("del"), space()])?;

match targets.as_slice() {
[] => {
write!(
f,
[
// Handle special case of delete statements without targets.
// ```
// del (
// # Dangling comment
// )
&text("("),
block_indent(&dangling_node_comments(item)),
&text(")"),
]
)
}
// TODO(cnpryer): single and multiple targets should be handled the same since
// tuples require special formatting whereas this is just a sequence of expressions (tuple-like).
[single] => {
write!(f, [single.format().with_options(Parenthesize::IfBreaks)])
}
targets => optional_parentheses(&ExprSequence::new(targets)).fmt(f),
}
}
}

// TODO(cnpryer): Shared `ExprSequence` (see expr_tuple.rs)
#[derive(Debug)]
struct ExprSequence<'a> {
targets: &'a [Expr],
}

impl<'a> ExprSequence<'a> {
const fn new(targets: &'a [Expr]) -> Self {
Self { targets }
}
}

impl Format<PyFormatContext<'_>> for ExprSequence<'_> {
fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
f.join_comma_separated().nodes(self.targets.iter()).finish()
}
}

// NOTE: `default_expression_needs_parentheses` is reserved for expression nodes.
impl NeedsParentheses for StmtDelete {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
source: &str,
comments: &Comments,
) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
Parentheses::Optional => Parentheses::Never,
parentheses => parentheses,
}
}
}
221 changes: 221 additions & 0 deletions crates/ruff_python_formatter/tests/snapshots/format@statement__delete.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/delete.py
---
## Input
```py
x = 1
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = 1
a, b, c, d = 1, 2, 3, 4

del a, b, c, d
del a, b, c, d # Trailing

del a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a
del a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a # Trailing

del (
a,
a
)

del (
# Dangling comment
)

# Delete something
del x # Deleted something
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x, # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
# Dangling comment
) # Completed
# Done deleting

# Delete something
del x, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, b, c, d # Delete these
# Ready to delete

# Delete something
del (
x,
# Deleting this
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
b,
c,
d,
# Deleted
) # Completed
# Done
```

## Output
```py
x = 1
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = 1
a, b, c, d = 1, 2, 3, 4

del a, b, c, d
del a, b, c, d # Trailing

del (
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
)
del (
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
a,
) # Trailing

del a, a
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh. Actually black will format this as

del (x, x)

Copy link
Contributor Author

@cnpryer cnpryer Jul 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe this is because it could be interpreted as del tuple(x, x) instead of del obj, obj?

A more reasonable example would be

a, b, c = 1, 2, 3
del (
    a, 
    b, 
    c
)

But I'd guess that's not actually performing a delete of a, b, and c but instead a new object of tuple(a, b, c)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense

>>> print(ast.dump(ast.parse('del (a, b, c)'), indent=4))
Module(
    body=[
        Delete(
            targets=[
                Tuple(
                    elts=[
                        Name(id='a', ctx=Del()),
                        Name(id='b', ctx=Del()),
                        Name(id='c', ctx=Del())],
                    ctx=Del())])],
    type_ignores=[])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

695df60


del (
# Dangling comment
)

# Delete something
del x # Deleted something
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x, # Deleted something
# Finishing deletes
) # Completed
# Done deleting

# Delete something
del (
# Deleting something
x # Deleted something
# Finishing deletes
# Dangling comment
) # Completed
# Done deleting

# Delete something
del (
x,
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
b,
c,
d,
) # Delete these
# Ready to delete

# Delete something
del (
x,
# Deleting this
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
b,
c,
d,
# Deleted
) # Completed
# Done
```