Fix for issues 4603 and 4609 - unlimitted indentation of macro code

Author: davidBar-On

src/formatting.rs

@@ -58,18 +58,45 @@ pub(crate) mod visitor;
 
 pub(crate) mod report;
 
+// As `catch_unwind` in `format_snippet()` does not allow the the of a `mut bool`,
+// this function is used to `format_input_inner()` using a reference to a new
+// `bool` variable, and return its value as part of the function return value.
+pub(crate) fn format_input_inner_for_catch_unwind(
+    input: Input,
+    config: &Config,
+    operation_setting: OperationSetting,
+    is_macro_def: bool,
+) -> (Result<FormatReport, OperationError>, bool) {
+    let mut macro_original_code_was_used = false;
+    let result = crate::format_input_inner(
+        input,
+        &config,
+        operation_setting,
+        is_macro_def,
+        &mut macro_original_code_was_used,
+    );
+    return (result, macro_original_code_was_used);
+}
+
 pub(crate) fn format_input_inner(
     input: Input,
     config: &Config,
     operation_setting: OperationSetting,
     is_macro_def: bool,
+    macro_original_code_was_used: &mut bool,
 ) -> Result<FormatReport, OperationError> {
     if !config.version_meets_requirement() {
         return Err(OperationError::VersionMismatch);
     }
 
     rustc_span::with_session_globals(config.edition().into(), || {
-        format_project(input, config, operation_setting, is_macro_def)
+        format_project(
+            input,
+            config,
+            operation_setting,
+            is_macro_def,
+            macro_original_code_was_used,
+        )
     })
 }
 
