Skip to content

Commit

Permalink
Use parenthesized_with_dangling_comments in arguments formatter (#6376
Browse files Browse the repository at this point in the history
)

## Summary

Fixes an instability whereby this:

```python
def get_recent_deployments(threshold_days: int) -> Set[str]:
    # Returns a list of deployments not older than threshold days
    # including `/root/zulip` directory if it exists.
    recent = set()
    threshold_date = datetime.datetime.now() - datetime.timedelta(  # noqa: DTZ005
        days=threshold_days
    )
```

Was being formatted as:

```python
def get_recent_deployments(threshold_days: int) -> Set[str]:
    # Returns a list of deployments not older than threshold days
    # including `/root/zulip` directory if it exists.
    recent = set()
    threshold_date = (
        datetime.datetime.now()
        - datetime.timedelta(days=threshold_days)  # noqa: DTZ005
    )
```

Which was in turn being formatted as:

```python
def get_recent_deployments(threshold_days: int) -> Set[str]:
    # Returns a list of deployments not older than threshold days
    # including `/root/zulip` directory if it exists.
    recent = set()
    threshold_date = (
        datetime.datetime.now() - datetime.timedelta(days=threshold_days)  # noqa: DTZ005
    )
```

The second-to-third formattings still differs from Black because we
aren't taking the line suffix into account when splitting
(#6377), but the first
formatting is correct and should be unchanged (i.e., the first-to-second
formattings is incorrect, and fixed here).

## Test Plan

`cargo run --bin ruff_dev -- format-dev --stability-check ../zulip`
  • Loading branch information
charliermarsh authored Aug 7, 2023
1 parent 89e4e03 commit 63692b3
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,7 @@ def f(*args, **kwargs):
1
# abc
)

threshold_date = datetime.datetime.now() - datetime.timedelta( # comment
days=threshold_days_threshold_days
)
33 changes: 18 additions & 15 deletions crates/ruff_python_formatter/src/other/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{TextRange, TextSize};

use crate::builders::empty_parenthesized_with_dangling_comments;
use crate::comments::trailing_comments;
use crate::expression::expr_generator_exp::GeneratorExpParentheses;
use crate::expression::parentheses::{parenthesized, Parentheses};
use crate::expression::parentheses::{parenthesized_with_dangling_comments, Parentheses};
use crate::prelude::*;
use crate::FormatNodeRule;

Expand Down Expand Up @@ -35,18 +34,6 @@ impl FormatNodeRule<Arguments> for FormatArguments {
);
}

// If the arguments are non-empty, then a dangling comment indicates a comment on the
// same line as the opening parenthesis, e.g.:
// ```python
// f( # This call has a dangling comment.
// a,
// b,
// c,
// )
let comments = f.context().comments().clone();
let dangling_comments = comments.dangling_comments(item.as_any_node_ref());
write!(f, [trailing_comments(dangling_comments)])?;

let all_arguments = format_with(|f: &mut PyFormatter| {
let source = f.context().source();
let mut joiner = f.join_comma_separated(item.end());
Expand Down Expand Up @@ -84,6 +71,17 @@ impl FormatNodeRule<Arguments> for FormatArguments {
joiner.finish()
});

// If the arguments are non-empty, then a dangling comment indicates a comment on the
// same line as the opening parenthesis, e.g.:
// ```python
// f( # This call has a dangling comment.
// a,
// b,
// c,
// )
let comments = f.context().comments().clone();
let dangling_comments = comments.dangling_comments(item.as_any_node_ref());

write!(
f,
[
Expand All @@ -103,7 +101,12 @@ impl FormatNodeRule<Arguments> for FormatArguments {
// )
// ```
// TODO(konstin): Doesn't work see wrongly formatted test
parenthesized("(", &group(&all_arguments), ")")
parenthesized_with_dangling_comments(
"(",
dangling_comments,
&group(&all_arguments),
")"
)
]
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ f (
1
# abc
)
threshold_date = datetime.datetime.now() - datetime.timedelta( # comment
days=threshold_days_threshold_days
)
```

## Output
Expand Down Expand Up @@ -230,6 +234,10 @@ f(
1
# abc
)
threshold_date = datetime.datetime.now() - datetime.timedelta( # comment
days=threshold_days_threshold_days
)
```


Expand Down

0 comments on commit 63692b3

Please sign in to comment.