about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-06-20 15:08:32 +0000
committerbors <bors@rust-lang.org>2022-06-20 15:08:32 +0000
commit93c6f9ebed65eb4d77a5cf1ccf670cef3b1fca9e (patch)
tree42c135810bfe8dd6b87d1fd2ef1b65a8c1f309ec
parent97d451397de3f97fa586297a7bc2d16908d3d842 (diff)
parent39ffda014debd9d1d4126a8996d39af934fe8d94 (diff)
downloadrust-93c6f9ebed65eb4d77a5cf1ccf670cef3b1fca9e.tar.gz
rust-93c6f9ebed65eb4d77a5cf1ccf670cef3b1fca9e.zip
Auto merge of #9006 - kyoto7250:issue-8836-v2, r=Jarcho
feat(fix): ignore `todo!` and `unimplemented!` in `if_same_then_else`

close: #8836
take over:  #8853

This PR adds  check `todo!` and `unimplemented!` in if_same_then_else.
( I thought `unimplemented` should not be checked as well as todo!.)

Thank you in advance.

changelog: ignore todo! and unimplemented! in if_same_then_else

r? `@Jarcho`
-rw-r--r--clippy_utils/src/hir_utils.rs38
-rw-r--r--tests/ui/if_same_then_else.rs61
-rw-r--r--tests/ui/if_same_then_else.stderr20
3 files changed, 107 insertions, 12 deletions
diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs
index af62c4afd5a..74ef2c1bebb 100644
--- a/clippy_utils/src/hir_utils.rs
+++ b/clippy_utils/src/hir_utils.rs
@@ -1,4 +1,5 @@
 use crate::consts::constant_simple;
+use crate::macros::macro_backtrace;
 use crate::source::snippet_opt;
 use rustc_ast::ast::InlineAsmTemplatePiece;
 use rustc_data_structures::fx::FxHasher;
