Skip to content

Commit 0919c67

Browse files
samueltardieusamueltardieu
committed
New lint: unit_as_impl_trait
1 parent d7b27ec commit 0919c67

12 files changed

+270
-4
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6371,6 +6371,7 @@ Released 2018-09-13
63716371
[`uninit_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_vec
63726372
[`uninlined_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
63736373
[`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
6374+
[`unit_as_impl_trait`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_as_impl_trait
63746375
[`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp
63756376
[`unit_hash`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_hash
63766377
[`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
738738
crate::unicode::UNICODE_NOT_NFC_INFO,
739739
crate::uninhabited_references::UNINHABITED_REFERENCES_INFO,
740740
crate::uninit_vec::UNINIT_VEC_INFO,
741+
crate::unit_as_impl_trait::UNIT_AS_IMPL_TRAIT_INFO,
741742
crate::unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD_INFO,
742743
crate::unit_types::LET_UNIT_VALUE_INFO,
743744
crate::unit_types::UNIT_ARG_INFO,

‎clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,7 @@ mod undocumented_unsafe_blocks;
369369
mod unicode;
370370
mod uninhabited_references;
371371
mod uninit_vec;
372+
mod unit_as_impl_trait;
372373
mod unit_return_expecting_ord;
373374
mod unit_types;
374375
mod unnecessary_box_returns;
@@ -948,5 +949,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
948949
store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix));
949950
store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
950951
store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
952+
store.register_late_pass(|_| Box::new(unit_as_impl_trait::UnitAsImplTrait));
951953
// add lints here, do not remove this comment, it's used in `new_lint`
952954
}

‎clippy_lints/src/unit_as_impl_trait.rs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use rustc_hir::def_id::LocalDefId;
3+
use rustc_hir::intravisit::FnKind;
4+
use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy, TyKind};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::declare_lint_pass;
7+
use rustc_span::{BytePos, Span};
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Checks for function whose return type is an `impl` trait
12+
/// implemented by `()`, and whose `()` return value is implicit.
13+
///
14+
/// ### Why is this bad?
15+
/// Since the required trait is implemented by `()`, adding an extra `;`
16+
/// at the end of the function last expression will compile with no
17+
/// indication that the expected value may have been silently swallowed.
18+
///
19+
/// ### Example
20+
/// ```no_run
21+
/// fn key() -> impl Eq {
22+
/// [1, 10, 2, 0].sort_unstable()
23+
/// }
24+
/// ```
25+
/// Use instead:
26+
/// ```no_run
27+
/// fn key() -> impl Eq {
28+
/// let mut arr = [1, 10, 2, 0];
29+
/// arr.sort_unstable();
30+
/// arr
31+
/// }
32+
/// ```
33+
/// or
34+
/// ```no_run
35+
/// fn key() -> impl Eq {
36+
/// [1, 10, 2, 0].sort_unstable();
37+
/// ()
38+
/// }
39+
/// ```
40+
/// if returning `()` is intentional.
41+
#[clippy::version = "1.86.0"]
42+
pub UNIT_AS_IMPL_TRAIT,
43+
suspicious,
44+
"`()` which implements the required trait might be returned unexpectedly"
45+
}
46+
47+
declare_lint_pass!(UnitAsImplTrait => [UNIT_AS_IMPL_TRAIT]);
48+
49+
impl<'tcx> LateLintPass<'tcx> for UnitAsImplTrait {
50+
fn check_fn(
51+
&mut self,
52+
cx: &LateContext<'tcx>,
53+
_: FnKind<'tcx>,
54+
decl: &'tcx FnDecl<'tcx>,
55+
body: &'tcx Body<'tcx>,
56+
span: Span,
57+
_: LocalDefId,
58+
) {
59+
if !span.from_expansion()
60+
&& let FnRetTy::Return(ty) = decl.output
61+
&& let TyKind::OpaqueDef(_) = ty.kind
62+
&& cx.typeck_results().expr_ty(body.value).is_unit()
63+
&& let ExprKind::Block(block, None) = body.value.kind
64+
&& block.expr.is_none_or(|e| !matches!(e.kind, ExprKind::Tup([])))
65+
{
66+
let block_end_span = block.span.with_hi(block.span.hi() - BytePos(1)).shrink_to_hi();
67+
68+
span_lint_and_then(
69+
cx,
70+
UNIT_AS_IMPL_TRAIT,
71+
ty.span,
72+
"this function returns `()` which implements the required trait",
73+
|diag| {
74+
if let Some(expr) = block.expr {
75+
diag.span_note(expr.span, "this expression evaluates to `()`")
76+
.span_help(
77+
expr.span.shrink_to_hi(),
78+
"consider being explicit, and terminate the body with `()`",
79+
);
80+
} else if let Some(last) = block.stmts.last() {
81+
diag.span_note(last.span, "this statement evaluates to `()` because it ends with `;`")
82+
.span_note(block.span, "therefore the function body evaluates to `()`")
83+
.span_help(
84+
block_end_span,
85+
"if this is intentional, consider being explicit, and terminate the body with `()`",
86+
);
87+
} else {
88+
diag.span_note(block.span, "the empty body evaluates to `()`")
89+
.span_help(block_end_span, "consider being explicit and use `()` in the body");
90+
}
91+
},
92+
);
93+
}
94+
}
95+
}

‎tests/ui/crashes/ice-11422.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::implied_bounds_in_impls)]
2+
#![allow(clippy::unit_as_impl_trait)]
23

34
use std::fmt::Debug;
45
use std::ops::*;

‎tests/ui/crashes/ice-11422.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::implied_bounds_in_impls)]
2+
#![allow(clippy::unit_as_impl_trait)]
23

