Skip to content

Commit 9767afe

Browse files
calebcartwrightcalebcartwright
committed
fix: chain wrapping with long child item
1 parent 159756d commit 9767afe

File tree

5 files changed

+264
-53
lines changed

5 files changed

+264
-53
lines changed

‎src/chains.rs

Lines changed: 75 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -414,8 +414,12 @@ impl Chain {
414414
_ => expr.clone(),
415415
}
416416
}
417+
}
418+
419+
impl Rewrite for Chain {
420+
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
421+
debug!("rewrite chain {:?} {:?}", self, shape);
417422

418-
fn format(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
419423
let mut formatter = match context.config.indent_style() {
420424
IndentStyle::Block => {
421425
Box::new(ChainFormatterBlock::new(self)) as Box<dyn ChainFormatter>
@@ -436,33 +440,7 @@ impl Chain {
436440
formatter.format_children(context, child_shape)?;
437441
formatter.format_last_child(context, shape, child_shape)?;
438442

439-
formatter.join_rewrites(context, child_shape)
440-
}
441-
}
442-
443-
impl Rewrite for Chain {
444-
fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
445-
debug!("rewrite chain {:?} {:?}", self, shape);
446-
447-
let result = self.format(context, shape)?;
448-
match context.config.indent_style() {
449-
IndentStyle::Block => Some(result),
450-
IndentStyle::Visual => match wrap_str(result, context.config.max_width(), shape) {
451-
Some(r) => Some(r),
452-
None => {
453-
let new_shape =
454-
Shape::indented(shape.indent.block_indent(context.config), context.config);
455-
if let Some(result) = self.format(context, new_shape) {
456-
let prefix = shape
457-
.indent
458-
.block_indent(context.config)
459-
.to_string_with_newline(context.config);
460-
return Some(format!("{}{}", prefix, result));
461-
}
462-
None
463-
}
464-
},
465-
}
443+
formatter.join_rewrites(context, child_shape, shape)
466444
}
467445
}
468446

@@ -493,7 +471,12 @@ trait ChainFormatter {
493471
shape: Shape,
494472
child_shape: Shape,
495473
) -> Option<()>;
496-
fn join_rewrites(&self, context: &RewriteContext<'_>, child_shape: Shape) -> Option<String>;
474+
fn join_rewrites(
475+
&self,
476+
context: &RewriteContext<'_>,
477+
child_shape: Shape,
478+
shape: Shape,
479+
) -> Option<String>;
497480
// Returns `Some` if the chain is only a root, None otherwise.
498481
fn pure_root(&mut self) -> Option<String>;
499482
}
@@ -511,6 +494,12 @@ struct ChainFormatterShared<'a> {
511494
// The number of children in the chain. This is not equal to `self.children.len()`
512495
// because `self.children` will change size as we process the chain.
513496
child_count: usize,
497+
// Whether the parent element of the chain exceeds the max width
498+
has_long_parent: bool,
499+
// Whether one or more of the inner chain items exceed the max width
500+
has_long_inner_item: bool,
501+
// Whether the final chain item exceeds the max width
502+
has_long_tail: bool,
514503
}
515504

516505
impl<'a> ChainFormatterShared<'a> {
@@ -520,6 +509,9 @@ impl<'a> ChainFormatterShared<'a> {
520509
rewrites: Vec::with_capacity(chain.children.len() + 1),
521510
fits_single_line: false,
522511
child_count: chain.children.len(),
512+
has_long_parent: false,
513+
has_long_inner_item: false,
514+
has_long_tail: false,
523515
}
524516
}
525517

@@ -658,8 +650,10 @@ impl<'a> ChainFormatterShared<'a> {
658650
child_shape.sub_width(shape.rhs_overhead(context.config) + last.tries)?
659651
};
660652

661-
last_subexpr_str = last_subexpr_str.or_else(|| last.rewrite(context, last_shape));
662-
self.rewrites.push(last_subexpr_str?);
653+
let (format_last, is_long) = format_chain_item(last, context, last_shape)?;
654+
self.has_long_tail = is_long;
655+
let last_rewrite = last_subexpr_str.unwrap_or(format_last);
656+
self.rewrites.push(last_rewrite);
663657
Some(())
664658
}
665659

