Prevent ranges from getting collapsed in patterns

src/expr.rs

@@ -23,6 +23,7 @@ use crate::macros::{MacroPosition, rewrite_macro};
 use crate::matches::rewrite_match;
 use crate::overflow::{self, IntoOverflowableItem, OverflowableItem};
 use crate::pairs::{PairParts, rewrite_all_pairs, rewrite_pair};
+use crate::range::rewrite_range;
 use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
 use crate::shape::{Indent, Shape};
 use crate::source_map::{LineRangeUtils, SpanUtils};
@@ -315,78 +316,13 @@ pub(crate) fn format_expr(
             shape,
             SeparatorPlace::Back,
         ),
-        ast::ExprKind::Range(ref lhs, ref rhs, limits) => {
-            let delim = match limits {
-                ast::RangeLimits::HalfOpen => "..",
-                ast::RangeLimits::Closed => "..=",
-            };
-
-            fn needs_space_before_range(context: &RewriteContext<'_>, lhs: &ast::Expr) -> bool {
-                match lhs.kind {
-                    ast::ExprKind::Lit(token_lit) => lit_ends_in_dot(&token_lit, context),
-                    ast::ExprKind::Unary(_, ref expr) => needs_space_before_range(context, expr),
-                    ast::ExprKind::Binary(_, _, ref rhs_expr) => {
-                        needs_space_before_range(context, rhs_expr)
-                    }
-                    _ => false,
-                }
-            }
-
-            fn needs_space_after_range(rhs: &ast::Expr) -> bool {
-                // Don't format `.. ..` into `....`, which is invalid.
-                //
-                // This check is unnecessary for `lhs`, because a range
-                // starting from another range needs parentheses as `(x ..) ..`
-                // (`x .. ..` is a range from `x` to `..`).
-                matches!(rhs.kind, ast::ExprKind::Range(None, _, _))
-            }
-
-            let default_sp_delim = |lhs: Option<&ast::Expr>, rhs: Option<&ast::Expr>| {
-                let space_if = |b: bool| if b { " " } else { "" };
-
-                format!(
-                    "{}{}{}",
-                    lhs.map_or("", |lhs| space_if(needs_space_before_range(context, lhs))),
-                    delim,
-                    rhs.map_or("", |rhs| space_if(needs_space_after_range(rhs))),
-                )
-            };
-
-            match (lhs.as_ref().map(|x| &**x), rhs.as_ref().map(|x| &**x)) {
-                (Some(lhs), Some(rhs)) => {
-                    let sp_delim = if context.config.spaces_around_ranges() {
-                        format!(" {delim} ")
-                    } else {
-                        default_sp_delim(Some(lhs), Some(rhs))
-                    };
-                    rewrite_pair(
-                        &*lhs,
-                        &*rhs,
-                        PairParts::infix(&sp_delim),
-                        context,
-                        shape,
-                        context.config.binop_separator(),
-                    )
-                }
-                (None, Some(rhs)) => {
-                    let sp_delim = if context.config.spaces_around_ranges() {
-                        format!("{delim} ")
-                    } else {
-                        default_sp_delim(None, Some(rhs))
-                    };
-                    rewrite_unary_prefix(context, &sp_delim, &*rhs, shape)
-                }
-                (Some(lhs), None) => {
-                    let sp_delim = if context.config.spaces_around_ranges() {
-                        format!(" {delim}")
-                    } else {
-                        default_sp_delim(Some(lhs), None)
-                    };
-                    rewrite_unary_suffix(context, &sp_delim, &*lhs, shape)
-                }
-                (None, None) => Ok(delim.to_owned()),
-            }
-        }
+        ast::ExprKind::Range(ref lhs, ref rhs, limits) => rewrite_range(
+            context,
+            shape,
+            lhs.as_deref(),
+            rhs.as_deref(),
+            limits.as_str(),
+        ),
         // We do not format these expressions yet, but they should still
         // satisfy our width restrictions.
         // Style Guide RFC for InlineAsm variant pending

src/lib.rs

@@ -84,6 +84,7 @@ mod overflow;
 mod pairs;
 mod parse;
 mod patterns;
+mod range;
 mod release_channel;
 mod reorder;
 mod rewrite;

src/patterns.rs

@@ -1,4 +1,4 @@
-use rustc_ast::ast::{self, BindingMode, ByRef, Pat, PatField, PatKind, RangeEnd, RangeSyntax};
+use rustc_ast::ast::{self, BindingMode, ByRef, Pat, PatField, PatKind};
 use rustc_span::{BytePos, Span};
 
 use crate::comment::{FindUncommented, combine_strs_with_missing_comments};
@@ -11,14 +11,15 @@ use crate::lists::{
 };
 use crate::macros::{MacroPosition, rewrite_macro};
 use crate::overflow;
-use crate::pairs::{PairParts, rewrite_pair};
+use crate::range::rewrite_range;
 use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
 use crate::shape::Shape;
 use crate::source_map::SpanUtils;
 use crate::spanned::Spanned;
 use crate::types::{PathContext, rewrite_path};
 use crate::utils::{
-    format_mutability, format_pinnedness_and_mutability, mk_sp, mk_sp_lo_plus_one, rewrite_ident,
+    format_mutability, format_pinnedness_and_mutability, format_range_end, mk_sp,
+    mk_sp_lo_plus_one, rewrite_ident,
 };
 
 /// Returns `true` if the given pattern is "short".
@@ -77,24 +78,6 @@ fn is_short_pattern_inner(context: &RewriteContext<'_>, pat: &ast::Pat) -> bool
     }
 }
 
-pub(crate) struct RangeOperand<'a, T> {
-    pub operand: &'a Option<Box<T>>,
-    pub span: Span,
-}
-
-impl<'a, T: Rewrite> Rewrite for RangeOperand<'a, T> {
-    fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
-        self.rewrite_result(context, shape).ok()
-    }
-
-    fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
-        match &self.operand {
-            None => Ok("".to_owned()),
-            Some(ref exp) => exp.rewrite_result(context, shape),
-        }
-    }
-}
-
 impl Rewrite for Pat {
     fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option<String> {
         self.rewrite_result(context, shape).ok()
@@ -291,9 +274,13 @@ impl Rewrite for Pat {
                 }
             }
             PatKind::Never => Err(RewriteError::Unknown),
-            PatKind::Range(ref lhs, ref rhs, ref end_kind) => {
-                rewrite_range_pat(context, shape, lhs, rhs, end_kind, self.span)
-            }
+            PatKind::Range(ref lhs, ref rhs, ref end_kind) => rewrite_range(
+                context,
+                shape,
+                lhs.as_deref(),
+                rhs.as_deref(),
+                format_range_end(end_kind.node),
+            ),
             PatKind::Ref(ref pat, pinnedness, mutability) => {
                 let (pin_prefix, mut_prefix) =
                     format_pinnedness_and_mutability(pinnedness, mutability);
@@ -356,50 +343,6 @@ impl Rewrite for Pat {
     }
 }
 
-pub(crate) fn rewrite_range_pat<T: Rewrite>(
-    context: &RewriteContext<'_>,
-    shape: Shape,
-    lhs: &Option<Box<T>>,
-    rhs: &Option<Box<T>>,
-    end_kind: &rustc_span::source_map::Spanned<RangeEnd>,
-    span: Span,
-) -> RewriteResult {
-    let infix = match end_kind.node {
-        RangeEnd::Included(RangeSyntax::DotDotDot) => "...",
-        RangeEnd::Included(RangeSyntax::DotDotEq) => "..=",
-        RangeEnd::Excluded => "..",
-    };
-    let infix = if context.config.spaces_around_ranges() {
-        let lhs_spacing = match lhs {
-            None => "",
-            Some(_) => " ",
-        };
-        let rhs_spacing = match rhs {
-            None => "",
-            Some(_) => " ",
-        };
-        format!("{lhs_spacing}{infix}{rhs_spacing}")
-    } else {
-        infix.to_owned()
-    };
-    let lspan = span.with_hi(end_kind.span.lo());
-    let rspan = span.with_lo(end_kind.span.hi());
-    rewrite_pair(
-        &RangeOperand {
-            operand: lhs,
-            span: lspan,
-        },
-        &RangeOperand {
-            operand: rhs,
-            span: rspan,
-        },
-        PairParts::infix(&infix),
-        context,
-        shape,
-        SeparatorPlace::Front,
-    )
-}
-
 fn rewrite_struct_pat(
     qself: &Option<Box<ast::QSelf>>,
     path: &ast::Path,

src/range.rs

@@ -0,0 +1,78 @@
+use crate::expr::{lit_ends_in_dot, rewrite_unary_prefix, rewrite_unary_suffix};
+use crate::pairs::{PairParts, rewrite_pair};
+use crate::rewrite::{RewriteContext, RewriteResult};
+use crate::shape::Shape;
+
+use rustc_ast::ast;
+
+fn needs_space_before_range(context: &RewriteContext<'_>, lhs: &ast::Expr) -> bool {
+    match lhs.kind {
+        ast::ExprKind::Lit(token_lit) => lit_ends_in_dot(&token_lit, context),
+        ast::ExprKind::Unary(_, ref expr) => needs_space_before_range(context, expr),
+        ast::ExprKind::Binary(_, _, ref rhs_expr) => needs_space_before_range(context, rhs_expr),
+        _ => false,
+    }
+}
+
+fn needs_space_after_range(rhs: &ast::Expr) -> bool {
+    // Don't format `.. ..` into `....`, which is invalid.
+    //
+    // This check is unnecessary for `lhs`, because a range
+    // starting from another range needs parentheses as `(x ..) ..`
+    // (`x .. ..` is a range from `x` to `..`).
+    matches!(rhs.kind, ast::ExprKind::Range(None, _, _))
+}
+
+pub(crate) fn rewrite_range(
+    context: &RewriteContext<'_>,
+    shape: Shape,
+    lhs: Option<&ast::Expr>,
+    rhs: Option<&ast::Expr>,
+    delim: &str,
+) -> RewriteResult {
+    let default_sp_delim = |lhs: Option<&ast::Expr>, rhs: Option<&ast::Expr>| {
+        let space_if = |b: bool| if b { " " } else { "" };
+
+        format!(
+            "{}{}{}",
+            lhs.map_or("", |lhs| space_if(needs_space_before_range(context, lhs))),
+            delim,
+            rhs.map_or("", |rhs| space_if(needs_space_after_range(rhs))),
+        )
+    };
+
+    match (lhs, rhs) {
+        (Some(lhs), Some(rhs)) => {
+            let sp_delim = if context.config.spaces_around_ranges() {
+                format!(" {delim} ")
+            } else {
+                default_sp_delim(Some(lhs), Some(rhs))
+            };
+            rewrite_pair(
+                lhs,
+                rhs,
+                PairParts::infix(&sp_delim),
+                context,
+                shape,
+                context.config.binop_separator(),
+            )
+        }
+        (None, Some(rhs)) => {
+            let sp_delim = if context.config.spaces_around_ranges() {
+                format!("{delim} ")
+            } else {
+                default_sp_delim(None, Some(rhs))
+            };
+            rewrite_unary_prefix(context, &sp_delim, rhs, shape)
+        }
+        (Some(lhs), None) => {
+            let sp_delim = if context.config.spaces_around_ranges() {
+                format!(" {delim}")
+            } else {
+                default_sp_delim(Some(lhs), None)
+            };
+            rewrite_unary_suffix(context, &sp_delim, lhs, shape)
+        }
+        (None, None) => Ok(delim.to_owned()),
+    }
+}

src/spanned.rs

@@ -4,7 +4,6 @@ use rustc_ast::ast;
 use rustc_span::{Span, source_map};
 
 use crate::macros::MacroArg;
-use crate::patterns::RangeOperand;
 use crate::utils::{mk_sp, outer_attributes};
 
 /// Spanned returns a span including attributes, if available.
@@ -205,9 +204,3 @@ impl Spanned for ast::PreciseCapturingArg {
         }
     }
 }
