fix additional semicolon during changing parentheses style

rust-lang · May 4, 2024 · e547ace · e547ace
1 parent d5f1200
commit e547ace
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 38 deletions.
diff --git a/src/expr.rs b/src/expr.rs
@@ -225,7 +225,8 @@ 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(|| {
+            let (rewrite, _) = rewrite_macro(mac, None, context, shape, MacroPosition::Expression);
+            rewrite.or_else(|| {
                 wrap_str(
                     context.snippet(expr.span).to_owned(),
                     context.config.max_width(),

diff --git a/src/items.rs b/src/items.rs
@@ -3374,7 +3374,8 @@ 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)
+                let (rewrite, _) = rewrite_macro(mac, None, context, shape, MacroPosition::Item);
+                rewrite
             }
         }?;
 

diff --git a/src/macros.rs b/src/macros.rs
@@ -155,13 +155,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(|| {
@@ -175,27 +175,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 in some situations
 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);
         }
     }
 
@@ -213,7 +215,7 @@ 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 {
+        let rewrite = match style {
             Delimiter::Parenthesis if position == MacroPosition::Item => {
                 Some(format!("{macro_name}();"))
             }
@@ -225,11 +227,12 @@ fn rewrite_macro_inner(
             Delimiter::Brace => Some(format!("{macro_name} {{}}")),
             _ => unreachable!(),
         };
+        return (rewrite, Some(style));
     }
     // 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()) {
-            return success;
+            return (success, Some(Delimiter::Brace));
         }
     }
 
@@ -240,17 +243,14 @@ 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(),
-            );
+            let rewrite =
+                return_macro_parse_failure_fallback(context, shape.indent, position, mac.span());
+            return (rewrite, None);
         }
     };
 
     if !arg_vec.is_empty() && arg_vec.iter().all(MacroArg::is_item) {
-        return rewrite_macro_with_items(
+        let rewrite = rewrite_macro_with_items(
             context,
             &arg_vec,
             &macro_name,
@@ -260,9 +260,10 @@ fn rewrite_macro_inner(
             position,
             mac.span(),
         );
+        return (rewrite, Some(style));
     }
 
-    match style {
+    let rewrite = match style {
         Delimiter::Parenthesis => {
             // Handle special case: `vec!(expr; expr)`
             if vec_with_semi {
@@ -308,20 +309,22 @@ fn rewrite_macro_inner(
                         force_trailing_comma = Some(SeparatorTactic::Vertical);
                     };
                 }
-                let rewrite = rewrite_array(
+                let Some(rewrite) = rewrite_array(
                     &macro_name,
                     arg_vec.iter(),
                     mac.span(),
                     context,
                     shape,
                     force_trailing_comma,
                     Some(original_style),
-                )?;
+                ) else {
+                    return (None, None);
+                };
+
                 let comma = match position {
                     MacroPosition::Item => ";",
                     _ => "",
                 };
-
                 Some(format!("{rewrite}{comma}"))
             }
         }
@@ -336,7 +339,8 @@ fn rewrite_macro_inner(
             }
         }
         _ => unreachable!(),
-    }
+    };
+    return (rewrite, Some(style));
 }
 
 fn handle_vec_semi(

diff --git a/src/patterns.rs b/src/patterns.rs
@@ -269,7 +269,8 @@ impl Rewrite for Pat {
                 shape,
             ),
             PatKind::MacCall(ref mac) => {
-                rewrite_macro(mac, None, context, shape, MacroPosition::Pat)
+                let (rewrite, _) = rewrite_macro(mac, None, context, shape, MacroPosition::Pat);
+                rewrite
             }
             PatKind::Paren(ref pat) => pat
                 .rewrite(context, shape.offset_left(1)?.sub_width(1)?)

diff --git a/src/types.rs b/src/types.rs
@@ -829,7 +829,9 @@ 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)
+                let (rewrite, _) =
+                    rewrite_macro(mac, None, context, shape, MacroPosition::Expression);
+                rewrite
             }
             ast::TyKind::ImplicitSelf => Some(String::from("")),
             ast::TyKind::ImplTrait(_, ref it) => {

diff --git a/src/visitor.rs b/src/visitor.rs
@@ -683,28 +683,38 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
 
         // 1 = ;
         let shape = self.shape().saturating_sub_width(1);
-        let rewrite = self.with_context(|ctx| rewrite_macro(mac, ident, ctx, shape, pos));
+        let original_style = macro_style(mac, &self.get_context());
+        let (rewrite, rewrite_style) =
+            self.with_context(|ctx| rewrite_macro(mac, ident, ctx, shape, pos));
         // As of v638 of the rustc-ap-* crates, the associated span no longer includes
         // the trailing semicolon. This determines the correct span to ensure scenarios
         // with whitespace between the delimiters and trailing semi (i.e. `foo!(abc)     ;`)
         // are formatted correctly.
-        let (span, rewrite) = match macro_style(mac, &self.get_context()) {
-            Delimiter::Bracket | Delimiter::Parenthesis if MacroPosition::Item == pos => {
-                let search_span = mk_sp(mac.span().hi(), self.snippet_provider.end_pos());
-                let hi = self.snippet_provider.span_before(search_span, ";");
-                let target_span = mk_sp(mac.span().lo(), hi + BytePos(1));
-                let rewrite = rewrite.map(|rw| {
+        let rewrite = match rewrite_style.unwrap_or(original_style) {
+            Delimiter::Bracket | Delimiter::Parenthesis if MacroPosition::Item == pos => rewrite
+                .map(|rw| {
                     if !rw.ends_with(';') {
                         format!("{};", rw)
                     } else {
                         rw
                     }
-                });
-                (target_span, rewrite)
-            }
-            _ => (mac.span(), rewrite),
+                }),
+            _ => rewrite,
         };
 
+        let span = match original_style {
+            Delimiter::Bracket | Delimiter::Parenthesis
+                if (MacroPosition::Item == pos)
+                    || (MacroPosition::Statement == pos)
+                        && rewrite_style
+                            .is_some_and(|x| x != original_style && x == Delimiter::Brace) =>
+            {
+                let search_span = mk_sp(mac.span().hi(), self.snippet_provider.end_pos());
+                let hi = self.snippet_provider.span_before(search_span, ";");
+                mk_sp(mac.span().lo(), hi + BytePos(1))
+            }
+            _ => mac.span(),
+        };
         self.push_rewrite(span, rewrite);
     }
 
@@ -989,13 +999,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
         }
     }
 
-    pub(crate) fn with_context<F>(&mut self, f: F) -> Option<String>
+    pub(crate) fn with_context<F, RESULT>(&mut self, f: F) -> RESULT
     where
-        F: Fn(&RewriteContext<'_>) -> Option<String>,
+        F: Fn(&RewriteContext<'_>) -> RESULT,
     {
         let context = self.get_context();
         let result = f(&context);
-
         self.macro_rewrite_failure |= context.macro_rewrite_failure.get();
         result
     }

diff --git a/tests/source/issue-6013/main.rs b/tests/source/issue-6013/main.rs
@@ -0,0 +1,7 @@
+fn main() {
+lazy_static!(
+        static ref DYNAMODB_CLIENT: Option<aws_sdk_dynamodb::Client> = None;
+            static ref CASCADE_IP: String = std::env::var("CASCADE_IP").unwrap_or("127.0.0.1".to_string());
+                static ref CASCADE_PORT: String = std::env::var("CASCADE_PORT").unwrap_or("4000".to_string());
+)     ;
+}
diff --git a/tests/target/issue-6013/main.rs b/tests/target/issue-6013/main.rs
@@ -0,0 +1,9 @@
+fn main() {
+    lazy_static! {
+        static ref DYNAMODB_CLIENT: Option<aws_sdk_dynamodb::Client> = None;
+        static ref CASCADE_IP: String =
+            std::env::var("CASCADE_IP").unwrap_or("127.0.0.1".to_string());
+        static ref CASCADE_PORT: String =
+            std::env::var("CASCADE_PORT").unwrap_or("4000".to_string());
+    }
+}