1 mod too_many_arguments;
4 use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_then};
5 use clippy_utils::source::snippet_opt;
6 use clippy_utils::ty::{is_must_use_ty, is_type_diagnostic_item, type_is_unsafe_function};
8 attr_by_name, attrs::is_proc_macro, iter_input_pats, match_def_path, must_use_attr, path_to_local, return_ty,
11 use if_chain::if_chain;
12 use rustc_ast::ast::Attribute;
13 use rustc_data_structures::fx::FxHashSet;
14 use rustc_errors::Applicability;
16 use rustc_hir::intravisit;
17 use rustc_hir::{def::Res, def_id::DefId, QPath};
18 use rustc_lint::{LateContext, LateLintPass, LintContext};
19 use rustc_middle::hir::map::Map;
20 use rustc_middle::lint::in_external_macro;
21 use rustc_middle::ty::{self, Ty};
22 use rustc_session::{declare_tool_lint, impl_lint_pass};
23 use rustc_span::{sym, Span};
24 use rustc_typeck::hir_ty_to_ty;
26 declare_clippy_lint! {
27 /// **What it does:** Checks for functions with too many parameters.
29 /// **Why is this bad?** Functions with lots of parameters are considered bad
30 /// style and reduce readability (“what does the 5th parameter mean?”). Consider
31 /// grouping some parameters into a new type.
33 /// **Known problems:** None.
38 /// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) {
42 pub TOO_MANY_ARGUMENTS,
44 "functions with too many arguments"
47 declare_clippy_lint! {
48 /// **What it does:** Checks for functions with a large amount of lines.
50 /// **Why is this bad?** Functions with a lot of lines are harder to understand
51 /// due to having to look at a larger amount of code to understand what the
52 /// function is doing. Consider splitting the body of the function into
53 /// multiple functions.
55 /// **Known problems:** None.
59 /// fn im_too_long() {
61 /// // ... 100 more LoC
67 "functions with too many lines"
70 declare_clippy_lint! {
71 /// **What it does:** Checks for public functions that dereference raw pointer
72 /// arguments but are not marked unsafe.
74 /// **Why is this bad?** The function should probably be marked `unsafe`, since
75 /// for an arbitrary raw pointer, there is no way of telling for sure if it is
78 /// **Known problems:**
80 /// * It does not check functions recursively so if the pointer is passed to a
81 /// private non-`unsafe` function which does the dereferencing, the lint won't
83 /// * It only checks for arguments whose type are raw pointers, not raw pointers
84 /// got from an argument in some other way (`fn foo(bar: &[*const u8])` or
85 /// `some_argument.get_raw_ptr()`).
90 /// pub fn foo(x: *const u8) {
91 /// println!("{}", unsafe { *x });
95 /// pub unsafe fn foo(x: *const u8) {
96 /// println!("{}", unsafe { *x });
99 pub NOT_UNSAFE_PTR_ARG_DEREF,
101 "public functions dereferencing raw pointer arguments but not marked `unsafe`"
104 declare_clippy_lint! {
105 /// **What it does:** Checks for a [`#[must_use]`] attribute on
106 /// unit-returning functions and methods.
108 /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
110 /// **Why is this bad?** Unit values are useless. The attribute is likely
111 /// a remnant of a refactoring that removed the return type.
113 /// **Known problems:** None.
122 "`#[must_use]` attribute on a unit-returning function / method"
125 declare_clippy_lint! {
126 /// **What it does:** Checks for a [`#[must_use]`] attribute without
127 /// further information on functions and methods that return a type already
128 /// marked as `#[must_use]`.
130 /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
132 /// **Why is this bad?** The attribute isn't needed. Not using the result
133 /// will already be reported. Alternatively, one can add some text to the
134 /// attribute to improve the lint message.
136 /// **Known problems:** None.
141 /// fn double_must_use() -> Result<(), ()> {
142 /// unimplemented!();
147 "`#[must_use]` attribute on a `#[must_use]`-returning function / method"
150 declare_clippy_lint! {
151 /// **What it does:** Checks for public functions that have no
152 /// [`#[must_use]`] attribute, but return something not already marked
153 /// must-use, have no mutable arg and mutate no statics.
155 /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
157 /// **Why is this bad?** Not bad at all, this lint just shows places where
158 /// you could add the attribute.
160 /// **Known problems:** The lint only checks the arguments for mutable
161 /// types without looking if they are actually changed. On the other hand,
162 /// it also ignores a broad range of potentially interesting side effects,
163 /// because we cannot decide whether the programmer intends the function to
164 /// be called for the side effect or the result. Expect many false
165 /// positives. At least we don't lint if the result type is unit or already
170 /// // this could be annotated with `#[must_use]`.
171 /// fn id<T>(t: T) -> T { t }
173 pub MUST_USE_CANDIDATE,
175 "function or method that could take a `#[must_use]` attribute"
178 declare_clippy_lint! {
179 /// **What it does:** Checks for public functions that return a `Result`
180 /// with an `Err` type of `()`. It suggests using a custom type that
181 /// implements [`std::error::Error`].
183 /// **Why is this bad?** Unit does not implement `Error` and carries no
184 /// further information about what went wrong.
186 /// **Known problems:** Of course, this lint assumes that `Result` is used
187 /// for a fallible operation (which is after all the intended use). However
188 /// code may opt to (mis)use it as a basic two-variant-enum. In that case,
189 /// the suggestion is misguided, and the code should use a custom enum
194 /// pub fn read_u8() -> Result<u8, ()> { Err(()) }
197 /// ```rust,should_panic
201 /// pub struct EndOfStream;
203 /// impl fmt::Display for EndOfStream {
204 /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
205 /// write!(f, "End of Stream")
209 /// impl std::error::Error for EndOfStream { }
211 /// pub fn read_u8() -> Result<u8, EndOfStream> { Err(EndOfStream) }
213 ///# read_u8().unwrap();
217 /// Note that there are crates that simplify creating the error type, e.g.
218 /// [`thiserror`](https://docs.rs/thiserror).
221 "public function returning `Result` with an `Err` type of `()`"
224 #[derive(Copy, Clone)]
225 pub struct Functions {
226 too_many_arguments_threshold: u64,
227 too_many_lines_threshold: u64,
231 pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64) -> Self {
233 too_many_arguments_threshold,
234 too_many_lines_threshold,
239 impl_lint_pass!(Functions => [
242 NOT_UNSAFE_PTR_ARG_DEREF,
249 impl<'tcx> LateLintPass<'tcx> for Functions {
252 cx: &LateContext<'tcx>,
253 kind: intravisit::FnKind<'tcx>,
254 decl: &'tcx hir::FnDecl<'_>,
255 body: &'tcx hir::Body<'_>,
259 too_many_arguments::check_fn(cx, kind, decl, span, hir_id, self.too_many_arguments_threshold);
260 too_many_lines::check(cx, span, body, self.too_many_lines_threshold);
262 let unsafety = match kind {
263 intravisit::FnKind::ItemFn(_, _, hir::FnHeader { unsafety, .. }, _) => unsafety,
264 intravisit::FnKind::Method(_, sig, _) => sig.header.unsafety,
265 intravisit::FnKind::Closure => return,
268 Self::check_raw_ptr(cx, unsafety, decl, body, hir_id);
271 fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
272 let attrs = cx.tcx.hir().attrs(item.hir_id());
273 let attr = must_use_attr(attrs);
274 if let hir::ItemKind::Fn(ref sig, ref _generics, ref body_id) = item.kind {
275 let is_public = cx.access_levels.is_exported(item.hir_id());
276 let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
278 check_result_unit_err(cx, &sig.decl, item.span, fn_header_span);
280 if let Some(attr) = attr {
281 check_needless_must_use(cx, &sig.decl, item.hir_id(), item.span, fn_header_span, attr);
284 if is_public && !is_proc_macro(cx.sess(), attrs) && attr_by_name(attrs, "no_mangle").is_none() {
285 check_must_use_candidate(
288 cx.tcx.hir().body(*body_id),
291 item.span.with_hi(sig.decl.output.span().hi()),
292 "this function could have a `#[must_use]` attribute",
298 fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
299 if let hir::ImplItemKind::Fn(ref sig, ref body_id) = item.kind {
300 let is_public = cx.access_levels.is_exported(item.hir_id());
301 let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
302 if is_public && trait_ref_of_method(cx, item.hir_id()).is_none() {
303 check_result_unit_err(cx, &sig.decl, item.span, fn_header_span);
305 let attrs = cx.tcx.hir().attrs(item.hir_id());
306 let attr = must_use_attr(attrs);
307 if let Some(attr) = attr {
308 check_needless_must_use(cx, &sig.decl, item.hir_id(), item.span, fn_header_span, attr);
309 } else if is_public && !is_proc_macro(cx.sess(), attrs) && trait_ref_of_method(cx, item.hir_id()).is_none()
311 check_must_use_candidate(
314 cx.tcx.hir().body(*body_id),
317 item.span.with_hi(sig.decl.output.span().hi()),
318 "this method could have a `#[must_use]` attribute",
324 fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) {
325 too_many_arguments::check_trait_item(cx, item, self.too_many_arguments_threshold);
327 if let hir::TraitItemKind::Fn(ref sig, ref eid) = item.kind {
328 let is_public = cx.access_levels.is_exported(item.hir_id());
329 let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
331 check_result_unit_err(cx, &sig.decl, item.span, fn_header_span);
334 let attrs = cx.tcx.hir().attrs(item.hir_id());
335 let attr = must_use_attr(attrs);
336 if let Some(attr) = attr {
337 check_needless_must_use(cx, &sig.decl, item.hir_id(), item.span, fn_header_span, attr);
339 if let hir::TraitFn::Provided(eid) = *eid {
340 let body = cx.tcx.hir().body(eid);
341 Self::check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.hir_id());
343 if attr.is_none() && is_public && !is_proc_macro(cx.sess(), attrs) {
344 check_must_use_candidate(
350 item.span.with_hi(sig.decl.output.span().hi()),
351 "this method could have a `#[must_use]` attribute",
359 impl<'tcx> Functions {
361 cx: &LateContext<'tcx>,
362 unsafety: hir::Unsafety,
363 decl: &'tcx hir::FnDecl<'_>,
364 body: &'tcx hir::Body<'_>,
367 let expr = &body.value;
368 if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(hir_id) {
369 let raw_ptrs = iter_input_pats(decl, body)
370 .zip(decl.inputs.iter())
371 .filter_map(|(arg, ty)| raw_ptr_arg(arg, ty))
372 .collect::<FxHashSet<_>>();
374 if !raw_ptrs.is_empty() {
375 let typeck_results = cx.tcx.typeck_body(body.id());
376 let mut v = DerefVisitor {
382 intravisit::walk_expr(&mut v, expr);
388 fn check_result_unit_err(cx: &LateContext<'_>, decl: &hir::FnDecl<'_>, item_span: Span, fn_header_span: Span) {
390 if !in_external_macro(cx.sess(), item_span);
391 if let hir::FnRetTy::Return(ref ty) = decl.output;
392 let ty = hir_ty_to_ty(cx.tcx, ty);
393 if is_type_diagnostic_item(cx, ty, sym::result_type);
394 if let ty::Adt(_, substs) = ty.kind();
395 let err_ty = substs.type_at(1);
402 "this returns a `Result<_, ()>",
404 "use a custom Error type instead",
410 fn check_needless_must_use(
411 cx: &LateContext<'_>,
412 decl: &hir::FnDecl<'_>,
415 fn_header_span: Span,
418 if in_external_macro(cx.sess(), item_span) {
421 if returns_unit(decl) {
426 "this unit-returning function has a `#[must_use]` attribute",
428 diag.span_suggestion(
430 "remove the attribute",
432 Applicability::MachineApplicable,
436 } else if !attr.is_value_str() && is_must_use_ty(cx, return_ty(cx, item_id)) {
441 "this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]`",
443 "either add some descriptive text or remove the attribute",
448 fn check_must_use_candidate<'tcx>(
449 cx: &LateContext<'tcx>,
450 decl: &'tcx hir::FnDecl<'_>,
451 body: &'tcx hir::Body<'_>,
457 if has_mutable_arg(cx, body)
458 || mutates_static(cx, body)
459 || in_external_macro(cx.sess(), item_span)
460 || returns_unit(decl)
461 || !cx.access_levels.is_exported(item_id)
462 || is_must_use_ty(cx, return_ty(cx, item_id))
466 span_lint_and_then(cx, MUST_USE_CANDIDATE, fn_span, msg, |diag| {
467 if let Some(snippet) = snippet_opt(cx, fn_span) {
468 diag.span_suggestion(
471 format!("#[must_use] {}", snippet),
472 Applicability::MachineApplicable,
478 fn returns_unit(decl: &hir::FnDecl<'_>) -> bool {
480 hir::FnRetTy::DefaultReturn(_) => true,
481 hir::FnRetTy::Return(ref ty) => match ty.kind {
482 hir::TyKind::Tup(ref tys) => tys.is_empty(),
483 hir::TyKind::Never => true,
489 fn has_mutable_arg(cx: &LateContext<'_>, body: &hir::Body<'_>) -> bool {
490 let mut tys = FxHashSet::default();
491 body.params.iter().any(|param| is_mutable_pat(cx, ¶m.pat, &mut tys))
494 fn is_mutable_pat(cx: &LateContext<'_>, pat: &hir::Pat<'_>, tys: &mut FxHashSet<DefId>) -> bool {
495 if let hir::PatKind::Wild = pat.kind {
496 return false; // ignore `_` patterns
498 if cx.tcx.has_typeck_results(pat.hir_id.owner.to_def_id()) {
499 is_mutable_ty(cx, &cx.tcx.typeck(pat.hir_id.owner).pat_ty(pat), pat.span, tys)
505 static KNOWN_WRAPPER_TYS: &[&[&str]] = &[&["alloc", "rc", "Rc"], &["std", "sync", "Arc"]];
507 fn is_mutable_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span, tys: &mut FxHashSet<DefId>) -> bool {
509 // primitive types are never mutable
510 ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => false,
511 ty::Adt(ref adt, ref substs) => {
512 tys.insert(adt.did) && !ty.is_freeze(cx.tcx.at(span), cx.param_env)
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 ty::Tuple(ref substs) => substs.types().any(|ty| is_mutable_ty(cx, ty, span, tys)),
517 ty::Array(ty, _) | ty::Slice(ty) => is_mutable_ty(cx, ty, span, tys),
518 ty::RawPtr(ty::TypeAndMut { ty, mutbl }) | ty::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<'tcx>,
537 ptrs: FxHashSet<hir::HirId>,
538 typeck_results: &'a ty::TypeckResults<'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.typeck_results.expr_ty(f);
549 if type_is_unsafe_function(self.cx, ty) {
555 hir::ExprKind::MethodCall(_, _, args, _) => {
556 let def_id = self.typeck_results.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::Deref, 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 Some(id) = path_to_local(ptr) {
580 if self.ptrs.contains(&id) {
583 NOT_UNSAFE_PTR_ARG_DEREF,
585 "this public function dereferences a raw pointer but is not marked `unsafe`",
592 struct StaticMutVisitor<'a, 'tcx> {
593 cx: &'a LateContext<'tcx>,
594 mutates_static: bool,
597 impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
598 type Map = Map<'tcx>;
600 fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
601 use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall};
603 if self.mutates_static {
607 Call(_, args) | MethodCall(_, _, args, _) => {
608 let mut tys = FxHashSet::default();
610 if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
613 self.cx.tcx.typeck(arg.hir_id.owner).expr_ty(arg),
617 && is_mutated_static(arg)
619 self.mutates_static = true;
625 Assign(ref target, ..) | AssignOp(_, ref target, _) | AddrOf(_, hir::Mutability::Mut, ref target) => {
626 self.mutates_static |= is_mutated_static(target)
632 fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
633 intravisit::NestedVisitorMap::None
637 fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
638 use hir::ExprKind::{Field, Index, Path};
641 Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
643 Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(inner),
648 fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool {
649 let mut v = StaticMutVisitor {
651 mutates_static: false,
653 intravisit::walk_expr(&mut v, &body.value);