From b2df15d65a7baf1f8d9b8b776027a34dd6e1db10 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 21 Aug 2015 18:28:17 +0200 Subject: [PATCH 1/4] ptr_arg improvements (fixes #214) * do not trigger on mutable references * use "real" type from ty, not AST type --- src/ptr_arg.rs | 45 ++++++++++++++++++++--------------- tests/compile-fail/ptr_arg.rs | 19 ++++++++------- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index 2748d187a..35c61b102 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -4,10 +4,9 @@ use rustc::lint::*; use syntax::ast::*; -use syntax::codemap::Span; +use rustc::middle::ty; -use types::match_ty_unwrap; -use utils::span_lint; +use utils::{span_lint, match_def_path}; declare_lint! { pub PTR_ARG, @@ -43,24 +42,32 @@ impl LintPass for PtrArg { } } +#[allow(unused_imports)] fn check_fn(cx: &Context, decl: &FnDecl) { + { + // In case stuff gets moved around + use collections::vec::Vec; + use collections::string::String; + } for arg in &decl.inputs { - match &arg.ty.node { - &TyPtr(ref p) | &TyRptr(_, ref p) => - check_ptr_subtype(cx, arg.ty.span, &p.ty), - _ => () + if arg.ty.node == TyInfer { // "self" arguments + continue; + } + let ref sty = cx.tcx.pat_ty(&*arg.pat).sty; + if let &ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = sty { + if let ty::TyStruct(did, _) = ty.sty { + if match_def_path(cx, did.did, &["collections", "vec", "Vec"]) { + span_lint(cx, PTR_ARG, arg.ty.span, + "writing `&Vec<_>` instead of `&[_]` involves one more reference \ + and cannot be used with non-Vec-based slices. Consider changing \ + the type to `&[...]`"); + } + else if match_def_path(cx, did.did, &["collections", "string", "String"]) { + span_lint(cx, PTR_ARG, arg.ty.span, + "writing `&String` instead of `&str` involves a new object \ + where a slice will do. Consider changing the type to `&str`"); + } + } } } } - -fn check_ptr_subtype(cx: &Context, span: Span, ty: &Ty) { - match_ty_unwrap(ty, &["Vec"]).map_or_else(|| match_ty_unwrap(ty, - &["String"]).map_or((), |_| { - span_lint(cx, PTR_ARG, span, - "writing `&String` instead of `&str` involves a new object \ - where a slice will do. Consider changing the type to `&str`") - }), |_| span_lint(cx, PTR_ARG, span, - "writing `&Vec<_>` instead of \ - `&[_]` involves one more reference and cannot be used with \ - non-Vec-based slices. Consider changing the type to `&[...]`")) -} diff --git a/tests/compile-fail/ptr_arg.rs b/tests/compile-fail/ptr_arg.rs index d4b6d2260..d0615be49 100755 --- a/tests/compile-fail/ptr_arg.rs +++ b/tests/compile-fail/ptr_arg.rs @@ -1,20 +1,23 @@ #![feature(plugin)] #![plugin(clippy)] +#![allow(unused)] +#![deny(ptr_arg)] -#[deny(ptr_arg)] -#[allow(unused)] fn do_vec(x: &Vec) { //~ERROR writing `&Vec<_>` instead of `&[_]` //Nothing here } -#[deny(ptr_arg)] -#[allow(unused)] +fn do_vec_mut(x: &mut Vec) { // no error here + //Nothing here +} + fn do_str(x: &String) { //~ERROR writing `&String` instead of `&str` //Nothing here either } -fn main() { - let x = vec![1i64, 2, 3]; - do_vec(&x); - do_str(&"hello".to_owned()); +fn do_str_mut(x: &mut String) { // no error here + //Nothing here either +} + +fn main() { } From 707e95f2e5b9f6e20998508d99959dd1642d26ec Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 21 Aug 2015 18:40:36 +0200 Subject: [PATCH 2/4] types: use middle::ty types instead of ast types This gets rid of the match_ty_unwrap function. --- src/types.rs | 71 ++++++++++++------------------------- tests/compile-fail/dlist.rs | 2 +- 2 files changed, 24 insertions(+), 49 deletions(-) mode change 100644 => 100755 tests/compile-fail/dlist.rs diff --git a/src/types.rs b/src/types.rs index 915ffb15f..986fb1016 100644 --- a/src/types.rs +++ b/src/types.rs @@ -2,11 +2,10 @@ use rustc::lint::*; use syntax::ast; use syntax::ast::*; use syntax::ast_util::{is_comparison_binop, binop_to_string}; -use syntax::ptr::P; use rustc::middle::ty; use syntax::codemap::ExpnInfo; -use utils::{in_macro, snippet, span_lint, span_help_and_lint, in_external_macro}; +use utils::{in_macro, match_def_path, snippet, span_lint, span_help_and_lint, in_external_macro}; /// Handles all the linting of funky types #[allow(missing_copy_implementations)] @@ -18,61 +17,37 @@ declare_lint!(pub LINKEDLIST, Warn, "usage of LinkedList, usually a vector is faster, or a more specialized data \ structure like a RingBuf"); -/// Matches a type with a provided string, and returns its type parameters if successful -pub fn match_ty_unwrap<'a>(ty: &'a Ty, segments: &[&str]) -> Option<&'a [P]> { - match ty.node { - TyPath(_, Path {segments: ref seg, ..}) => { - // So ast::Path isn't the full path, just the tokens that were provided. - // I could muck around with the maps and find the full path - // however the more efficient way is to simply reverse the iterators and zip them - // which will compare them in reverse until one of them runs out of segments - if seg.iter().rev().zip(segments.iter().rev()).all(|(a,b)| a.identifier.name == b) { - match seg[..].last() { - Some(&PathSegment {parameters: AngleBracketedParameters(ref a), ..}) => { - Some(&a.types[..]) - } - _ => None - } - } else { - None - } - }, - _ => None - } -} - #[allow(unused_imports)] impl LintPass for TypePass { fn get_lints(&self) -> LintArray { lint_array!(BOX_VEC, LINKEDLIST) } - fn check_ty(&mut self, cx: &Context, ty: &ast::Ty) { + fn check_ty(&mut self, cx: &Context, ast_ty: &ast::Ty) { { // In case stuff gets moved around - use std::boxed::Box; - use std::vec::Vec; + use collections::vec::Vec; + use collections::linked_list::LinkedList; } - match_ty_unwrap(ty, &["std", "boxed", "Box"]).and_then(|t| t.first()) - .and_then(|t| match_ty_unwrap(&**t, &["std", "vec", "Vec"])) - .map(|_| { - span_help_and_lint(cx, BOX_VEC, ty.span, - "you seem to be trying to use `Box>`. Did you mean to use `Vec`?", - "`Vec` is already on the heap, `Box>` makes an extra allocation"); - }); - { - // In case stuff gets moved around - use collections::linked_list::LinkedList as DL1; - use std::collections::linked_list::LinkedList as DL2; - } - let dlists = [vec!["std","collections","linked_list","LinkedList"], - vec!["collections","linked_list","LinkedList"]]; - for path in &dlists { - if match_ty_unwrap(ty, &path[..]).is_some() { - span_help_and_lint(cx, LINKEDLIST, ty.span, - "I see you're using a LinkedList! Perhaps you meant some other data structure?", - "a RingBuf might work"); - return; + if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) { + if let ty::TyBox(ref inner) = ty.sty { + if let ty::TyStruct(did, _) = inner.sty { + if match_def_path(cx, did.did, &["collections", "vec", "Vec"]) { + span_help_and_lint( + cx, BOX_VEC, ast_ty.span, + "you seem to be trying to use `Box>`. Did you mean to use `Vec`?", + "`Vec` is already on the heap, `Box>` makes an extra allocation"); + } + } + } + if let ty::TyStruct(did, _) = ty.sty { + if match_def_path(cx, did.did, &["collections", "linked_list", "LinkedList"]) { + span_help_and_lint( + cx, LINKEDLIST, ast_ty.span, + "I see you're using a LinkedList! Perhaps you meant some other data structure?", + "a RingBuf might work"); + return; + } } } } diff --git a/tests/compile-fail/dlist.rs b/tests/compile-fail/dlist.rs old mode 100644 new mode 100755 index a2343c339..a800c045a --- a/tests/compile-fail/dlist.rs +++ b/tests/compile-fail/dlist.rs @@ -12,4 +12,4 @@ pub fn test(foo: LinkedList) { //~ ERROR I see you're using a LinkedList! fn main(){ test(LinkedList::new()); -} \ No newline at end of file +} From a437936d49081faa82227c7d216ff3b0d6363ed3 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 21 Aug 2015 18:48:36 +0200 Subject: [PATCH 3/4] all: put often used DefPaths into utils as consts Also remove the "use xxx;" blocks to ensure import paths don't change. They don't work anyway since stuff may still be re-exported at the old location, while we need the "canonical" location for the type checks. Plus, the test suite catches all these cases. --- src/methods.rs | 14 ++++---------- src/ptr_arg.rs | 11 +++-------- src/strings.rs | 3 ++- src/types.rs | 11 +++-------- src/utils.rs | 7 +++++++ 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/src/methods.rs b/src/methods.rs index f2df736be..70cf32e50 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -3,6 +3,7 @@ use rustc::lint::*; use rustc::middle::ty; use utils::{span_lint, match_def_path, walk_ptrs_ty}; +use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH}; #[derive(Copy,Clone)] pub struct MethodsPass; @@ -16,30 +17,23 @@ declare_lint!(pub STR_TO_STRING, Warn, declare_lint!(pub STRING_TO_STRING, Warn, "calling `String.to_string()` which is a no-op"); -#[allow(unused_imports)] impl LintPass for MethodsPass { fn get_lints(&self) -> LintArray { lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING) } fn check_expr(&mut self, cx: &Context, expr: &Expr) { - { - // In case stuff gets moved around - use core::option::Option; - use core::result::Result; - use collections::string::String; - } if let ExprMethodCall(ref ident, _, ref args) = expr.node { let ref obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(&*args[0])).sty; if ident.node.name == "unwrap" { if let ty::TyEnum(did, _) = *obj_ty { - if match_def_path(cx, did.did, &["core", "option", "Option"]) { + if match_def_path(cx, did.did, &OPTION_PATH) { span_lint(cx, OPTION_UNWRAP_USED, expr.span, "used unwrap() on an Option value. If you don't want \ to handle the None case gracefully, consider using expect() to provide a better panic message"); } - else if match_def_path(cx, did.did, &["core", "result", "Result"]) { + else if match_def_path(cx, did.did, &RESULT_PATH) { span_lint(cx, RESULT_UNWRAP_USED, expr.span, "used unwrap() on a Result value. Graceful handling \ of Err values is preferred"); @@ -51,7 +45,7 @@ impl LintPass for MethodsPass { span_lint(cx, STR_TO_STRING, expr.span, "`str.to_owned()` is faster"); } else if let ty::TyStruct(did, _) = *obj_ty { - if match_def_path(cx, did.did, &["collections", "string", "String"]) { + if match_def_path(cx, did.did, &STRING_PATH) { span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op") } diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index 35c61b102..cdf4ecb48 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -7,6 +7,7 @@ use syntax::ast::*; use rustc::middle::ty; use utils::{span_lint, match_def_path}; +use utils::{STRING_PATH, VEC_PATH}; declare_lint! { pub PTR_ARG, @@ -42,13 +43,7 @@ impl LintPass for PtrArg { } } -#[allow(unused_imports)] fn check_fn(cx: &Context, decl: &FnDecl) { - { - // In case stuff gets moved around - use collections::vec::Vec; - use collections::string::String; - } for arg in &decl.inputs { if arg.ty.node == TyInfer { // "self" arguments continue; @@ -56,13 +51,13 @@ fn check_fn(cx: &Context, decl: &FnDecl) { let ref sty = cx.tcx.pat_ty(&*arg.pat).sty; if let &ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = sty { if let ty::TyStruct(did, _) = ty.sty { - if match_def_path(cx, did.did, &["collections", "vec", "Vec"]) { + if match_def_path(cx, did.did, &VEC_PATH) { span_lint(cx, PTR_ARG, arg.ty.span, "writing `&Vec<_>` instead of `&[_]` involves one more reference \ and cannot be used with non-Vec-based slices. Consider changing \ the type to `&[...]`"); } - else if match_def_path(cx, did.did, &["collections", "string", "String"]) { + else if match_def_path(cx, did.did, &STRING_PATH) { span_lint(cx, PTR_ARG, arg.ty.span, "writing `&String` instead of `&str` involves a new object \ where a slice will do. Consider changing the type to `&str`"); diff --git a/src/strings.rs b/src/strings.rs index b1da93d71..c4fbca934 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -10,6 +10,7 @@ use syntax::codemap::Spanned; use eq_op::is_exp_equal; use utils::{match_def_path, span_lint, walk_ptrs_ty, get_parent_expr}; +use utils::STRING_PATH; declare_lint! { pub STRING_ADD_ASSIGN, @@ -63,7 +64,7 @@ impl LintPass for StringAdd { fn is_string(cx: &Context, e: &Expr) -> bool { let ty = walk_ptrs_ty(cx.tcx.expr_ty(e)); if let TyStruct(did, _) = ty.sty { - match_def_path(cx, did.did, &["collections", "string", "String"]) + match_def_path(cx, did.did, &STRING_PATH) } else { false } } diff --git a/src/types.rs b/src/types.rs index 986fb1016..57649adee 100644 --- a/src/types.rs +++ b/src/types.rs @@ -6,6 +6,7 @@ use rustc::middle::ty; use syntax::codemap::ExpnInfo; use utils::{in_macro, match_def_path, snippet, span_lint, span_help_and_lint, in_external_macro}; +use utils::{LL_PATH, VEC_PATH}; /// Handles all the linting of funky types #[allow(missing_copy_implementations)] @@ -17,22 +18,16 @@ declare_lint!(pub LINKEDLIST, Warn, "usage of LinkedList, usually a vector is faster, or a more specialized data \ structure like a RingBuf"); -#[allow(unused_imports)] impl LintPass for TypePass { fn get_lints(&self) -> LintArray { lint_array!(BOX_VEC, LINKEDLIST) } fn check_ty(&mut self, cx: &Context, ast_ty: &ast::Ty) { - { - // In case stuff gets moved around - use collections::vec::Vec; - use collections::linked_list::LinkedList; - } if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) { if let ty::TyBox(ref inner) = ty.sty { if let ty::TyStruct(did, _) = inner.sty { - if match_def_path(cx, did.did, &["collections", "vec", "Vec"]) { + if match_def_path(cx, did.did, &VEC_PATH) { span_help_and_lint( cx, BOX_VEC, ast_ty.span, "you seem to be trying to use `Box>`. Did you mean to use `Vec`?", @@ -41,7 +36,7 @@ impl LintPass for TypePass { } } if let ty::TyStruct(did, _) = ty.sty { - if match_def_path(cx, did.did, &["collections", "linked_list", "LinkedList"]) { + if match_def_path(cx, did.did, &LL_PATH) { span_help_and_lint( cx, LINKEDLIST, ast_ty.span, "I see you're using a LinkedList! Perhaps you meant some other data structure?", diff --git a/src/utils.rs b/src/utils.rs index 47e3a3456..fe5c1433c 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -6,6 +6,13 @@ use rustc::ast_map::Node::NodeExpr; use rustc::middle::ty; use std::borrow::Cow; +// module DefPaths for certain structs/enums we check for +pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"]; +pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"]; +pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; +pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"]; +pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; + /// returns true if the macro that expanded the crate was outside of /// the current crate or was a compiler plugin pub fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool { From 8a10440641f7ec89236b1f40129738426ad4d702 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 21 Aug 2015 19:00:33 +0200 Subject: [PATCH 4/4] utils: add match_type() helper function which saves one level of matching when checking for type paths --- src/methods.rs | 35 ++++++++++++++--------------------- src/ptr_arg.rs | 23 ++++++++++------------- src/ranges.rs | 11 ++++------- src/strings.rs | 8 ++------ src/types.rs | 27 +++++++++++---------------- src/utils.rs | 12 ++++++++++++ 6 files changed, 53 insertions(+), 63 deletions(-) diff --git a/src/methods.rs b/src/methods.rs index 70cf32e50..df8e35d98 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -2,7 +2,7 @@ use syntax::ast::*; use rustc::lint::*; use rustc::middle::ty; -use utils::{span_lint, match_def_path, walk_ptrs_ty}; +use utils::{span_lint, match_type, walk_ptrs_ty}; use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH}; #[derive(Copy,Clone)] @@ -24,31 +24,24 @@ impl LintPass for MethodsPass { fn check_expr(&mut self, cx: &Context, expr: &Expr) { if let ExprMethodCall(ref ident, _, ref args) = expr.node { - let ref obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(&*args[0])).sty; + let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(&*args[0])); if ident.node.name == "unwrap" { - if let ty::TyEnum(did, _) = *obj_ty { - if match_def_path(cx, did.did, &OPTION_PATH) { - span_lint(cx, OPTION_UNWRAP_USED, expr.span, - "used unwrap() on an Option value. If you don't want \ - to handle the None case gracefully, consider using - expect() to provide a better panic message"); - } - else if match_def_path(cx, did.did, &RESULT_PATH) { - span_lint(cx, RESULT_UNWRAP_USED, expr.span, - "used unwrap() on a Result value. Graceful handling \ - of Err values is preferred"); - } + if match_type(cx, obj_ty, &OPTION_PATH) { + span_lint(cx, OPTION_UNWRAP_USED, expr.span, + "used unwrap() on an Option value. If you don't want \ + to handle the None case gracefully, consider using \ + expect() to provide a better panic message"); + } else if match_type(cx, obj_ty, &RESULT_PATH) { + span_lint(cx, RESULT_UNWRAP_USED, expr.span, + "used unwrap() on a Result value. Graceful handling \ + of Err values is preferred"); } } else if ident.node.name == "to_string" { - if let ty::TyStr = *obj_ty { + if obj_ty.sty == ty::TyStr { span_lint(cx, STR_TO_STRING, expr.span, "`str.to_owned()` is faster"); - } - else if let ty::TyStruct(did, _) = *obj_ty { - if match_def_path(cx, did.did, &STRING_PATH) { - span_lint(cx, STRING_TO_STRING, expr.span, - "`String.to_string()` is a no-op") - } + } else if match_type(cx, obj_ty, &STRING_PATH) { + span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op"); } } } diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index cdf4ecb48..f0a0592f5 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -6,7 +6,7 @@ use rustc::lint::*; use syntax::ast::*; use rustc::middle::ty; -use utils::{span_lint, match_def_path}; +use utils::{span_lint, match_type}; use utils::{STRING_PATH, VEC_PATH}; declare_lint! { @@ -50,18 +50,15 @@ fn check_fn(cx: &Context, decl: &FnDecl) { } let ref sty = cx.tcx.pat_ty(&*arg.pat).sty; if let &ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = sty { - if let ty::TyStruct(did, _) = ty.sty { - if match_def_path(cx, did.did, &VEC_PATH) { - span_lint(cx, PTR_ARG, arg.ty.span, - "writing `&Vec<_>` instead of `&[_]` involves one more reference \ - and cannot be used with non-Vec-based slices. Consider changing \ - the type to `&[...]`"); - } - else if match_def_path(cx, did.did, &STRING_PATH) { - span_lint(cx, PTR_ARG, arg.ty.span, - "writing `&String` instead of `&str` involves a new object \ - where a slice will do. Consider changing the type to `&str`"); - } + if match_type(cx, ty, &VEC_PATH) { + span_lint(cx, PTR_ARG, arg.ty.span, + "writing `&Vec<_>` instead of `&[_]` involves one more reference \ + and cannot be used with non-Vec-based slices. Consider changing \ + the type to `&[...]`"); + } else if match_type(cx, ty, &STRING_PATH) { + span_lint(cx, PTR_ARG, arg.ty.span, + "writing `&String` instead of `&str` involves a new object \ + where a slice will do. Consider changing the type to `&str`"); } } } diff --git a/src/ranges.rs b/src/ranges.rs index bbe65285d..d1a0a7e70 100644 --- a/src/ranges.rs +++ b/src/ranges.rs @@ -1,8 +1,7 @@ use rustc::lint::{Context, LintArray, LintPass}; -use rustc::middle::ty::TypeVariants::TyStruct; use syntax::ast::*; use syntax::codemap::Spanned; -use utils::{match_def_path}; +use utils::match_type; declare_lint! { pub RANGE_STEP_BY_ZERO, Warn, @@ -34,11 +33,9 @@ impl LintPass for StepByZero { fn is_range(cx: &Context, expr: &Expr) -> bool { // No need for walk_ptrs_ty here because step_by moves self, so it // can't be called on a borrowed range. - if let TyStruct(did, _) = cx.tcx.expr_ty(expr).sty { - // Note: RangeTo and RangeFull don't have step_by - match_def_path(cx, did.did, &["core", "ops", "Range"]) || - match_def_path(cx, did.did, &["core", "ops", "RangeFrom"]) - } else { false } + let ty = cx.tcx.expr_ty(expr); + // Note: RangeTo and RangeFull don't have step_by + match_type(cx, ty, &["core", "ops", "Range"]) || match_type(cx, ty, &["core", "ops", "RangeFrom"]) } fn is_lit_zero(expr: &Expr) -> bool { diff --git a/src/strings.rs b/src/strings.rs index c4fbca934..64d18eeb2 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -4,12 +4,11 @@ //! disable the subsumed lint unless it has a higher level use rustc::lint::*; -use rustc::middle::ty::TypeVariants::TyStruct; use syntax::ast::*; use syntax::codemap::Spanned; use eq_op::is_exp_equal; -use utils::{match_def_path, span_lint, walk_ptrs_ty, get_parent_expr}; +use utils::{match_type, span_lint, walk_ptrs_ty, get_parent_expr}; use utils::STRING_PATH; declare_lint! { @@ -62,10 +61,7 @@ impl LintPass for StringAdd { } fn is_string(cx: &Context, e: &Expr) -> bool { - let ty = walk_ptrs_ty(cx.tcx.expr_ty(e)); - if let TyStruct(did, _) = ty.sty { - match_def_path(cx, did.did, &STRING_PATH) - } else { false } + match_type(cx, walk_ptrs_ty(cx.tcx.expr_ty(e)), &STRING_PATH) } fn is_add(cx: &Context, src: &Expr, target: &Expr) -> bool { diff --git a/src/types.rs b/src/types.rs index 57649adee..622f733f8 100644 --- a/src/types.rs +++ b/src/types.rs @@ -5,7 +5,7 @@ use syntax::ast_util::{is_comparison_binop, binop_to_string}; use rustc::middle::ty; use syntax::codemap::ExpnInfo; -use utils::{in_macro, match_def_path, snippet, span_lint, span_help_and_lint, in_external_macro}; +use utils::{in_macro, match_type, snippet, span_lint, span_help_and_lint, in_external_macro}; use utils::{LL_PATH, VEC_PATH}; /// Handles all the linting of funky types @@ -26,23 +26,18 @@ impl LintPass for TypePass { fn check_ty(&mut self, cx: &Context, ast_ty: &ast::Ty) { if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) { if let ty::TyBox(ref inner) = ty.sty { - if let ty::TyStruct(did, _) = inner.sty { - if match_def_path(cx, did.did, &VEC_PATH) { - span_help_and_lint( - cx, BOX_VEC, ast_ty.span, - "you seem to be trying to use `Box>`. Did you mean to use `Vec`?", - "`Vec` is already on the heap, `Box>` makes an extra allocation"); - } + if match_type(cx, inner, &VEC_PATH) { + span_help_and_lint( + cx, BOX_VEC, ast_ty.span, + "you seem to be trying to use `Box>`. Did you mean to use `Vec`?", + "`Vec` is already on the heap, `Box>` makes an extra allocation"); } } - if let ty::TyStruct(did, _) = ty.sty { - if match_def_path(cx, did.did, &LL_PATH) { - span_help_and_lint( - cx, LINKEDLIST, ast_ty.span, - "I see you're using a LinkedList! Perhaps you meant some other data structure?", - "a RingBuf might work"); - return; - } + else if match_type(cx, ty, &LL_PATH) { + span_help_and_lint( + cx, LINKEDLIST, ast_ty.span, + "I see you're using a LinkedList! Perhaps you meant some other data structure?", + "a RingBuf might work"); } } } diff --git a/src/utils.rs b/src/utils.rs index fe5c1433c..4fd36fb91 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -44,6 +44,18 @@ pub fn match_def_path(cx: &Context, def_id: DefId, path: &[&str]) -> bool { .zip(path.iter()).all(|(nm, p)| nm == p)) } +/// check if type is struct or enum type with given def path +pub fn match_type(cx: &Context, ty: ty::Ty, path: &[&str]) -> bool { + match ty.sty { + ty::TyEnum(ref adt, _) | ty::TyStruct(ref adt, _) => { + match_def_path(cx, adt.did, path) + } + _ => { + false + } + } +} + /// match a Path against a slice of segment string literals, e.g. /// `match_path(path, &["std", "rt", "begin_unwind"])` pub fn match_path(path: &Path, segments: &[&str]) -> bool {