Skip to content

Commit 25e9acb

Browse files
borsbors
committed
Auto merge of #6476 - 1c3t3a:1c3t3a-from-over-into, r=llogiq
Added from_over_into lint Closes #6456 Added a lint that searches for implementations of `Into<..>` and suggests to implement `From<..>` instead, as it comes with a default implementation of `Into`. Category: style. changelog: added `from_over_into` lint
2 parents 4b233c8 + 53f4d43 commit 25e9acb

10 files changed

+158
-23
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,6 +1841,7 @@ Released 2018-09-13
18411841
[`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
18421842
[`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
18431843
[`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect
1844+
[`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into
18441845
[`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send
18451846
[`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
18461847
[`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap

‎clippy_lints/src/from_over_into.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use crate::utils::paths::INTO;
2+
use crate::utils::{match_def_path, meets_msrv, span_lint_and_help};
3+
use if_chain::if_chain;
4+
use rustc_hir as hir;
5+
use rustc_lint::{LateContext, LateLintPass, LintContext};
6+
use rustc_semver::RustcVersion;
7+
use rustc_session::{declare_tool_lint, impl_lint_pass};
8+
9+
const FROM_OVER_INTO_MSRV: RustcVersion = RustcVersion::new(1, 41, 0);
10+
11+
declare_clippy_lint! {
12+
/// **What it does:** Searches for implementations of the `Into<..>` trait and suggests to implement `From<..>` instead.
13+
///
14+
/// **Why is this bad?** According the std docs implementing `From<..>` is preferred since it gives you `Into<..>` for free where the reverse isn't true.
15+
///
16+
/// **Known problems:** None.
17+
///
18+
/// **Example:**
19+
///
20+
/// ```rust
21+
/// struct StringWrapper(String);
22+
///
23+
/// impl Into<StringWrapper> for String {
24+
/// fn into(self) -> StringWrapper {
25+
/// StringWrapper(self)
26+
/// }
27+
/// }
28+
/// ```
29+
/// Use instead:
30+
/// ```rust
31+
/// struct StringWrapper(String);
32+
///
33+
/// impl From<String> for StringWrapper {
34+
/// fn from(s: String) -> StringWrapper {
35+
/// StringWrapper(s)
36+
/// }
37+
/// }
38+
/// ```
39+
pub FROM_OVER_INTO,
40+
style,
41+
"Warns on implementations of `Into<..>` to use `From<..>`"
42+
}
43+
44+
pub struct FromOverInto {
45+
msrv: Option<RustcVersion>,
46+
}
47+
48+
impl FromOverInto {
49+
#[must_use]
50+
pub fn new(msrv: Option<RustcVersion>) -> Self {
51+
FromOverInto { msrv }
52+
}
53+
}
54+
55+
impl_lint_pass!(FromOverInto => [FROM_OVER_INTO]);
56+
57+
impl LateLintPass<'_> for FromOverInto {
58+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
59+
if !meets_msrv(self.msrv.as_ref(), &FROM_OVER_INTO_MSRV) {
60+
return;
61+
}
62+
63+
let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id);
64+
if_chain! {
65+
if let hir::ItemKind::Impl{ .. } = &item.kind;
66+
if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id);
67+
if match_def_path(cx, impl_trait_ref.def_id, &INTO);
68+
69+
then {
70+
span_lint_and_help(
71+
cx,
72+
FROM_OVER_INTO,
73+
item.span,
74+
"an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true",
75+
None,
76+
"consider to implement `From` instead",
77+
);
78+
}
79+
}
80+
}
81+
82+
extract_msrv_attr!(LateContext);
83+
}

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ mod float_literal;
207207
mod floating_point_arithmetic;
208208
mod format;
209209
mod formatting;
210+
mod from_over_into;
210211
mod functions;
211212
mod future_not_send;
212213
mod get_last_with_len;
@@ -614,6 +615,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
614615
&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
615616
&formatting::SUSPICIOUS_ELSE_FORMATTING,
616617
&formatting::SUSPICIOUS_UNARY_OP_FORMATTING,
618+
&from_over_into::FROM_OVER_INTO,
617619
&functions::DOUBLE_MUST_USE,
618620
&functions::MUST_USE_CANDIDATE,
619621
&functions::MUST_USE_UNIT,
@@ -1014,6 +1016,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10141016
store.register_late_pass(move || box checked_conversions::CheckedConversions::new(msrv));
10151017
store.register_late_pass(move || box mem_replace::MemReplace::new(msrv));
10161018
store.register_late_pass(move || box ranges::Ranges::new(msrv));
1019+
store.register_late_pass(move || box from_over_into::FromOverInto::new(msrv));
10171020
store.register_late_pass(move || box use_self::UseSelf::new(msrv));
10181021
store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv));
10191022