@@ -12,7 +13,7 @@ use rustc_hir::{
 use rustc_lexer::{tokenize, TokenKind};
 use rustc_lint::LateContext;
 use rustc_middle::ty::TypeckResults;
-use rustc_span::Symbol;
+use rustc_span::{sym, Symbol};
 use std::hash::{Hash, Hasher};
 
 /// Type used to check whether two ast are the same. This is different from the
@@ -121,6 +122,9 @@ impl HirEqInterExpr<'_, '_, '_> {
 
     /// Checks whether two blocks are the same.
     fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
+        if self.cannot_be_compared_block(left) || self.cannot_be_compared_block(right) {
+            return false;
+        }
         match (left.stmts, left.expr, right.stmts, right.expr) {
             ([], None, [], None) => {
                 // For empty blocks, check to see if the tokens are equal. This will catch the case where a macro
@@ -171,6 +175,38 @@ impl HirEqInterExpr<'_, '_, '_> {
         }
     }
 
+    fn cannot_be_compared_block(&mut self, block: &Block<'_>) -> bool {
+        if block.stmts.last().map_or(false, |stmt| {
+            matches!(
+                stmt.kind,
+                StmtKind::Semi(semi_expr) if self.should_ignore(semi_expr)
+            )
+        }) {
+            return true;
+        }
+
+        if let Some(block_expr) = block.expr
+            && self.should_ignore(block_expr)
+        {
+            return true
+        }
+
+        false
+    }
+
+    fn should_ignore(&mut self, expr: &Expr<'_>) -> bool {
+        if macro_backtrace(expr.span).last().map_or(false, |macro_call| {
+            matches!(
+                &self.inner.cx.tcx.get_diagnostic_name(macro_call.def_id),
+                Some(sym::todo_macro | sym::unimplemented_macro)
+            )
+        }) {
+            return true;
+        }
+
+        false
+    }
+
     pub fn eq_array_length(&mut self, left: ArrayLen, right: ArrayLen) -> bool {
         match (left, right) {
             (ArrayLen::Infer(..), ArrayLen::Infer(..)) => true,
diff --git a/tests/ui/if_same_then_else.rs b/tests/ui/if_same_then_else.rs
index ef956745500..2598c2ab426 100644
--- a/tests/ui/if_same_then_else.rs
+++ b/tests/ui/if_same_then_else.rs
@@ -6,7 +6,9 @@
     clippy::no_effect,
     clippy::unused_unit,
     clippy::zero_divided_by_zero,
-    clippy::branches_sharing_code
+    clippy::branches_sharing_code,
+    dead_code,
+    unreachable_code
 )]
 
 struct Foo {
@@ -155,4 +157,61 @@ mod issue_5698 {
     }
 }
 
+mod issue_8836 {
+    fn do_not_lint() {
+        if true {
+            todo!()
+        } else {
+            todo!()
+        }
+        if true {
+            todo!();
+        } else {
+            todo!();
+        }
+        if true {
+            unimplemented!()
+        } else {
+            unimplemented!()
+        }
+        if true {
+            unimplemented!();
+        } else {
+            unimplemented!();
+        }
+
+        if true {
+            println!("FOO");
+            todo!();
+        } else {
+            println!("FOO");
+            todo!();
+        }
+
+        if true {
+            println!("FOO");
+            unimplemented!();
+        } else {
+            println!("FOO");
+            unimplemented!();
+        }
+
+        if true {
+            println!("FOO");
+            todo!()
+        } else {
+            println!("FOO");
+            todo!()
+        }
+
+        if true {
+            println!("FOO");
+            unimplemented!()
+        } else {
+            println!("FOO");
+            unimplemented!()
+        }
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/if_same_then_else.stderr b/tests/ui/if_same_then_else.stderr
index 2f38052fc20..2cdf442486a 100644
--- a/tests/ui/if_same_then_else.stderr
+++ b/tests/ui/if_same_then_else.stderr
@@ -1,5 +1,5 @@
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else.rs:21:13
+  --> $DIR/if_same_then_else.rs:23:13
    |
 LL |       if true {
    |  _____________^
@@ -13,7 +13,7 @@ LL | |     } else {
    |
    = note: `-D clippy::if-same-then-else` implied by `-D warnings`
 note: same as this
-  --> $DIR/if_same_then_else.rs:29:12
+  --> $DIR/if_same_then_else.rs:31:12
    |
 LL |       } else {
    |  ____________^
@@ -26,7 +26,7 @@ LL | |     }
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else.rs:65:21
+  --> $DIR/if_same_then_else.rs:67:21
    |
 LL |       let _ = if true {
    |  _____________________^
@@ -35,7 +35,7 @@ LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else.rs:67:12
+  --> $DIR/if_same_then_else.rs:69:12
    |
 LL |       } else {
    |  ____________^
@@ -45,7 +45,7 @@ LL | |     };
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else.rs:72:21
+  --> $DIR/if_same_then_else.rs:74:21
    |
 LL |       let _ = if true {
    |  _____________________^
@@ -54,7 +54,7 @@ LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else.rs:74:12
+  --> $DIR/if_same_then_else.rs:76:12
    |
 LL |       } else {
    |  ____________^
@@ -64,7 +64,7 @@ LL | |     };
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else.rs:88:21
+  --> $DIR/if_same_then_else.rs:90:21
    |
 LL |       let _ = if true {
    |  _____________________^
@@ -73,7 +73,7 @@ LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else.rs:90:12
+  --> $DIR/if_same_then_else.rs:92:12
    |
 LL |       } else {
    |  ____________^
@@ -83,7 +83,7 @@ LL | |     };
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else.rs:95:13
+  --> $DIR/if_same_then_else.rs:97:13
    |
 LL |       if true {
    |  _____________^
@@ -96,7 +96,7 @@ LL | |     } else {
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else.rs:102:12
+  --> $DIR/if_same_then_else.rs:104:12
    |
 LL |       } else {
    |  ____________^