Skip to content

Commit 3a1159e

Browse files
authored
needless_return: look inside else if parts as well (#14798)
`if` expressions don't necessarily contain a block in the `else` part in the presence of an `else if`. The `else` part, if present, must be handled as a regular expression, not necessarily as a block expression. Found while applying Clippy to triagebot and looking at the result. This also found an issue in Clippy itself. changelog: [`needless_return`]: look inside `else if` parts as well
2 parents 291b8fd + c364717 commit 3a1159e

File tree

5 files changed

+230
-3
lines changed

5 files changed

+230
-3
lines changed

clippy_lints/src/panic_unimplemented.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,6 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented {
152152
expr.span,
153153
"`panic_any` should not be present in production code",
154154
);
155-
return;
156155
}
157156
}
158157
}

clippy_lints/src/returns.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,9 @@ fn check_final_expr<'tcx>(
435435
ExprKind::If(_, then, else_clause_opt) => {
436436
check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone());
437437
if let Some(else_clause) = else_clause_opt {
438-
check_block_return(cx, &else_clause.kind, peeled_drop_expr.span, semi_spans);
438+
// The `RetReplacement` won't be used there as `else_clause` will be either a block or
439+
// a `if` expression.
440+
check_final_expr(cx, else_clause, semi_spans, RetReplacement::Empty, match_ty_opt);
439441
}
440442
},
441443
// a match expr, check all arms

tests/ui/needless_return.fixed

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,3 +452,68 @@ pub unsafe fn issue_12157() -> *const i32 {
452452
(unsafe { todo() } as *const i32)
453453
//~^ needless_return
454454
}
455+
456+
mod else_ifs {
457+
fn test1(a: i32) -> u32 {
458+
if a == 0 {
459+
1
460+
//~^ needless_return
461+
} else if a < 10 {
462+
2
463+
//~^ needless_return
464+
} else {
465+
3
466+
//~^ needless_return
467+
}
468+
}
469+
470+
fn test2(a: i32) -> u32 {
471+
if a == 0 {
472+
1
473+
//~^ needless_return
474+
} else if a < 10 {
475+
2
476+
} else {
477+
3
478+
//~^ needless_return
479+
}
480+
}
481+
482+
fn test3(a: i32) -> u32 {
483+
if a == 0 {
484+
1
485+
//~^ needless_return
486+
} else if a < 10 {
487+
2
488+
} else {
489+
3
490+
//~^ needless_return
491+
}
492+
}
493+
494+
#[allow(clippy::match_single_binding, clippy::redundant_pattern)]
495+
fn test4(a: i32) -> u32 {
496+
if a == 0 {
497+
1
498+
//~^ needless_return
499+
} else if if if a > 0x1_1 {
500+
return 2;
501+
} else {
502+
return 5;
503+
} {
504+
true
505+
} else {
506+
true
507+
} {
508+
0xDEADC0DE
509+
} else if match a {
510+
b @ _ => {
511+
return 1;
512+
},
513+
} {
514+
0xDEADBEEF
515+
} else {
516+
1
517+
}
518+
}
519+
}

tests/ui/needless_return.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,3 +461,68 @@ pub unsafe fn issue_12157() -> *const i32 {
461461
return unsafe { todo() } as *const i32;
462462
//~^ needless_return
463463
}
464+
465+
mod else_ifs {
466+
fn test1(a: i32) -> u32 {
467+
if a == 0 {
468+
return 1;
469+
//~^ needless_return
470+
} else if a < 10 {
471+
return 2;
472+
//~^ needless_return
473+
} else {
474+
return 3;
475+
//~^ needless_return
476+
}
477+
}
478+
479+
fn test2(a: i32) -> u32 {
480+
if a == 0 {
481+
return 1;
482+
//~^ needless_return
483+
} else if a < 10 {
484+
2
485+
} else {
486+
return 3;
487+
//~^ needless_return
488+
}
489+
}
490+
491+
fn test3(a: i32) -> u32 {
492+
if a == 0 {
493+
return 1;
494+
//~^ needless_return
495+
} else if a < 10 {
496+
2
497+
} else {
498+
return 3;
499+
//~^ needless_return
500+
}
501+
}
502+
503+
#[allow(clippy::match_single_binding, clippy::redundant_pattern)]
504+
fn test4(a: i32) -> u32 {
505+
if a == 0 {
506+
return 1;
507+
//~^ needless_return
508+
} else if if if a > 0x1_1 {
509+
return 2;
510+
} else {
511+
return 5;
512+
} {
513+
true
514+
} else {
515+
true
516+
} {
517+
0xDEADC0DE
518+
} else if match a {
519+
b @ _ => {
520+
return 1;
521+
},
522+
} {
523+
0xDEADBEEF
524+
} else {
525+
1
526+
}
527+
}
528+
}

tests/ui/needless_return.stderr

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,5 +685,101 @@ LL - return unsafe { todo() } as *const i32;
685685
LL + (unsafe { todo() } as *const i32)
686686
|
687687

688-
error: aborting due to 55 previous errors
688+
error: unneeded `return` statement
689+
--> tests/ui/needless_return.rs:468:13
690+
|
691+
LL | return 1;
692+
| ^^^^^^^^
693+
|
694+
help: remove `return`
695+
|
696+
LL - return 1;
697+
LL + 1
698+
|
699+
700+
error: unneeded `return` statement
701+
--> tests/ui/needless_return.rs:471:13
702+
|
703+
LL | return 2;
704+
| ^^^^^^^^
705+
|
706+
help: remove `return`
707+
|
708+
LL - return 2;
709+
LL + 2
710+
|
711+
712+
error: unneeded `return` statement
713+
--> tests/ui/needless_return.rs:474:13
714+
|
715+
LL | return 3;
716+
| ^^^^^^^^
717+
|
718+
help: remove `return`
719+
|
720+
LL - return 3;
721+
LL + 3
722+
|
723+
724+
error: unneeded `return` statement
725+
--> tests/ui/needless_return.rs:481:13
726+
|
727+
LL | return 1;
728+
| ^^^^^^^^
729+
|
730+
help: remove `return`
731+
|
732+
LL - return 1;
733+
LL + 1
734+
|
735+
736+
error: unneeded `return` statement
737+
--> tests/ui/needless_return.rs:486:13
738+
|
739+
LL | return 3;
740+
| ^^^^^^^^
741+
|
742+
help: remove `return`
743+
|
744+
LL - return 3;
745+
LL + 3
746+
|
747+
748+
error: unneeded `return` statement
749+
--> tests/ui/needless_return.rs:493:13
750+
|
751+
LL | return 1;
752+
| ^^^^^^^^
753+
|
754+
help: remove `return`
755+
|
756+
LL - return 1;
757+
LL + 1
758+
|
759+
760+
error: unneeded `return` statement
761+
--> tests/ui/needless_return.rs:498:13
762+
|
763+
LL | return 3;
764+
| ^^^^^^^^
765+
|
766+
help: remove `return`
767+
|
768+
LL - return 3;
769+
LL + 3
770+
|
771+
772+
error: unneeded `return` statement
773+
--> tests/ui/needless_return.rs:506:13
774+
|
775+
LL | return 1;
776+
| ^^^^^^^^
777+
|
778+
help: remove `return`
779+
|
780+
LL - return 1;
781+
LL + 1
782+
|
783+
784+
error: aborting due to 63 previous errors
689785

0 commit comments

Comments
 (0)