Fix for issue 4603 about extra macro body indentation (third version)

Author: davidBar-On

src/formatting.rs

@@ -331,6 +331,9 @@ pub(crate) struct ReportedErrors {
 
     /// Formatted code differs from existing code (--check only).
     pub(crate) has_diff: bool,
+
+    /// Formatted code missed something, like lost comments or extra trailing space
+    pub(crate) has_unformatted_code_errors: bool,
 }
 
 impl ReportedErrors {
@@ -342,6 +345,7 @@ impl ReportedErrors {
         self.has_macro_format_failure |= other.has_macro_format_failure;
         self.has_check_errors |= other.has_check_errors;
         self.has_diff |= other.has_diff;
+        self.has_unformatted_code_errors |= other.has_unformatted_code_errors;
     }
 }
 

src/lib.rs

@@ -210,14 +210,22 @@ impl FormatReport {
         if !new_errors.is_empty() {
             errs.has_formatting_errors = true;
         }
-        if errs.has_operational_errors && errs.has_check_errors {
+        if errs.has_operational_errors && errs.has_check_errors && errs.has_unformatted_code_errors
+        {
             return;
         }
         for err in new_errors {
             match err.kind {
-                ErrorKind::LineOverflow(..) | ErrorKind::TrailingWhitespace => {
+                ErrorKind::LineOverflow(..) => {
                     errs.has_operational_errors = true;
                 }
+                ErrorKind::TrailingWhitespace => {
+                    errs.has_operational_errors = true;
+                    errs.has_unformatted_code_errors = true;
+                }
+                ErrorKind::LostComment => {
+                    errs.has_unformatted_code_errors = true;
+                }
                 ErrorKind::BadIssue(_)
                 | ErrorKind::LicenseCheck
                 | ErrorKind::DeprecatedAttr
@@ -294,6 +302,9 @@ fn format_snippet(snippet: &str, config: &Config, is_macro_def: bool) -> Option<
         config.set().emit_mode(config::EmitMode::Stdout);
         config.set().verbose(Verbosity::Quiet);
         config.set().hide_parse_errors(true);
+        if is_macro_def {
+            config.set().error_on_unformatted(true);
+        }
 
         let (formatting_error, result) = {
             let input = Input::Text(snippet.into());
@@ -302,7 +313,8 @@ fn format_snippet(snippet: &str, config: &Config, is_macro_def: bool) -> Option<
             (
                 session.errors.has_macro_format_failure
                     || session.out.as_ref().unwrap().is_empty() && !snippet.is_empty()
-                    || result.is_err(),
+                    || result.is_err()
+                    || (is_macro_def && session.has_unformatted_code_errors()),
                 result,
             )
         };
@@ -477,13 +489,18 @@ impl<'b, T: Write + 'b> Session<'b, T> {
         self.errors.has_diff
     }
 
+    pub fn has_unformatted_code_errors(&self) -> bool {
+        self.errors.has_unformatted_code_errors
+    }
+
     pub fn has_no_errors(&self) -> bool {
         !(self.has_operational_errors()
             || self.has_parsing_errors()
             || self.has_formatting_errors()
             || self.has_check_errors()
-            || self.has_diff())
-            || self.errors.has_macro_format_failure
+            || self.has_diff()
+            || self.has_unformatted_code_errors()
+            || self.errors.has_macro_format_failure)
     }
 }
 

tests/source/issue-4603.rs

@@ -0,0 +1,47 @@
+// Formatting when original macro snippet is used
+
+// Original issue #4603 code
+#![feature(or_patterns)]
+macro_rules! t_or_f {
+    () => {
+        (true // some comment
+        | false)
+    };
+}
+
+// Other test cases variations
+macro_rules! RULES {
+    () => {
+        (
+		xxxxxxx // COMMENT
+        | yyyyyyy
+        )
+    };
+}
+macro_rules! RULES {
+    () => {
+        (xxxxxxx // COMMENT
+            | yyyyyyy)
+    };
+}
+
+fn main() {
+	macro_rules! RULES {
+		() => {
+			(xxxxxxx // COMMENT
+			| yyyyyyy)
+		};
+	}
+}
+
+macro_rules! RULES {
+    () => {
+        (xxxxxxx /* COMMENT */ | yyyyyyy)
+    };
+}
+macro_rules! RULES {
+    () => {
+        (xxxxxxx /* COMMENT */
+        | yyyyyyy)
+    };
+}

tests/target/issue-4603.rs

@@ -0,0 +1,47 @@
+// Formatting when original macro snippet is used
+
+// Original issue #4603 code
+#![feature(or_patterns)]
+macro_rules! t_or_f {
+    () => {
+        (true // some comment
+        | false)
+    };
+}
+
+// Other test cases variations
+macro_rules! RULES {
+    () => {
+        (
+		xxxxxxx // COMMENT
+        | yyyyyyy
+        )
+    };
+}
+macro_rules! RULES {
+    () => {
+        (xxxxxxx // COMMENT
+            | yyyyyyy)
+    };
+}
+
+fn main() {
+    macro_rules! RULES {
+		() => {
+			(xxxxxxx // COMMENT
+			| yyyyyyy)
+		};
+	}
+}
+
+macro_rules! RULES {
+    () => {
+        (xxxxxxx /* COMMENT */ | yyyyyyy)
+    };
+}
+macro_rules! RULES {
+    () => {
+        (xxxxxxx /* COMMENT */
+        | yyyyyyy)
+    };
+}