Skip to content

Commit

Permalink
Rollup merge of rust-lang#114819 - estebank:issue-78124, r=compiler-e…
Browse files Browse the repository at this point in the history
…rrors

Point at return type when it influences non-first `match` arm

When encountering code like

```rust
fn foo() -> i32 {
    match 0 {
        1 => return 0,
        2 => "",
        _ => 1,
    }
}
```

Point at the return type and not at the prior arm, as that arm has type `!` which isn't influencing the arm corresponding to arm `2`.

Fix rust-lang#78124.
  • Loading branch information
matthiaskrgr authored Aug 15, 2023
2 parents 9e33b69 + d0a26f1 commit 39219a6
Show file tree
Hide file tree
Showing 16 changed files with 17 additions and 35 deletions.
3 changes: 2 additions & 1 deletion clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,8 @@ fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> boo
match parent.kind {
ExprKind::Call(child, _) | ExprKind::MethodCall(_, child, _, _) | ExprKind::Index(child, _, _)
if child.hir_id == e.hir_id => true,
ExprKind::Field(_, _) | ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) => true,
ExprKind::Match(.., MatchSource::TryDesugar(_) | MatchSource::AwaitDesugar)
| ExprKind::Field(_, _) => true,
_ => false,
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
wild_in_or_pats::check(cx, arms);
}

if source == MatchSource::TryDesugar {
if let MatchSource::TryDesugar(_) = source {
try_err::check(cx, expr, ex);
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/matches/try_err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine

/// Finds function return type by examining return expressions in match arms.
fn find_return_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx ExprKind<'_>) -> Option<Ty<'tcx>> {
if let ExprKind::Match(_, arms, MatchSource::TryDesugar) = expr {
if let ExprKind::Match(_, arms, MatchSource::TryDesugar(_)) = expr {
for arm in *arms {
if let ExprKind::Ret(Some(ret)) = arm.body.kind {
return Some(cx.typeck_results().expr_ty(ret));
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/clone_on_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub(super) fn check(
ExprKind::Path(QPath::LangItem(rustc_hir::LangItem::TryTraitBranch, _, _))
),
ExprKind::MethodCall(_, self_arg, ..) if expr.hir_id == self_arg.hir_id => true,
ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar)
ExprKind::Match(_, _, MatchSource::TryDesugar(_) | MatchSource::AwaitDesugar)
| ExprKind::Field(..)
| ExprKind::Index(..) => true,
_ => false,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/str_splitn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ fn indirect_usage<'tcx>(
!matches!(
node,
Node::Expr(Expr {
kind: ExprKind::Match(.., MatchSource::TryDesugar),
kind: ExprKind::Match(.., MatchSource::TryDesugar(_)),
..
})
)
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/needless_question_mark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
} else {
return;
};
if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &arg.kind;
if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar(_)) = &arg.kind;
if let ExprKind::Call(called, [inner_expr]) = &inner_expr_with_q.kind;
if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, ..)) = &called.kind;
if expr.span.ctxt() == inner_expr.span.ctxt();
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/question_mark_used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ declare_lint_pass!(QuestionMarkUsed => [QUESTION_MARK_USED]);

impl<'tcx> LateLintPass<'tcx> for QuestionMarkUsed {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Match(_, _, MatchSource::TryDesugar) = expr.kind {
if let ExprKind::Match(_, _, MatchSource::TryDesugar(_)) = expr.kind {
if !span_is_local(expr.span) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/redundant_closure_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ReturnVisitor {

impl<'tcx> Visitor<'tcx> for ReturnVisitor {
fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) {
if let hir::ExprKind::Ret(_) | hir::ExprKind::Match(.., hir::MatchSource::TryDesugar) = ex.kind {
if let hir::ExprKind::Ret(_) | hir::ExprKind::Match(.., hir::MatchSource::TryDesugar(_)) = ex.kind {
self.found_return = true;
} else {
hir_visit::walk_expr(self, ex);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/returns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ impl<'tcx> LateLintPass<'tcx> for Return {
if !in_external_macro(cx.sess(), stmt.span)
&& let StmtKind::Semi(expr) = stmt.kind
&& let ExprKind::Ret(Some(ret)) = expr.kind
&& let ExprKind::Match(.., MatchSource::TryDesugar) = ret.kind
&& let ExprKind::Match(.., MatchSource::TryDesugar(_)) = ret.kind
// Ensure this is not the final stmt, otherwise removing it would cause a compile error
&& let OwnerNode::Item(item) = cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id))
&& let ItemKind::Fn(_, _, body) = item.kind
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/unit_types/unit_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if cx.typeck_results().expr_ty(arg).is_unit() && !utils::is_unit_literal(arg) {
!matches!(
&arg.kind,
ExprKind::Match(.., MatchSource::TryDesugar) | ExprKind::Path(..)
ExprKind::Match(.., MatchSource::TryDesugar(_)) | ExprKind::Path(..)
)
} else {
false
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/useless_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
}

match e.kind {
ExprKind::Match(_, arms, MatchSource::TryDesugar) => {
ExprKind::Match(_, arms, MatchSource::TryDesugar(_)) => {
let (ExprKind::Ret(Some(e)) | ExprKind::Break(_, Some(e))) = arms[0].body.kind else {
return;
};
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/check_proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) {
(Pat::Str("for"), Pat::Str("}"))
},
ExprKind::Match(_, _, MatchSource::Normal) => (Pat::Str("match"), Pat::Str("}")),
ExprKind::Match(e, _, MatchSource::TryDesugar) => (expr_search_pat(tcx, e).0, Pat::Str("?")),
ExprKind::Match(e, _, MatchSource::TryDesugar(_)) => (expr_search_pat(tcx, e).0, Pat::Str("?")),
ExprKind::Match(e, _, MatchSource::AwaitDesugar) | ExprKind::Yield(e, YieldSource::Await { .. }) => {
(expr_search_pat(tcx, e).0, Pat::Str("await"))
},
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1765,7 +1765,7 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc

if let ExprKind::Match(_, arms, ref source) = expr.kind {
// desugared from a `?` operator
if *source == MatchSource::TryDesugar {
if let MatchSource::TryDesugar(_) = *source {
return Some(expr);
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/visitors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub fn for_each_expr_with_closures<'tcx, B, C: Continue>(
/// returns `true` if expr contains match expr desugared from try
fn contains_try(expr: &hir::Expr<'_>) -> bool {
for_each_expr(expr, |e| {
if matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar)) {
if matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar(_))) {
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/if_same_then_else2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ fn if_same_then_else2() -> Result<&'static str, ()> {
};

if true {
//~^ ERROR: this `if` has identical blocks
// FIXME: should emit "this `if` has identical blocks"
Ok("foo")?;
} else {
Ok("foo")?;
Expand Down
21 changes: 1 addition & 20 deletions tests/ui/if_same_then_else2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -82,25 +82,6 @@ LL | | f32::NAN
LL | | };
| |_____^

error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:100:13
|
LL | if true {
| _____________^
LL | |
LL | | Ok("foo")?;
LL | | } else {
| |_____^
|
note: same as this
--> $DIR/if_same_then_else2.rs:103:12
|
LL | } else {
| ____________^
LL | | Ok("foo")?;
LL | | }
| |_____^

error: this `if` has identical blocks
--> $DIR/if_same_then_else2.rs:124:20
|
Expand All @@ -122,5 +103,5 @@ LL | | return Ok(&foo[0..]);
LL | | }
| |_____^

error: aborting due to 6 previous errors
error: aborting due to 5 previous errors

0 comments on commit 39219a6

Please sign in to comment.