From 3713fd3dce39e94fb5903554300df1db63a5c422 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 18 Jan 2016 15:35:50 +0100 Subject: [PATCH] Check types in the CMP_OWNED lint --- src/methods.rs | 2 +- src/misc.rs | 29 ++++++++++++++++++++--------- src/utils.rs | 4 ++-- tests/compile-fail/cmp_owned.rs | 2 ++ 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/methods.rs b/src/methods.rs index b5ce82f1a43..254b68deb11 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -318,7 +318,7 @@ fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P]) return false; }; - if implements_trait(cx, arg_ty, default_trait_id) { + if implements_trait(cx, arg_ty, default_trait_id, None) { span_lint(cx, OR_FUN_CALL, span, &format!("use of `{}` followed by a call to `{}`", name, path)) .span_suggestion(span, "try this", diff --git a/src/misc.rs b/src/misc.rs index a8fabaa3fcf..52234fd61af 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -11,7 +11,7 @@ use rustc::middle::const_eval::eval_const_expr_partial; use rustc::middle::const_eval::EvalHint::ExprTypeChecked; use utils::{get_item_name, match_path, snippet, get_parent_expr, span_lint}; -use utils::{span_help_and_lint, walk_ptrs_ty, is_integer_literal}; +use utils::{span_help_and_lint, walk_ptrs_ty, is_integer_literal, implements_trait}; /// **What it does:** This lint checks for function arguments and let bindings denoted as `ref`. It is `Warn` by default. /// @@ -210,18 +210,18 @@ impl LateLintPass for CmpOwned { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { if let ExprBinary(ref cmp, ref left, ref right) = expr.node { if is_comparison_binop(cmp.node) { - check_to_owned(cx, left, right.span, true, cmp.span); - check_to_owned(cx, right, left.span, false, cmp.span) + check_to_owned(cx, left, right, true, cmp.span); + check_to_owned(cx, right, left, false, cmp.span) } } } } -fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, op: Span) { - let snip = match expr.node { +fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr, left: bool, op: Span) { + let (arg_ty, snip) = match expr.node { ExprMethodCall(Spanned{node: ref name, ..}, _, ref args) if args.len() == 1 => { if name.as_str() == "to_string" || name.as_str() == "to_owned" && is_str_arg(cx, args) { - snippet(cx, args[0].span, "..") + (cx.tcx.expr_ty(&args[0]), snippet(cx, args[0].span, "..")) } else { return; } @@ -229,7 +229,7 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, o ExprCall(ref path, ref v) if v.len() == 1 => { if let ExprPath(None, ref path) = path.node { if match_path(path, &["String", "from_str"]) || match_path(path, &["String", "from"]) { - snippet(cx, v[0].span, "..") + (cx.tcx.expr_ty(&v[0]), snippet(cx, v[0].span, "..")) } else { return; } @@ -239,6 +239,17 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, o } _ => return, }; + + let other_ty = cx.tcx.expr_ty(other); + let partial_eq_trait_id = match cx.tcx.lang_items.eq_trait() { + Some(id) => id, + None => return, + }; + + if !implements_trait(cx, arg_ty, partial_eq_trait_id, Some(vec![other_ty])) { + return; + } + if left { span_lint(cx, CMP_OWNED, @@ -247,14 +258,14 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, o compare without allocation", snip, snippet(cx, op, "=="), - snippet(cx, other_span, ".."))); + snippet(cx, other.span, ".."))); } else { span_lint(cx, CMP_OWNED, expr.span, &format!("this creates an owned instance just for comparison. Consider using `{} {} {}` to \ compare without allocation", - snippet(cx, other_span, ".."), + snippet(cx, other.span, ".."), snippet(cx, op, "=="), snip)); } diff --git a/src/utils.rs b/src/utils.rs index 446303f8bb1..65a7bc86031 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -242,14 +242,14 @@ pub fn get_trait_def_id(cx: &LateContext, path: &[&str]) -> Option { /// Check whether a type implements a trait. /// See also `get_trait_def_id`. -pub fn implements_trait<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: ty::Ty<'tcx>, trait_id: DefId) -> bool { +pub fn implements_trait<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: ty::Ty<'tcx>, trait_id: DefId, ty_params: Option>>) -> bool { cx.tcx.populate_implementations_for_trait_if_necessary(trait_id); let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, None); let obligation = traits::predicate_for_trait_def(cx.tcx, traits::ObligationCause::dummy(), trait_id, 0, ty, - vec![]); + ty_params.unwrap_or_default()); traits::SelectionContext::new(&infcx).evaluate_obligation_conservatively(&obligation) } diff --git a/tests/compile-fail/cmp_owned.rs b/tests/compile-fail/cmp_owned.rs index afca83e1d32..c4c9ee60fab 100644 --- a/tests/compile-fail/cmp_owned.rs +++ b/tests/compile-fail/cmp_owned.rs @@ -21,4 +21,6 @@ fn main() { // as of 2015-08-14 x != String::from("foo"); //~ERROR this creates an owned instance + + 42.to_string() == "42"; }