From ff16aa7ef09d0663a44cc9c7f1673f608fe9e6bc Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 31 Mar 2018 08:29:57 +0200 Subject: [PATCH] Make too-many-arguments errors span fn header only The HIR API has no span for just the function header. This commit handles that by using the entire function span until the start of the body expression. This isn't perfect because it includes spaces after the header. Closes #2488 --- clippy_lints/src/functions.rs | 28 ++++++++++----- tests/ui/functions.rs | 11 ++++++ tests/ui/functions.stderr | 68 +++++++++++++++++++++-------------- 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index cb359daba441..67cb5010a9ca 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -102,7 +102,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { // don't lint extern functions decls, it's not their fault either match kind { hir::intravisit::FnKind::Method(_, &hir::MethodSig { abi: Abi::Rust, .. }, _, _) | - hir::intravisit::FnKind::ItemFn(_, _, _, _, Abi::Rust, _, _) => self.check_arg_number(cx, decl, span), + hir::intravisit::FnKind::ItemFn(_, _, _, _, Abi::Rust, _, _) => { + self.check_arg_number(cx, decl, Some(body), span); + } _ => {}, } } @@ -113,26 +115,34 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) { if let hir::TraitItemKind::Method(ref sig, ref eid) = item.node { // don't lint extern functions decls, it's not their fault - if sig.abi == Abi::Rust { - self.check_arg_number(cx, &sig.decl, item.span); - } + let not_extern = sig.abi == Abi::Rust; + match *eid { + hir::TraitMethod::Required(_) => { + if not_extern { + self.check_arg_number(cx, &sig.decl, None, item.span); + } + }, - if let hir::TraitMethod::Provided(eid) = *eid { - let body = cx.tcx.hir.body(eid); - self.check_raw_ptr(cx, sig.unsafety, &sig.decl, body, item.id); + hir::TraitMethod::Provided(eid) => { + let body = cx.tcx.hir.body(eid); + if not_extern { + self.check_arg_number(cx, &sig.decl, Some(&body), item.span); + } + self.check_raw_ptr(cx, sig.unsafety, &sig.decl, body, item.id); + }, } } } } impl<'a, 'tcx> Functions { - fn check_arg_number(&self, cx: &LateContext, decl: &hir::FnDecl, span: Span) { + fn check_arg_number(&self, cx: &LateContext, decl: &hir::FnDecl, body: Option<&hir::Body>, span: Span) { let args = decl.inputs.len() as u64; if args > self.threshold { span_lint( cx, TOO_MANY_ARGUMENTS, - span, + body.map(|b| span.until(b.value.span)).unwrap_or(span), &format!("this function has too many arguments ({}/{})", args, self.threshold), ); } diff --git a/tests/ui/functions.rs b/tests/ui/functions.rs index 5688c471d86b..fc2abc2ea047 100644 --- a/tests/ui/functions.rs +++ b/tests/ui/functions.rs @@ -11,6 +11,17 @@ fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) { } +fn bad_with_ret(_a: u32, _b: u32, _c: u32, _d: u32, _e: u32, _f: u32, _g: u32, _h: u32) -> u32 { + 0 +} + +fn bad_with_where(_a: T, _b: u32, _c: u32, _d: u32, _e: u32, _f: u32, _g: u32, _h: u32) -> u32 +where + T: Copy +{ + 0 +} + // don't lint extern fns extern fn extern_fn(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} diff --git a/tests/ui/functions.stderr b/tests/ui/functions.stderr index 0a97748954f3..ff2ab2fb4ad4 100644 --- a/tests/ui/functions.stderr +++ b/tests/ui/functions.stderr @@ -1,79 +1,93 @@ error: this function has too many arguments (8/7) --> $DIR/functions.rs:11:1 | -11 | / fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) { -12 | | } - | |_^ +11 | fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D too-many-arguments` implied by `-D warnings` error: this function has too many arguments (8/7) - --> $DIR/functions.rs:19:5 + --> $DIR/functions.rs:14:1 | -19 | fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()); +14 | fn bad_with_ret(_a: u32, _b: u32, _c: u32, _d: u32, _e: u32, _f: u32, _g: u32, _h: u32) -> u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this function has too many arguments (8/7) + --> $DIR/functions.rs:18:1 + | +18 | / fn bad_with_where(_a: T, _b: u32, _c: u32, _d: u32, _e: u32, _f: u32, _g: u32, _h: u32) -> u32 +19 | | where +20 | | T: Copy +21 | | { + | |_ + +error: this function has too many arguments (8/7) + --> $DIR/functions.rs:30:5 + | +30 | 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) - --> $DIR/functions.rs:28:5 + --> $DIR/functions.rs:39:5 | -28 | fn bad_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +39 | fn bad_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:37:34 + --> $DIR/functions.rs:48:34 | -37 | println!("{}", unsafe { *p }); +48 | println!("{}", unsafe { *p }); | ^ | = note: `-D not-unsafe-ptr-arg-deref` implied by `-D warnings` error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:38:35 + --> $DIR/functions.rs:49:35 | -38 | println!("{:?}", unsafe { p.as_ref() }); +49 | println!("{:?}", unsafe { p.as_ref() }); | ^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:39:33 + --> $DIR/functions.rs:50:33 | -39 | unsafe { std::ptr::read(p) }; +50 | unsafe { std::ptr::read(p) }; | ^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:50:30 + --> $DIR/functions.rs:61:30 | -50 | println!("{}", unsafe { *p }); +61 | println!("{}", unsafe { *p }); | ^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:51:31 + --> $DIR/functions.rs:62:31 | -51 | println!("{:?}", unsafe { p.as_ref() }); +62 | println!("{:?}", unsafe { p.as_ref() }); | ^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:52:29 + --> $DIR/functions.rs:63:29 | -52 | unsafe { std::ptr::read(p) }; +63 | unsafe { std::ptr::read(p) }; | ^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:61:34 + --> $DIR/functions.rs:72:34 | -61 | println!("{}", unsafe { *p }); +72 | println!("{}", unsafe { *p }); | ^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:62:35 + --> $DIR/functions.rs:73:35 | -62 | println!("{:?}", unsafe { p.as_ref() }); +73 | println!("{:?}", unsafe { p.as_ref() }); | ^ error: this public function dereferences a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:63:33 + --> $DIR/functions.rs:74:33 | -63 | unsafe { std::ptr::read(p) }; +74 | unsafe { std::ptr::read(p) }; | ^ -error: aborting due to 12 previous errors +error: aborting due to 14 previous errors