34
use std::fmt::Debug;
45
use std::ops::*;

‎tests/ui/crashes/ice-11422.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this bound is already specified as the supertrait of `PartialOrd`
2-
--> tests/ui/crashes/ice-11422.rs:6:33
2+
--> tests/ui/crashes/ice-11422.rs:7:33
33
|
44
LL | fn r#gen() -> impl PartialOrd + PartialEq + Debug {}
55
| ^^^^^^^^^

‎tests/ui/implied_bounds_in_impls.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::implied_bounds_in_impls)]
2-
#![allow(dead_code)]
2+
#![allow(dead_code, clippy::unit_as_impl_trait)]
33
#![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
44

55
use std::ops::{Deref, DerefMut};

‎tests/ui/implied_bounds_in_impls.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::implied_bounds_in_impls)]
2-
#![allow(dead_code)]
2+
#![allow(dead_code, clippy::unit_as_impl_trait)]
33
#![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
44

55
use std::ops::{Deref, DerefMut};

‎tests/ui/new_ret_no_self.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![feature(type_alias_impl_trait)]
22
#![warn(clippy::new_ret_no_self)]
3-
#![allow(dead_code)]
3+
#![allow(dead_code, clippy::unit_as_impl_trait)]
44

55
fn main() {}
66

‎tests/ui/unit_as_impl_trait.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#![warn(clippy::unit_as_impl_trait)]
2+
#![allow(clippy::unused_unit)]
3+
4+
fn implicit_unit() -> impl Copy {
5+
//~^ ERROR: function returns `()` which implements the required trait
6+
}
7+
8+
fn explicit_unit() -> impl Copy {
9+
()
10+
}
11+
12+
fn not_unit(x: u32) -> impl Copy {
13+
x
14+
}
15+
16+
fn never(x: u32) -> impl Copy {
17+
panic!();
18+
}
19+
20+
fn with_generic_param<T: Eq>(x: T) -> impl Eq {
21+
//~^ ERROR: function returns `()` which implements the required trait
22+
x;
23+
}
24+
25+
fn non_empty_implicit_unit() -> impl Copy {
26+
//~^ ERROR: function returns `()` which implements the required trait
27+
println!("foobar");
28+
}
29+
30+
fn last_expression_returning_unit() -> impl Eq {
31+
//~^ ERROR: function returns `()` which implements the required trait
32+
[1, 10, 2, 0].sort_unstable()
33+
}
34+
35+
#[derive(Clone)]
36+
struct S;
37+
38+
impl S {
39+
fn clone(&self) -> impl Clone {
40+
//~^ ERROR: function returns `()` which implements the required trait
41+
S;
42+
}
43+
}
44+
45+
fn main() {}

