unused_unit: lint also in type parameters and where clauses

diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 5c9117d5b81cd6c62de819caa00c87e626fe8b77..35464f629c3646e1e3031eb82844e676c1e92eea 100644 (file)
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -248,28 +248,7 @@ fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: a
             if let ast::TyKind::Tup(ref vals) = ty.kind;
             if vals.is_empty() && !ty.span.from_expansion() && 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("->") {
-                        #[allow(clippy::cast_possible_truncation)]
-                        (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_sugg(
-                    cx,
-                    UNUSED_UNIT,
-                    rspan,
-                    "unneeded unit return type",
-                    "remove the `-> ()`",
-                    String::new(),
-                    appl,
-                );
+                lint_unneeded_unit_return(cx, ty, span);
             }
         }
     }
@@ -313,6 +292,22 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
             _ => (),
         }
     }
+
+    fn check_poly_trait_ref(&mut self, cx: &EarlyContext<'_>, poly: &ast::PolyTraitRef, _: &ast::TraitBoundModifier) {
+        let segments = &poly.trait_ref.path.segments;
+
+        if_chain! {
+            if segments.len() == 1;
+            if ["Fn", "FnMut", "FnOnce"].contains(&&*segments[0].ident.name.as_str());
+            if let Some(args) = &segments[0].args;
+            if let ast::GenericArgs::Parenthesized(generic_args) = &**args;
+            if let ast::FnRetTy::Ty(ty) = &generic_args.output;
+            if ty.kind.is_unit();
+            then {
+                lint_unneeded_unit_return(cx, ty, generic_args.span);
+            }
+        }
+    }
 }
 
 fn attr_is_cfg(attr: &ast::Attribute) -> bool {
@@ -337,3 +332,28 @@ fn is_unit_expr(expr: &ast::Expr) -> bool {
         false
     }
 }
+
+fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) {
+    let (ret_span, 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("->") {
+            #[allow(clippy::cast_possible_truncation)]
+            (
+                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_sugg(
+        cx,
+        UNUSED_UNIT,
+        ret_span,
+        "unneeded unit return type",
+        "remove the `-> ()`",
+        String::new(),
+        appl,
+    );
+}

diff --git a/tests/ui/unused_unit.fixed b/tests/ui/unused_unit.fixed
index 3f63624720f7553276aea8c6f721bed2c573066e..07f2791786d7f48711378386377019c698b33af4 100644 (file)
--- a/tests/ui/unused_unit.fixed
+++ b/tests/ui/unused_unit.fixed
@@ -14,11 +14,10 @@
 
 struct Unitter;
 impl Unitter {
-    // try to disorient the lint with multiple unit returns and newlines
     #[allow(clippy::no_effect)]
-    pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) 
-    where G: Fn() -> () {
-        let _y: &dyn Fn() -> () = &f;
+    pub fn get_unit<F: Fn() , G>(&self, f: F, _g: G) 
+    where G: Fn()  {
+        let _y: &dyn Fn()  = &f;
         (); // this should not lint, as it's not in return type position
     }
 }
@@ -30,6 +29,20 @@ impl Into<()> for Unitter {
     }
 }
 
+trait Trait {
+    fn redundant<F: FnOnce() , G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() ,
+        H: Fn() ;
+}
+
+impl Trait for Unitter {
+    fn redundant<F: FnOnce() , G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() ,
+        H: Fn()  {}
+}
+
 fn return_unit()  {  }
 
 #[allow(clippy::needless_return)]

diff --git a/tests/ui/unused_unit.rs b/tests/ui/unused_unit.rs
index 8fc072ebd69f848ffc41dd09306febf07e459a96..e2c6afb020f5887dc175b3f8a832fb8293173637 100644 (file)
--- a/tests/ui/unused_unit.rs
+++ b/tests/ui/unused_unit.rs
@@ -14,10 +14,8 @@
 
 struct Unitter;
 impl Unitter {
-    // try to disorient the lint with multiple unit returns and newlines
     #[allow(clippy::no_effect)]
-    pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) ->
-        ()
+    pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
     where G: Fn() -> () {
         let _y: &dyn Fn() -> () = &f;
         (); // this should not lint, as it's not in return type position
@@ -31,6 +29,20 @@ fn into(self) -> () {
     }
 }
 
+trait Trait {
+    fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() -> (),
+        H: Fn() -> ();
+}
+
+impl Trait for Unitter {
+    fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() -> (),
+        H: Fn() -> () {}
+}
+
 fn return_unit() -> () { () }
 
 #[allow(clippy::needless_return)]

diff --git a/tests/ui/unused_unit.stderr b/tests/ui/unused_unit.stderr
index a013d2b3495ba40a4d824e042e71ce6f3d7e873e..81e6738e6bf67b8cdec71e21fc20b5027cfe19b4 100644 (file)
--- a/tests/ui/unused_unit.stderr
+++ b/tests/ui/unused_unit.stderr
@@ -1,10 +1,8 @@
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:19:59
+  --> $DIR/unused_unit.rs:18:29
    |
-LL |       pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) ->
-   |  ___________________________________________________________^
-LL | |         ()
-   | |__________^ help: remove the `-> ()`
+LL |     pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
+   |                             ^^^^^ help: remove the `-> ()`
    |
 note: the lint level is defined here
   --> $DIR/unused_unit.rs:12:9
@@ -13,40 +11,94 @@ LL | #![deny(clippy::unused_unit)]
    |         ^^^^^^^^^^^^^^^^^^^
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:29:19
+  --> $DIR/unused_unit.rs:19:19
+   |
+LL |     where G: Fn() -> () {
+   |                   ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:18:59
+   |
+LL |     pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
+   |                                                           ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:20:27
+   |
+LL |         let _y: &dyn Fn() -> () = &f;
+   |                           ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:27:19
    |
 LL |     fn into(self) -> () {
    |                   ^^^^^ help: remove the `-> ()`
 
 error: unneeded unit expression
-  --> $DIR/unused_unit.rs:30:9
+  --> $DIR/unused_unit.rs:28:9
    |
 LL |         ()
    |         ^^ help: remove the final `()`
 
 error: unneeded unit return type
-  --> $DIR/unused_unit.rs:34:18
+  --> $DIR/unused_unit.rs:33:30
+   |
+LL |     fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
+   |                              ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:35:20
+   |
+LL |         G: FnMut() -> (),
+   |                    ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:36:17
+   |
+LL |         H: Fn() -> ();
+   |                 ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:40:30
+   |
+LL |     fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
+   |                              ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:42:20
+   |
+LL |         G: FnMut() -> (),
+   |                    ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:43:17
+   |
+LL |         H: Fn() -> () {}
+   |                 ^^^^^ help: remove the `-> ()`
+
+error: unneeded unit return type
+  --> $DIR/unused_unit.rs:46:18
    |
 LL | fn return_unit() -> () { () }
    |                  ^^^^^ help: remove the `-> ()`
 
 error: unneeded unit expression
-  --> $DIR/unused_unit.rs:34:26
+  --> $DIR/unused_unit.rs:46:26
    |
 LL | fn return_unit() -> () { () }
    |                          ^^ help: remove the final `()`
 
 error: unneeded `()`
-  --> $DIR/unused_unit.rs:44:14
+  --> $DIR/unused_unit.rs:56:14
    |
 LL |         break();
    |              ^^ help: remove the `()`
 
 error: unneeded `()`
-  --> $DIR/unused_unit.rs:46:11
+  --> $DIR/unused_unit.rs:58:11
    |
 LL |     return();
    |           ^^ help: remove the `()`
 
-error: aborting due to 7 previous errors
+error: aborting due to 16 previous errors