Merge pull request #2381 from HMPerson1/remove_is_unit_expr

Replace `is_unit_expr`

Author: oli-obk

clippy_lints/src/is_unit_expr.rs

@@ -111,7 +111,6 @@ pub mod if_not_else;
 pub mod infinite_iter;
 pub mod int_plus_one;
 pub mod invalid_ref;
-pub mod is_unit_expr;
 pub mod items_after_statements;
 pub mod large_enum_variant;
 pub mod len_zero;
@@ -248,6 +247,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         "string_to_string",
         "using `string::to_string` is common even today and specialization will likely happen soon",
     );
+    store.register_removed(
+        "unit_expr",
+        "superseded by `let_unit_value` and `unit_arg`",
+    );
     // end deprecated lints, do not remove this comment, it’s used in `update_lints`
 
     reg.register_late_lint_pass(box serde_api::Serde);
@@ -268,7 +271,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box approx_const::Pass);
     reg.register_late_lint_pass(box misc::Pass);
     reg.register_early_lint_pass(box precedence::Precedence);
-    reg.register_early_lint_pass(box is_unit_expr::UnitExpr);
     reg.register_early_lint_pass(box needless_continue::NeedlessContinue);
     reg.register_late_lint_pass(box eta_reduction::EtaPass);
     reg.register_late_lint_pass(box identity_op::IdentityOp);
@@ -365,6 +367,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_early_lint_pass(box const_static_lifetime::StaticConst);
     reg.register_late_lint_pass(box fallible_impl_from::FallibleImplFrom);
     reg.register_late_lint_pass(box replace_consts::ReplaceConsts);
+    reg.register_late_lint_pass(box types::UnitArg);
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -478,7 +481,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
         infinite_iter::INFINITE_ITER,
         invalid_ref::INVALID_REF,
-        is_unit_expr::UNIT_EXPR,
         large_enum_variant::LARGE_ENUM_VARIANT,
         len_zero::LEN_WITHOUT_IS_EMPTY,
         len_zero::LEN_ZERO,
@@ -608,6 +610,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         types::OPTION_OPTION,
         types::TYPE_COMPLEXITY,
         types::UNIT_CMP,
+        types::UNIT_ARG,
         types::UNNECESSARY_CAST,
         unicode::ZERO_WIDTH_SPACE,
         unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,

clippy_lints/src/lib.rs

@@ -361,25 +361,22 @@ declare_lint! {
 
 fn check_let_unit(cx: &LateContext, decl: &Decl) {
     if let DeclLocal(ref local) = decl.node {
-        match cx.tables.pat_ty(&local.pat).sty {
-            ty::TyTuple(slice, _) if slice.is_empty() => {
-                if in_external_macro(cx, decl.span) || in_macro(local.pat.span) {
-                    return;
-                }
-                if higher::is_from_for_desugar(decl) {
-                    return;
-                }
-                span_lint(
-                    cx,
-                    LET_UNIT_VALUE,
-                    decl.span,
-                    &format!(
-                        "this let-binding has unit value. Consider omitting `let {} =`",
-                        snippet(cx, local.pat.span, "..")
-                    ),
-                );
-            },
-            _ => (),
+        if is_unit(cx.tables.pat_ty(&local.pat)) {
+            if in_external_macro(cx, decl.span) || in_macro(local.pat.span) {
+                return;
+            }
+            if higher::is_from_for_desugar(decl) {
+                return;
+            }
+            span_lint(
+                cx,
+                LET_UNIT_VALUE,
+                decl.span,
+                &format!(
+                    "this let-binding has unit value. Consider omitting `let {} =`",
+                    snippet(cx, local.pat.span, "..")
+                ),
+            );
         }
     }
 }
@@ -434,31 +431,118 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitCmp {
         }
         if let ExprBinary(ref cmp, ref left, _) = expr.node {
             let op = cmp.node;
-            if op.is_comparison() {
-                match cx.tables.expr_ty(left).sty {
-                    ty::TyTuple(slice, _) if slice.is_empty() => {
-                        let result = match op {
-                            BiEq | BiLe | BiGe => "true",
-                            _ => "false",
-                        };
-                        span_lint(
-                            cx,
-                            UNIT_CMP,
-                            expr.span,
-                            &format!(
-                                "{}-comparison of unit values detected. This will always be {}",
-                                op.as_str(),
-                                result
-                            ),
-                        );
-                    },
-                    _ => (),
-                }
+            if op.is_comparison() && is_unit(cx.tables.expr_ty(left)) {
+                let result = match op {
+                    BiEq | BiLe | BiGe => "true",
+                    _ => "false",
+                };
+                span_lint(
+                    cx,
+                    UNIT_CMP,
+                    expr.span,
+                    &format!(
+                        "{}-comparison of unit values detected. This will always be {}",
+                        op.as_str(),
+                        result
+                    ),
+                );
             }
         }
     }
 }
 
+/// **What it does:** Checks for passing a unit value as an argument to a function without using a unit literal (`()`).
+///
+/// **Why is this bad?** This is likely the result of an accidental semicolon.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// foo({
+///   let a = bar();
+///   baz(a);
+/// })
+/// ```
+declare_lint! {
+    pub UNIT_ARG,
+    Warn,
+    "passing unit to a function"
+}
+
+pub struct UnitArg;
+
+impl LintPass for UnitArg {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(UNIT_ARG)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitArg {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if in_macro(expr.span) {
+            return;
+        }
+        match expr.node {
+            ExprCall(_, ref args) | ExprMethodCall(_, _, ref args) => {
+                for arg in args {
+                    if is_unit(cx.tables.expr_ty(arg)) && !is_unit_literal(arg) {
+                        let map = &cx.tcx.hir;
+                        // apparently stuff in the desugaring of `?` can trigger this
+                        // so check for that here
+                        // only the calls to `Try::from_error` is marked as desugared,
+                        // so we need to check both the current Expr and its parent.
+                        if !is_questionmark_desugar_marked_call(expr) {
+                            if_chain!{
+                                let opt_parent_node = map.find(map.get_parent_node(expr.id));
+                                if let Some(hir::map::NodeExpr(parent_expr)) = opt_parent_node;
+                                if is_questionmark_desugar_marked_call(parent_expr);
+                                then {}
+                                else {
+                                    // `expr` and `parent_expr` where _both_ not from
+                                    // desugaring `?`, so lint
+                                    span_lint_and_sugg(
+                                        cx,
+                                        UNIT_ARG,
+                                        arg.span,
+                                        "passing a unit value to a function",
+                                        "if you intended to pass a unit value, use a unit literal instead",
+                                        "()".to_string(),
+                                    );
+                                }
+                            }
+                        }
+                    }
+                }
+            },
+            _ => (),
+        }
+    }
+}
+
+fn is_questionmark_desugar_marked_call(expr: &Expr) -> bool {
+    use syntax_pos::hygiene::CompilerDesugaringKind;
+    if let ExprCall(ref callee, _) = expr.node {
+        callee.span.is_compiler_desugaring(CompilerDesugaringKind::QuestionMark)
+    } else {
+        false
+    }
+}
+
+fn is_unit(ty: Ty) -> bool {
+    match ty.sty {
+        ty::TyTuple(slice, _) if slice.is_empty() => true,
+        _ => false,
+    }
+}
+
+fn is_unit_literal(expr: &Expr) -> bool {
+    match expr.node {
+        ExprTup(ref slice) if slice.is_empty() => true,
+        _ => false,
+    }
+}
+
 pub struct CastPass;
 
 /// **What it does:** Checks for casts from any numerical to a float type where