‎tests/ui/unit_as_impl_trait.stderr

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
error: this function returns `()` which implements the required trait
2+
--> tests/ui/unit_as_impl_trait.rs:4:23
3+
|
4+
LL | fn implicit_unit() -> impl Copy {
5+
| ^^^^^^^^^
6+
|
7+
note: the empty body evaluates to `()`
8+
--> tests/ui/unit_as_impl_trait.rs:4:33
9+
|
10+
LL | fn implicit_unit() -> impl Copy {
11+
| _________________________________^
12+
LL | |
13+
LL | | }
14+
| |_^
15+
help: consider being explicit and use `()` in the body
16+
--> tests/ui/unit_as_impl_trait.rs:6:1
17+
|
18+
LL | }
19+
| ^
20+
= note: `-D clippy::unit-as-impl-trait` implied by `-D warnings`
21+
= help: to override `-D warnings` add `#[allow(clippy::unit_as_impl_trait)]`
22+
23+
error: this function returns `()` which implements the required trait
24+
--> tests/ui/unit_as_impl_trait.rs:20:39
25+
|
26+
LL | fn with_generic_param<T: Eq>(x: T) -> impl Eq {
27+
| ^^^^^^^
28+
|
29+
note: this statement evaluates to `()` because it ends with `;`
30+
--> tests/ui/unit_as_impl_trait.rs:22:5
31+
|
32+
LL | x;
33+
| ^^
34+
note: therefore the function body evaluates to `()`
35+
--> tests/ui/unit_as_impl_trait.rs:20:47
36+
|
37+
LL | fn with_generic_param<T: Eq>(x: T) -> impl Eq {
38+
| _______________________________________________^
39+
LL | |
40+
LL | | x;
41+
LL | | }
42+
| |_^
43+
help: if this is intentional, consider being explicit, and terminate the body with `()`
44+
--> tests/ui/unit_as_impl_trait.rs:23:1
45+
|
46+
LL | }
47+
| ^
48+
49+
error: this function returns `()` which implements the required trait
50+
--> tests/ui/unit_as_impl_trait.rs:25:33
51+
|
52+
LL | fn non_empty_implicit_unit() -> impl Copy {
53+
| ^^^^^^^^^
54+
|
55+
note: this statement evaluates to `()` because it ends with `;`
56+
--> tests/ui/unit_as_impl_trait.rs:27:5
57+
|
58+
LL | println!("foobar");
59+
| ^^^^^^^^^^^^^^^^^^
60+
note: therefore the function body evaluates to `()`
61+
--> tests/ui/unit_as_impl_trait.rs:25:43
62+
|
63+
LL | fn non_empty_implicit_unit() -> impl Copy {
64+
| ___________________________________________^
65+
LL | |
66+
LL | | println!("foobar");
67+
LL | | }
68+
| |_^
69+
help: if this is intentional, consider being explicit, and terminate the body with `()`
70+
--> tests/ui/unit_as_impl_trait.rs:28:1
71+
|
72+
LL | }
73+
| ^
74+
= note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
75+
76+
error: this function returns `()` which implements the required trait
77+
--> tests/ui/unit_as_impl_trait.rs:30:40
78+
|
79+
LL | fn last_expression_returning_unit() -> impl Eq {
80+
| ^^^^^^^
81+
|
82+
note: this expression evaluates to `()`
83+
--> tests/ui/unit_as_impl_trait.rs:32:5
84+
|
85+
LL | [1, 10, 2, 0].sort_unstable()
86+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
87+
help: consider being explicit, and terminate the body with `()`
88+
--> tests/ui/unit_as_impl_trait.rs:32:34
89+
|
90+
LL | [1, 10, 2, 0].sort_unstable()
91+
| ^
92+
93+
error: this function returns `()` which implements the required trait
94+
--> tests/ui/unit_as_impl_trait.rs:39:24
95+
|
96+
LL | fn clone(&self) -> impl Clone {
97+
| ^^^^^^^^^^
98+
|
99+
note: this statement evaluates to `()` because it ends with `;`
100+
--> tests/ui/unit_as_impl_trait.rs:41:9
101+
|
102+
LL | S;
103+
| ^^
104+
note: therefore the function body evaluates to `()`
105+
--> tests/ui/unit_as_impl_trait.rs:39:35
106+
|
107+
LL | fn clone(&self) -> impl Clone {
108+
| ___________________________________^
109+
LL | |
110+
LL | | S;
111+
LL | | }
112+
| |_____^
113+
help: if this is intentional, consider being explicit, and terminate the body with `()`
114+
--> tests/ui/unit_as_impl_trait.rs:42:5
115+
|
116+
LL | }
117+
| ^
118+
119+
error: aborting due to 5 previous errors
120+

0 commit comments

Comments
 (0)