From cc5521d9b2fff696a852454dd2aa89e7942464dd Mon Sep 17 00:00:00 2001 From: David Bar-On <61089727+davidBar-On@users.noreply.github.com> Date: Sat, 23 Oct 2021 21:58:25 +0300 Subject: [PATCH] Fix for issue 4689 - indentation on multiline single generic bound (#4730) --- src/formatting/types.rs | 49 +++++++++++- tests/source/issue-4689.rs | 147 ++++++++++++++++++++++++++++++++++++ tests/target/issue-4689.rs | 150 +++++++++++++++++++++++++++++++++++++ 3 files changed, 342 insertions(+), 4 deletions(-) create mode 100644 tests/source/issue-4689.rs create mode 100644 tests/target/issue-4689.rs diff --git a/src/formatting/types.rs b/src/formatting/types.rs index 27768d50040..efdb5aff410 100644 --- a/src/formatting/types.rs +++ b/src/formatting/types.rs @@ -952,6 +952,24 @@ fn join_bounds_inner( ast::GenericBound::Trait(..) => last_line_extendable(s), }; + // Whether a PathSegment segment includes internal array containing more than one item + let is_segment_with_multi_items_array = |seg: &ast::PathSegment| { + if let Some(args_in) = &seg.args { + match &**args_in { + ast::AngleBracketed(args) => { + if args.args.len() > 1 { + true + } else { + false + } + } + _ => false, + } + } else { + false + } + }; + let result = items.iter().enumerate().try_fold( (String::new(), None, false), |(strs, prev_trailing_span, prev_extendable), (i, item)| { @@ -1046,10 +1064,33 @@ fn join_bounds_inner( }, )?; - if !force_newline - && items.len() > 1 - && (result.0.contains('\n') || result.0.len() > shape.width) - { + // Whether retry the function with forced newline is needed: + // Only if result is not already multiline and did not exceed line width, + // and either there is more than one item; + // or the single item is of type `Trait`, + // and any of the internal arrays contains more than one item; + let retry_with_force_newline = + if force_newline || (!result.0.contains('\n') && result.0.len() <= shape.width) { + false + } else { + if items.len() > 1 { + true + } else { + match items[0] { + ast::GenericBound::Trait(ref poly_trait_ref, ..) => { + let segments = &poly_trait_ref.trait_ref.path.segments; + if segments.len() > 1 { + true + } else { + is_segment_with_multi_items_array(&segments[0]) + } + } + _ => false, + } + } + }; + + if retry_with_force_newline { join_bounds_inner(context, shape, items, need_indent, true) } else { Some(result.0) diff --git a/tests/source/issue-4689.rs b/tests/source/issue-4689.rs new file mode 100644 index 00000000000..83bbfde2b11 --- /dev/null +++ b/tests/source/issue-4689.rs @@ -0,0 +1,147 @@ +// Based on the issue description +pub trait PrettyPrinter<'tcx>: +Printer< +'tcx, +Error = fmt::Error, +Path = Self, +Region = Self, +Type = Self, +DynExistential = Self, +Const = Self, +> +{ +// +} +pub trait PrettyPrinter<'tcx>: +Printer< +'tcx, +Error = fmt::Error, +Path = Self, +Region = Self, +Type = Self, +DynExistential = Self, +Const = Self, +> + fmt::Write +{ +// +} +pub trait PrettyPrinter<'tcx>: +Printer< +'tcx, +Error = fmt::Error, +Path = Self, +Region = Self, +Type = Self, +DynExistential = Self, +Const = Self, +> + fmt::Write1 + fmt::Write2 +{ +// +} +pub trait PrettyPrinter<'tcx>: +fmt::Write + Printer< +'tcx, +Error = fmt::Error, +Path = Self, +Region = Self, +Type = Self, +DynExistential = Self, +Const = Self, +> +{ +// +} +pub trait PrettyPrinter<'tcx>: +fmt::Write + Printer1< +'tcx, +Error = fmt::Error, +Path = Self, +Region = Self, +Type = Self, +DynExistential = Self, +Const = Self, +> + Printer2< +'tcx, +Error = fmt::Error, +Path = Self, +Region = Self, +Type = Self, +DynExistential = Self, +Const = Self, +> +{ +// +} + +// Some test cases to ensure other cases formatting were not changed +fn f() -> Box< +FnMut() -> Thing< +WithType = LongItemName, +Error = LONGLONGLONGLONGLONGONGEvenLongerErrorNameLongerLonger, +>, +> { +} +fn f() -> Box< +FnMut() -> Thing< +WithType = LongItemName, +Error = LONGLONGLONGLONGLONGONGEvenLongerErrorNameLongerLonger, +> + fmt::Write1 ++ fmt::Write2, +> { +} + +fn foo(foo2: F) +where +F: Fn( +// this comment is deleted +) +{ +} +fn foo(foo2: F) +where +F: Fn( +// this comment is deleted +) + fmt::Write +{ +} + +fn elaborate_bounds(mut mk_cand: F) +where +F: for<> FnMut( +&mut ProbeContext<>, +ty::PolyTraitRefffffffffffffffffffffffffffffffff<>, +tyyyyyyyyyyyyyyyyyyyyy::AssociatedItem, +), +{ +} +fn elaborate_bounds(mut mk_cand: F) +where +F: for<> FnMut( +&mut ProbeContext<>, +ty::PolyTraitRefffffffffffffffffffffffffffffffff<>, +tyyyyyyyyyyyyyyyyyyyyy::AssociatedItem, +) + fmt::Write, +{ +} + +fn build_sorted_static_get_entry_names( +mut entries: entryyyyyyyy, +) -> ( +impl Fn( +AlphabeticalTraversal, +Seconddddddddddddddddddddddddddddddddddd +) -> Parammmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm ++ Sendddddddddddddddddddddddddddddddddddddddddddd +) { +} + +pub trait SomeTrait: +Cloneeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee ++ Eqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq +{ +} + +trait B = where +for<'b> &'b Self: Send ++ Cloneeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee ++ Copyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy; diff --git a/tests/target/issue-4689.rs b/tests/target/issue-4689.rs new file mode 100644 index 00000000000..9ad9474693c --- /dev/null +++ b/tests/target/issue-4689.rs @@ -0,0 +1,150 @@ +// Based on the issue description +pub trait PrettyPrinter<'tcx>: + Printer< + 'tcx, + Error = fmt::Error, + Path = Self, + Region = Self, + Type = Self, + DynExistential = Self, + Const = Self, + > +{ + // +} +pub trait PrettyPrinter<'tcx>: + Printer< + 'tcx, + Error = fmt::Error, + Path = Self, + Region = Self, + Type = Self, + DynExistential = Self, + Const = Self, + > + fmt::Write +{ + // +} +pub trait PrettyPrinter<'tcx>: + Printer< + 'tcx, + Error = fmt::Error, + Path = Self, + Region = Self, + Type = Self, + DynExistential = Self, + Const = Self, + > + fmt::Write1 + + fmt::Write2 +{ + // +} +pub trait PrettyPrinter<'tcx>: + fmt::Write + + Printer< + 'tcx, + Error = fmt::Error, + Path = Self, + Region = Self, + Type = Self, + DynExistential = Self, + Const = Self, + > +{ + // +} +pub trait PrettyPrinter<'tcx>: + fmt::Write + + Printer1< + 'tcx, + Error = fmt::Error, + Path = Self, + Region = Self, + Type = Self, + DynExistential = Self, + Const = Self, + > + Printer2< + 'tcx, + Error = fmt::Error, + Path = Self, + Region = Self, + Type = Self, + DynExistential = Self, + Const = Self, + > +{ + // +} + +// Some test cases to ensure other cases formatting were not changed +fn f() -> Box< + FnMut() -> Thing< + WithType = LongItemName, + Error = LONGLONGLONGLONGLONGONGEvenLongerErrorNameLongerLonger, + >, +> { +} +fn f() -> Box< + FnMut() -> Thing< + WithType = LongItemName, + Error = LONGLONGLONGLONGLONGONGEvenLongerErrorNameLongerLonger, + > + fmt::Write1 + + fmt::Write2, +> { +} + +fn foo(foo2: F) +where + F: Fn( + // this comment is deleted + ), +{ +} +fn foo(foo2: F) +where + F: Fn( + // this comment is deleted + ) + fmt::Write, +{ +} + +fn elaborate_bounds(mut mk_cand: F) +where + F: FnMut( + &mut ProbeContext, + ty::PolyTraitRefffffffffffffffffffffffffffffffff, + tyyyyyyyyyyyyyyyyyyyyy::AssociatedItem, + ), +{ +} +fn elaborate_bounds(mut mk_cand: F) +where + F: FnMut( + &mut ProbeContext, + ty::PolyTraitRefffffffffffffffffffffffffffffffff, + tyyyyyyyyyyyyyyyyyyyyy::AssociatedItem, + ) + fmt::Write, +{ +} + +fn build_sorted_static_get_entry_names( + mut entries: entryyyyyyyy, +) -> ( + impl Fn( + AlphabeticalTraversal, + Seconddddddddddddddddddddddddddddddddddd, + ) -> Parammmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm + + Sendddddddddddddddddddddddddddddddddddddddddddd +) { +} + +pub trait SomeTrait: + Cloneeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + + Eqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq +{ +} + +trait B = where + for<'b> &'b Self: Send + + Cloneeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + + Copyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy;