Remove spaces_within_parens_and_brackets

cc #1974

Author: Nick Cameron

Configurations.md

@@ -1589,52 +1589,6 @@ fn main() {
 }
 ```
 
-## `spaces_within_parens_and_brackets`
-
-Put spaces within non-empty generic arguments, parentheses, and square brackets
-
-- **Default value**: `false`
-- **Possible values**: `true`, `false`
-- **Stable**: No
-
-#### `false` (default):
-
-```rust
-// generic arguments
-fn lorem<T: Eq>(t: T) {
-    // body
-}
-
-// non-empty parentheses
-fn lorem<T: Eq>(t: T) {
-    let lorem = (ipsum, dolor);
-}
-
-// non-empty square brackets
-fn lorem<T: Eq>(t: T) {
-    let lorem: [usize; 2] = [ipsum, dolor];
-}
-```
-
-#### `true`:
-
-```rust
-// generic arguments
-fn lorem< T: Eq >( t: T ) {
-    // body
-}
-
-// non-empty parentheses
-fn lorem< T: Eq >( t: T ) {
-    let lorem = ( ipsum, dolor );
-}
-
-// non-empty square brackets
-fn lorem< T: Eq >( t: T ) {
-    let lorem: [ usize; 2 ] = [ ipsum, dolor ];
-}
-```
-
 ## `struct_lit_single_line`
 
 Put small struct literals on a single line

src/chains.rs

@@ -498,12 +498,7 @@ fn rewrite_method_call(
             .map(|ty| ty.rewrite(context, shape))
             .collect::<Option<Vec<_>>>()?;
 
-        let type_str =
-            if context.config.spaces_within_parens_and_brackets() && !type_list.is_empty() {
-                format!("::< {} >", type_list.join(", "))
-            } else {
-                format!("::<{}>", type_list.join(", "))
-            };
+        let type_str = format!("::<{}>", type_list.join(", "));
 
         (types.last().unwrap().span.hi(), type_str)
     };

src/config/mod.rs

@@ -77,9 +77,7 @@ create_config! {
         "Determines if '+' or '=' are wrapped in spaces in the punctuation of types";
     space_before_colon: bool, false, false, "Leave a space before the colon";
     space_after_colon: bool, true, false, "Leave a space after the colon";
-    spaces_around_ranges: bool, false, false, "Put spaces around the  .. and ... range operators";
-    spaces_within_parens_and_brackets: bool, false, false,
-        "Put spaces within non-empty parentheses or brackets";
+    spaces_around_ranges: bool, false, false, "Put spaces around the  .. and ..= range operators";
     binop_separator: SeparatorPlace, SeparatorPlace::Front, false,
         "Where to put a binary operator when a binary expression goes multiline.";
 

src/expr.rs

@@ -39,8 +39,8 @@ use string::{rewrite_string, StringFormat};
 use types::{can_be_overflowed_type, rewrite_path, PathContext};
 use utils::{
     colon_spaces, contains_skip, count_newlines, first_line_width, inner_attributes,
-    last_line_extendable, last_line_width, mk_sp, outer_attributes, paren_overhead,
-    ptr_vec_to_ref_vec, semicolon_for_stmt, wrap_str,
+    last_line_extendable, last_line_width, mk_sp, outer_attributes, ptr_vec_to_ref_vec,
+    semicolon_for_stmt, wrap_str,
 };
 use vertical::rewrite_with_alignment;
 use visitor::FmtVisitor;
@@ -223,21 +223,14 @@ pub fn format_expr(
         ast::ExprKind::Index(ref expr, ref index) => {
             rewrite_index(&**expr, &**index, context, shape)
         }
-        ast::ExprKind::Repeat(ref expr, ref repeats) => {
-            let (lbr, rbr) = if context.config.spaces_within_parens_and_brackets() {
-                ("[ ", " ]")
-            } else {
-                ("[", "]")
-            };
-            rewrite_pair(
-                &**expr,
-                &**repeats,
-                PairParts::new(lbr, "; ", rbr),
-                context,
-                shape,
-                SeparatorPlace::Back,
-            )
-        }
+        ast::ExprKind::Repeat(ref expr, ref repeats) => rewrite_pair(
+            &**expr,
+            &**repeats,
+            PairParts::new("[", "; ", "]"),
+            context,
+            shape,
+            SeparatorPlace::Back,
+        ),
         ast::ExprKind::Range(ref lhs, ref rhs, limits) => {
             let delim = match limits {
                 ast::RangeLimits::HalfOpen => "..",
@@ -1581,27 +1574,15 @@ fn rewrite_paren(
         break;
     }
 
-    let total_paren_overhead = paren_overhead(context);
-    let paren_overhead = total_paren_overhead / 2;
-    let sub_shape = shape
-        .offset_left(paren_overhead)
-        .and_then(|s| s.sub_width(paren_overhead))?;
-
-    let paren_wrapper = |s: &str| {
-        if context.config.spaces_within_parens_and_brackets() && !s.is_empty() {
-            format!("( {}{}{} )", pre_comment, s, post_comment)
-        } else {
-            format!("({}{}{})", pre_comment, s, post_comment)
-        }
-    };
+    // 1 `(`
+    let sub_shape = shape.offset_left(1).and_then(|s| s.sub_width(1))?;
 
     let subexpr_str = subexpr.rewrite(context, sub_shape)?;
     debug!("rewrite_paren, subexpr_str: `{:?}`", subexpr_str);
 
-    if subexpr_str.contains('\n')
-        || first_line_width(&subexpr_str) + total_paren_overhead <= shape.width
-    {
-        Some(paren_wrapper(&subexpr_str))
+    // 2 = `()`
+    if subexpr_str.contains('\n') || first_line_width(&subexpr_str) + 2 <= shape.width {
+        Some(format!("({}{}{})", pre_comment, &subexpr_str, post_comment))
     } else {
         None
     }
@@ -1615,54 +1596,44 @@ fn rewrite_index(
 ) -> Option<String> {
     let expr_str = expr.rewrite(context, shape)?;
 
-    let (lbr, rbr) = if context.config.spaces_within_parens_and_brackets() {
-        ("[ ", " ]")
-    } else {
-        ("[", "]")
-    };
-
-    let offset = last_line_width(&expr_str) + lbr.len();
+    let offset = last_line_width(&expr_str) + 1;
     let rhs_overhead = shape.rhs_overhead(context.config);
     let index_shape = if expr_str.contains('\n') {
         Shape::legacy(context.config.max_width(), shape.indent)
             .offset_left(offset)
-            .and_then(|shape| shape.sub_width(rbr.len() + rhs_overhead))
+            .and_then(|shape| shape.sub_width(1 + rhs_overhead))
     } else {
-        shape.visual_indent(offset).sub_width(offset + rbr.len())
+        shape.visual_indent(offset).sub_width(offset + 1)
     };
     let orig_index_rw = index_shape.and_then(|s| index.rewrite(context, s));
 
     // Return if index fits in a single line.
     match orig_index_rw {
         Some(ref index_str) if !index_str.contains('\n') => {
-            return Some(format!("{}{}{}{}", expr_str, lbr, index_str, rbr));
+            return Some(format!("{}[{}]", expr_str, index_str));
         }
         _ => (),
     }
 
     // Try putting index on the next line and see if it fits in a single line.
     let indent = shape.indent.block_indent(context.config);
-    let index_shape = Shape::indented(indent, context.config).offset_left(lbr.len())?;
-    let index_shape = index_shape.sub_width(rbr.len() + rhs_overhead)?;
+    let index_shape = Shape::indented(indent, context.config).offset_left(1)?;
+    let index_shape = index_shape.sub_width(1 + rhs_overhead)?;
     let new_index_rw = index.rewrite(context, index_shape);
     match (orig_index_rw, new_index_rw) {
         (_, Some(ref new_index_str)) if !new_index_str.contains('\n') => Some(format!(
-            "{}{}{}{}{}",
+            "{}{}[{}]",
             expr_str,
             indent.to_string_with_newline(context.config),
-            lbr,
             new_index_str,
-            rbr
         )),
         (None, Some(ref new_index_str)) => Some(format!(
-            "{}{}{}{}{}",
+            "{}{}[{}]",
             expr_str,
             indent.to_string_with_newline(context.config),
-            lbr,
             new_index_str,
-            rbr
         )),
-        (Some(ref index_str), _) => Some(format!("{}{}{}{}", expr_str, lbr, index_str, rbr)),
+        (Some(ref index_str), _) => Some(format!("{}[{}]", expr_str, index_str)),
         _ => None,
     }
 }
@@ -1877,13 +1848,7 @@ where
             .next()
             .unwrap()
             .rewrite(context, nested_shape)
-            .map(|s| {
-                if context.config.spaces_within_parens_and_brackets() {
-                    format!("( {}, )", s)
-                } else {
-                    format!("({},)", s)
-                }
-            });
+            .map(|s| format!("({},)", s));
     }
 
     let list_lo = context.snippet_provider.span_after(span, "(");
@@ -1919,11 +1884,7 @@ where
     };
     let list_str = write_list(&item_vec, &fmt)?;
 
-    if context.config.spaces_within_parens_and_brackets() && !list_str.is_empty() {
-        Some(format!("( {} )", list_str))
-    } else {
-        Some(format!("({})", list_str))
-    }
+    Some(format!("({})", list_str))
 }
 
 pub fn rewrite_tuple<'a, T>(

src/items.rs

@@ -1915,12 +1915,6 @@ fn rewrite_fn_base(
     } else {
         result.push('(');
     }
-    if context.config.spaces_within_parens_and_brackets()
-        && !fd.inputs.is_empty()
-        && result.ends_with('(')
-    {
-        result.push(' ')
-    }
 
     // Skip `pub(crate)`.
     let lo_after_visibility = get_bytepos_after_visibility(&fn_sig.visibility, span);
@@ -1978,9 +1972,6 @@ fn rewrite_fn_base(
         if fd.inputs.is_empty() && used_width + 1 > context.config.max_width() {
             result.push('\n');
         }
-        if context.config.spaces_within_parens_and_brackets() && !fd.inputs.is_empty() {
-            result.push(' ')
-        }
         // If the last line of args contains comment, we cannot put the closing paren
         // on the same line.
         if arg_str

src/macros.rs

@@ -259,32 +259,25 @@ pub fn rewrite_macro_inner(
             // Handle special case: `vec![expr; expr]`
             if vec_with_semi {
                 let mac_shape = shape.offset_left(macro_name.len())?;
-                let (lbr, rbr) = if context.config.spaces_within_parens_and_brackets() {
-                    ("[ ", " ]")
-                } else {
-                    ("[", "]")
-                };
-                // 6 = `vec!` + `; `
-                let total_overhead = lbr.len() + rbr.len() + 6;
+                // 8 = `vec![]` + `; `
+                let total_overhead = 8;
                 let nested_shape = mac_shape.block_indent(context.config.tab_spaces());
                 let lhs = arg_vec[0].rewrite(context, nested_shape)?;
                 let rhs = arg_vec[1].rewrite(context, nested_shape)?;
                 if !lhs.contains('\n')
                     && !rhs.contains('\n')
                     && lhs.len() + rhs.len() + total_overhead <= shape.width
                 {
-                    Some(format!("{}{}{}; {}{}", macro_name, lbr, lhs, rhs, rbr))
+                    Some(format!("{}[{}; {}]", macro_name, lhs, rhs))
                 } else {
                     Some(format!(
-                        "{}{}{}{};{}{}{}{}",
+                        "{}[{}{};{}{}{}]",
                         macro_name,
-                        lbr,
                         nested_shape.indent.to_string_with_newline(context.config),
                         lhs,
                         nested_shape.indent.to_string_with_newline(context.config),
                         rhs,
                         shape.indent.to_string_with_newline(context.config),
-                        rbr
                     ))
                 }
             } else {

src/overflow.rs

@@ -23,9 +23,7 @@ use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListIte
 use rewrite::{Rewrite, RewriteContext};
 use shape::Shape;
 use spanned::Spanned;
-use utils::{
-    count_newlines, extra_offset, first_line_width, last_line_width, mk_sp, paren_overhead,
-};
+use utils::{count_newlines, extra_offset, first_line_width, last_line_width, mk_sp};
 
 use std::cmp::min;
 
@@ -140,26 +138,16 @@ impl<'a, T: 'a + Rewrite + ToExpr + Spanned> Context<'a, T> {
         force_separator_tactic: Option<SeparatorTactic>,
         custom_delims: Option<(&'a str, &'a str)>,
     ) -> Context<'a, T> {
-        // 2 = `( `, 1 = `(`
-        let paren_overhead = if context.config.spaces_within_parens_and_brackets() {
-            2
-        } else {
-            1
-        };
         let used_width = extra_offset(ident, shape);
-        let one_line_width = shape.width.saturating_sub(used_width + 2 * paren_overhead);
+        // 1 = `()`
+        let one_line_width = shape.width.saturating_sub(used_width + 2);
 
         // 1 = "(" or ")"
         let one_line_shape = shape
             .offset_left(last_line_width(ident) + 1)
             .and_then(|shape| shape.sub_width(1))
             .unwrap_or(Shape { width: 0, ..shape });
-        let nested_shape = shape_from_indent_style(
-            context,
-            shape,
-            used_width + 2 * paren_overhead,
-            used_width + paren_overhead,
-        );
+        let nested_shape = shape_from_indent_style(context, shape, used_width + 2, used_width + 1);
         Context {
             context,
             items,
@@ -417,12 +405,13 @@ impl<'a, T: 'a + Rewrite + ToExpr + Spanned> Context<'a, T> {
             Some((lhs, rhs)) => (lhs, rhs),
             _ => (self.prefix, self.suffix),
         };
-        let paren_overhead = paren_overhead(self.context);
-        let fits_one_line = items_str.len() + paren_overhead <= shape.width;
+
+        // 2 = `()`
+        let fits_one_line = items_str.len() + 2 <= shape.width;
         let extend_width = if items_str.is_empty() {
-            paren_overhead
+            2
         } else {
-            first_line_width(items_str) + (paren_overhead / 2)
+            first_line_width(items_str) + 1
         };
         let nested_indent_str = self
             .nested_shape
@@ -441,13 +430,7 @@ impl<'a, T: 'a + Rewrite + ToExpr + Spanned> Context<'a, T> {
             || (self.context.inside_macro() && !items_str.contains('\n') && fits_one_line)
             || (is_extendable && extend_width <= shape.width)
         {
-            if self.context.config.spaces_within_parens_and_brackets() && !items_str.is_empty() {
-                result.push(' ');
-                result.push_str(items_str);
-                result.push(' ');
-            } else {
-                result.push_str(items_str);
-            }
+            result.push_str(items_str);
         } else {
             if !items_str.is_empty() {
                 result.push_str(&nested_indent_str);

src/patterns.rs

@@ -156,12 +156,7 @@ impl Rewrite for Pat {
                 let pats = pats?;
 
                 // Unwrap all the sub-strings and join them with commas.
-                let result = if context.config.spaces_within_parens_and_brackets() {
-                    format!("[ {} ]", pats.join(", "))
-                } else {
-                    format!("[{}]", pats.join(", "))
-                };
-                Some(result)
+                Some(format!("[{}]", pats.join(", ")))
             }
             PatKind::Struct(ref path, ref fields, ellipsis) => {
                 rewrite_struct_pat(path, fields, ellipsis, self.span, context, shape)