Skip to content

Commit a451b2a

Browse files
1c3t3a1c3t3a
committed
Added from_over_into lint
1 parent 0718eeb commit a451b2a

File tree

8 files changed

+126
-19
lines changed

8 files changed

+126
-19
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: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
use crate::utils::paths::INTO;
2+
use crate::utils::{match_def_path, span_lint_and_help};
3+
use if_chain::if_chain;
4+
use rustc_hir as hir;
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
8+
declare_clippy_lint! {
9+
/// **What it does:** Searches for implementations of the `Into<..>` trait and suggests to implement `From<..>` instead.
10+
///
11+
/// **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.
12+
///
13+
/// **Known problems:** None.
14+
///
15+
/// **Example:**
16+
///
17+
/// ```rust
18+
/// struct StringWrapper(String);
19+
///
20+
/// impl Into<StringWrapper> for String {
21+
/// fn into(self) -> StringWrapper {
22+
/// StringWrapper(self)
23+
/// }
24+
/// }
25+
/// ```
26+
/// Use instead:
27+
/// ```rust
28+
/// struct StringWrapper(String);
29+
///
30+
/// impl From<String> for StringWrapper {
31+
/// fn from(s: String) -> StringWrapper {
32+
/// StringWrapper(s)
33+
/// }
34+
/// }
35+
/// ```
36+
pub FROM_OVER_INTO,
37+
style,
38+
"Warns on implementations of `Into<..>` to use `From<..>`"
39+
}
40+
41+
declare_lint_pass!(FromOverInto => [FROM_OVER_INTO]);
42+
43+
impl LateLintPass<'_> for FromOverInto {
44+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
45+
let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id);
46+
if_chain! {
47+
if let hir::ItemKind::Impl{ .. } = &item.kind;
48+
if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id);
49+
if match_def_path(cx, impl_trait_ref.def_id, &INTO);
50+
51+
then {
52+
span_lint_and_help(
53+
cx,
54+
FROM_OVER_INTO,
55+
item.span,
56+
"An implementation of From is preferred since it gives you Into<..> for free where the reverse isn't true.",
57+
None,
58+
"consider implement From instead",
59+
);
60+
}
61+
}
62+
}
63+
}

‎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,
@@ -1203,6 +1205,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12031205
store.register_late_pass(|| box manual_unwrap_or::ManualUnwrapOr);
12041206
store.register_late_pass(|| box manual_ok_or::ManualOkOr);
12051207
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
1208+
store.register_late_pass(|| box from_over_into::FromOverInto);
12061209
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
12071210
let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::<FxHashSet<_>>();
12081211
store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods));
@@ -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 implement From instead
13+
14+
error: aborting due to previous error
15+

‎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)