Skip to content

Commit 3d2a487

Browse files
authored
[prism] Fix comment check for expression-led call chains (#786)
* [prism] Add tests around comments after call chains with leading expressions * Fix comment check for expression-led call chains
1 parent 800110f commit 3d2a487

File tree

3 files changed

+123
-17
lines changed

3 files changed

+123
-17
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
[
2+
CommonFields::OBVIOUSLY,
3+
CommonFields::THESE,
4+
CommonFields::ARE,
5+
CommonFields::FAKE_RESOURCE.with_wombo(true).with_combo(true).with_explosion(true)
6+
].map do |field|
7+
# Swap things out, to make sure.
8+
override || field
9+
end
10+
11+
{
12+
having: "Having a Coke With You",
13+
with: you
14+
}.freeze
15+
16+
{
17+
having: "Having a Coke With You",
18+
with: you
19+
} # by Mark Leidner
20+
.freeze
21+
22+
{
23+
having: "Having a Coke With You",
24+
with: you
25+
}.# by Mark Leidner
26+
freeze
27+
28+
{
29+
having: "Having a Coke With You",
30+
with: you
31+
}
32+
# by Mark Leidner
33+
.freeze
34+
35+
{
36+
something: "wicked"
37+
}.this_way {
38+
comes
39+
# spookyyyy
40+
}
41+
42+
{musical: "wicked"}
43+
# The original broadway cast recording!
44+
.sing("Defying Gravity")
45+
46+
{musical: "wicked"}
47+
.sing("Defying Gravity") # The original broadway cast recording!
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
[
2+
CommonFields::OBVIOUSLY,
3+
CommonFields::THESE,
4+
CommonFields::ARE,
5+
CommonFields::FAKE_RESOURCE.with_wombo(true).with_combo(true).with_explosion(true)
6+
].map do |field|
7+
# Swap things out, to make sure.
8+
override || field
9+
end
10+
11+
{
12+
having: "Having a Coke With You",
13+
with: you
14+
}.freeze
15+
16+
{
17+
having: "Having a Coke With You",
18+
with: you
19+
# by Mark Leidner
20+
}.freeze
21+
22+
{
23+
having: "Having a Coke With You",
24+
with: you
25+
# by Mark Leidner
26+
}.freeze
27+
28+
{
29+
having: "Having a Coke With You",
30+
with: you
31+
}
32+
# by Mark Leidner
33+
.freeze
34+
35+
{
36+
something: "wicked"
37+
}.this_way {
38+
comes
39+
# spookyyyy
40+
}
41+
42+
{musical: "wicked"}
43+
# The original broadway cast recording!
44+
.sing("Defying Gravity")
45+
46+
{musical: "wicked"}
47+
# The original broadway cast recording!
48+
.sing("Defying Gravity")

librubyfmt/src/format_prism.rs

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2351,23 +2351,34 @@ fn call_chain_elements_are_user_multilined<'src>(
23512351
| prism::Node::ParenthesesNode { .. }
23522352
);
23532353

2354-
// _However_, don't ignore this if there are comments in the call chain though; this check may
2355-
// cause it to single-lined, which breaks comment rendering. Specifically, we're checking
2356-
// for comments in between the receiver expression and the following message, e.g.
2357-
// ```ruby
2358-
// [stuff]
2359-
// # spooky comment
2360-
// .freeze
2361-
// ```
2362-
// For cases without the comment, we'd usually put this all on one line, but if we force
2363-
// it all on one line, this will break the comment insertion logic, and given the comment's
2364-
// placement, the user probably intended to break this onto multiple lines anyways.
2365-
let has_comment = ps.has_comment_in_offset_span(
2366-
call_chain_elements[0].location().end_offset(),
2367-
start_loc_for_call_node_in_chain(&call_chain_elements[1].as_call_node().unwrap()),
2368-
);
2369-
if is_literal_expression && !has_comment {
2370-
call_chain_elements = &call_chain_elements[1..];
2354+
if is_literal_expression {
2355+
// _However_, don't ignore this if there are comments in the call chain though; this check may
2356+
// cause it to single-lined, which breaks comment rendering. Specifically, we're checking
2357+
// for comments in between the receiver expression and the following message, e.g.
2358+
// ```ruby
2359+
// [stuff]
2360+
// # spooky comment
2361+
// .freeze
2362+
// ```
2363+
// For cases without the comment, we'd usually put this all on one line, but if we force
2364+
// it all on one line, this will break the comment insertion logic, and given the comment's
2365+
// placement, the user probably intended to break this onto multiple lines anyways.
2366+
let first_call_start_line = ps.get_line_number_for_offset(
2367+
start_loc_for_call_node_in_chain(&call_chain_elements[1].as_call_node().unwrap()),
2368+
);
2369+
let leading_expr_end_line =
2370+
ps.get_line_number_for_offset(call_chain_elements[0].location().end_offset());
2371+
2372+
// Note: We check from `leading_expr_end_line + 1` because comments on the same line as
2373+
// the closing brace will be rendered into the breakable during formatting, so they don't
2374+
// affect whether we should multiline the call chain. We check `first_call_start_line + 1`
2375+
// because the range checked by `has_comments_in_line` is non-inclusive.
2376+
let has_comment_between_expression_and_call =
2377+
ps.has_comments_in_line(leading_expr_end_line + 1, first_call_start_line + 1);
2378+
2379+
if !has_comment_between_expression_and_call {
2380+
call_chain_elements = &call_chain_elements[1..];
2381+
}
23712382
}
23722383
}
23732384

0 commit comments

Comments
 (0)