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

Avoid printing continuations within import identifiers #7744

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

charliermarsh
Copy link
Member

Summary

It turns out that some identifiers can contain newlines -- specifically, dot-delimited import identifiers, like:

import foo\
    .bar

At present, we print all identifiers verbatim, which causes us to retain the \ in the formatted output. This also leads to violating some debug assertions (see the linked issue, though that's a symptom of this formatting failure).

This PR adds detection for import identifiers that contain newlines, and formats them via text (slow) rather than source_code_slice (fast) in those cases.

Closes #7734.

Test Plan

cargo test

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 1, 2023

CodSpeed Performance Report

Merging #7744 will improve performances by 2.02%

Comparing charlie/continuation (a307da9) with main (d8a6279)

Summary

⚡ 1 improvements
✅ 24 untouched benchmarks

Benchmarks breakdown

Benchmark main charlie/continuation Change
linter/default-rules[numpy/ctypeslib.py] 19 ms 18.6 ms +2.02%

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

I think that fundamentally that is bug in our AST: Indentifiers in python can not contain dots, and import foo.bar consists of two dot-separated keywords, not one.

I think we should merge this fix and then solve this properly with #7760

@charliermarsh charliermarsh merged commit c71ff7e into main Oct 2, 2023
16 checks passed
@charliermarsh charliermarsh deleted the charlie/continuation branch October 2, 2023 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when formatting file editor_main.main_ui' contains an unsupported '\r' line terminator character
2 participants