Skip to content

Commit 7cb4cef

Browse files
kyoto7250kyoto7250
committed
feat(fix): ignore todo! and unimplemented! in if_same_then_else
1 parent 9306e9a commit 7cb4cef

File tree

3 files changed

+106
-29
lines changed

3 files changed

+106
-29
lines changed

‎clippy_lints/src/copies.rs

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
2+
use clippy_utils::macros::macro_backtrace;
23
use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
34
use clippy_utils::{
45
eq_expr_value, get_enclosing_block, hash_expr, hash_stmt, if_sequence, is_else_clause, is_lint_allowed,
@@ -7,14 +8,16 @@ use clippy_utils::{
78
use core::iter;
89
use rustc_errors::Applicability;
910
use rustc_hir::intravisit;
10-
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Stmt, StmtKind};
11+
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, QPath, Stmt, StmtKind};
1112
use rustc_lint::{LateContext, LateLintPass};
1213
use rustc_session::{declare_lint_pass, declare_tool_lint};
1314
use rustc_span::hygiene::walk_chain;
1415
use rustc_span::source_map::SourceMap;
15-
use rustc_span::{sym, BytePos, Span, Symbol};
16+
use rustc_span::{BytePos, Span, Symbol};
1617
use std::borrow::Cow;
1718

19+
const ACCEPTABLE_MACRO: [&str; 2] = ["todo", "unimplemented"];
20+
1821
declare_clippy_lint! {
1922
/// ### What it does
2023
/// Checks for consecutive `if`s with the same condition.
@@ -195,7 +198,12 @@ fn lint_if_same_then_else(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&
195198
.array_windows::<2>()
196199
.enumerate()
197200
.fold(true, |all_eq, (i, &[lhs, rhs])| {
198-
if eq.eq_block(lhs, rhs) && !contains_let(conds[i]) && conds.get(i + 1).map_or(true, |e| !contains_let(e)) {
201+
if eq.eq_block(lhs, rhs)
202+
&& !contains_acceptable_macro(cx, lhs)
203+
&& !contains_acceptable_macro(cx, rhs)
204+
&& !contains_let(conds[i])
205+
&& conds.get(i + 1).map_or(true, |e| !contains_let(e))
206+
{
199207
span_lint_and_note(
200208
cx,
201209
IF_SAME_THEN_ELSE,
@@ -365,19 +373,33 @@ fn eq_stmts(
365373
.all(|b| get_stmt(b).map_or(false, |s| eq.eq_stmt(s, stmt)))
366374
}
367375

368-
fn block_contains_todo_macro(cx: &LateContext<'_>, block: &Block<'_>) -> bool {
369-
dbg!(block);
370-
if let Some(macro_def_id) = block.span.ctxt().outer_expn_data().macro_def_id {
371-
dbg!(macro_def_id);
372-
if let Some(diagnostic_name) = cx.tcx.get_diagnostic_name(macro_def_id) {
373-
dbg!(diagnostic_name);
374-
diagnostic_name == sym::todo_macro
375-
} else {
376-
false
376+
fn contains_acceptable_macro(cx: &LateContext<'_>, block: &Block<'_>) -> bool {
377+
for stmt in block.stmts {
378+
match stmt.kind {
379+
StmtKind::Semi(semi_expr) if acceptable_macro(cx, semi_expr) => return true,
380+
_ => {},
377381
}
378-
} else {
379-
false
380382
}
383+
384+
if let Some(block_expr) = block.expr
385+
&& acceptable_macro(cx, block_expr)
386+
{
387+
return true
388+
}
389+
390+
false
391+
}
392+
393+
fn acceptable_macro(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
394+
if let ExprKind::Call(call_expr, _) = expr.kind
395+
&& let ExprKind::Path(QPath::Resolved(None, path)) = call_expr.kind
396+
&& macro_backtrace(path.span).any(|macro_call| {
397+
ACCEPTABLE_MACRO.contains(&cx.tcx.item_name(macro_call.def_id).as_str())
398+
}) {
399+
return true;
400+
}
401+
402+
false
381403
}
382404

383405
fn scan_block_for_eq(cx: &LateContext<'_>, _conds: &[&Expr<'_>], block: &Block<'_>, blocks: &[&Block<'_>]) -> BlockEq {
@@ -413,7 +435,6 @@ fn scan_block_for_eq(cx: &LateContext<'_>, _conds: &[&Expr<'_>], block: &Block<'
413435
moved_locals,
414436
};
415437
}
416-
417438
let end_search_start = block.stmts[start_end_eq..]
418439
.iter()
419440
.rev()

‎tests/ui/if_same_then_else.rs

Lines changed: 60 additions & 4 deletions
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 {
@@ -126,9 +128,6 @@ fn if_same_then_else() {
126128
_ => 4,
127129
};
128130
}
129-
130-
// Issue #8836
131-
if true { todo!() } else { todo!() }
132131
}
133132

134133
// Issue #2423. This was causing an ICE.
@@ -158,4 +157,61 @@ mod issue_5698 {
158157
}
159158
}
160159

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+
161217
fn main() {}

‎tests/ui/if_same_then_else.stderr

Lines changed: 10 additions & 10 deletions
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)