Merge pull request #1694 from dtolnay/jumps

Parenthesize closures and jumps which are not rightmost subexpressions

src/expr.rs

@@ -3119,6 +3119,13 @@ pub(crate) mod printing {
         }
     }
 
+    fn precedence_of_rhs(e: &Expr, #[cfg(feature = "full")] fixup: FixupContext) -> Precedence {
+        #[cfg(feature = "full")]
+        return fixup.precedence_of_rhs(e);
+        #[cfg(not(feature = "full"))]
+        return Precedence::of(e);
+    }
+
     #[cfg(feature = "full")]
     #[cfg_attr(docsrs, doc(cfg(feature = "printing")))]
     impl ToTokens for ExprArray {
@@ -3150,7 +3157,7 @@ pub(crate) mod printing {
         e.eq_token.to_tokens(tokens);
         print_subexpression(
             &e.right,
-            Precedence::of_rhs(&e.right) < Precedence::Assign,
+            precedence_of_rhs(&e.right, fixup) < Precedence::Assign,
             tokens,
             fixup.subsequent_subexpression(),
         );
@@ -3209,7 +3216,11 @@ pub(crate) mod printing {
 
         let binop_prec = Precedence::of_binop(&e.op);
         let left_prec = Precedence::of(&e.left);
-        let right_prec = Precedence::of_rhs(&e.right);
+        let right_prec = precedence_of_rhs(
+            &e.right,
+            #[cfg(feature = "full")]
+            fixup,
+        );
         let (mut left_needs_group, right_needs_group) = if let Precedence::Assign = binop_prec {
             (left_prec <= binop_prec, right_prec < binop_prec)
         } else {
@@ -3696,7 +3707,7 @@ pub(crate) mod printing {
         if let Some(end) = &e.end {
             print_subexpression(
                 end,
-                Precedence::of_rhs(end) <= Precedence::Range,
+                precedence_of_rhs(end, fixup) <= Precedence::Range,
                 tokens,
                 fixup.subsequent_subexpression(),
             );
@@ -3723,9 +3734,14 @@ pub(crate) mod printing {
         outer_attrs_to_tokens(&e.attrs, tokens);
         e.and_token.to_tokens(tokens);
         e.mutability.to_tokens(tokens);
+        let prec = precedence_of_rhs(
+            &e.expr,
+            #[cfg(feature = "full")]
+            fixup,
+        );
         print_subexpression(
             &e.expr,
-            Precedence::of_rhs(&e.expr) < Precedence::Prefix,
+            prec < Precedence::Prefix,
             tokens,
             #[cfg(feature = "full")]
             fixup.subsequent_subexpression(),
@@ -3844,9 +3860,14 @@ pub(crate) mod printing {
     ) {
         outer_attrs_to_tokens(&e.attrs, tokens);
         e.op.to_tokens(tokens);
+        let prec = precedence_of_rhs(
+            &e.expr,
+            #[cfg(feature = "full")]
+            fixup,
+        );
         print_subexpression(
             &e.expr,
-            Precedence::of_rhs(&e.expr) < Precedence::Prefix,
+            prec < Precedence::Prefix,
             tokens,
             #[cfg(feature = "full")]
             fixup.subsequent_subexpression(),

src/fixup.rs

@@ -85,6 +85,14 @@ pub(crate) struct FixupContext {
     //     }
     //
     parenthesize_exterior_struct_lit: bool,
+
+    // This is the difference between:
+    //
+    //     let _ = 1 + return 1;  // no parens if rightmost subexpression
+    //
+    //     let _ = 1 + (return 1) + 1;  // needs parens
+    //
+    parenthesize_exterior_jump: bool,
 }
 
 impl FixupContext {
@@ -96,6 +104,7 @@ impl FixupContext {
         match_arm: false,
         leftmost_subexpression_in_match_arm: false,
         parenthesize_exterior_struct_lit: false,
+        parenthesize_exterior_jump: false,
     };
 
     /// Create the initial fixup for printing an expression in statement
@@ -145,6 +154,7 @@ impl FixupContext {
             match_arm: false,
             leftmost_subexpression_in_match_arm: self.match_arm
                 || self.leftmost_subexpression_in_match_arm,
+            parenthesize_exterior_jump: true,
             ..self
         }
     }
@@ -159,6 +169,7 @@ impl FixupContext {
             leftmost_subexpression_in_stmt: false,
             match_arm: self.match_arm || self.leftmost_subexpression_in_match_arm,
             leftmost_subexpression_in_match_arm: false,
+            parenthesize_exterior_jump: true,
             ..self
         }
     }
@@ -205,7 +216,23 @@ impl FixupContext {
     ///     "let chain".
     pub fn needs_group_as_let_scrutinee(self, expr: &Expr) -> bool {
         self.parenthesize_exterior_struct_lit && classify::confusable_with_adjacent_block(expr)
-            || Precedence::of_rhs(expr) <= Precedence::And
+            || self.precedence_of_rhs(expr) <= Precedence::And
+    }
+
+    /// Determines the effective precedence of a subexpression. Some expressions
+    /// have higher precedence on the right side of a binary operator than on
+    /// the left.
+    pub fn precedence_of_rhs(self, expr: &Expr) -> Precedence {
+        if !self.parenthesize_exterior_jump {
+            match expr {
+                Expr::Break(_) | Expr::Closure(_) | Expr::Return(_) | Expr::Yield(_) => {
+                    return Precedence::Prefix;
+                }
+                Expr::Range(e) if e.start.is_none() => return Precedence::Prefix,
+                _ => {}
+            }
+        }
+        Precedence::of(expr)
     }
 }
 

src/precedence.rs

@@ -122,18 +122,6 @@ impl Precedence {
             Expr::Closure(_) => unreachable!(),
         }
     }
-
-    #[cfg(feature = "printing")]
-    pub(crate) fn of_rhs(e: &Expr) -> Self {
-        match e {
-            Expr::Break(_) | Expr::Closure(_) | Expr::Return(_) | Expr::Yield(_) => {
-                Precedence::Prefix
-            }
-            #[cfg(feature = "full")]
-            Expr::Range(e) if e.start.is_none() => Precedence::Prefix,
-            _ => Precedence::of(e),
-        }
-    }
 }
 
 impl Copy for Precedence {}

tests/test_expr.rs

@@ -673,6 +673,7 @@ fn test_fixup() {
         quote! { if let _ = (a && b) && c {} },
         quote! { if let _ = (S {}) {} },
         quote! { break ('a: loop { break 'a 1 } + 1) },
+        quote! { a + (|| b) + c },
     ] {
         let original: Expr = syn::parse2(tokens).unwrap();