2 attr_by_name, attrs::is_proc_macro, is_must_use_ty, iter_input_pats, match_def_path, must_use_attr, qpath_res,
3 return_ty, snippet, snippet_opt, span_lint, span_lint_and_help, span_lint_and_then, trait_ref_of_method,
4 type_is_unsafe_function,
7 use rustc::hir::map::Map;
8 use rustc::lint::in_external_macro;
9 use rustc::ty::{self, Ty};
10 use rustc_data_structures::fx::FxHashSet;
11 use rustc_errors::Applicability;
13 use rustc_hir::intravisit;
14 use rustc_hir::{def::Res, def_id::DefId};
15 use rustc_lint::{LateContext, LateLintPass, LintContext};
16 use rustc_session::{declare_tool_lint, impl_lint_pass};
17 use rustc_span::source_map::Span;
18 use rustc_target::spec::abi::Abi;
19 use syntax::ast::Attribute;
21 declare_clippy_lint! {
22 /// **What it does:** Checks for functions with too many parameters.
24 /// **Why is this bad?** Functions with lots of parameters are considered bad
25 /// style and reduce readability (“what does the 5th parameter mean?”). Consider
26 /// grouping some parameters into a new type.
28 /// **Known problems:** None.
33 /// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) {
37 pub TOO_MANY_ARGUMENTS,
39 "functions with too many arguments"
42 declare_clippy_lint! {
43 /// **What it does:** Checks for functions with a large amount of lines.
45 /// **Why is this bad?** Functions with a lot of lines are harder to understand
46 /// due to having to look at a larger amount of code to understand what the
47 /// function is doing. Consider splitting the body of the function into
48 /// multiple functions.
50 /// **Known problems:** None.
54 /// fn im_too_long() {
56 /// // ... 100 more LoC
62 "functions with too many lines"
65 declare_clippy_lint! {
66 /// **What it does:** Checks for public functions that dereference raw pointer
67 /// arguments but are not marked unsafe.
69 /// **Why is this bad?** The function should probably be marked `unsafe`, since
70 /// for an arbitrary raw pointer, there is no way of telling for sure if it is
73 /// **Known problems:**
75 /// * It does not check functions recursively so if the pointer is passed to a
76 /// private non-`unsafe` function which does the dereferencing, the lint won't
78 /// * It only checks for arguments whose type are raw pointers, not raw pointers
79 /// got from an argument in some other way (`fn foo(bar: &[*const u8])` or
80 /// `some_argument.get_raw_ptr()`).
84 /// pub fn foo(x: *const u8) {
85 /// println!("{}", unsafe { *x });
88 pub NOT_UNSAFE_PTR_ARG_DEREF,
90 "public functions dereferencing raw pointer arguments but not marked `unsafe`"
93 declare_clippy_lint! {
94 /// **What it does:** Checks for a [`#[must_use]`] attribute on
95 /// unit-returning functions and methods.
97 /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
99 /// **Why is this bad?** Unit values are useless. The attribute is likely
100 /// a remnant of a refactoring that removed the return type.
102 /// **Known problems:** None.
111 "`#[must_use]` attribute on a unit-returning function / method"
114 declare_clippy_lint! {
115 /// **What it does:** Checks for a [`#[must_use]`] attribute without
116 /// further information on functions and methods that return a type already
117 /// marked as `#[must_use]`.
119 /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
121 /// **Why is this bad?** The attribute isn't needed. Not using the result
122 /// will already be reported. Alternatively, one can add some text to the
123 /// attribute to improve the lint message.
125 /// **Known problems:** None.
130 /// fn double_must_use() -> Result<(), ()> {
131 /// unimplemented!();
136 "`#[must_use]` attribute on a `#[must_use]`-returning function / method"
139 declare_clippy_lint! {
140 /// **What it does:** Checks for public functions that have no
141 /// [`#[must_use]`] attribute, but return something not already marked
142 /// must-use, have no mutable arg and mutate no statics.
144 /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
146 /// **Why is this bad?** Not bad at all, this lint just shows places where
147 /// you could add the attribute.
149 /// **Known problems:** The lint only checks the arguments for mutable
150 /// types without looking if they are actually changed. On the other hand,
151 /// it also ignores a broad range of potentially interesting side effects,
152 /// because we cannot decide whether the programmer intends the function to
153 /// be called for the side effect or the result. Expect many false
154 /// positives. At least we don't lint if the result type is unit or already
159 /// // this could be annotated with `#[must_use]`.
160 /// fn id<T>(t: T) -> T { t }
162 pub MUST_USE_CANDIDATE,
164 "function or method that could take a `#[must_use]` attribute"
167 #[derive(Copy, Clone)]
168 pub struct Functions {
174 pub fn new(threshold: u64, max_lines: u64) -> Self {
175 Self { threshold, max_lines }
179 impl_lint_pass!(Functions => [
182 NOT_UNSAFE_PTR_ARG_DEREF,
188 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions {
191 cx: &LateContext<'a, 'tcx>,
192 kind: intravisit::FnKind<'tcx>,
193 decl: &'tcx hir::FnDecl<'_>,
194 body: &'tcx hir::Body<'_>,
198 let is_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) {
199 matches!(item.kind, hir::ItemKind::Impl{ of_trait: Some(_), .. })
204 let unsafety = match kind {
205 intravisit::FnKind::ItemFn(_, _, hir::FnHeader { unsafety, .. }, _, _) => unsafety,
206 intravisit::FnKind::Method(_, sig, _, _) => sig.header.unsafety,
207 intravisit::FnKind::Closure(_) => return,
210 // don't warn for implementations, it's not their fault
212 // don't lint extern functions decls, it's not their fault either
214 intravisit::FnKind::Method(
217 header: hir::FnHeader { abi: Abi::Rust, .. },
223 | intravisit::FnKind::ItemFn(_, _, hir::FnHeader { abi: Abi::Rust, .. }, _, _) => {
224 self.check_arg_number(cx, decl, span.with_hi(decl.output.span().hi()))
230 Self::check_raw_ptr(cx, unsafety, decl, body, hir_id);
231 self.check_line_number(cx, span, body);
234 fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) {
235 let attr = must_use_attr(&item.attrs);
236 if let hir::ItemKind::Fn(ref sig, ref _generics, ref body_id) = item.kind {
237 if let Some(attr) = attr {
238 let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
239 check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr);
242 if cx.access_levels.is_exported(item.hir_id)
243 && !is_proc_macro(&item.attrs)
244 && attr_by_name(&item.attrs, "no_mangle").is_none()
246 check_must_use_candidate(
249 cx.tcx.hir().body(*body_id),
252 item.span.with_hi(sig.decl.output.span().hi()),
253 "this function could have a `#[must_use]` attribute",
259 fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ImplItem<'_>) {
260 if let hir::ImplItemKind::Method(ref sig, ref body_id) = item.kind {
261 let attr = must_use_attr(&item.attrs);
262 if let Some(attr) = attr {
263 let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
264 check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr);
265 } else if cx.access_levels.is_exported(item.hir_id)
266 && !is_proc_macro(&item.attrs)
267 && trait_ref_of_method(cx, item.hir_id).is_none()
269 check_must_use_candidate(
272 cx.tcx.hir().body(*body_id),
275 item.span.with_hi(sig.decl.output.span().hi()),
276 "this method could have a `#[must_use]` attribute",
282 fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem<'_>) {
283 if let hir::TraitItemKind::Method(ref sig, ref eid) = item.kind {
284 // don't lint extern functions decls, it's not their fault
285 if sig.header.abi == Abi::Rust {
286 self.check_arg_number(cx, &sig.decl, item.span.with_hi(sig.decl.output.span().hi()));
289 let attr = must_use_attr(&item.attrs);
290 if let Some(attr) = attr {
291 let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
292 check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr);
294 if let hir::TraitMethod::Provided(eid) = *eid {
295 let body = cx.tcx.hir().body(eid);
296 Self::check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.hir_id);
298 if attr.is_none() && cx.access_levels.is_exported(item.hir_id) && !is_proc_macro(&item.attrs) {
299 check_must_use_candidate(
305 item.span.with_hi(sig.decl.output.span().hi()),
306 "this method could have a `#[must_use]` attribute",
314 impl<'a, 'tcx> Functions {
315 fn check_arg_number(self, cx: &LateContext<'_, '_>, decl: &hir::FnDecl<'_>, fn_span: Span) {
316 let args = decl.inputs.len() as u64;
317 if args > self.threshold {
322 &format!("this function has too many arguments ({}/{})", args, self.threshold),
327 fn check_line_number(self, cx: &LateContext<'_, '_>, span: Span, body: &'tcx hir::Body<'_>) {
328 if in_external_macro(cx.sess(), span) {
332 let code_snippet = snippet(cx, body.value.span, "..");
333 let mut line_count: u64 = 0;
334 let mut in_comment = false;
335 let mut code_in_line;
337 // Skip the surrounding function decl.
338 let start_brace_idx = code_snippet.find('{').map_or(0, |i| i + 1);
339 let end_brace_idx = code_snippet.rfind('}').unwrap_or_else(|| code_snippet.len());
340 let function_lines = code_snippet[start_brace_idx..end_brace_idx].lines();
342 for mut line in function_lines {
343 code_in_line = false;
345 line = line.trim_start();
350 match line.find("*/") {
352 line = &line[i + 2..];
359 let multi_idx = line.find("/*").unwrap_or_else(|| line.len());
360 let single_idx = line.find("//").unwrap_or_else(|| line.len());
361 code_in_line |= multi_idx > 0 && single_idx > 0;
362 // Implies multi_idx is below line.len()
363 if multi_idx < single_idx {
364 line = &line[multi_idx + 2..];
376 if line_count > self.max_lines {
377 span_lint(cx, TOO_MANY_LINES, span, "This function has a large number of lines.")
382 cx: &LateContext<'a, 'tcx>,
383 unsafety: hir::Unsafety,
384 decl: &'tcx hir::FnDecl<'_>,
385 body: &'tcx hir::Body<'_>,
388 let expr = &body.value;
389 if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(hir_id) {
390 let raw_ptrs = iter_input_pats(decl, body)
391 .zip(decl.inputs.iter())
392 .filter_map(|(arg, ty)| raw_ptr_arg(arg, ty))
393 .collect::<FxHashSet<_>>();
395 if !raw_ptrs.is_empty() {
396 let tables = cx.tcx.body_tables(body.id());
397 let mut v = DerefVisitor {
403 intravisit::walk_expr(&mut v, expr);
409 fn check_needless_must_use(
410 cx: &LateContext<'_, '_>,
411 decl: &hir::FnDecl<'_>,
414 fn_header_span: Span,
417 if in_external_macro(cx.sess(), item_span) {
420 if returns_unit(decl) {
425 "this unit-returning function has a `#[must_use]` attribute",
429 "remove the attribute",
431 Applicability::MachineApplicable,
435 } else if !attr.is_value_str() && is_must_use_ty(cx, return_ty(cx, item_id)) {
440 "this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]`",
441 "either add some descriptive text or remove the attribute",
446 fn check_must_use_candidate<'a, 'tcx>(
447 cx: &LateContext<'a, 'tcx>,
448 decl: &'tcx hir::FnDecl<'_>,
449 body: &'tcx hir::Body<'_>,
455 if has_mutable_arg(cx, body)
456 || mutates_static(cx, body)
457 || in_external_macro(cx.sess(), item_span)
458 || returns_unit(decl)
459 || !cx.access_levels.is_exported(item_id)
460 || is_must_use_ty(cx, return_ty(cx, item_id))
464 span_lint_and_then(cx, MUST_USE_CANDIDATE, fn_span, msg, |db| {
465 if let Some(snippet) = snippet_opt(cx, fn_span) {
469 format!("#[must_use] {}", snippet),
470 Applicability::MachineApplicable,
476 fn returns_unit(decl: &hir::FnDecl<'_>) -> bool {
478 hir::FunctionRetTy::DefaultReturn(_) => true,
479 hir::FunctionRetTy::Return(ref ty) => match ty.kind {
480 hir::TyKind::Tup(ref tys) => tys.is_empty(),
481 hir::TyKind::Never => true,
487 fn has_mutable_arg(cx: &LateContext<'_, '_>, body: &hir::Body<'_>) -> bool {
488 let mut tys = FxHashSet::default();
489 body.params.iter().any(|param| is_mutable_pat(cx, ¶m.pat, &mut tys))
492 fn is_mutable_pat(cx: &LateContext<'_, '_>, pat: &hir::Pat<'_>, tys: &mut FxHashSet<DefId>) -> bool {
493 if let hir::PatKind::Wild = pat.kind {
494 return false; // ignore `_` patterns
496 let def_id = pat.hir_id.owner_def_id();
497 if cx.tcx.has_typeck_tables(def_id) {
498 is_mutable_ty(cx, &cx.tcx.typeck_tables_of(def_id).pat_ty(pat), pat.span, tys)
504 static KNOWN_WRAPPER_TYS: &[&[&str]] = &[&["alloc", "rc", "Rc"], &["std", "sync", "Arc"]];
506 fn is_mutable_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, span: Span, tys: &mut FxHashSet<DefId>) -> bool {
509 // primitive types are never mutable
510 Bool | Char | Int(_) | Uint(_) | Float(_) | Str => false,
511 Adt(ref adt, ref substs) => {
512 tys.insert(adt.did) && !ty.is_freeze(cx.tcx, cx.param_env, span)
513 || KNOWN_WRAPPER_TYS.iter().any(|path| match_def_path(cx, adt.did, path))
514 && substs.types().any(|ty| is_mutable_ty(cx, ty, span, tys))
516 Tuple(ref substs) => substs.types().any(|ty| is_mutable_ty(cx, ty, span, tys)),
517 Array(ty, _) | Slice(ty) => is_mutable_ty(cx, ty, span, tys),
518 RawPtr(ty::TypeAndMut { ty, mutbl }) | Ref(_, ty, mutbl) => {
519 mutbl == hir::Mutability::Mut || is_mutable_ty(cx, ty, span, tys)
521 // calling something constitutes a side effect, so return true on all callables
522 // also never calls need not be used, so return true for them, too
527 fn raw_ptr_arg(arg: &hir::Param<'_>, ty: &hir::Ty<'_>) -> Option<hir::HirId> {
528 if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.kind, &ty.kind) {
535 struct DerefVisitor<'a, 'tcx> {
536 cx: &'a LateContext<'a, 'tcx>,
537 ptrs: FxHashSet<hir::HirId>,
538 tables: &'a ty::TypeckTables<'tcx>,
541 impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> {
542 type Map = Map<'tcx>;
544 fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
546 hir::ExprKind::Call(ref f, args) => {
547 let ty = self.tables.expr_ty(f);
549 if type_is_unsafe_function(self.cx, ty) {
555 hir::ExprKind::MethodCall(_, _, args) => {
556 let def_id = self.tables.type_dependent_def_id(expr.hir_id).unwrap();
557 let base_type = self.cx.tcx.type_of(def_id);
559 if type_is_unsafe_function(self.cx, base_type) {
565 hir::ExprKind::Unary(hir::UnOp::UnDeref, ref ptr) => self.check_arg(ptr),
569 intravisit::walk_expr(self, expr);
572 fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<'_, Self::Map> {
573 intravisit::NestedVisitorMap::None
577 impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
578 fn check_arg(&self, ptr: &hir::Expr<'_>) {
579 if let hir::ExprKind::Path(ref qpath) = ptr.kind {
580 if let Res::Local(id) = qpath_res(self.cx, qpath, ptr.hir_id) {
581 if self.ptrs.contains(&id) {
584 NOT_UNSAFE_PTR_ARG_DEREF,
586 "this public function dereferences a raw pointer but is not marked `unsafe`",
594 struct StaticMutVisitor<'a, 'tcx> {
595 cx: &'a LateContext<'a, 'tcx>,
596 mutates_static: bool,
599 impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
600 type Map = Map<'tcx>;
602 fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
603 use hir::ExprKind::*;
605 if self.mutates_static {
609 Call(_, args) | MethodCall(_, _, args) => {
610 let mut tys = FxHashSet::default();
612 let def_id = arg.hir_id.owner_def_id();
613 if self.cx.tcx.has_typeck_tables(def_id)
616 self.cx.tcx.typeck_tables_of(def_id).expr_ty(arg),
620 && is_mutated_static(self.cx, arg)
622 self.mutates_static = true;
628 Assign(ref target, ..) | AssignOp(_, ref target, _) | AddrOf(_, hir::Mutability::Mut, ref target) => {
629 self.mutates_static |= is_mutated_static(self.cx, target)
635 fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<'_, Self::Map> {
636 intravisit::NestedVisitorMap::None
640 fn is_mutated_static(cx: &LateContext<'_, '_>, e: &hir::Expr<'_>) -> bool {
641 use hir::ExprKind::*;
645 if let Res::Local(_) = qpath_res(cx, qpath, e.hir_id) {
651 Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(cx, inner),
656 fn mutates_static<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, body: &'tcx hir::Body<'_>) -> bool {
657 let mut v = StaticMutVisitor {
659 mutates_static: false,
661 intravisit::walk_expr(&mut v, &body.value);