Add identity_conversion lint (fixes #1051)
This commit is contained in:
parent
1ab7f36e14
commit
771d2220d2
96
clippy_lints/src/identity_conversion.rs
Normal file
96
clippy_lints/src/identity_conversion.rs
Normal file
@ -0,0 +1,96 @@
|
||||
use rustc::lint::*;
|
||||
use rustc::hir::*;
|
||||
use syntax::ast::NodeId;
|
||||
use utils::{in_macro, match_def_path, match_trait_method, same_tys, snippet, span_lint_and_then};
|
||||
use utils::{opt_def_id, paths, resolve_node};
|
||||
|
||||
/// **What it does:** Checks for always-identical `Into`/`From` conversions.
|
||||
///
|
||||
/// **Why is this bad?** Redundant code.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// // format!() returns a `String`
|
||||
/// let s: String = format!("hello").into();
|
||||
/// ```
|
||||
declare_lint! {
|
||||
pub IDENTITY_CONVERSION,
|
||||
Warn,
|
||||
"using always-identical `Into`/`From` conversions"
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub struct IdentityConversion {
|
||||
try_desugar_arm: Vec<NodeId>,
|
||||
}
|
||||
|
||||
impl LintPass for IdentityConversion {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(IDENTITY_CONVERSION)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion {
|
||||
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
|
||||
if in_macro(e.span) {
|
||||
return;
|
||||
}
|
||||
|
||||
if Some(&e.id) == self.try_desugar_arm.last() {
|
||||
return;
|
||||
}
|
||||
|
||||
match e.node {
|
||||
ExprMatch(_, ref arms, MatchSource::TryDesugar) => {
|
||||
let e = match arms[0].body.node {
|
||||
ExprRet(Some(ref e)) | ExprBreak(_, Some(ref e)) => e,
|
||||
_ => return,
|
||||
};
|
||||
if let ExprCall(_, ref args) = e.node {
|
||||
self.try_desugar_arm.push(args[0].id);
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
},
|
||||
|
||||
ExprMethodCall(ref name, .., ref args) => {
|
||||
if match_trait_method(cx, e, &paths::INTO[..]) && &*name.name.as_str() == "into" {
|
||||
let a = cx.tables.expr_ty(e);
|
||||
let b = cx.tables.expr_ty(&args[0]);
|
||||
if same_tys(cx, a, b) {
|
||||
let sugg = snippet(cx, args[0].span, "<expr>").into_owned();
|
||||
span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |db| {
|
||||
db.span_suggestion(e.span, "consider removing `.into()`", sugg);
|
||||
});
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
ExprCall(ref path, ref args) => if let ExprPath(ref qpath) = path.node {
|
||||
if let Some(def_id) = opt_def_id(resolve_node(cx, qpath, path.hir_id)) {
|
||||
if match_def_path(cx.tcx, def_id, &paths::FROM_FROM[..]) {
|
||||
let a = cx.tables.expr_ty(e);
|
||||
let b = cx.tables.expr_ty(&args[0]);
|
||||
if same_tys(cx, a, b) {
|
||||
let sugg = snippet(cx, args[0].span, "<expr>").into_owned();
|
||||
let sugg_msg = format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
|
||||
span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |db| {
|
||||
db.span_suggestion(e.span, &sugg_msg, sugg);
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
|
||||
fn check_expr_post(&mut self, _: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
|
||||
if Some(&e.id) == self.try_desugar_arm.last() {
|
||||
self.try_desugar_arm.pop();
|
||||
}
|
||||
}
|
||||
}
|
@ -93,6 +93,7 @@ pub mod eval_order_dependence;
|
||||
pub mod format;
|
||||
pub mod formatting;
|
||||
pub mod functions;
|
||||
pub mod identity_conversion;
|
||||
pub mod identity_op;
|
||||
pub mod if_let_redundant_pattern_matching;
|
||||
pub mod if_not_else;
|
||||
@ -331,6 +332,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
reg.register_late_lint_pass(box bytecount::ByteCount);
|
||||
reg.register_late_lint_pass(box infinite_iter::Pass);
|
||||
reg.register_late_lint_pass(box invalid_ref::InvalidRef);
|
||||
reg.register_late_lint_pass(box identity_conversion::IdentityConversion::default());
|
||||
|
||||
reg.register_lint_group("clippy_restrictions", vec![
|
||||
arithmetic::FLOAT_ARITHMETIC,
|
||||
@ -431,6 +433,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
formatting::SUSPICIOUS_ELSE_FORMATTING,
|
||||
functions::NOT_UNSAFE_PTR_ARG_DEREF,
|
||||
functions::TOO_MANY_ARGUMENTS,
|
||||
identity_conversion::IDENTITY_CONVERSION,
|
||||
identity_op::IDENTITY_OP,
|
||||
if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
|
||||
infinite_iter::INFINITE_ITER,
|
||||
|
@ -26,11 +26,13 @@ pub const DOUBLE_ENDED_ITERATOR: [&'static str; 4] = ["core", "iter", "traits",
|
||||
pub const DROP: [&'static str; 3] = ["core", "mem", "drop"];
|
||||
pub const FMT_ARGUMENTS_NEWV1: [&'static str; 4] = ["core", "fmt", "Arguments", "new_v1"];
|
||||
pub const FMT_ARGUMENTV1_NEW: [&'static str; 4] = ["core", "fmt", "ArgumentV1", "new"];
|
||||
pub const FROM_FROM: [&'static str; 4] = ["core", "convert", "From", "from"];
|
||||
pub const HASH: [&'static str; 2] = ["hash", "Hash"];
|
||||
pub const HASHMAP: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"];
|
||||
pub const HASHMAP_ENTRY: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"];
|
||||
pub const HASHSET: [&'static str; 5] = ["std", "collections", "hash", "set", "HashSet"];
|
||||
pub const INIT: [&'static str; 4] = ["core", "intrinsics", "", "init"];
|
||||
pub const INTO: [&'static str; 3] = ["core", "convert", "Into"];
|
||||
pub const INTO_ITERATOR: [&'static str; 4] = ["core", "iter", "traits", "IntoIterator"];
|
||||
pub const IO_PRINT: [&'static str; 4] = ["std", "io", "stdio", "_print"];
|
||||
pub const IO_READ: [&'static str; 3] = ["std", "io", "Read"];
|
||||
|
@ -67,7 +67,7 @@ fn check_vec_macro<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, vec_args: &higher::VecA
|
||||
.eval(len)
|
||||
.is_ok()
|
||||
{
|
||||
format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into()
|
||||
format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len"))
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
@ -75,7 +75,7 @@ fn check_vec_macro<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, vec_args: &higher::VecA
|
||||
higher::VecArgs::Vec(args) => if let Some(last) = args.iter().last() {
|
||||
let span = args[0].span.to(last.span);
|
||||
|
||||
format!("&[{}]", snippet(cx, span, "..")).into()
|
||||
format!("&[{}]", snippet(cx, span, ".."))
|
||||
} else {
|
||||
"&[]".into()
|
||||
},
|
||||
|
35
tests/ui/identity_conversion.rs
Normal file
35
tests/ui/identity_conversion.rs
Normal file
@ -0,0 +1,35 @@
|
||||
#![deny(identity_conversion)]
|
||||
|
||||
fn test_generic<T: Copy>(val: T) -> T {
|
||||
let _ = T::from(val);
|
||||
val.into()
|
||||
}
|
||||
|
||||
fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
|
||||
// ok
|
||||
let _: i32 = val.into();
|
||||
let _: U = val.into();
|
||||
let _ = U::from(val);
|
||||
}
|
||||
|
||||
fn test_questionmark() -> Result<(), ()> {
|
||||
{
|
||||
let _: i32 = 0i32.into();
|
||||
Ok(Ok(()))
|
||||
}??;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn main() {
|
||||
test_generic(10i32);
|
||||
test_generic2::<i32, i32>(10i32);
|
||||
test_questionmark().unwrap();
|
||||
|
||||
let _: String = "foo".into();
|
||||
let _: String = From::from("foo");
|
||||
let _ = String::from("foo");
|
||||
|
||||
let _: String = "foo".to_string().into();
|
||||
let _: String = From::from("foo".to_string());
|
||||
let _ = String::from("foo".to_string());
|
||||
}
|
42
tests/ui/identity_conversion.stderr
Normal file
42
tests/ui/identity_conversion.stderr
Normal file
@ -0,0 +1,42 @@
|
||||
error: identical conversion
|
||||
--> $DIR/identity_conversion.rs:4:13
|
||||
|
|
||||
4 | let _ = T::from(val);
|
||||
| ^^^^^^^^^^^^ help: consider removing `T::from()`: `val`
|
||||
|
|
||||
note: lint level defined here
|
||||
--> $DIR/identity_conversion.rs:1:9
|
||||
|
|
||||
1 | #![deny(identity_conversion)]
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: identical conversion
|
||||
--> $DIR/identity_conversion.rs:5:5
|
||||
|
|
||||
5 | val.into()
|
||||
| ^^^^^^^^^^ help: consider removing `.into()`: `val`
|
||||
|
||||
error: identical conversion
|
||||
--> $DIR/identity_conversion.rs:17:22
|
||||
|
|
||||
17 | let _: i32 = 0i32.into();
|
||||
| ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
|
||||
|
||||
error: identical conversion
|
||||
--> $DIR/identity_conversion.rs:32:21
|
||||
|
|
||||
32 | let _: String = "foo".to_string().into();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
|
||||
|
||||
error: identical conversion
|
||||
--> $DIR/identity_conversion.rs:33:21
|
||||
|
|
||||
33 | let _: String = From::from("foo".to_string());
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()`
|
||||
|
||||
error: identical conversion
|
||||
--> $DIR/identity_conversion.rs:34:13
|
||||
|
|
||||
34 | let _ = String::from("foo".to_string());
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
|
||||
|
Loading…
Reference in New Issue
Block a user