From d35b94349c9973fe372541376f96da682b80d24f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 1 Jul 2016 22:59:42 -0400 Subject: [PATCH 1/6] typo: use commas around "e.g." --- clippy_lints/src/methods.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index a959dbe1d6ad..c59453333a4d 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -73,7 +73,7 @@ declare_lint! { /// |`is_` |`&self` or none | /// |`to_` |`&self` | /// -/// **Why is this bad?** Consistency breeds readability. If you follow the conventions, your users won't be surprised that they e.g. need to supply a mutable reference to a `as_..` function. +/// **Why is this bad?** Consistency breeds readability. If you follow the conventions, your users won't be surprised that they, e.g., need to supply a mutable reference to a `as_..` function. /// /// **Known problems:** None /// From f609ac5b587f92947b3341a75e663b77ca73037c Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 2 Jul 2016 16:02:27 +0200 Subject: [PATCH 2/6] Bump to 0.0.78 --- CHANGELOG.md | 3 ++- Cargo.toml | 4 ++-- clippy_lints/Cargo.toml | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fece611c1dac..7fedd3749487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Change Log All notable changes to this project will be documented in this file. -## 0.0.78 - TBA +## 0.0.78 - 2016-07-02 +* Rustup to *rustc 1.11.0-nightly (01411937f 2016-07-01)* * New lints: [`wrong_transmute`, `double_neg`] * For compatibility, `cargo clippy` does not defines the `clippy` feature introduced in 0.0.76 anymore diff --git a/Cargo.toml b/Cargo.toml index 5dc1363f3f53..f8b63a56257c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.0.77" +version = "0.0.78" authors = [ "Manish Goregaokar ", "Andre Bogus ", @@ -25,7 +25,7 @@ test = false [dependencies] # begin automatic update -clippy_lints = { version = "0.0.77", path = "clippy_lints" } +clippy_lints = { version = "0.0.78", path = "clippy_lints" } # end automatic update [dev-dependencies] diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index a1012e94ee5a..e74db2b17a64 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clippy_lints" # begin automatic update -version = "0.0.77" +version = "0.0.78" # end automatic update authors = [ "Manish Goregaokar ", From 7781f1d7c5a8ec31f9994ebad0ae1ca140e69ef7 Mon Sep 17 00:00:00 2001 From: mcarton Date: Tue, 7 Jun 2016 16:55:55 +0200 Subject: [PATCH 3/6] Add a new `not_unsafe_ptr_arg_deref` lint --- CHANGELOG.md | 2 + README.md | 1 + clippy_lints/src/functions.rs | 128 ++++++++++++++++++++++++++++---- clippy_lints/src/lib.rs | 1 + clippy_lints/src/utils/mod.rs | 9 +++ tests/compile-fail/functions.rs | 50 ++++++++++++- 6 files changed, 174 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fedd3749487..ae316a90b05a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file. ## 0.0.76 — 2016-06-10 * Rustup to *rustc 1.11.0-nightly (7d2f75a95 2016-06-09)* * `cargo clippy` now automatically defines the `clippy` feature +* New lint: [`not_unsafe_ptr_arg_deref`] ## 0.0.75 — 2016-06-08 * Rustup to *rustc 1.11.0-nightly (763f9234b 2016-06-06)* @@ -220,6 +221,7 @@ All notable changes to this project will be documented in this file. [`non_ascii_literal`]: https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal [`nonminimal_bool`]: https://github.com/Manishearth/rust-clippy/wiki#nonminimal_bool [`nonsensical_open_options`]: https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options +[`not_unsafe_ptr_arg_deref`]: https://github.com/Manishearth/rust-clippy/wiki#not_unsafe_ptr_arg_deref [`ok_expect`]: https://github.com/Manishearth/rust-clippy/wiki#ok_expect [`option_map_unwrap_or`]: https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or [`option_map_unwrap_or_else`]: https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else diff --git a/README.md b/README.md index 2ae53db2d050..b1f8e7aabe4c 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,7 @@ name [non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal) | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead [nonminimal_bool](https://github.com/Manishearth/rust-clippy/wiki#nonminimal_bool) | allow | checks for boolean expressions that can be written more concisely [nonsensical_open_options](https://github.com/Manishearth/rust-clippy/wiki#nonsensical_open_options) | warn | nonsensical combination of options for opening a file +[not_unsafe_ptr_arg_deref](https://github.com/Manishearth/rust-clippy/wiki#not_unsafe_ptr_arg_deref) | warn | public functions dereferencing raw pointer arguments but not marked `unsafe` [ok_expect](https://github.com/Manishearth/rust-clippy/wiki#ok_expect) | warn | using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result [option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)` [option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)` diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 3d3423a47435..c02f02f064d3 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -1,9 +1,11 @@ -use rustc::lint::*; -use rustc::hir; use rustc::hir::intravisit; +use rustc::hir; +use rustc::ty; +use rustc::lint::*; +use std::collections::HashSet; use syntax::ast; use syntax::codemap::Span; -use utils::span_lint; +use utils::{span_lint, type_is_unsafe_function}; /// **What it does:** Check for functions with too many parameters. /// @@ -15,7 +17,7 @@ use utils::span_lint; /// /// **Example:** /// -/// ``` +/// ```rust /// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) { .. } /// ``` declare_lint! { @@ -24,6 +26,30 @@ declare_lint! { "functions with too many arguments" } +/// **What it does:** Check for public functions that dereferences raw pointer arguments but are +/// not marked unsafe. +/// +/// **Why is this bad?** The function should probably be marked `unsafe`, since for an arbitrary +/// raw pointer, there is no way of telling for sure if it is valid. +/// +/// **Known problems:** +/// +/// * It does not check functions recursively so if the pointer is passed to a private non- +/// `unsafe` function which does the dereferencing, the lint won't trigger. +/// * It only checks for arguments whose type are raw pointers, not raw pointers got from an +/// argument in some other way (`fn foo(bar: &[*const u8])` or `some_argument.get_raw_ptr()`). +/// +/// **Example:** +/// +/// ```rust +/// pub fn foo(x: *const u8) { println!("{}", unsafe { *x }); } +/// ``` +declare_lint! { + pub NOT_UNSAFE_PTR_ARG_DEREF, + Warn, + "public functions dereferencing raw pointer arguments but not marked `unsafe`" +} + #[derive(Copy,Clone)] pub struct Functions { threshold: u64, @@ -37,29 +63,41 @@ impl Functions { impl LintPass for Functions { fn get_lints(&self) -> LintArray { - lint_array!(TOO_MANY_ARGUMENTS) + lint_array!(TOO_MANY_ARGUMENTS, NOT_UNSAFE_PTR_ARG_DEREF) } } impl LateLintPass for Functions { - fn check_fn(&mut self, cx: &LateContext, _: intravisit::FnKind, decl: &hir::FnDecl, _: &hir::Block, span: Span, - nodeid: ast::NodeId) { + fn check_fn(&mut self, cx: &LateContext, kind: intravisit::FnKind, decl: &hir::FnDecl, block: &hir::Block, span: Span, nodeid: ast::NodeId) { use rustc::hir::map::Node::*; - if let Some(NodeItem(ref item)) = cx.tcx.map.find(cx.tcx.map.get_parent_node(nodeid)) { - match item.node { - hir::ItemImpl(_, _, _, Some(_), _, _) | - hir::ItemDefaultImpl(..) => return, - _ => (), - } + let is_impl = if let Some(NodeItem(ref item)) = cx.tcx.map.find(cx.tcx.map.get_parent_node(nodeid)) { + matches!(item.node, hir::ItemImpl(_, _, _, Some(_), _, _) | hir::ItemDefaultImpl(..)) + } else { + false + }; + + let unsafety = match kind { + hir::intravisit::FnKind::ItemFn(_, _, unsafety, _, _, _, _) => unsafety, + hir::intravisit::FnKind::Method(_, sig, _, _) => sig.unsafety, + hir::intravisit::FnKind::Closure(_) => return, + }; + + // don't warn for implementations, it's not their fault + if !is_impl { + self.check_arg_number(cx, decl, span); } - self.check_arg_number(cx, decl, span); + self.check_raw_ptr(cx, unsafety, decl, block, span, nodeid); } fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) { - if let hir::MethodTraitItem(ref sig, _) = item.node { + if let hir::MethodTraitItem(ref sig, ref block) = item.node { self.check_arg_number(cx, &sig.decl, item.span); + + if let Some(ref block) = *block { + self.check_raw_ptr(cx, sig.unsafety, &sig.decl, block, item.span, item.id); + } } } } @@ -74,4 +112,64 @@ impl Functions { &format!("this function has too many arguments ({}/{})", args, self.threshold)); } } + + fn check_raw_ptr(&self, cx: &LateContext, unsafety: hir::Unsafety, decl: &hir::FnDecl, block: &hir::Block, span: Span, nodeid: ast::NodeId) { + if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(nodeid) { + let raw_ptrs = decl.inputs.iter().filter_map(|arg| raw_ptr_arg(cx, arg)).collect::>(); + + if !raw_ptrs.is_empty() { + let mut v = DerefVisitor { + cx: cx, + ptrs: raw_ptrs, + }; + + hir::intravisit::walk_block(&mut v, block); + } + } + } +} + +fn raw_ptr_arg(cx: &LateContext, arg: &hir::Arg) -> Option { + if let (&hir::PatKind::Binding(_, _, _), &hir::TyPtr(_)) = (&arg.pat.node, &arg.ty.node) { + cx.tcx.def_map.borrow().get(&arg.pat.id).map(hir::def::PathResolution::def_id) + } else { + None + } +} + +struct DerefVisitor<'a, 'tcx: 'a> { + cx: &'a LateContext<'a, 'tcx>, + ptrs: HashSet, +} + +impl<'a, 'tcx, 'v> hir::intravisit::Visitor<'v> for DerefVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'v hir::Expr) { + let ptr = match expr.node { + hir::ExprUnary(hir::UnDeref, ref ptr) => Some(ptr), + hir::ExprMethodCall(_, _, ref args) => { + let method_call = ty::MethodCall::expr(expr.id); + let base_type = self.cx.tcx.tables.borrow().method_map[&method_call].ty; + + if type_is_unsafe_function(base_type) { + Some(&args[0]) + } else { + None + } + } + _ => None, + }; + + if let Some(ptr) = ptr { + if let Some(def) = self.cx.tcx.def_map.borrow().get(&ptr.id) { + if self.ptrs.contains(&def.def_id()) { + span_lint(self.cx, + NOT_UNSAFE_PTR_ARG_DEREF, + ptr.span, + "this public function dereferences a raw pointer but is not marked `unsafe`"); + } + } + } + + hir::intravisit::walk_expr(self, expr); + } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 040096dcbb4b..5ba52374135e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -322,6 +322,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { format::USELESS_FORMAT, formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, formatting::SUSPICIOUS_ELSE_FORMATTING, + functions::NOT_UNSAFE_PTR_ARG_DEREF, functions::TOO_MANY_ARGUMENTS, identity_op::IDENTITY_OP, len_zero::LEN_WITHOUT_IS_EMPTY, diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 9eb54c2a0139..30bb6dfeb765 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -813,3 +813,12 @@ pub fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> { }} None } + +/// Return whether the given type is an `unsafe` function. +pub fn type_is_unsafe_function(ty: ty::Ty) -> bool { + match ty.sty { + ty::TyFnDef(_, _, ref f) | + ty::TyFnPtr(ref f) => f.unsafety == Unsafety::Unsafe, + _ => false, + } +} diff --git a/tests/compile-fail/functions.rs b/tests/compile-fail/functions.rs index 2cc165686005..311da1ed2fe7 100644 --- a/tests/compile-fail/functions.rs +++ b/tests/compile-fail/functions.rs @@ -3,20 +3,24 @@ #![deny(clippy)] #![allow(dead_code)] +#![allow(unused_unsafe)] +// TOO_MANY_ARGUMENTS fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {} fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) { //~^ ERROR: this function has too many arguments (8/7) } -trait Foo { +pub trait Foo { fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool); fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()); //~^ ERROR: this function has too many arguments (8/7) + + fn ptr(p: *const u8); } -struct Bar; +pub struct Bar; impl Bar { fn good_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {} @@ -28,6 +32,48 @@ impl Bar { impl Foo for Bar { fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {} fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} + + fn ptr(p: *const u8) { + println!("{}", unsafe { *p }); + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + } +} + +// NOT_UNSAFE_PTR_ARG_DEREF + +fn private(p: *const u8) { + println!("{}", unsafe { *p }); +} + +pub fn public(p: *const u8) { + println!("{}", unsafe { *p }); + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + println!("{:?}", unsafe { p.as_ref() }); + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` +} + +impl Bar { + fn private(self, p: *const u8) { + println!("{}", unsafe { *p }); + } + + pub fn public(self, p: *const u8) { + println!("{}", unsafe { *p }); + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + println!("{:?}", unsafe { p.as_ref() }); + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + } + + pub fn public_ok(self, p: *const u8) { + if !p.is_null() { + println!("{:p}", p); + } + } + + pub unsafe fn public_unsafe(self, p: *const u8) { + println!("{}", unsafe { *p }); + println!("{:?}", unsafe { p.as_ref() }); + } } fn main() {} From 0e3dcd13765c5fed9795227ca75e15b8849ff5a4 Mon Sep 17 00:00:00 2001 From: mcarton Date: Tue, 7 Jun 2016 17:29:22 +0200 Subject: [PATCH 4/6] Improve `NOT_UNSAFE_PTR_ARG_DEREF` with functions --- README.md | 2 +- clippy_lints/src/functions.rs | 51 ++++++++++++++++++++------------- tests/compile-fail/functions.rs | 8 ++++++ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index b1f8e7aabe4c..3159a3e8979c 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 157 lints included in this crate: +There are 158 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index c02f02f064d3..a1918aed69f9 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -88,7 +88,7 @@ impl LateLintPass for Functions { self.check_arg_number(cx, decl, span); } - self.check_raw_ptr(cx, unsafety, decl, block, span, nodeid); + self.check_raw_ptr(cx, unsafety, decl, block, nodeid); } fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) { @@ -96,7 +96,7 @@ impl LateLintPass for Functions { self.check_arg_number(cx, &sig.decl, item.span); if let Some(ref block) = *block { - self.check_raw_ptr(cx, sig.unsafety, &sig.decl, block, item.span, item.id); + self.check_raw_ptr(cx, sig.unsafety, &sig.decl, block, item.id); } } } @@ -113,7 +113,7 @@ impl Functions { } } - fn check_raw_ptr(&self, cx: &LateContext, unsafety: hir::Unsafety, decl: &hir::FnDecl, block: &hir::Block, span: Span, nodeid: ast::NodeId) { + fn check_raw_ptr(&self, cx: &LateContext, unsafety: hir::Unsafety, decl: &hir::FnDecl, block: &hir::Block, nodeid: ast::NodeId) { if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(nodeid) { let raw_ptrs = decl.inputs.iter().filter_map(|arg| raw_ptr_arg(cx, arg)).collect::>(); @@ -144,32 +144,43 @@ struct DerefVisitor<'a, 'tcx: 'a> { impl<'a, 'tcx, 'v> hir::intravisit::Visitor<'v> for DerefVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'v hir::Expr) { - let ptr = match expr.node { - hir::ExprUnary(hir::UnDeref, ref ptr) => Some(ptr), + match expr.node { + hir::ExprCall(ref f, ref args) => { + let ty = self.cx.tcx.expr_ty(f); + + if type_is_unsafe_function(ty) { + for arg in args { + self.check_arg(arg); + } + } + } hir::ExprMethodCall(_, _, ref args) => { let method_call = ty::MethodCall::expr(expr.id); let base_type = self.cx.tcx.tables.borrow().method_map[&method_call].ty; if type_is_unsafe_function(base_type) { - Some(&args[0]) - } else { - None - } - } - _ => None, - }; - - if let Some(ptr) = ptr { - if let Some(def) = self.cx.tcx.def_map.borrow().get(&ptr.id) { - if self.ptrs.contains(&def.def_id()) { - span_lint(self.cx, - NOT_UNSAFE_PTR_ARG_DEREF, - ptr.span, - "this public function dereferences a raw pointer but is not marked `unsafe`"); + for arg in args { + self.check_arg(arg); + } } } + hir::ExprUnary(hir::UnDeref, ref ptr) => self.check_arg(ptr), + _ => (), } hir::intravisit::walk_expr(self, expr); } } + +impl<'a, 'tcx: 'a> DerefVisitor<'a, 'tcx> { + fn check_arg(&self, ptr: &hir::Expr) { + if let Some(def) = self.cx.tcx.def_map.borrow().get(&ptr.id) { + if self.ptrs.contains(&def.def_id()) { + span_lint(self.cx, + NOT_UNSAFE_PTR_ARG_DEREF, + ptr.span, + "this public function dereferences a raw pointer but is not marked `unsafe`"); + } + } + } +} diff --git a/tests/compile-fail/functions.rs b/tests/compile-fail/functions.rs index 311da1ed2fe7..f7ee41d28169 100644 --- a/tests/compile-fail/functions.rs +++ b/tests/compile-fail/functions.rs @@ -36,6 +36,10 @@ impl Foo for Bar { fn ptr(p: *const u8) { println!("{}", unsafe { *p }); //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + println!("{:?}", unsafe { p.as_ref() }); + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + unsafe { std::ptr::read(p) }; + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` } } @@ -50,6 +54,8 @@ pub fn public(p: *const u8) { //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` println!("{:?}", unsafe { p.as_ref() }); //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + unsafe { std::ptr::read(p) }; + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` } impl Bar { @@ -62,6 +68,8 @@ impl Bar { //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` println!("{:?}", unsafe { p.as_ref() }); //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` + unsafe { std::ptr::read(p) }; + //~^ ERROR: this public function dereferences a raw pointer but is not marked `unsafe` } pub fn public_ok(self, p: *const u8) { From 31948c481524247ddfe3dd600c51b0f501891414 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 3 Jul 2016 13:55:23 +0530 Subject: [PATCH 5/6] Make #991 work with current rust --- clippy_lints/src/functions.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index a1918aed69f9..0dec4e94c0b8 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -131,7 +131,7 @@ impl Functions { fn raw_ptr_arg(cx: &LateContext, arg: &hir::Arg) -> Option { if let (&hir::PatKind::Binding(_, _, _), &hir::TyPtr(_)) = (&arg.pat.node, &arg.ty.node) { - cx.tcx.def_map.borrow().get(&arg.pat.id).map(hir::def::PathResolution::def_id) + cx.tcx.def_map.borrow().get(&arg.pat.id).map(|pr| pr.full_def().def_id()) } else { None } @@ -175,7 +175,7 @@ impl<'a, 'tcx, 'v> hir::intravisit::Visitor<'v> for DerefVisitor<'a, 'tcx> { impl<'a, 'tcx: 'a> DerefVisitor<'a, 'tcx> { fn check_arg(&self, ptr: &hir::Expr) { if let Some(def) = self.cx.tcx.def_map.borrow().get(&ptr.id) { - if self.ptrs.contains(&def.def_id()) { + if self.ptrs.contains(&def.full_def().def_id()) { span_lint(self.cx, NOT_UNSAFE_PTR_ARG_DEREF, ptr.span, From 10b545e30b18216d536fd4799da65589e499b588 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Sun, 3 Jul 2016 12:12:43 -0700 Subject: [PATCH 6/6] Check for constant expression in useless_vec lint --- clippy_lints/src/vec.rs | 10 +++++++++- tests/compile-fail/vec.rs | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index 73f2fb7caa81..a6289837d4e9 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -1,6 +1,8 @@ use rustc::lint::*; use rustc::ty::TypeVariants; use rustc::hir::*; +use rustc_const_eval::EvalHint::ExprTypeChecked; +use rustc_const_eval::eval_const_expr_partial; use syntax::codemap::Span; use syntax::ptr::P; use utils::{is_expn_of, match_path, paths, recover_for_loop, snippet, span_lint_and_then}; @@ -52,9 +54,15 @@ impl LateLintPass for Pass { fn check_vec_macro(cx: &LateContext, vec: &Expr, span: Span) { if let Some(vec_args) = unexpand(cx, vec) { + let snippet = match vec_args { Args::Repeat(elem, len) => { - format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into() + // Check that the length is a constant expression + if eval_const_expr_partial(cx.tcx, len, ExprTypeChecked, None).is_ok() { + format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into() + } else { + return; + } } Args::Vec(args) => { if let Some(last) = args.iter().last() { diff --git a/tests/compile-fail/vec.rs b/tests/compile-fail/vec.rs index eda75a2fe8a4..92c99c20e36e 100644 --- a/tests/compile-fail/vec.rs +++ b/tests/compile-fail/vec.rs @@ -7,6 +7,16 @@ fn on_slice(_: &[u8]) {} #[allow(ptr_arg)] fn on_vec(_: &Vec) {} +struct Line { + length: usize, +} + +impl Line { + fn length(&self) -> usize { + self.length + } +} + fn main() { on_slice(&vec![]); //~^ ERROR useless use of `vec!` @@ -42,6 +52,12 @@ fn main() { on_vec(&vec![1, 2]); on_vec(&vec![1; 2]); + // Now with non-constant expressions + let line = Line { length: 2 }; + + on_slice(&vec![2; line.length]); + on_slice(&vec![2; line.length()]); + for a in vec![1, 2, 3] { //~^ ERROR useless use of `vec!` //~| HELP you can use