Skip to content

Commit 3d646f6

Browse files
committed
Auto merge of #3714 - mikerite:fix-2945, r=oli-obk
Fix `unit_arg` false positive Ignore arguments with the question mark operator. Closes #2945
2 parents 410d5ba + df04238 commit 3d646f6

File tree

3 files changed

+53
-24
lines changed

3 files changed

+53
-24
lines changed

clippy_lints/src/types.rs

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -609,36 +609,43 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitArg {
609609
if in_macro(expr.span) {
610610
return;
611611
}
612+
613+
// apparently stuff in the desugaring of `?` can trigger this
614+
// so check for that here
615+
// only the calls to `Try::from_error` is marked as desugared,
616+
// so we need to check both the current Expr and its parent.
617+
if is_questionmark_desugar_marked_call(expr) {
618+
return;
619+
}
620+
if_chain! {
621+
let map = &cx.tcx.hir();
622+
let opt_parent_node = map.find(map.get_parent_node(expr.id));
623+
if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
624+
if is_questionmark_desugar_marked_call(parent_expr);
625+
then {
626+
return;
627+
}
628+
}
629+
612630
match expr.node {
613631
ExprKind::Call(_, ref args) | ExprKind::MethodCall(_, _, ref args) => {
614632
for arg in args {
615633
if is_unit(cx.tables.expr_ty(arg)) && !is_unit_literal(arg) {
616-
let map = &cx.tcx.hir();
617-
// apparently stuff in the desugaring of `?` can trigger this
618-
// so check for that here
619-
// only the calls to `Try::from_error` is marked as desugared,
620-
// so we need to check both the current Expr and its parent.
621-
if !is_questionmark_desugar_marked_call(expr) {
622-
if_chain! {
623-
let opt_parent_node = map.find(map.get_parent_node(expr.id));
624-
if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
625-
if is_questionmark_desugar_marked_call(parent_expr);
626-
then {}
627-
else {
628-
// `expr` and `parent_expr` where _both_ not from
629-
// desugaring `?`, so lint
630-
span_lint_and_sugg(
631-
cx,
632-
UNIT_ARG,
633-
arg.span,
634-
"passing a unit value to a function",
635-
"if you intended to pass a unit value, use a unit literal instead",
636-
"()".to_string(),
637-
Applicability::MachineApplicable,
638-
);
639-
}
634+
if let ExprKind::Match(.., match_source) = &arg.node {
635+
if *match_source == MatchSource::TryDesugar {
636+
continue;
640637
}
641638
}
639+
640+
span_lint_and_sugg(
641+
cx,
642+
UNIT_ARG,
643+
arg.span,
644+
"passing a unit value to a function",
645+
"if you intended to pass a unit value, use a unit literal instead",
646+
"()".to_string(),
647+
Applicability::MachineApplicable,
648+
);
642649
}
643650
}
644651
},

tests/ui/unit_arg.fixed

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,17 @@ fn question_mark() -> Result<(), ()> {
4747
Ok(())
4848
}
4949

50+
#[allow(dead_code)]
51+
mod issue_2945 {
52+
fn unit_fn() -> Result<(), i32> {
53+
Ok(())
54+
}
55+
56+
fn fallible() -> Result<(), i32> {
57+
Ok(unit_fn()?)
58+
}
59+
}
60+
5061
fn main() {
5162
bad();
5263
ok();

tests/ui/unit_arg.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@ fn question_mark() -> Result<(), ()> {
5454
Ok(())
5555
}
5656

57+
#[allow(dead_code)]
58+
mod issue_2945 {
59+
fn unit_fn() -> Result<(), i32> {
60+
Ok(())
61+
}
62+
63+
fn fallible() -> Result<(), i32> {
64+
Ok(unit_fn()?)
65+
}
66+
}
67+
5768
fn main() {
5869
bad();
5970
ok();

0 commit comments

Comments
 (0)