@@ -78,6 +105,7 @@ fn format_project(
     config: &Config,
     operation_setting: OperationSetting,
     is_macro_def: bool,
+    macro_original_code_was_used: &mut bool,
 ) -> Result<FormatReport, OperationError> {
     let mut timer = Timer::start();
 
@@ -87,6 +115,7 @@ fn format_project(
     let input_is_stdin = main_file == FileName::Stdin;
 
     let mut parse_session = ParseSess::new(config)?;
+
     if !operation_setting.recursive && parse_session.ignore_file(&main_file) {
         format_report.add_ignored_file(main_file);
         return Ok(format_report);
@@ -152,6 +181,7 @@ fn format_project(
             &files,
             original_snippet.clone(),
             is_macro_def,
+            macro_original_code_was_used,
         )?;
     }
     timer = timer.done_formatting();
@@ -177,15 +207,18 @@ fn format_file(
     file_mod_map: &FileModMap<'_>,
     original_snippet: Option<String>,
     is_macro_def: bool,
+    macro_original_code_was_used: &mut bool,
 ) -> Result<(), OperationError> {
     let snippet_provider = parse_session.snippet_provider(module.as_ref().inner);
+
     let mut visitor = FmtVisitor::from_parse_sess(
         &parse_session,
         config,
         &snippet_provider,
         file_mod_map,
         report.clone(),
     );
+
     visitor.is_macro_def = is_macro_def;
     visitor.skip_context.update_with_attrs(&krate.attrs);
     visitor.last_pos = snippet_provider.start_pos();
@@ -237,6 +270,10 @@ fn format_file(
     );
     report.add_format_result(path.clone(), format_result);
 
+    if visitor.macro_original_code_was_used.get() {
+        *macro_original_code_was_used = true;
+    }
+
     Ok(())
 }
 

src/formatting/comment.rs

@@ -687,9 +687,13 @@ impl<'a> CommentRewrite<'a> {
                         let mut config = self.fmt.config.clone();
                         config.set().wrap_comments(false);
                         if config.format_code_in_doc_comments() {
-                            if let Some(s) =
-                                format_code_block(&self.code_block_buffer, &config, false)
-                            {
+                            let mut macro_original_code_was_used = false;
+                            if let Some(s) = format_code_block(
+                                &self.code_block_buffer,
+                                &config,
+                                false,
+                                &mut macro_original_code_was_used,
+                            ) {
                                 trim_custom_comment_prefix(s.as_ref())
                             } else {
                                 trim_custom_comment_prefix(&self.code_block_buffer)
@@ -1604,6 +1608,7 @@ pub(crate) fn recover_comment_removed(
     let snippet = context.snippet(span);
     let includes_comment = contains_comment(snippet);
     if snippet != new && includes_comment && changed_comment_content(snippet, &new) {
+        context.macro_original_code_was_used.replace(true);
         snippet.to_owned()
     } else {
         new

src/formatting/expr.rs

@@ -606,6 +606,11 @@ pub(crate) fn rewrite_block_with_visitor(
         .skipped_range
         .borrow_mut()
         .append(&mut visitor_context.skipped_range.borrow_mut());
+
+    if visitor.macro_original_code_was_used.get() {
+        context.macro_original_code_was_used.replace(true);
+    }
+
     Some(format!("{}{}{}", prefix, label_str, visitor.buffer))
 }
 

src/formatting/items.rs

@@ -890,6 +890,10 @@ pub(crate) fn format_impl(
             result.push_str(&inner_indent_str);
             result.push_str(visitor.buffer.trim());
             result.push_str(&outer_indent_str);
+
+            if visitor.macro_original_code_was_used.get() {
+                context.macro_original_code_was_used.replace(true);
+            }
         } else if need_newline || !context.config.empty_item_single_line() {
             result.push_str(&sep);
         }
@@ -1266,6 +1270,10 @@ pub(crate) fn format_trait(
             result.push_str(&inner_indent_str);
             result.push_str(visitor.buffer.trim());
             result.push_str(&outer_indent_str);
+
+            if visitor.macro_original_code_was_used.get() {
+                context.macro_original_code_was_used.replace(true);
+            }
         } else if result.contains('\n') {
             result.push_str(&outer_indent_str);
         }
@@ -3264,6 +3272,11 @@ impl Rewrite for ast::ForeignItem {
                     defaultness,
                     Some(&inner_attrs),
                 );
+
+                if visitor.macro_original_code_was_used.get() {
+                    context.macro_original_code_was_used.replace(true);
+                }
+
                 Some(visitor.buffer.to_owned())
             }
             ast::ForeignItemKind::Fn(_, ref fn_sig, ref generics, None) => rewrite_fn_base(

src/formatting/macros.rs

@@ -170,7 +170,7 @@ fn return_macro_parse_failure_fallback(
 ) -> Option<String> {
     // Mark this as a failure however we format it
     context.macro_rewrite_failure.replace(true);
-
+    context.macro_original_code_was_used.replace(true);
     // Heuristically determine whether the last line of the macro uses "Block" style
     // rather than using "Visual" style, or another indentation style.
     let is_like_block_indent_style = context
@@ -229,6 +229,7 @@ pub(crate) fn rewrite_macro(
                 guard.is_nested(),
             )
         }));
+
         match result {
             Err(..) | Ok(None) => {
                 context.macro_rewrite_failure.replace(true);
@@ -510,13 +511,17 @@ pub(crate) fn rewrite_macro_def(
 ) -> Option<String> {
     let snippet = Some(remove_trailing_white_spaces(context.snippet(span)));
     if snippet.as_ref().map_or(true, |s| s.ends_with(';')) {
+        context.macro_original_code_was_used.replace(true);
         return snippet;
     }
     let ts = def.body.inner_tokens();
     let mut parser = MacroParser::new(ts.into_trees());
     let parsed_def = match parser.parse(def.macro_rules) {
         Some(def) => def,
-        None => return snippet,
+        None => {
+            context.macro_original_code_was_used.replace(true);
+            return snippet;
+        }
     };
 
     let mut result = if def.macro_rules {
@@ -527,7 +532,6 @@ pub(crate) fn rewrite_macro_def(
 
     result += " ";
     result += rewrite_ident(context, ident);
-
     let multi_branch_style = def.macro_rules || parsed_def.branches.len() != 1;
 
     let arm_shape = if multi_branch_style {
@@ -550,6 +554,7 @@ pub(crate) fn rewrite_macro_def(
             // if the rewrite returned None because a macro could not be rewritten, then return the
             // original body
             None if context.macro_rewrite_failure.get() => {
+                context.macro_original_code_was_used.replace(true);
                 Some(context.snippet(branch.body).trim().to_string())
             }
             None => None,
@@ -576,14 +581,16 @@ pub(crate) fn rewrite_macro_def(
 
     match write_list(&branch_items, &fmt) {
         Some(ref s) => result += s,
-        None => return snippet,
+        None => {
+            context.macro_original_code_was_used.replace(true);
+            return snippet;
+        }
     }
 
     if multi_branch_style {
         result += &indent.to_string_with_newline(context.config);
         result += "}";
     }
-
     Some(result)
 }
 
@@ -1274,7 +1281,8 @@ impl MacroParser {
             branches.push(self.parse_branch(is_macro_rules)?);
         }
 
-        Some(Macro { branches })
+        let ret = Some(Macro { branches });
+        ret
     }
 
     // `(` ... `)` `=>` `{` ... `}`
@@ -1356,6 +1364,10 @@ impl MacroBranch {
             result += " =>";
         }
 
+        // Prepare for restoring original macro body if original code was use during formatting
+        // (since in this case indentation done by this function doesn't work properly).
+        let result_without_body = result.clone();
+
         if !context.config.format_macro_bodies() {
             result += " ";
             result += context.snippet(self.whole_body);
@@ -1386,23 +1398,44 @@ impl MacroBranch {
         config.set().max_width(new_width);
 
         // First try to format as items, then as statements.
-        let new_body_snippet = match format_snippet(&body_str, &config, true) {
-            Some(new_body) => new_body,
-            None => {
-                let new_width = new_width + config.tab_spaces();
-                config.set().max_width(new_width);
-                match format_code_block(&body_str, &config, true) {
-                    Some(new_body) => new_body,
-                    None => return None,
+
+        let mut macro_original_code_was_used = false;
+
+        let new_body_snippet =
+            match format_snippet(&body_str, &config, true, &mut macro_original_code_was_used) {
+                Some(new_body) => new_body,
+                None => {
+                    let new_width = new_width + config.tab_spaces();
+                    config.set().max_width(new_width);
+                    match format_code_block(
+                        &body_str,
+                        &config,
+                        true,
+                        &mut macro_original_code_was_used,
+                    ) {
+                        Some(new_body) => new_body,
+                        None => {
+                            return None;
+                        }
+                    }
                 }
-            }
-        };
+            };
+
         let new_body = wrap_str(
             new_body_snippet.as_ref().to_owned(),
             config.max_width(),
             shape,
         )?;
 
+        // If original code was used during the macro body formatting -
+        // use whole original body without formatting
+        if macro_original_code_was_used {
+            result = result_without_body;
+            result += " ";
+            result += context.snippet(self.whole_body);
+            return Some(result);
+        }
+
         // Indent the body since it is in a block.
         let indent_str = body_indent.to_string(&config);
         let mut new_body = LineClasses::new(new_body.trim_end())
@@ -1425,7 +1458,6 @@ impl MacroBranch {
         // FIXME: this could be *much* more efficient.
         for (old, new) in &substs {
             if old_body.contains(new) {
-                debug!("rewrite_macro_def: bailing matching variable: `{}`", new);
                 return None;
             }
             new_body = new_body.replace(new, old);
@@ -1576,6 +1608,11 @@ fn rewrite_macro_with_items(
     result.push_str(&shape.indent.to_string_with_newline(context.config));
     result.push_str(closer);
     result.push_str(trailing_semicolon);
+
+    if visitor.macro_original_code_was_used.get() {
+        context.macro_original_code_was_used.replace(true);
+    }
+
     Some(result)
 }
 

src/formatting/rewrite.rs

@@ -44,6 +44,7 @@ pub(crate) struct RewriteContext<'a> {
     pub(crate) snippet_provider: &'a SnippetProvider,
     // Used for `format_snippet`
     pub(crate) macro_rewrite_failure: Cell<bool>,
+    pub(crate) macro_original_code_was_used: Cell<bool>,
     pub(crate) report: FormatReport,
     pub(crate) skip_context: SkipContext,
     pub(crate) skipped_range: Rc<RefCell<Vec<NonFormattedRange>>>,