Skip to content

Commit 93c6f9e

Browse files
committed
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`
2 parents 97d4513 + 39ffda0 commit 93c6f9e

File tree

3 files changed

+107
-12
lines changed

3 files changed

+107
-12
lines changed

clippy_utils/src/hir_utils.rs

+37-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::consts::constant_simple;
2+
use crate::macros::macro_backtrace;
23
use crate::source::snippet_opt;
34
use rustc_ast::ast::InlineAsmTemplatePiece;
45
use rustc_data_structures::fx::FxHasher;
@@ -12,7 +13,7 @@ use rustc_hir::{
1213
use rustc_lexer::{tokenize, TokenKind};
1314
use rustc_lint::LateContext;
1415
use rustc_middle::ty::TypeckResults;
15-
use rustc_span::Symbol;
16+
use rustc_span::{sym, Symbol};
1617
use std::hash::{Hash, Hasher};
1718

1819
/// Type used to check whether two ast are the same. This is different from the
@@ -121,6 +122,9 @@ impl HirEqInterExpr<'_, '_, '_> {
121122

122123
/// Checks whether two blocks are the same.
123124
fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool {
125+
if self.cannot_be_compared_block(left) || self.cannot_be_compared_block(right) {
126+
return false;
127+
}
124128
match (left.stmts, left.expr, right.stmts, right.expr) {
125129
([], None, [], None) => {
126130
// 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<'_, '_, '_> {
171175
}
172176
}
173177

178+
fn cannot_be_compared_block(&mut self, block: &Block<'_>) -> bool {
179+
if block.stmts.last().map_or(false, |stmt| {
180+
matches!(
181+
stmt.kind,
182+
StmtKind::Semi(semi_expr) if self.should_ignore(semi_expr)
183+
)
184+
}) {
185+
return true;
186+
}
187+
188+
if let Some(block_expr) = block.expr
189+
&& self.should_ignore(block_expr)
190+
{
191+
return true
192+
}
193+
194+
false
195+
}
196+
197+
fn should_ignore(&mut self, expr: &Expr<'_>) -> bool {
198+
if macro_backtrace(expr.span).last().map_or(false, |macro_call| {
199+
matches!(
200+
&self.inner.cx.tcx.get_diagnostic_name(macro_call.def_id),
201+
Some(sym::todo_macro | sym::unimplemented_macro)
202+
)
203+
}) {
204+
return true;
205+
}
206+
207+
false
208+
}
209+
174210
pub fn eq_array_length(&mut self, left: ArrayLen, right: ArrayLen) -> bool {
175211
match (left, right) {
176212
(ArrayLen::Infer(..), ArrayLen::Infer(..)) => true,

tests/ui/if_same_then_else.rs

+60-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
clippy::no_effect,
77
clippy::unused_unit,
88
clippy::zero_divided_by_zero,
9-
clippy::branches_sharing_code
9+
clippy::branches_sharing_code,
10+
dead_code,
11+
unreachable_code
1012
)]
1113

1214
struct Foo {
@@ -155,4 +157,61 @@ mod issue_5698 {
155157
}
156158
}
157159

160+
mod issue_8836 {
161+
fn do_not_lint() {
162+
if true {
163+
todo!()
164+
} else {
165+
todo!()
166+
}
167+
if true {
168+
todo!();
169+
} else {
170+
todo!();
171+
}
172+
if true {
173+
unimplemented!()
174+
} else {
175+
unimplemented!()
176+
}
177+
if true {
178+
unimplemented!();
179+
} else {
180+
unimplemented!();
181+
}
182+
183+
if true {
184+
println!("FOO");
185+
todo!();
186+
} else {
187+
println!("FOO");
188+
todo!();
189+
}
190+
191+
if true {
192+
println!("FOO");
193+
unimplemented!();
194+
} else {
195+
println!("FOO");
196+
unimplemented!();
197+
}
198+
199+
if true {
200+
println!("FOO");
201+
todo!()
202+
} else {
203+
println!("FOO");
204+
todo!()
205+
}
206+
207+
if true {
208+
println!("FOO");
209+
unimplemented!()
210+
} else {
211+
println!("FOO");
212+
unimplemented!()
213+
}
214+
}
215+
}
216+
158217
fn main() {}

tests/ui/if_same_then_else.stderr

+10-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `if` has identical blocks
2-
--> $DIR/if_same_then_else.rs:21:13
2+
--> $DIR/if_same_then_else.rs:23:13
33
|
44
LL | if true {
55
| _____________^
@@ -13,7 +13,7 @@ LL | | } else {
1313
|
1414
= note: `-D clippy::if-same-then-else` implied by `-D warnings`
1515
note: same as this
16-
--> $DIR/if_same_then_else.rs:29:12
16+
--> $DIR/if_same_then_else.rs:31:12
1717
|
1818
LL | } else {
1919
| ____________^
@@ -26,7 +26,7 @@ LL | | }
2626
| |_____^
2727

2828
error: this `if` has identical blocks
29-
--> $DIR/if_same_then_else.rs:65:21
29+
--> $DIR/if_same_then_else.rs:67:21
3030
|
3131
LL | let _ = if true {
3232
| _____________________^
@@ -35,7 +35,7 @@ LL | | } else {
3535
| |_____^
3636
|
3737
note: same as this
38-
--> $DIR/if_same_then_else.rs:67:12
38+
--> $DIR/if_same_then_else.rs:69:12
3939
|
4040
LL | } else {
4141
| ____________^
@@ -45,7 +45,7 @@ LL | | };
4545
| |_____^
4646

4747
error: this `if` has identical blocks
48-
--> $DIR/if_same_then_else.rs:72:21
48+
--> $DIR/if_same_then_else.rs:74:21
4949
|
5050
LL | let _ = if true {
5151
| _____________________^
@@ -54,7 +54,7 @@ LL | | } else {
5454
| |_____^
5555
|
5656
note: same as this
57-
--> $DIR/if_same_then_else.rs:74:12
57+
--> $DIR/if_same_then_else.rs:76:12
5858
|
5959
LL | } else {
6060
| ____________^
@@ -64,7 +64,7 @@ LL | | };
6464
| |_____^
6565

6666
error: this `if` has identical blocks
67-
--> $DIR/if_same_then_else.rs:88:21
67+
--> $DIR/if_same_then_else.rs:90:21
6868
|
6969
LL | let _ = if true {
7070
| _____________________^
@@ -73,7 +73,7 @@ LL | | } else {
7373
| |_____^
7474
|
7575
note: same as this
76-
--> $DIR/if_same_then_else.rs:90:12
76+
--> $DIR/if_same_then_else.rs:92:12
7777
|
7878
LL | } else {
7979
| ____________^
@@ -83,7 +83,7 @@ LL | | };
8383
| |_____^
8484

8585
error: this `if` has identical blocks
86-
--> $DIR/if_same_then_else.rs:95:13
86+
--> $DIR/if_same_then_else.rs:97:13
8787
|
8888
LL | if true {
8989
| _____________^
@@ -96,7 +96,7 @@ LL | | } else {
9696
| |_____^
9797
|
9898
note: same as this
99-
--> $DIR/if_same_then_else.rs:102:12
99+
--> $DIR/if_same_then_else.rs:104:12
100100
|
101101
LL | } else {
102102
| ____________^

0 commit comments

Comments
 (0)