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.

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

handle_parenthesized_comment can't deal with certain kinds of match comments #6866

Closed
konstin opened this issue Aug 25, 2023 · 1 comment · Fixed by #6881
Closed

handle_parenthesized_comment can't deal with certain kinds of match comments #6866

konstin opened this issue Aug 25, 2023 · 1 comment · Fixed by #6881
Assignees
Labels
bug Something isn't working formatter Related to the formatter

Comments

@konstin
Copy link
Member

konstin commented Aug 25, 2023

The following code is valid python, but crashes handle_parenthesized_comment:

match a:
    case A(
        # a
        b # b
        = # c
        2 # d
        # e
    ):
        pass
thread 'main' panicked at 'Unexpected token between nodes: `"\n        b # b\n        = # c\n        "`', crates/ruff_python_formatter/src/comments/placement.rs:147:17

The assert in question:

if tokenizer
.skip_trivia()
.take_while(|token| {
!matches!(
token.kind,
SimpleTokenKind::As | SimpleTokenKind::Def | SimpleTokenKind::Class
)
})
.any(|token| {
debug_assert!(
!matches!(token.kind, SimpleTokenKind::Bogus),
"Unexpected token between nodes: `{:?}`",
locator.slice(TextRange::new(comment.end(), following.start()))
);
token.kind() == SimpleTokenKind::RParen
})

@konstin konstin added bug Something isn't working formatter Related to the formatter labels Aug 25, 2023
@konstin konstin changed the title handle_parenthesized_comment can't deal with certain kinds handle_parenthesized_comment can't deal with certain kinds of match comments Aug 25, 2023
@MichaReiser MichaReiser added this to the Formatter: Alpha milestone Aug 25, 2023
@charliermarsh
Copy link
Member

Probably because the b in that example isn’t a node but rather an identifier? I can own.

@charliermarsh charliermarsh self-assigned this Aug 25, 2023
charliermarsh added a commit that referenced this issue Aug 26, 2023
## Summary

This PR introduces two new AST nodes to improve the representation of
`PatternMatchClass`. As a reminder, `PatternMatchClass` looks like this:

```python
case Point2D(0, 0, x=1, y=2):
  ...
```

Historically, this was represented as a vector of patterns (for the `0,
0` portion) and parallel vectors of keyword names (for `x` and `y`) and
values (for `1` and `2`). This introduces a bunch of challenges for the
formatter, but importantly, it's also really different from how we
represent similar nodes, like arguments (`func(0, 0, x=1, y=2)`) or
parameters (`def func(x, y)`).

So, firstly, we now use a single node (`PatternArguments`) for the
entire parenthesized region, making it much more consistent with our
other nodes. So, above, `PatternArguments` would be `(0, 0, x=1, y=2)`.

Secondly, we now have a `PatternKeyword` node for `x=1` and `y=2`. This
is much more similar to the how `Keyword` is represented within
`Arguments` for call expressions.

Closes #6866.

Closes #6880.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants