Auto merge of rust-lang#7013 - Y-Nak:fix-needless-paren, r=flip1995

clippy_utils: fix needless parenthesis output from sugg::Sugg::maybe_par changelog: clippy_utils: fix needless parenthesis output from `sugg::Sugg::maybe_par` fixes: rust-lang#6767
flip1995 · Apr 1, 2021 · 72eb60a · 72eb60a
2 parents 92c4fc3 + 6325fe1
commit 72eb60a
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 6 deletions.
diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs
@@ -267,17 +267,44 @@ impl<'a> Sugg<'a> {
             Sugg::NonParen(..) => self,
             // `(x)` and `(x).y()` both don't need additional parens.
             Sugg::MaybeParen(sugg) => {
-                if sugg.starts_with('(') && sugg.ends_with(')') {
+                if has_enclosing_paren(&sugg) {
                     Sugg::MaybeParen(sugg)
                 } else {
                     Sugg::NonParen(format!("({})", sugg).into())
                 }
             },
-            Sugg::BinOp(_, sugg) => Sugg::NonParen(format!("({})", sugg).into()),
+            Sugg::BinOp(_, sugg) => {
+                if has_enclosing_paren(&sugg) {
+                    Sugg::NonParen(sugg)
+                } else {
+                    Sugg::NonParen(format!("({})", sugg).into())
+                }
+            },
         }
     }
 }
 
+/// Return `true` if `sugg` is enclosed in parenthesis.
+fn has_enclosing_paren(sugg: impl AsRef<str>) -> bool {
+    let mut chars = sugg.as_ref().chars();
+    if let Some('(') = chars.next() {
+        let mut depth = 1;
+        while let Some(c) = chars.next() {
+            if c == '(' {
+                depth += 1;
+            } else if c == ')' {
+                depth -= 1;
+            }
+            if depth == 0 {
+                break;
+            }
+        }
+        chars.next().is_none()
+    } else {
+        false
+    }
+}
+
 // Copied from the rust standart library, and then edited
 macro_rules! forward_binop_impls_to_ref {
     (impl $imp:ident, $method:ident for $t:ty, type Output = $o:ty) => {
@@ -668,6 +695,8 @@ impl<T: LintContext> DiagnosticBuilderExt<T> for rustc_errors::DiagnosticBuilder
 #[cfg(test)]
 mod test {
     use super::Sugg;
+
+    use rustc_ast::util::parser::AssocOp;
     use std::borrow::Cow;
 
     const SUGGESTION: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("function_call()"));
@@ -681,4 +710,13 @@ mod test {
     fn blockify_transforms_sugg_into_a_block() {
         assert_eq!("{ function_call() }", SUGGESTION.blockify().to_string());
     }
+
+    #[test]
+    fn binop_maybe_par() {
+        let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1)".into());
+        assert_eq!("(1 + 1)", sugg.maybe_par().to_string());
+
+        let sugg = Sugg::BinOp(AssocOp::Add, "(1 + 1) + (1 + 1)".into());
+        assert_eq!("((1 + 1) + (1 + 1))", sugg.maybe_par().to_string());
+    }
 }
diff --git a/tests/ui/floating_point_log.fixed b/tests/ui/floating_point_log.fixed
@@ -27,7 +27,7 @@ fn check_ln1p() {
     let _ = (x / 2.0).ln_1p();
     let _ = x.powi(3).ln_1p();
     let _ = (x.powi(3) / 2.0).ln_1p();
-    let _ = ((std::f32::consts::E - 1.0)).ln_1p();
+    let _ = (std::f32::consts::E - 1.0).ln_1p();
     let _ = x.ln_1p();
     let _ = x.powi(3).ln_1p();
     let _ = (x + 2.0).ln_1p();

diff --git a/tests/ui/floating_point_log.stderr b/tests/ui/floating_point_log.stderr
@@ -90,7 +90,7 @@ error: ln(1 + x) can be computed more accurately
   --> $DIR/floating_point_log.rs:30:13
    |
 LL |     let _ = (1.0 + (std::f32::consts::E - 1.0)).ln();
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `((std::f32::consts::E - 1.0)).ln_1p()`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(std::f32::consts::E - 1.0).ln_1p()`
 
 error: ln(1 + x) can be computed more accurately
   --> $DIR/floating_point_log.rs:31:13

diff --git a/tests/ui/from_str_radix_10.stderr b/tests/ui/from_str_radix_10.stderr
@@ -28,7 +28,7 @@ error: this call to `from_str_radix` can be replaced with a call to `str::parse`
   --> $DIR/from_str_radix_10.rs:32:5
    |
 LL |     u16::from_str_radix(&("10".to_owned() + "5"), 10)?;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `(("10".to_owned() + "5")).parse::<u16>()`
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `("10".to_owned() + "5").parse::<u16>()`
 
 error: this call to `from_str_radix` can be replaced with a call to `str::parse`
   --> $DIR/from_str_radix_10.rs:33:5

diff --git a/tests/ui/manual_memcpy/with_loop_counters.stderr b/tests/ui/manual_memcpy/with_loop_counters.stderr
@@ -43,7 +43,7 @@ LL | /     for i in 3..(3 + src.len()) {
 LL | |         dst[i] = src[count];
 LL | |         count += 1;
 LL | |     }
-   | |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
+   | |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
 
 error: it looks like you're manually copying between slices
   --> $DIR/with_loop_counters.rs:35:5