unused_unit: lint also in type parameters and where clauses

rust-lang · May 13, 2020 · f20b962 · f20b962
1 parent b20a9cd
commit f20b962
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 41 deletions.
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
@@ -248,28 +248,7 @@ impl EarlyLintPass for Return {
  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 @@ impl EarlyLintPass for Return {
  _ => (),
  }
  }
+
+ 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
@@ -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
@@ -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 @@ 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.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