@@ -1417,6 +1420,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14171420
LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
14181421
LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
14191422
LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
1423+
LintId::of(&from_over_into::FROM_OVER_INTO),
14201424
LintId::of(&functions::DOUBLE_MUST_USE),
14211425
LintId::of(&functions::MUST_USE_UNIT),
14221426
LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF),
@@ -1663,6 +1667,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16631667
LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
16641668
LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
16651669
LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
1670+
LintId::of(&from_over_into::FROM_OVER_INTO),
16661671
LintId::of(&functions::DOUBLE_MUST_USE),
16671672
LintId::of(&functions::MUST_USE_UNIT),
16681673
LintId::of(&functions::RESULT_UNIT_ERR),

‎tests/ui/from_over_into.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![warn(clippy::from_over_into)]
2+
3+
// this should throw an error
4+
struct StringWrapper(String);
5+
6+
impl Into<StringWrapper> for String {
7+
fn into(self) -> StringWrapper {
8+
StringWrapper(self)
9+
}
10+
}
11+
12+
// this is fine
13+
struct A(String);
14+
15+
impl From<String> for A {
16+
fn from(s: String) -> A {
17+
A(s)
18+
}
19+
}
20+
21+
fn main() {}

‎tests/ui/from_over_into.stderr

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true
2+
--> $DIR/from_over_into.rs:6:1
3+
|
4+
LL | / impl Into<StringWrapper> for String {
5+
LL | | fn into(self) -> StringWrapper {
6+
LL | | StringWrapper(self)
7+
LL | | }
8+
LL | | }
9+
| |_^
10+
|
11+
= note: `-D clippy::from-over-into` implied by `-D warnings`
12+
= help: consider to implement `From` instead
13+
14+
error: aborting due to previous error
15+

‎tests/ui/min_rust_version_attr.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ pub fn checked_conversion() {
5757
let _ = value <= (u32::MAX as i64) && value >= 0;
5858
}
5959

