unused unit lint

clippy_lints/src/lib.rs

@@ -685,6 +685,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         regex::TRIVIAL_REGEX,
         returns::LET_AND_RETURN,
         returns::NEEDLESS_RETURN,
+        returns::UNUSED_UNIT,
         serde_api::SERDE_API_MISUSE,
         strings::STRING_LIT_AS_BYTES,
         suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
@@ -801,6 +802,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         regex::TRIVIAL_REGEX,
         returns::LET_AND_RETURN,
         returns::NEEDLESS_RETURN,
+        returns::UNUSED_UNIT,
         strings::STRING_LIT_AS_BYTES,
         types::FN_TO_NUMERIC_CAST,
         types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION,

clippy_lints/src/returns.rs

@@ -14,8 +14,8 @@ use if_chain::if_chain;
 use crate::syntax::ast;
 use crate::syntax::source_map::Span;
 use crate::syntax::visit::FnKind;
+use crate::syntax_pos::BytePos;
 use crate::rustc_errors::Applicability;
-
 use crate::utils::{in_macro, match_path_ast, snippet_opt, span_lint_and_then, span_note_and_lint};
 
 /// **What it does:** Checks for return statements at the end of a block.
@@ -68,6 +68,25 @@ declare_clippy_lint! {
      the end of a block"
 }
 
+/// **What it does:** Checks for unit (`()`) expressions that can be removed.
+///
+/// **Why is this bad?** Such expressions add no value, but can make the code
+/// less readable. Depending on formatting they can make a `break` or `return`
+/// statement look like a function call.
+///
+/// **Known problems:** The lint currently misses unit return types in types,
+/// e.g. the `F` in `fn generic_unit<F: Fn() -> ()>(f: F) { .. }`.
+///
+/// **Example:**
+/// ```rust
+/// fn return_unit() -> () { () }
+/// ```
+declare_clippy_lint! {
+    pub UNUSED_UNIT,
+    style,
+    "needless unit expression"
+}
+
 #[derive(Copy, Clone)]
 pub struct ReturnPass;
 
@@ -162,23 +181,98 @@ impl ReturnPass {
 
 impl LintPass for ReturnPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(NEEDLESS_RETURN, LET_AND_RETURN)
+        lint_array!(NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT)
     }
 }
 
 impl EarlyLintPass for ReturnPass {
-    fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, _: &ast::FnDecl, _: Span, _: ast::NodeId) {
+    fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, decl: &ast::FnDecl, span: Span, _: ast::NodeId) {
         match kind {
             FnKind::ItemFn(.., block) | FnKind::Method(.., block) => self.check_block_return(cx, block),
             FnKind::Closure(body) => self.check_final_expr(cx, body, Some(body.span)),
         }
+        if_chain! {
+            if let ast::FunctionRetTy::Ty(ref ty) = decl.output;
+            if let ast::TyKind::Tup(ref vals) = ty.node;
+            if vals.is_empty() && !in_macro(ty.span) && get_def(span) == get_def(ty.span);
+            then {
+                let (rspan, appl) = if let Ok(fn_source) =
+                        cx.sess().source_map()
+                                 .span_to_snippet(span.with_hi(ty.span.hi())) {
+                    if let Some(rpos) = fn_source.rfind("->") {
+                        (ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
+                            Applicability::MachineApplicable)
+                    } else {
+                        (ty.span, Applicability::MaybeIncorrect)
+                    }
+                } else {
+                    (ty.span, Applicability::MaybeIncorrect)
+                };
+                span_lint_and_then(cx, UNUSED_UNIT, rspan, "unneeded unit return type", |db| {
+                    db.span_suggestion_with_applicability(
+                        rspan,
+                        "remove the `-> ()`",
+                        String::new(),
+                        appl,
+                    );
+                });
+            }
+        }
     }
 
     fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
         self.check_let_return(cx, block);
+        if_chain! {
+            if let Some(ref stmt) = block.stmts.last();
+            if let ast::StmtKind::Expr(ref expr) = stmt.node;
+            if is_unit_expr(expr) && !in_macro(expr.span);
+            then {
+                let sp = expr.span;
+                span_lint_and_then(cx, UNUSED_UNIT, sp, "unneeded unit expression", |db| {
+                    db.span_suggestion_with_applicability(
+                        sp,
+                        "remove the final `()`",
+                        String::new(),
+                        Applicability::MachineApplicable,
+                    );
+                });
+            }
+        }
+    }
+
+    fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
+        match e.node {
+            ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => {
+                if is_unit_expr(expr) && !in_macro(expr.span) {
+                    span_lint_and_then(cx, UNUSED_UNIT, expr.span, "unneeded `()`", |db| {
+                        db.span_suggestion_with_applicability(
+                            expr.span,
+                            "remove the `()`",
+                            String::new(),
+                            Applicability::MachineApplicable,
+                        );
+                    });
+                }
+            }
+            _ => ()
+        }
     }
 }
 
 fn attr_is_cfg(attr: &ast::Attribute) -> bool {
     attr.meta_item_list().is_some() && attr.name() == "cfg"
 }
+
+// get the def site
+fn get_def(span: Span) -> Option<Span> {
+    span.ctxt().outer().expn_info().and_then(|info| info.def_site)
+}
+
+// is this expr a `()` unit?
+fn is_unit_expr(expr: &ast::Expr) -> bool {
+    if let ast::ExprKind::Tup(ref vals) = expr.node {
+        vals.is_empty()
+    } else {
+        false
+    }
+}