Skip to content

Commit 7206023

Browse files
HMPerson1HMPerson1
committed
Change to TryFrom 
1 parent e5076d0 commit 7206023

File tree

5 files changed

+117
-117
lines changed

5 files changed

+117
-117
lines changed

‎clippy_lints/src/impl_from_str.rs renamed to ‎clippy_lints/src/fallible_impl_from.rs

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ use rustc::hir;
33
use rustc::ty;
44
use syntax_pos::Span;
55
use utils::{method_chain_args, match_def_path, span_lint_and_then, walk_ptrs_ty};
6-
use utils::paths::{BEGIN_PANIC, BEGIN_PANIC_FMT, FROM_TRAIT, OPTION, RESULT, STRING};
6+
use utils::paths::{BEGIN_PANIC, BEGIN_PANIC_FMT, FROM_TRAIT, OPTION, RESULT};
77

8-
/// **What it does:** Checks for impls of `From<&str>` and `From<String>` that contain `panic!()` or
9-
/// `unwrap()`
8+
/// **What it does:** Checks for impls of `From<..>` that contain `panic!()` or `unwrap()`
109
///
11-
/// **Why is this bad?** `FromStr` should be used if there's a possibility of failure.
10+
/// **Why is this bad?** `TryFrom` should be used if there's a possibility of failure.
1211
///
1312
/// **Known problems:** None.
1413
///
@@ -22,19 +21,19 @@ use utils::paths::{BEGIN_PANIC, BEGIN_PANIC_FMT, FROM_TRAIT, OPTION, RESULT, STR
2221
/// }
2322
/// ```
2423
declare_lint! {
25-
pub IMPL_FROM_STR, Warn,
26-
"Warn on impls of `From<&str>` and `From<String>` that contain `panic!()` or `unwrap()`"
24+
pub FALLIBLE_IMPL_FROM, Allow,
25+
"Warn on impls of `From<..>` that contain `panic!()` or `unwrap()`"
2726
}
2827

29-
pub struct ImplFromStr;
28+
pub struct FallibleImplFrom;
3029

31-
impl LintPass for ImplFromStr {
30+
impl LintPass for FallibleImplFrom {
3231
fn get_lints(&self) -> LintArray {
33-
lint_array!(IMPL_FROM_STR)
32+
lint_array!(FALLIBLE_IMPL_FROM)
3433
}
3534
}
3635

37-
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplFromStr {
36+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FallibleImplFrom {
3837
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
3938
// check for `impl From<???> for ..`
4039
let impl_def_id = cx.tcx.hir.local_def_id(item.id);
@@ -43,15 +42,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplFromStr {
4342
let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id),
4443
match_def_path(cx.tcx, impl_trait_ref.def_id, &FROM_TRAIT),
4544
], {
46-
// check if the type parameter is `str` or `String`
47-
let from_ty_param = impl_trait_ref.substs.type_at(1);
48-
let base_from_ty_param =
49-
walk_ptrs_ty(cx.tcx.normalize_associated_type(&from_ty_param));
50-
if base_from_ty_param.sty == ty::TyStr ||
51-
match_type(cx.tcx, base_from_ty_param, &STRING)
52-
{
53-
lint_impl_body(cx, item.span, impl_items);
54-
}
45+
lint_impl_body(cx, item.span, impl_items);
5546
}}
5647
}
5748
}
@@ -117,10 +108,13 @@ fn lint_impl_body<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, impl_span: Span, impl_it
117108
if !fpu.result.is_empty() {
118109
span_lint_and_then(
119110
cx,
120-
IMPL_FROM_STR,
111+
FALLIBLE_IMPL_FROM,
121112
impl_span,
122-
"consider implementing `FromStr` instead",
113+
"consider implementing `TryFrom` instead",
123114
move |db| {
115+
db.help(
116+
"`From` is intended for infallible conversions only. \
117+
Use `TryFrom` if there's a possibility for the conversion to fail.");
124118
db.span_note(fpu.result, "potential failure(s)");
125119
});
126120
}

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ pub mod identity_conversion;
100100
pub mod identity_op;
101101
pub mod if_let_redundant_pattern_matching;
102102
pub mod if_not_else;
103-
pub mod impl_from_str;
103+
pub mod fallible_impl_from;
104104
pub mod infinite_iter;
105105
pub mod int_plus_one;
106106
pub mod invalid_ref;
@@ -342,7 +342,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
342342
reg.register_late_lint_pass(box identity_conversion::IdentityConversion::default());
343343
reg.register_late_lint_pass(box types::ImplicitHasher);
344344
reg.register_early_lint_pass(box const_static_lifetime::StaticConst);
345-
reg.register_late_lint_pass(box impl_from_str::ImplFromStr);
345+
reg.register_late_lint_pass(box fallible_impl_from::FallibleImplFrom);
346346

347347
reg.register_lint_group("clippy_restrictions", vec![
348348
arithmetic::FLOAT_ARITHMETIC,
@@ -448,7 +448,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
448448
identity_conversion::IDENTITY_CONVERSION,
449449
identity_op::IDENTITY_OP,
450450
if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
451-
impl_from_str::IMPL_FROM_STR,
451+
fallible_impl_from::FALLIBLE_IMPL_FROM,
452452
infinite_iter::INFINITE_ITER,
453453
invalid_ref::INVALID_REF,
454454
is_unit_expr::UNIT_EXPR,

‎tests/ui/impl_from_str.rs renamed to ‎tests/ui/fallible_impl_from.rs

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![deny(fallible_impl_from)]
2+
13
// docs example
24
struct Foo(i32);
35
impl From<String> for Foo {
@@ -14,34 +16,27 @@ impl<'a> From<&'a str> for Valid {
1416
Valid(s.to_owned().into_bytes())
1517
}
1618
}
17-
impl From<String> for Valid {
18-
fn from(s: String) -> Valid {
19-
Valid(s.into_bytes())
20-
}
21-
}
2219
impl From<usize> for Valid {
2320
fn from(i: usize) -> Valid {
24-
if i == 0 {
25-
panic!();
26-
}
2721
Valid(Vec::with_capacity(i))
2822
}
2923
}
3024

3125

3226
struct Invalid;
3327

34-
impl<'a> From<&'a str> for Invalid {
35-
fn from(s: &'a str) -> Invalid {
36-
if !s.is_empty() {
28+
impl From<usize> for Invalid {
29+
fn from(i: usize) -> Invalid {
30+
if i != 42 {
3731
panic!();
3832
}
3933
Invalid
4034
}
4135
}
4236

43-
impl From<String> for Invalid {
44-
fn from(s: String) -> Invalid {
37+
impl From<Option<String>> for Invalid {
38+
fn from(s: Option<String>) -> Invalid {
39+
let s = s.unwrap();
4540
if !s.is_empty() {
4641
panic!(42);
4742
} else if s.parse::<u32>().unwrap() != 42 {

‎tests/ui/fallible_impl_from.stderr

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
error: consider implementing `TryFrom` instead
2+
--> $DIR/fallible_impl_from.rs:5:1
3+
|
4+
5 | / impl From<String> for Foo {
5+
6 | | fn from(s: String) -> Self {
6+
7 | | Foo(s.parse().unwrap())
7+
8 | | }
8+
9 | | }
9+
| |_^
10+
|
11+
note: lint level defined here
12+
--> $DIR/fallible_impl_from.rs:1:9
13+
|
14+
1 | #![deny(fallible_impl_from)]
15+
| ^^^^^^^^^^^^^^^^^^
16+
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail.
17+
note: potential failure(s)
18+
--> $DIR/fallible_impl_from.rs:7:13
19+
|
20+
7 | Foo(s.parse().unwrap())
21+
| ^^^^^^^^^^^^^^^^^^
22+
23+
error: consider implementing `TryFrom` instead
24+
--> $DIR/fallible_impl_from.rs:28:1
25+
|
26+
28 | / impl From<usize> for Invalid {
27+
29 | | fn from(i: usize) -> Invalid {
28+
30 | | if i != 42 {
29+
31 | | panic!();
30+
... |
31+
34 | | }
32+
35 | | }
33+
| |_^
34+
|
35+
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail.
36+
note: potential failure(s)
37+
--> $DIR/fallible_impl_from.rs:31:13
38+
|
39+
31 | panic!();
40+
| ^^^^^^^^^
41+
= note: this error originates in a macro outside of the current crate
42+
43+
error: consider implementing `TryFrom` instead
44+
--> $DIR/fallible_impl_from.rs:37:1
45+
|
46+
37 | / impl From<Option<String>> for Invalid {
47+
38 | | fn from(s: Option<String>) -> Invalid {
48+
39 | | let s = s.unwrap();
49+
40 | | if !s.is_empty() {
50+
... |
51+
46 | | }
52+
47 | | }
53+
| |_^
54+
|
55+
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail.
56+
note: potential failure(s)
57+
--> $DIR/fallible_impl_from.rs:39:17
58+
|
59+
39 | let s = s.unwrap();
60+
| ^^^^^^^^^^
61+
40 | if !s.is_empty() {
62+
41 | panic!(42);
63+
| ^^^^^^^^^^^
64+
42 | } else if s.parse::<u32>().unwrap() != 42 {
65+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
66+
43 | panic!("{:?}", s);
67+
| ^^^^^^^^^^^^^^^^^^
68+
= note: this error originates in a macro outside of the current crate
69+
70+
error: consider implementing `TryFrom` instead
71+
--> $DIR/fallible_impl_from.rs:55:1
72+
|
73+
55 | / impl<'a> From<&'a mut <Box<u32> as ProjStrTrait>::ProjString> for Invalid {
74+
56 | | fn from(s: &'a mut <Box<u32> as ProjStrTrait>::ProjString) -> Invalid {
75+
57 | | if s.parse::<u32>().ok().unwrap() != 42 {
76+
58 | | panic!("{:?}", s);
77+
... |
78+
61 | | }
79+
62 | | }
80+
| |_^
81+
|
82+
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail.
83+
note: potential failure(s)
84+
--> $DIR/fallible_impl_from.rs:57:12
85+
|
86+
57 | if s.parse::<u32>().ok().unwrap() != 42 {
87+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
88+
58 | panic!("{:?}", s);
89+
| ^^^^^^^^^^^^^^^^^^
90+
= note: this error originates in a macro outside of the current crate
91+

‎tests/ui/impl_from_str.stderr

Lines changed: 0 additions & 80 deletions
This file was deleted.

0 commit comments

Comments
 (0)