60+
pub struct FromOverInto(String);
61+
62+
impl Into<FromOverInto> for String {
63+
fn into(self) -> FromOverInto {
64+
FromOverInto(self)
65+
}
66+
}
67+
6068
pub fn filter_map_next() {
6169
let a = ["1", "lol", "3", "NaN", "5"];
6270

‎tests/ui/min_rust_version_attr.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
error: stripping a prefix manually
2-
--> $DIR/min_rust_version_attr.rs:142:24
2+
--> $DIR/min_rust_version_attr.rs:150:24
33
|
44
LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!");
55
| ^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::manual-strip` implied by `-D warnings`
88
note: the prefix was tested here
9-
--> $DIR/min_rust_version_attr.rs:141:9
9+
--> $DIR/min_rust_version_attr.rs:149:9
1010
|
1111
LL | if s.starts_with("hello, ") {
1212
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -17,13 +17,13 @@ LL | assert_eq!(<stripped>.to_uppercase(), "WORLD!");
1717
|
1818

1919
error: stripping a prefix manually
20-
--> $DIR/min_rust_version_attr.rs:154:24
20+
--> $DIR/min_rust_version_attr.rs:162:24
2121
|
2222
LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!");
2323
| ^^^^^^^^^^^^^^^^^^^^
2424
|
2525
note: the prefix was tested here
26-
--> $DIR/min_rust_version_attr.rs:153:9
26+
--> $DIR/min_rust_version_attr.rs:161:9
2727
|
2828
LL | if s.starts_with("hello, ") {
2929
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

‎tests/ui/unused_unit.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#![deny(clippy::unused_unit)]
1313
#![allow(dead_code)]
14+
#![allow(clippy::from_over_into)]
1415

1516
struct Unitter;
1617
impl Unitter {

‎tests/ui/unused_unit.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#![deny(clippy::unused_unit)]
1313
#![allow(dead_code)]
14+
#![allow(clippy::from_over_into)]
1415

1516
struct Unitter;
1617
impl Unitter {

‎tests/ui/unused_unit.stderr

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: unneeded unit return type
2-
--> $DIR/unused_unit.rs:18:28
2+
--> $DIR/unused_unit.rs:19:28
33
|
44
LL | pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
55
| ^^^^^^ help: remove the `-> ()`
@@ -11,109 +11,109 @@ LL | #![deny(clippy::unused_unit)]
1111
| ^^^^^^^^^^^^^^^^^^^
1212

1313
error: unneeded unit return type
14-
--> $DIR/unused_unit.rs:19:18
14+
--> $DIR/unused_unit.rs:20:18
1515
|
1616
LL | where G: Fn() -> () {
1717
| ^^^^^^ help: remove the `-> ()`
1818

1919
error: unneeded unit return type
20-
--> $DIR/unused_unit.rs:18:58
20+
--> $DIR/unused_unit.rs:19:58
2121
|
2222
LL | pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
2323
| ^^^^^^ help: remove the `-> ()`
2424

2525
error: unneeded unit return type
26-
--> $DIR/unused_unit.rs:20:26
26+
--> $DIR/unused_unit.rs:21:26
2727
|
2828
LL | let _y: &dyn Fn() -> () = &f;
2929
| ^^^^^^ help: remove the `-> ()`
3030

3131
error: unneeded unit return type
32-
--> $DIR/unused_unit.rs:27:18
32+
--> $DIR/unused_unit.rs:28:18
3333
|
3434
LL | fn into(self) -> () {
3535
| ^^^^^^ help: remove the `-> ()`
3636

3737
error: unneeded unit expression
38-
--> $DIR/unused_unit.rs:28:9
38+
--> $DIR/unused_unit.rs:29:9
3939
|
4040
LL | ()
4141
| ^^ help: remove the final `()`
4242

4343
error: unneeded unit return type
44-
--> $DIR/unused_unit.rs:33:29
44+
--> $DIR/unused_unit.rs:34:29
4545
|
4646
LL | fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
4747
| ^^^^^^ help: remove the `-> ()`
4848

4949
error: unneeded unit return type
50-
--> $DIR/unused_unit.rs:35:19
50+
--> $DIR/unused_unit.rs:36:19
5151
|
5252
LL | G: FnMut() -> (),
5353
| ^^^^^^ help: remove the `-> ()`
5454

5555
error: unneeded unit return type
56-
--> $DIR/unused_unit.rs:36:16
56+
--> $DIR/unused_unit.rs:37:16
5757
|
5858
LL | H: Fn() -> ();
5959
| ^^^^^^ help: remove the `-> ()`
6060

6161
error: unneeded unit return type
62-
--> $DIR/unused_unit.rs:40:29
62+
--> $DIR/unused_unit.rs:41:29
6363
|
6464
LL | fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
6565
| ^^^^^^ help: remove the `-> ()`
6666

6767
error: unneeded unit return type
68-
--> $DIR/unused_unit.rs:42:19
68+
--> $DIR/unused_unit.rs:43:19
6969
|
7070
LL | G: FnMut() -> (),
7171
| ^^^^^^ help: remove the `-> ()`
7272

7373
error: unneeded unit return type
74-
--> $DIR/unused_unit.rs:43:16
74+
--> $DIR/unused_unit.rs:44:16
7575
|
7676
LL | H: Fn() -> () {}
7777
| ^^^^^^ help: remove the `-> ()`
7878

7979
error: unneeded unit return type
80-
--> $DIR/unused_unit.rs:46:17
80+
--> $DIR/unused_unit.rs:47:17
8181
|
8282
LL | fn return_unit() -> () { () }
8383
| ^^^^^^ help: remove the `-> ()`
8484

8585
error: unneeded unit expression
86-
--> $DIR/unused_unit.rs:46:26
86+
--> $DIR/unused_unit.rs:47:26
8787
|
8888
LL | fn return_unit() -> () { () }
8989
| ^^ help: remove the final `()`
9090

9191
error: unneeded `()`
92-
--> $DIR/unused_unit.rs:56:14
92+
--> $DIR/unused_unit.rs:57:14
9393
|
9494
LL | break();
9595
| ^^ help: remove the `()`
9696

9797
error: unneeded `()`
98-
--> $DIR/unused_unit.rs:58:11
98+
--> $DIR/unused_unit.rs:59:11
9999
|
100100
LL | return();
101101
| ^^ help: remove the `()`
102102

103103
error: unneeded unit return type
104-
--> $DIR/unused_unit.rs:75:10
104+
--> $DIR/unused_unit.rs:76:10
105105
|
106106
LL | fn test()->(){}
107107
| ^^^^ help: remove the `-> ()`
108108

109109
error: unneeded unit return type
110-
--> $DIR/unused_unit.rs:78:11
110+
--> $DIR/unused_unit.rs:79:11
111111
|
112112
LL | fn test2() ->(){}
113113
| ^^^^^ help: remove the `-> ()`
114114

115115
error: unneeded unit return type
116-
--> $DIR/unused_unit.rs:81:11
116+
--> $DIR/unused_unit.rs:82:11
117117
|
118118
LL | fn test3()-> (){}
119119
| ^^^^^ help: remove the `-> ()`

0 commit comments

Comments
 (0)