check for semicolon

Author: J-ZhengLi

clippy_lints/src/question_mark.rs

@@ -3,7 +3,7 @@ use clippy_utils::higher;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{eq_expr_value, is_lang_ctor, path_to_local, peel_blocks, peel_blocks_with_stmt};
+use clippy_utils::{eq_expr_value, is_lang_ctor, path_to_local, path_to_local_id, peel_blocks, peel_blocks_with_stmt};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
@@ -101,14 +101,19 @@ impl QuestionMark {
             // Only check one of the blocks
             let nested_expr = if_else.unwrap_or(if_then);
             if Self::check_lang_items(cx, path1, &[OptionSome, ResultOk, ResultErr]);
-            if let PatKind::Binding(annot, _, ident, _) = fields[0].kind;
+            if let PatKind::Binding(annot, bind_id, ident, _) = fields[0].kind;
             if Self::result_check_and_early_return(cx, let_expr, nested_expr, Some(ident.name))
                 || Self::option_check_and_early_return(cx, let_expr, nested_expr);
             let by_ref = matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut);
             then {
                 let mut applicability = Applicability::MachineApplicable;
                 let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability);
-                let replacement = format!("{}{}?;", receiver_str, if by_ref { ".as_ref()" } else { "" },);
+                let replacement = format!(
+                    "{}{}?{}",
+                    receiver_str,
+                    if by_ref { ".as_ref()" } else { "" },
+                    if path_to_local_id(peel_blocks(if_then), bind_id) { "" } else { ";" }
+                );
 
                 span_lint_and_sugg(
                     cx,
@@ -178,8 +183,10 @@ impl QuestionMark {
         symbol: Option<Symbol>,
     ) -> bool {
         match &peel_blocks_with_stmt(expr).kind {
-            ExprKind::Ret(Some(ret_expr)) => Self::expression_returns_unmodified_err(cx, ret_expr, cond_expr, symbol),
-            ExprKind::Path(_) => path_to_local(expr).is_some() && path_to_local(expr) == path_to_local(cond_expr),
+            ExprKind::Ret(Some(ret_expr)) =>
+                Self::expression_returns_unmodified_err(cx, ret_expr, cond_expr, symbol),
+            ExprKind::Path(_) =>
+                path_to_local(expr).is_some() && path_to_local(expr) == path_to_local(cond_expr),
             ExprKind::Call(_, args_expr) => {
                 if let Some(arg) = args_expr.first() {
                     if let Some(name) = symbol {

tests/ui/question_mark.fixed

@@ -52,7 +52,7 @@ impl CopyStruct {
 
         let _ = Some(self.opt?);
 
-        let _ = self.opt?;;
+        let _ = self.opt?;
 
         self.opt
     }
@@ -82,13 +82,13 @@ impl MoveStruct {
     }
 
     pub fn if_let_ref_func(self) -> Option<Vec<u32>> {
-        let v: &Vec<_> = self.opt.as_ref()?;;
+        let v: &Vec<_> = self.opt.as_ref()?;
 
         Some(v.clone())
     }
 
     pub fn if_let_mov_func(self) -> Option<Vec<u32>> {
-        let v = self.opt?;;
+        let v = self.opt?;
 
         Some(v)
     }
@@ -109,7 +109,7 @@ fn func_returning_result() -> Result<i32, i32> {
 }
 
 fn result_func(x: Result<i32, i32>) -> Result<i32, i32> {
-    let _ = x?;;
+    let _ = x?;
 
     x?;
 

tests/ui/question_mark.stderr

@@ -44,7 +44,7 @@ LL | |             x
 LL | |         } else {
 LL | |             return None;
 LL | |         };
-   | |_________^ help: replace it with: `self.opt?;`
+   | |_________^ help: replace it with: `self.opt?`
 
 error: this block may be rewritten with the `?` operator
   --> $DIR/question_mark.rs:82:9
@@ -79,7 +79,7 @@ LL | |             v
 LL | |         } else {
 LL | |             return None;
 LL | |         };
-   | |_________^ help: replace it with: `self.opt.as_ref()?;`
+   | |_________^ help: replace it with: `self.opt.as_ref()?`
 
 error: this if-let-else may be rewritten with the `?` operator
   --> $DIR/question_mark.rs:115:17
@@ -90,7 +90,7 @@ LL | |             v
 LL | |         } else {
 LL | |             return None;
 LL | |         };
-   | |_________^ help: replace it with: `self.opt?;`
+   | |_________^ help: replace it with: `self.opt?`
 
 error: this block may be rewritten with the `?` operator
   --> $DIR/question_mark.rs:130:5
@@ -104,7 +104,7 @@ error: this if-let-else may be rewritten with the `?` operator
   --> $DIR/question_mark.rs:142:13
    |
 LL |     let _ = if let Ok(x) = x { x } else { return x };
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x?;`
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x?`
 
 error: this block may be rewritten with the `?` operator
   --> $DIR/question_mark.rs:144:5