Skip to content

Commit

Permalink
Auto merge of rust-lang#5592 - ebroto:extend_unused_unit, r=flip1995
Browse files Browse the repository at this point in the history 
unused_unit: lint also in type parameters and where clauses

changelog: unused_unit now also lints in type parameters and where clauses

Fixes rust-lang#5585
  • Loading branch information
@bors
bors committed May 15, 2020
2 parents e1842b0 + f20b962 commit e22ccf5
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 41 deletions.
64 changes: 42 additions & 22 deletions clippy_lints/src/returns.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
);
}
21 changes: 17 additions & 4 deletions tests/ui/unused_unit.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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)]
Expand Down
18 changes: 15 additions & 3 deletions tests/ui/unused_unit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)]
Expand Down
76 changes: 64 additions & 12 deletions tests/ui/unused_unit.stderr
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

0 comments on commit e22ccf5

Please sign in to comment.