mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-27 23:20:39 +00:00
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
This commit is contained in:
commit
25e9acb253
10 changed files with 158 additions and 23 deletions
|
@ -1841,6 +1841,7 @@ Released 2018-09-13
|
|||
[`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
|
||||
[`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
|
||||
[`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect
|
||||
[`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into
|
||||
[`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send
|
||||
[`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
|
||||
[`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap
|
||||
|
|
83
clippy_lints/src/from_over_into.rs
Normal file
83
clippy_lints/src/from_over_into.rs
Normal file
|
@ -0,0 +1,83 @@
|
|||
use crate::utils::paths::INTO;
|
||||
use crate::utils::{match_def_path, meets_msrv, span_lint_and_help};
|
||||
use if_chain::if_chain;
|
||||
use rustc_hir as hir;
|
||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||
use rustc_semver::RustcVersion;
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
|
||||
const FROM_OVER_INTO_MSRV: RustcVersion = RustcVersion::new(1, 41, 0);
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Searches for implementations of the `Into<..>` trait and suggests to implement `From<..>` instead.
|
||||
///
|
||||
/// **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.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
///
|
||||
/// ```rust
|
||||
/// struct StringWrapper(String);
|
||||
///
|
||||
/// impl Into<StringWrapper> for String {
|
||||
/// fn into(self) -> StringWrapper {
|
||||
/// StringWrapper(self)
|
||||
/// }
|
||||
/// }
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// struct StringWrapper(String);
|
||||
///
|
||||
/// impl From<String> for StringWrapper {
|
||||
/// fn from(s: String) -> StringWrapper {
|
||||
/// StringWrapper(s)
|
||||
/// }
|
||||
/// }
|
||||
/// ```
|
||||
pub FROM_OVER_INTO,
|
||||
style,
|
||||
"Warns on implementations of `Into<..>` to use `From<..>`"
|
||||
}
|
||||
|
||||
pub struct FromOverInto {
|
||||
msrv: Option<RustcVersion>,
|
||||
}
|
||||
|
||||
impl FromOverInto {
|
||||
#[must_use]
|
||||
pub fn new(msrv: Option<RustcVersion>) -> Self {
|
||||
FromOverInto { msrv }
|
||||
}
|
||||
}
|
||||
|
||||
impl_lint_pass!(FromOverInto => [FROM_OVER_INTO]);
|
||||
|
||||
impl LateLintPass<'_> for FromOverInto {
|
||||
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
|
||||
if !meets_msrv(self.msrv.as_ref(), &FROM_OVER_INTO_MSRV) {
|
||||
return;
|
||||
}
|
||||
|
||||
let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id);
|
||||
if_chain! {
|
||||
if let hir::ItemKind::Impl{ .. } = &item.kind;
|
||||
if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id);
|
||||
if match_def_path(cx, impl_trait_ref.def_id, &INTO);
|
||||
|
||||
then {
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
FROM_OVER_INTO,
|
||||
item.span,
|
||||
"an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true",
|
||||
None,
|
||||
"consider to implement `From` instead",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
extract_msrv_attr!(LateContext);
|
||||
}
|
|
@ -207,6 +207,7 @@ mod float_literal;
|
|||
mod floating_point_arithmetic;
|
||||
mod format;
|
||||
mod formatting;
|
||||
mod from_over_into;
|
||||
mod functions;
|
||||
mod future_not_send;
|
||||
mod get_last_with_len;
|
||||
|
@ -614,6 +615,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
|
||||
&formatting::SUSPICIOUS_ELSE_FORMATTING,
|
||||
&formatting::SUSPICIOUS_UNARY_OP_FORMATTING,
|
||||
&from_over_into::FROM_OVER_INTO,
|
||||
&functions::DOUBLE_MUST_USE,
|
||||
&functions::MUST_USE_CANDIDATE,
|
||||
&functions::MUST_USE_UNIT,
|
||||
|
@ -1014,6 +1016,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
store.register_late_pass(move || box checked_conversions::CheckedConversions::new(msrv));
|
||||
store.register_late_pass(move || box mem_replace::MemReplace::new(msrv));
|
||||
store.register_late_pass(move || box ranges::Ranges::new(msrv));
|
||||
store.register_late_pass(move || box from_over_into::FromOverInto::new(msrv));
|
||||
store.register_late_pass(move || box use_self::UseSelf::new(msrv));
|
||||
store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv));
|
||||
|
||||
|
@ -1417,6 +1420,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
|
||||
LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
|
||||
LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
|
||||
LintId::of(&from_over_into::FROM_OVER_INTO),
|
||||
LintId::of(&functions::DOUBLE_MUST_USE),
|
||||
LintId::of(&functions::MUST_USE_UNIT),
|
||||
LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF),
|
||||
|
@ -1663,6 +1667,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
|
||||
LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),
|
||||
LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
|
||||
LintId::of(&from_over_into::FROM_OVER_INTO),
|
||||
LintId::of(&functions::DOUBLE_MUST_USE),
|
||||
LintId::of(&functions::MUST_USE_UNIT),
|
||||
LintId::of(&functions::RESULT_UNIT_ERR),
|
||||
|
|
21
tests/ui/from_over_into.rs
Normal file
21
tests/ui/from_over_into.rs
Normal file
|
@ -0,0 +1,21 @@
|
|||
#![warn(clippy::from_over_into)]
|
||||
|
||||
// this should throw an error
|
||||
struct StringWrapper(String);
|
||||
|
||||
impl Into<StringWrapper> for String {
|
||||
fn into(self) -> StringWrapper {
|
||||
StringWrapper(self)
|
||||
}
|
||||
}
|
||||
|
||||
// this is fine
|
||||
struct A(String);
|
||||
|
||||
impl From<String> for A {
|
||||
fn from(s: String) -> A {
|
||||
A(s)
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
15
tests/ui/from_over_into.stderr
Normal file
15
tests/ui/from_over_into.stderr
Normal file
|
@ -0,0 +1,15 @@
|
|||
error: an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true
|
||||
--> $DIR/from_over_into.rs:6:1
|
||||
|
|
||||
LL | / impl Into<StringWrapper> for String {
|
||||
LL | | fn into(self) -> StringWrapper {
|
||||
LL | | StringWrapper(self)
|
||||
LL | | }
|
||||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= note: `-D clippy::from-over-into` implied by `-D warnings`
|
||||
= help: consider to implement `From` instead
|
||||
|
||||
error: aborting due to previous error
|
||||
|
|
@ -57,6 +57,14 @@ pub fn checked_conversion() {
|
|||
let _ = value <= (u32::MAX as i64) && value >= 0;
|
||||
}
|
||||
|
||||
pub struct FromOverInto(String);
|
||||
|
||||
impl Into<FromOverInto> for String {
|
||||
fn into(self) -> FromOverInto {
|
||||
FromOverInto(self)
|
||||
}
|
||||
}
|
||||
|
||||
pub fn filter_map_next() {
|
||||
let a = ["1", "lol", "3", "NaN", "5"];
|
||||
|
||||
|
|
|
@ -1,12 +1,12 @@
|
|||
error: stripping a prefix manually
|
||||
--> $DIR/min_rust_version_attr.rs:142:24
|
||||
--> $DIR/min_rust_version_attr.rs:150:24
|
||||
|
|
||||
LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!");
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::manual-strip` implied by `-D warnings`
|
||||
note: the prefix was tested here
|
||||
--> $DIR/min_rust_version_attr.rs:141:9
|
||||
--> $DIR/min_rust_version_attr.rs:149:9
|
||||
|
|
||||
LL | if s.starts_with("hello, ") {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
@ -17,13 +17,13 @@ LL | assert_eq!(<stripped>.to_uppercase(), "WORLD!");
|
|||
|
|
||||
|
||||
error: stripping a prefix manually
|
||||
--> $DIR/min_rust_version_attr.rs:154:24
|
||||
--> $DIR/min_rust_version_attr.rs:162:24
|
||||
|
|
||||
LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!");
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
note: the prefix was tested here
|
||||
--> $DIR/min_rust_version_attr.rs:153:9
|
||||
--> $DIR/min_rust_version_attr.rs:161:9
|
||||
|
|
||||
LL | if s.starts_with("hello, ") {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
|
@ -11,6 +11,7 @@
|
|||
|
||||
#![deny(clippy::unused_unit)]
|
||||
#![allow(dead_code)]
|
||||
#![allow(clippy::from_over_into)]
|
||||
|
||||
struct Unitter;
|
||||
impl Unitter {
|
||||
|
|
|
@ -11,6 +11,7 @@
|
|||
|
||||
#![deny(clippy::unused_unit)]
|
||||
#![allow(dead_code)]
|
||||
#![allow(clippy::from_over_into)]
|
||||
|
||||
struct Unitter;
|
||||
impl Unitter {
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:18:28
|
||||
--> $DIR/unused_unit.rs:19:28
|
||||
|
|
||||
LL | pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
@ -11,109 +11,109 @@ LL | #![deny(clippy::unused_unit)]
|
|||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:19:18
|
||||
--> $DIR/unused_unit.rs:20:18
|
||||
|
|
||||
LL | where G: Fn() -> () {
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:18:58
|
||||
--> $DIR/unused_unit.rs:19:58
|
||||
|
|
||||
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:26
|
||||
--> $DIR/unused_unit.rs:21:26
|
||||
|
|
||||
LL | let _y: &dyn Fn() -> () = &f;
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:27:18
|
||||
--> $DIR/unused_unit.rs:28:18
|
||||
|
|
||||
LL | fn into(self) -> () {
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit expression
|
||||
--> $DIR/unused_unit.rs:28:9
|
||||
--> $DIR/unused_unit.rs:29:9
|
||||
|
|
||||
LL | ()
|
||||
| ^^ help: remove the final `()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:33:29
|
||||
--> $DIR/unused_unit.rs:34:29
|
||||
|
|
||||
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:19
|
||||
--> $DIR/unused_unit.rs:36:19
|
||||
|
|
||||
LL | G: FnMut() -> (),
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:36:16
|
||||
--> $DIR/unused_unit.rs:37:16
|
||||
|
|
||||
LL | H: Fn() -> ();
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:40:29
|
||||
--> $DIR/unused_unit.rs:41:29
|
||||
|
|
||||
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:19
|
||||
--> $DIR/unused_unit.rs:43:19
|
||||
|
|
||||
LL | G: FnMut() -> (),
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:43:16
|
||||
--> $DIR/unused_unit.rs:44:16
|
||||
|
|
||||
LL | H: Fn() -> () {}
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:46:17
|
||||
--> $DIR/unused_unit.rs:47:17
|
||||
|
|
||||
LL | fn return_unit() -> () { () }
|
||||
| ^^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit expression
|
||||
--> $DIR/unused_unit.rs:46:26
|
||||
--> $DIR/unused_unit.rs:47:26
|
||||
|
|
||||
LL | fn return_unit() -> () { () }
|
||||
| ^^ help: remove the final `()`
|
||||
|
||||
error: unneeded `()`
|
||||
--> $DIR/unused_unit.rs:56:14
|
||||
--> $DIR/unused_unit.rs:57:14
|
||||
|
|
||||
LL | break();
|
||||
| ^^ help: remove the `()`
|
||||
|
||||
error: unneeded `()`
|
||||
--> $DIR/unused_unit.rs:58:11
|
||||
--> $DIR/unused_unit.rs:59:11
|
||||
|
|
||||
LL | return();
|
||||
| ^^ help: remove the `()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:75:10
|
||||
--> $DIR/unused_unit.rs:76:10
|
||||
|
|
||||
LL | fn test()->(){}
|
||||
| ^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:78:11
|
||||
--> $DIR/unused_unit.rs:79:11
|
||||
|
|
||||
LL | fn test2() ->(){}
|
||||
| ^^^^^ help: remove the `-> ()`
|
||||
|
||||
error: unneeded unit return type
|
||||
--> $DIR/unused_unit.rs:81:11
|
||||
--> $DIR/unused_unit.rs:82:11
|
||||
|
|
||||
LL | fn test3()-> (){}
|
||||
| ^^^^^ help: remove the `-> ()`
|
||||
|
|
Loading…
Reference in a new issue