Merge pull request #1513 from sinkuu/identical_conversion

Add identity_conversion lint
This commit is contained in:
Oliver Schneider 2017-10-04 16:06:12 +02:00 committed by GitHub
commit 346936a7c1
6 changed files with 185 additions and 2 deletions

View 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();
}
}
}

View File

@ -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,

View File

@ -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"];

View File

@ -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()
},

View File

@ -0,0 +1,40 @@
#![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");
#[allow(identity_conversion)]
{
let _: String = "foo".into();
let _ = String::from("foo");
}
let _: String = "foo".to_string().into();
let _: String = From::from("foo".to_string());
let _ = String::from("foo".to_string());
}

View 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:37:21
|
37 | let _: String = "foo".to_string().into();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
error: identical conversion
--> $DIR/identity_conversion.rs:38:21
|
38 | let _: String = From::from("foo".to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()`
error: identical conversion
--> $DIR/identity_conversion.rs:39:13
|
39 | let _ = String::from("foo".to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`