Format PatternMatchStar#6653
Conversation
|
As a test, I formatted this code: x = [1, 2, 3, 4]
match x:
case [
1,
*
rest,
]:
print(rest)
case [
1,
2,
*
# hello
rest,
]:
print(rest)
case [*_]:
print("no match")and got: x = [1, 2, 3, 4]
match x:
case [NOT_YET_IMPLEMENTED_PatternMatchSequence, 2]:
print(rest)
case [NOT_YET_IMPLEMENTED_PatternMatchSequence, 2]:
print(rest)
case [NOT_YET_IMPLEMENTED_PatternMatchSequence, 2]:
print("no match")How can I test this? |
PR Check ResultsBenchmarkLinuxWindows |
|
Tweaked the x = [1, 2, 3, 4]
match x:
case [
"NOT_YET_IMPLEMENTED_PatternMatchValue",
*rest,
]:
print("a")
print(rest)
case [
"NOT_YET_IMPLEMENTED_PatternMatchValue",
"NOT_YET_IMPLEMENTED_PatternMatchValue",
*rest# hello
,
]:
print(rest)
case [*_]:
print("no match")The comment location seems incorrect. |
| let comments = f.context().comments().clone(); | ||
| let dangling = comments.dangling_comments(item); |
There was a problem hiding this comment.
Should comments be handled in PatternMatchSequence?
There was a problem hiding this comment.
It depends on the comments. What's the code snipped that requires the dangling comment handling?
There was a problem hiding this comment.
|
Hmm yeah, it may be challenging to test this just now without match as, singleton formatting, or sequence formatting. There are PRs for |
Can I tackle #6645 first? |
Sure. You'll have to comment on the issue so that I can assign it to you. |
harupy
force-pushed
the
format-PatternMatchStar
branch
from
c0e1161 to
84ed8e8
Compare
crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py
Show resolved
Hide resolved
crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py
Outdated
Show resolved
Hide resolved
| case [1, 2, * # comment | ||
| rest]: | ||
| pass | ||
| case [1, 2, * # comment | ||
| _]: | ||
| pass |
There was a problem hiding this comment.
This is formatted first to:
case [
"NOT_YET_IMPLEMENTED_PatternMatchValue",
"NOT_YET_IMPLEMENTED_PatternMatchValue",
*rest # comment
,
]:
pass
case [
"NOT_YET_IMPLEMENTED_PatternMatchValue",
"NOT_YET_IMPLEMENTED_PatternMatchValue",
*_ # comment
,
]:
passthen formatted to:
case [
"NOT_YET_IMPLEMENTED_PatternMatchValue",
"NOT_YET_IMPLEMENTED_PatternMatchValue",
*rest, # comment
]:
pass
case [
"NOT_YET_IMPLEMENTED_PatternMatchValue",
"NOT_YET_IMPLEMENTED_PatternMatchValue",
*_, # comment
]:
pass
There was a problem hiding this comment.
If you look at
match foo:
case [* # comment
rest, 1]:
pass
case [*rest # comment
, 1]:
passyou get
{
Node {
kind: PatternMatchStar,
range: 21..45,
source: `* # comment⏎`,
}: {
"leading": [],
"dangling": [
SourceComment {
text: "# comment",
position: EndOfLine,
formatted: true,
},
],
"trailing": [],
},
Node {
kind: PatternMatchStar,
range: 74..79,
source: `*rest`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# comment",
position: EndOfLine,
formatted: true,
},
],
},
}
so i think the best solution is to manually reassign the comment from dangling to trailing.
There was a problem hiding this comment.
@konstin Thanks for the comment. How can we access this leading-dangiling-trailing mapping?
There was a problem hiding this comment.
@harupy - I typically run something like cargo run foo.py --emit=stdout --print-comments from ruff/crates/ruff_python_formatter.
There was a problem hiding this comment.
Oh sorry. You're asking how to modify these assignments. Any such modifications need to be done in placement.rs -- those are essentially custom rules that let us change the association of comments. See, e.g., handle_trailing_expression_starred_star_end_of_line_comment which is somewhat similar.
There was a problem hiding this comment.
You have to add a case in handle_enclosed_comment for when the enclosing node is PatternMatchStar. The general logic to fix comment placement is in placement.rs/place_comment.
| /// ``` | ||
| fn handle_pattern_match_star_comment(comment: DecoratedComment) -> CommentPlacement { | ||
| debug_assert!(comment.enclosing_node().is_pattern_match_star()); | ||
| let AnyNodeRef::PatternMatchStar(range, ..) = comment.enclosing_node() else { |
There was a problem hiding this comment.
You can get the ast::PatternMatchStar from the AnyNodeRef::PatternMatchStar(_) in the match and then pass it to the function (like handle_leading_class_with_decorators_comment)
There was a problem hiding this comment.
diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs
index 01dc6b905..a367e6043 100644
--- a/crates/ruff_python_formatter/src/comments/placement.rs
+++ b/crates/ruff_python_formatter/src/comments/placement.rs
@@ -206,7 +206,7 @@ fn handle_enclosed_comment<'a>(
}
AnyNodeRef::WithItem(_) => handle_with_item_comment(comment, locator),
AnyNodeRef::PatternMatchAs(_) => handle_pattern_match_as_comment(comment, locator),
- AnyNodeRef::PatternMatchStar(_) => handle_pattern_match_star_comment(comment),
+ AnyNodeRef::PatternMatchStar(range, ..) => CommentPlacement::trailing(range, comment),
AnyNodeRef::StmtFunctionDef(_) => handle_leading_function_with_decorators_comment(comment),
AnyNodeRef::StmtClassDef(class_def) => {
handle_leading_class_with_decorators_comment(comment, class_def)Can we just do this?
There was a problem hiding this comment.
Ah I see, this doesn't work for the following case:
case [
1,
*
# comment
rest,
]:
passThere was a problem hiding this comment.
I think I'd suggest treating these as dangling, and then rendering them as leading comments within the node.
There was a problem hiding this comment.
(Leading is more consistent with how we treat comments in starred expressions.)
|
@charliermarsh Thanks for the update and merging! |
|
Thanks @harupy for another great contribution! |
4 participants
Summary
Close #6558
Test Plan