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

formatter: debug panic in placement::find_pos_only_slash_offset #5176

Closed
davidszotten opened this issue Jun 19, 2023 · 4 comments
Closed

formatter: debug panic in placement::find_pos_only_slash_offset #5176

davidszotten opened this issue Jun 19, 2023 · 4 comments
Labels
formatter Related to the formatter

Comments

@davidszotten
Copy link
Contributor

while working on comments inside tuple unpacking between stars and name (following #5167 (comment) )

i was wondering if function arguments could have the same issue. in trying to test that i found my test case violates this debug assert

$ cat ../../scratch.py
def foo(
    a=2 ** 3 # trailing
):
    ...
$ cargo run --bin ruff_python_formatter -- --emit stdout ../../scratch.py
   [...]
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `RParen`,
 right: `Comma`', crates/ruff_python_formatter/src/comments/placement.rs:846:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

commit: be11cae

@MichaReiser MichaReiser added the formatter Related to the formatter label Jun 19, 2023
@MichaReiser
Copy link
Member

Thanks for reporting this issue. This is a bug in my implementation. We can fix it by either:

Changing the fallback from arguments.end() to arguments.end() - TextSize::new(1) to exclude the )

let trivia_end = comment
.following_node()
.map_or(arguments.end(), |following| following.start());

Gracefully handle a ) in the find_pos_only_slash

if let Some(comma) = tokens.next() {
debug_assert_eq!(comma.kind(), TokenKind::Comma);

I would prefer the first fix because between_arguments_range is explicit about the range being between arguments, which isn't true for ).

@MichaReiser
Copy link
Member

@davidszotten I tried to reproduce, but your provided snippet does not panic on main (although it gets formatted extremely weirdly):

def foo(
    a=2  # trailing
    **3,
):
    ...

@MichaReiser
Copy link
Member

The weird formatting should be fixed in #5204

@davidszotten
Copy link
Contributor Author

panic was fixed by #5192 (rustpython upgrade)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

No branches or pull requests

2 participants