Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Make too-many-arguments errors span fn header only #2599

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions clippy_lints/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
_ => {},
}
}
Expand All @@ -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),
);
}
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(_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: ()) {}

Expand Down
68 changes: 41 additions & 27 deletions tests/ui/functions.stderr
Original file line number Diff line number Diff line change
@@ -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<T>(_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