From b2df15d65a7baf1f8d9b8b776027a34dd6e1db10 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 21 Aug 2015 18:28:17 +0200 Subject: [PATCH] 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 2748d187a4e..35c61b10266 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 d4b6d22608f..d0615be492b 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() { }