-
-impl<'a, T> Spanned for RangeOperand<'a, T> {
-    fn span(&self) -> Span {
-        self.span
-    }
-}

src/types.rs

@@ -16,14 +16,14 @@ use crate::lists::{
 use crate::macros::{MacroPosition, rewrite_macro};
 use crate::overflow;
 use crate::pairs::{PairParts, rewrite_pair};
-use crate::patterns::rewrite_range_pat;
+use crate::range::rewrite_range;
 use crate::rewrite::{Rewrite, RewriteContext, RewriteError, RewriteErrorExt, RewriteResult};
 use crate::shape::Shape;
 use crate::source_map::SpanUtils;
 use crate::spanned::Spanned;
 use crate::utils::{
     colon_spaces, extra_offset, first_line_width, format_extern, format_mutability,
-    last_line_extendable, last_line_width, mk_sp, rewrite_ident,
+    format_range_end, last_line_extendable, last_line_width, mk_sp, rewrite_ident,
 };
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
@@ -1067,9 +1067,13 @@ impl Rewrite for ast::TyPat {
 
     fn rewrite_result(&self, context: &RewriteContext<'_>, shape: Shape) -> RewriteResult {
         match self.kind {
-            ast::TyPatKind::Range(ref lhs, ref rhs, ref end_kind) => {
-                rewrite_range_pat(context, shape, lhs, rhs, end_kind, self.span)
-            }
+            ast::TyPatKind::Range(ref lhs, ref rhs, ref end_kind) => rewrite_range(
+                context,
+                shape,
+                lhs.as_deref().map(|x| x.value.as_ref()),
+                rhs.as_deref().map(|x| x.value.as_ref()),
+                format_range_end(end_kind.node),
+            ),
             ast::TyPatKind::Or(ref variants) => {
                 let mut first = true;
                 let mut s = String::new();

src/utils.rs

@@ -146,6 +146,15 @@ pub(crate) fn format_pinnedness_and_mutability(
     }
 }
 
+#[inline]
+pub(crate) fn format_range_end(end: ast::RangeEnd) -> &'static str {
+    match end {
+        ast::RangeEnd::Included(ast::RangeSyntax::DotDotDot) => "...",
+        ast::RangeEnd::Included(ast::RangeSyntax::DotDotEq) => "..=",
+        ast::RangeEnd::Excluded => "..",
+    }
+}
+
 #[inline]
 pub(crate) fn format_extern(ext: ast::Extern, explicit_abi: bool) -> Cow<'static, str> {
     match ext {

tests/source/configs/float_literal_trailing_zero/always.rs

@@ -1,4 +1,5 @@
 // rustfmt-float_literal_trailing_zero: Always
+// spaces_around_ranges: false is implied since it's the default
 
 fn float_literals() {
     let a = 0.;
@@ -43,3 +44,6 @@ fn line_wrapping() {
     ];
     println!("This is floaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaat {}", 10e3);
 }
+
+fn pattern_in_function_parameters_exactly_max_width_before_zero___(2. ..10.: std::ops::Range<f32>) {
+}

tests/source/configs/float_literal_trailing_zero/if-no-postfix.rs

@@ -1,4 +1,5 @@
 // rustfmt-float_literal_trailing_zero: IfNoPostfix
+// spaces_around_ranges: false is implied since it's the default
 
 fn float_literals() {
     let a = 0.;
@@ -46,3 +47,6 @@ fn line_wrapping() {
         10.0e3
     );
 }
+
+fn pattern_in_function_parameters_exactly_max_width_before_zero___(2. ..10.: std::ops::Range<f32>) {
+}