Working with return values instead of state

Author: Alexander Glusker

src/expr.rs

@@ -225,13 +225,15 @@ pub(crate) fn format_expr(
         | ast::ExprKind::MethodCall(..)
         | ast::ExprKind::Await(_, _) => rewrite_chain(expr, context, shape),
         ast::ExprKind::MacCall(ref mac) => {
-            rewrite_macro(mac, None, context, shape, MacroPosition::Expression).or_else(|| {
-                wrap_str(
-                    context.snippet(expr.span).to_owned(),
-                    context.config.max_width(),
-                    shape,
-                )
-            })
+            rewrite_macro(mac, None, context, shape, MacroPosition::Expression)
+                .0
+                .or_else(|| {
+                    wrap_str(
+                        context.snippet(expr.span).to_owned(),
+                        context.config.max_width(),
+                        shape,
+                    )
+                })
         }
         ast::ExprKind::Ret(None) => Some("return".to_owned()),
         ast::ExprKind::Ret(Some(ref expr)) => {

src/items.rs

@@ -3374,7 +3374,7 @@ impl Rewrite for ast::ForeignItem {
                 rewrite_type_alias(ty_alias, context, shape.indent, kind, span)
             }
             ast::ForeignItemKind::MacCall(ref mac) => {
-                rewrite_macro(mac, None, context, shape, MacroPosition::Item)
+                rewrite_macro(mac, None, context, shape, MacroPosition::Item).0
             }
         }?;
 

src/macros.rs

@@ -156,13 +156,13 @@ pub(crate) fn rewrite_macro(
     context: &RewriteContext<'_>,
     shape: Shape,
     position: MacroPosition,
-) -> Option<String> {
+) -> (Option<String>, Option<Delimiter>) {
     let should_skip = context
         .skip_context
         .macros
         .skip(context.snippet(mac.path.span));
     if should_skip {
-        None
+        (None, None)
     } else {
         let guard = context.enter_macro();
         let result = catch_unwind(AssertUnwindSafe(|| {
@@ -176,27 +176,29 @@ pub(crate) fn rewrite_macro(
             )
         }));
         match result {
-            Err(..) | Ok(None) => {
+            Err(..) | Ok((None, ..)) => {
                 context.macro_rewrite_failure.replace(true);
-                None
+                (None, None)
             }
             Ok(rw) => rw,
         }
     }
 }
 
+//We return not only string but also new delimiter if it changes
+//It needs to remove semicolon if delimiter changes
 fn rewrite_macro_inner(
     mac: &ast::MacCall,
     extra_ident: Option<symbol::Ident>,
     context: &RewriteContext<'_>,
     shape: Shape,
     position: MacroPosition,
     is_nested_macro: bool,
-) -> Option<String> {
+) -> (Option<String>, Option<Delimiter>) {
     if context.config.use_try_shorthand() {
         if let Some(expr) = convert_try_mac(mac, context) {
             context.leave_macro();
-            return expr.rewrite(context, shape);
+            return (expr.rewrite(context, shape), None);
         }
     }
 
@@ -214,26 +216,37 @@ fn rewrite_macro_inner(
     let ts = mac.args.tokens.clone();
     let has_comment = contains_comment(context.snippet(mac.span()));
     if ts.is_empty() && !has_comment {
-        return match style {
-            Delimiter::Parenthesis if position == MacroPosition::Item => {
-                Some(format!("{macro_name}();"))
-            }
-            Delimiter::Bracket if position == MacroPosition::Item => {
-                Some(format!("{macro_name}[];"))
-            }
-            Delimiter::Parenthesis => Some(format!("{macro_name}()")),
-            Delimiter::Bracket => Some(format!("{macro_name}[]")),
-            Delimiter::Brace => Some(format!("{macro_name} {{}}")),
-            _ => unreachable!(),
-        };
+        return (
+            match style {
+                Delimiter::Parenthesis if position == MacroPosition::Item => {
+                    Some(format!("{macro_name}();"))
+                }
+                Delimiter::Bracket if position == MacroPosition::Item => {
+                    Some(format!("{macro_name}[];"))
+                }
+                Delimiter::Parenthesis => Some(format!("{macro_name}()")),
+                Delimiter::Bracket => Some(format!("{macro_name}[]")),
+                Delimiter::Brace => Some(format!("{macro_name} {{}}")),
+                _ => unreachable!(),
+            },
+            if original_style != style {
+                Some(style)
+            } else {
+                None
+            },
+        );
     }
     // Format well-known macros which cannot be parsed as a valid AST.
     if macro_name == "lazy_static!" && !has_comment {
         if let success @ Some(..) = format_lazy_static(context, shape, ts.clone()) {
-            if original_style == Delimiter::Parenthesis {
-                context.lazy_static_success.replace(true);
-            }
-            return success;
+            return (
+                success,
+                if original_style != Delimiter::Brace {
+                    Some(Delimiter::Brace)
+                } else {
+                    None
+                },
+            );
         }
     }
 
@@ -244,103 +257,117 @@ fn rewrite_macro_inner(
     } = match parse_macro_args(context, ts, style, is_forced_bracket) {
         Some(args) => args,
         None => {
-            return return_macro_parse_failure_fallback(
-                context,
-                shape.indent,
-                position,
-                mac.span(),
+            return (
+                return_macro_parse_failure_fallback(context, shape.indent, position, mac.span()),
+                None,
             );
         }
     };
 
     if !arg_vec.is_empty() && arg_vec.iter().all(MacroArg::is_item) {
-        return rewrite_macro_with_items(
-            context,
-            &arg_vec,
-            &macro_name,
-            shape,
-            style,
-            original_style,
-            position,
-            mac.span(),
+        return (
+            rewrite_macro_with_items(
+                context,
+                &arg_vec,
+                &macro_name,
+                shape,
+                style,
+                original_style,
+                position,
+                mac.span(),
+            ),
+            if original_style != style {
+                Some(style)
+            } else {
+                None
+            },
         );
     }
 
-    match style {
-        Delimiter::Parenthesis => {
-            // Handle special case: `vec!(expr; expr)`
-            if vec_with_semi {
-                handle_vec_semi(context, shape, arg_vec, macro_name, style)
-            } else {
-                // Format macro invocation as function call, preserve the trailing
-                // comma because not all macros support them.
-                overflow::rewrite_with_parens(
-                    context,
-                    &macro_name,
-                    arg_vec.iter(),
-                    shape,
-                    mac.span(),
-                    context.config.fn_call_width(),
-                    if trailing_comma {
+    (
+        match style {
+            Delimiter::Parenthesis => {
+                // Handle special case: `vec!(expr; expr)`
+                if vec_with_semi {
+                    handle_vec_semi(context, shape, arg_vec, macro_name, style)
+                } else {
+                    // Format macro invocation as function call, preserve the trailing
+                    // comma because not all macros support them.
+                    overflow::rewrite_with_parens(
+                        context,
+                        &macro_name,
+                        arg_vec.iter(),
+                        shape,
+                        mac.span(),
+                        context.config.fn_call_width(),
+                        if trailing_comma {
+                            Some(SeparatorTactic::Always)
+                        } else {
+                            Some(SeparatorTactic::Never)
+                        },
+                    )
+                    .map(|rw| match position {
+                        MacroPosition::Item => format!("{};", rw),
+                        _ => rw,
+                    })
+                }
+            }
+            Delimiter::Bracket => {
+                // Handle special case: `vec![expr; expr]`
+                if vec_with_semi {
+                    handle_vec_semi(context, shape, arg_vec, macro_name, style)
+                } else {
+                    // If we are rewriting `vec!` macro or other special macros,
+                    // then we can rewrite this as a usual array literal.
+                    // Otherwise, we must preserve the original existence of trailing comma.
+                    let mut force_trailing_comma = if trailing_comma {
                         Some(SeparatorTactic::Always)
                     } else {
                         Some(SeparatorTactic::Never)
-                    },
-                )
-                .map(|rw| match position {
-                    MacroPosition::Item => format!("{};", rw),
-                    _ => rw,
-                })
-            }
-        }
-        Delimiter::Bracket => {
-            // Handle special case: `vec![expr; expr]`
-            if vec_with_semi {
-                handle_vec_semi(context, shape, arg_vec, macro_name, style)
-            } else {
-                // If we are rewriting `vec!` macro or other special macros,
-                // then we can rewrite this as a usual array literal.
-                // Otherwise, we must preserve the original existence of trailing comma.
-                let mut force_trailing_comma = if trailing_comma {
-                    Some(SeparatorTactic::Always)
-                } else {
-                    Some(SeparatorTactic::Never)
-                };
-                if is_forced_bracket && !is_nested_macro {
-                    context.leave_macro();
-                    if context.use_block_indent() {
-                        force_trailing_comma = Some(SeparatorTactic::Vertical);
                     };
+                    if is_forced_bracket && !is_nested_macro {
+                        context.leave_macro();
+                        if context.use_block_indent() {
+                            force_trailing_comma = Some(SeparatorTactic::Vertical);
+                        };
+                    }
+                    rewrite_array(
+                        &macro_name,
+                        arg_vec.iter(),
+                        mac.span(),
+                        context,
+                        shape,
+                        force_trailing_comma,
+                        Some(original_style),
+                    )
+                    .map(|rewrite| {
+                        let comma = match position {
+                            MacroPosition::Item => ";",
+                            _ => "",
+                        };
+
+                        format!("{rewrite}{comma}")
+                    })
                 }
-                let rewrite = rewrite_array(
-                    &macro_name,
-                    arg_vec.iter(),
-                    mac.span(),
-                    context,
-                    shape,
-                    force_trailing_comma,
-                    Some(original_style),
-                )?;
-                let comma = match position {
-                    MacroPosition::Item => ";",
-                    _ => "",
-                };
-
-                Some(format!("{rewrite}{comma}"))
             }
-        }
-        Delimiter::Brace => {
-            // For macro invocations with braces, always put a space between
-            // the `macro_name!` and `{ /* macro_body */ }` but skip modifying
-            // anything in between the braces (for now).
-            let snippet = context.snippet(mac.span()).trim_start_matches(|c| c != '{');
-            match trim_left_preserve_layout(snippet, shape.indent, context.config) {
-                Some(macro_body) => Some(format!("{macro_name} {macro_body}")),
-                None => Some(format!("{macro_name} {snippet}")),
+            Delimiter::Brace => {
+                // For macro invocations with braces, always put a space between
+                // the `macro_name!` and `{ /* macro_body */ }` but skip modifying
+                // anything in between the braces (for now).
+                let snippet = context.snippet(mac.span()).trim_start_matches(|c| c != '{');
+                match trim_left_preserve_layout(snippet, shape.indent, context.config) {
+                    Some(macro_body) => Some(format!("{macro_name} {macro_body}")),
+                    None => Some(format!("{macro_name} {snippet}")),
+                }
             }
-        }
-        _ => unreachable!(),
-    }
+            _ => unreachable!(),
+        },
+        if original_style != style {
+            Some(style)
+        } else {
+            None
+        },
+    )
 }
 
 fn handle_vec_semi(

src/missed_spans.rs

@@ -50,15 +50,6 @@ impl<'a> FmtVisitor<'a> {
         self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet))
     }
 
-    pub(crate) fn format_missing_ignore_semicolon(&mut self, end: BytePos) {
-        let missing_snippet = self.snippet(mk_sp(self.last_pos, end));
-        if missing_snippet.trim() == ";" {
-            self.last_pos = end;
-            return;
-        }
-        self.format_missing_inner(end, |this, last_snippet, _| this.push_str(last_snippet))
-    }
-
     pub(crate) fn format_missing_with_indent(&mut self, end: BytePos) {
         self.format_missing_indent(end, true)
     }

src/patterns.rs

@@ -269,7 +269,7 @@ impl Rewrite for Pat {
                 shape,
             ),
             PatKind::MacCall(ref mac) => {
-                rewrite_macro(mac, None, context, shape, MacroPosition::Pat)
+                rewrite_macro(mac, None, context, shape, MacroPosition::Pat).0
             }
             PatKind::Paren(ref pat) => pat
                 .rewrite(context, shape.offset_left(1)?.sub_width(1)?)

src/rewrite.rs

@@ -43,7 +43,6 @@ pub(crate) struct RewriteContext<'a> {
     pub(crate) report: FormatReport,
     pub(crate) skip_context: SkipContext,
     pub(crate) skipped_range: Rc<RefCell<Vec<(usize, usize)>>>,
-    pub(crate) lazy_static_success: Cell<bool>,
 }
 
 pub(crate) struct InsideMacroGuard {

src/types.rs

@@ -829,7 +829,7 @@ impl Rewrite for ast::Ty {
             ast::TyKind::BareFn(ref bare_fn) => rewrite_bare_fn(bare_fn, self.span, context, shape),
             ast::TyKind::Never => Some(String::from("!")),
             ast::TyKind::MacCall(ref mac) => {
-                rewrite_macro(mac, None, context, shape, MacroPosition::Expression)
+                rewrite_macro(mac, None, context, shape, MacroPosition::Expression).0
             }
             ast::TyKind::ImplicitSelf => Some(String::from("")),
             ast::TyKind::ImplTrait(_, ref it) => {