@@ -691,6 +685,10 @@ impl<'a> ChainFormatterShared<'a> {
691685

692686
Some(result)
693687
}
688+
689+
fn has_long_item(&self) -> bool {
690+
self.has_long_parent || self.has_long_inner_item || self.has_long_tail
691+
}
694692
}
695693

696694
// Formats a chain using block indent.
@@ -708,7 +706,11 @@ impl<'a> ChainFormatterBlock<'a> {
708706
}
709707
}
710708

711-
fn format_chain_item(item: &ChainItem, context: &RewriteContext<'_>, shape: Shape) -> String {
709+
fn format_chain_item(
710+
item: &ChainItem,
711+
context: &RewriteContext<'_>,
712+
shape: Shape,
713+
) -> Option<(String, bool)> {
712714
fn contains_multiline_args(
713715
exprs: &Vec<ptr::P<ast::Expr>>,
714716
context: &RewriteContext<'_>,
@@ -768,7 +770,7 @@ fn format_chain_item(item: &ChainItem, context: &RewriteContext<'_>, shape: Shap
768770
.sub_width(shape.rhs_overhead(context.config))
769771
{
770772
Some(shape) => shape,
771-
None => return context.snippet(item.span).to_owned(),
773+
None => return Some((context.snippet(item.span).to_owned(), false)),
772774
};
773775
let next_line_result = item.rewrite(context, new_shape);
774776
let parent_indent_style = context.config.chains_block_parent_indent_parent_item();
@@ -777,24 +779,26 @@ fn format_chain_item(item: &ChainItem, context: &RewriteContext<'_>, shape: Shap
777779
if !orig_result.contains('\n')
778780
&& utils::unicode_str_width(&orig_result) <= shape.width =>
779781
{
780-
orig_result
782+
Some((orig_result, false))
781783
}
782784
(Some(orig_result), Some(next_line_result)) => match item.kind {
783785
ChainItemKind::Parent(ref expr) => match parent_indent_style {
784-
ChainsBlockParentElementIndent::Never => orig_result,
785-
ChainsBlockParentElementIndent::Always => next_line_result,
786+
ChainsBlockParentElementIndent::Never => Some((orig_result, false)),
787+
ChainsBlockParentElementIndent::Always => Some((next_line_result, false)),
786788
ChainsBlockParentElementIndent::OnlySimpleCalls => {
787789
match is_simple_call_like(&expr.kind, &context, shape) {
788-
true => next_line_result,
789-
false => orig_result,
790+
true => Some((next_line_result, false)),
791+
false => Some((orig_result, false)),
790792
}
791793
}
792794
},
793-
_ => orig_result,
795+
_ => Some((orig_result, false)),
794796
},
795-
(None, None) => context.snippet(item.span).to_owned(),
796-
(Some(orig_result), _) => orig_result,
797-
(None, Some(next_line_result)) => next_line_result,
797+
(None, None) => Some((context.snippet(item.span).to_owned(), true)),
798+
(Some(orig_result), _) => Some((orig_result, false)),
799+
// This will only occur when the chain is part of the rhs and
800+
// will fit with an indent on the next line
801+
(None, Some(_)) => None,
798802
}
799803
}
800804

@@ -805,7 +809,8 @@ impl<'a> ChainFormatter for ChainFormatterBlock<'a> {
805809
context: &RewriteContext<'_>,
806810
shape: Shape,
807811
) -> Option<()> {
808-
let mut root_rewrite = format_chain_item(parent, context, shape);
812+
let (mut root_rewrite, long_parent) = format_chain_item(parent, context, shape)?;
813+
self.shared.has_long_inner_item = long_parent;
809814

810815
let mut root_ends_with_block = parent.kind.is_block_like(context, &root_rewrite);
811816
let tab_width = context.config.tab_spaces().saturating_sub(shape.offset);
@@ -849,8 +854,10 @@ impl<'a> ChainFormatter for ChainFormatterBlock<'a> {
849854

850855
fn format_children(&mut self, context: &RewriteContext<'_>, child_shape: Shape) -> Option<()> {
851856
for item in &self.shared.children[..self.shared.children.len() - 1] {
852-
// let (rewrite, _) = format_chain_item(&item, &context, child_shape);
853-
let rewrite = format_chain_item(&item, &context, child_shape);
857+
let (rewrite, is_long) = format_chain_item(&item, &context, child_shape)?;
858+
if is_long {
859+
self.shared.has_long_inner_item = true;
860+
}
854861
self.shared.rewrites.push(rewrite);
855862
}
856863
Some(())
@@ -866,7 +873,12 @@ impl<'a> ChainFormatter for ChainFormatterBlock<'a> {
866873
.format_last_child(true, context, shape, child_shape)
867874
}
868875

869-
fn join_rewrites(&self, context: &RewriteContext<'_>, child_shape: Shape) -> Option<String> {
876+
fn join_rewrites(
877+
&self,
878+
context: &RewriteContext<'_>,
879+
child_shape: Shape,
880+
_shape: Shape,
881+
) -> Option<String> {
870882
self.shared.join_rewrites(context, child_shape)
871883
}
872884

@@ -899,7 +911,8 @@ impl<'a> ChainFormatter for ChainFormatterVisual<'a> {
899911
shape: Shape,
900912
) -> Option<()> {
901913
let parent_shape = shape.visual_indent(0);
902-
let mut root_rewrite = parent.rewrite(context, parent_shape)?;
914+
let (mut root_rewrite, is_long) = format_chain_item(parent, context, parent_shape)?;
915+
self.shared.has_long_parent = is_long;
903916
let multiline = root_rewrite.contains('\n');
904917
self.offset = if multiline {
905918
last_line_width(&root_rewrite).saturating_sub(shape.used_width())
@@ -944,8 +957,10 @@ impl<'a> ChainFormatter for ChainFormatterVisual<'a> {
944957

945958
fn format_children(&mut self, context: &RewriteContext<'_>, child_shape: Shape) -> Option<()> {
946959
for item in &self.shared.children[..self.shared.children.len() - 1] {
947-
// let (rewrite, _) = format_chain_item(&item, &context, child_shape);
948-
let rewrite = format_chain_item(&item, &context, child_shape);
960+
let (rewrite, is_long) = format_chain_item(&item, &context, child_shape)?;
961+
if is_long {
962+
self.shared.has_long_inner_item = true;
963+
}
949964
self.shared.rewrites.push(rewrite);
950965
}
951966
Some(())
@@ -961,8 +976,17 @@ impl<'a> ChainFormatter for ChainFormatterVisual<'a> {
961976
.format_last_child(false, context, shape, child_shape)
962977
}
963978

964-
fn join_rewrites(&self, context: &RewriteContext<'_>, child_shape: Shape) -> Option<String> {
965-
self.shared.join_rewrites(context, child_shape)
979+
fn join_rewrites(
980+
&self,
981+
context: &RewriteContext<'_>,
982+
child_shape: Shape,
983+
shape: Shape,
984+
) -> Option<String> {
985+
match self.shared.join_rewrites(context, child_shape) {
986+
Some(rewrite) if self.shared.has_long_item() => Some(rewrite),
987+
Some(rewrite) => wrap_str(rewrite, context.config.max_width(), shape),
988+
None => None,
989+
}
966990
}
967991

968992
fn pure_root(&mut self) -> Option<String> {

‎tests/source/chains_long_items_block.rs

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,45 @@
1+
// rustfmt-indent_style: Block
2+
13
// https://github.com/rust-lang/rustfmt/issues/3863
2-
fn f() {
4+
fn issue_3863() {
35
foo("This text is under the max_width limit, and shouldn't cause any problems on its own.").long("But this line is extra long, and doesn't fit within 100 max_width. 1234567890123456789 aBcDeFgHiJ").baz().collect().unwrap();
46
}
7+
8+
fn long_parent() {
9+
let foo = looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonnnnnnnnnnnnnnnnnnnnnnnnggggggggggggggggggggggggggggggggggggggggggg().foo().bar().baz();
10+
11+
asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff().foo().bar().baz();
12+
13+
// With args
14+
let bar = looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonnnnnnnnnnnnnnnnnnnnnnnnggggggggggggggggggggggggggggggggggggggggggg("ffffffffffffffffffffffffffffffffffff").foo().bar().baz();
15+
16+
asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff("ffffffffffffffffffffffffffffffffffff").foo().bar().baz();
17+
}
18+
19+
fn long_inner() {
20+
// Args that do not fit
21+
let bar = bar().baz("ffffffffffffffffffffffffffffffffffffasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfadfasdfasdfasdfadfasdfasdf").foo().bar().baz();
22+
23+
qux().baz("ffffffffffffffffffffffffffffffffffffasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfadfasdfasdfasdfadfasdfasdf").foo().bar().baz();
24+
25+
// Long element no args
26+
let foo = bar().asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff().foo().bar().baz();
27+
28+
qux().asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff().foo().bar().baz();
29+
30+
// Long element with args that fit
31+
let bar = bar().asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff("ffffffffffffffffffffffffffffffffffff").foo().bar().baz();
32+
33+
qux().asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff("ffffffffffffffffffffffffffffffffffff").foo().bar().baz();
34+
}
35+
36+
fn long_tail() {
37+
bar().xxxxxxx.map(|x| x + 5).map(|x| x / 2).fold(0, |acc, x| acc + x).doooooooooooooooooooooooooooooooooooooooooooooooooooooo_stufffffffffffffffffffffffffffffffffffffff();
38+
39+
let foo = bar().xxxxxxx.map(|x| x + 5).map(|x| x / 2).fold(0, |acc, x| acc + x).doooooooooooooooooooooooooooooooooooooooooooooooooooooo_stufffffffffffffffffffffffffffffffffffffff();
40+
41+
// With args
42+
bar().xxxxxxx.map(|x| x + 5).map(|x| x / 2).fold(0, |acc, x| acc + x).doooooooooooooooooooooooooooooooooooooooooooooooooooooo_stufffffffffffffffffffffffffffffffffffffff("abcdefghadfasdfasdfasdfasdfadf");
43+
44+
let foo = bar().xxxxxxx.map(|x| x + 5).map(|x| x / 2).fold(0, |acc, x| acc + x).doooooooooooooooooooooooooooooooooooooooooooooooooooooo_stufffffffffffffffffffffffffffffffffffffff("abcdefghadfasdfasdfasdfasdfadf");
45+
}

‎tests/source/chains_long_items_visual.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// rustfmt-indent_style: Visual
2+
3+
fn long_inner() {
4+
let foo = bar().asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff().foo().bar().baz();
5+
qux().asdffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff().foo().bar().baz();
6+
}
7+
8+
fn long_tail() {
9+
bar().xxxxxxx.map(|x| x + 5).map(|x| x / 2).fold(0, |acc, x| acc + x).doooooooooooooooooooooooooooooooooooooooooooooooooooooo_stufffffffffffffffffffffffffffffffffffffff("abcdefghadfasdfasdfasdfasdfadf");
10+
}
11+
12+
fn assignment_long_tail() {
13+
let foo = bar().xxxxxxx.map(|x| x + 5).map(|x| x / 2).fold(0, |acc, x| acc + x).doooooooooooooooooooooooooooooooooooooooooooooooooooooo_stufffffffffffffffffffffffffffffffffffffff("abcdefghadfasdfasdfasdfasdfadf");
14+
15+
}

0 commit comments

Comments
 (0)