clippy_lints/src/functions.rs

   1 use std::convert::TryFrom;
   2
   3 use crate::utils::{iter_input_pats, snippet, snippet_opt, span_lint, type_is_unsafe_function};
   4 use matches::matches;
   5 use rustc::hir;
   6 use rustc::hir::def::Res;
   7 use rustc::hir::intravisit;
   8 use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
   9 use rustc::ty;
  10 use rustc::{declare_tool_lint, impl_lint_pass};
  11 use rustc_data_structures::fx::FxHashSet;
  12 use rustc_target::spec::abi::Abi;
  13 use syntax::source_map::{BytePos, Span};
  14
  15 declare_clippy_lint! {
  16     /// **What it does:** Checks for functions with too many parameters.
  17     ///
  18     /// **Why is this bad?** Functions with lots of parameters are considered bad
  19     /// style and reduce readability (“what does the 5th parameter mean?”). Consider
  20     /// grouping some parameters into a new type.
  21     ///
  22     /// **Known problems:** None.
  23     ///
  24     /// **Example:**
  25     /// ```rust
  26     /// # struct Color;
  27     /// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) {
  28     ///     // ..
  29     /// }
  30     /// ```
  31     pub TOO_MANY_ARGUMENTS,
  32     complexity,
  33     "functions with too many arguments"
  34 }
  35
  36 declare_clippy_lint! {
  37     /// **What it does:** Checks for functions with a large amount of lines.
  38     ///
  39     /// **Why is this bad?** Functions with a lot of lines are harder to understand
  40     /// due to having to look at a larger amount of code to understand what the
  41     /// function is doing. Consider splitting the body of the function into
  42     /// multiple functions.
  43     ///
  44     /// **Known problems:** None.
  45     ///
  46     /// **Example:**
  47     /// ``` rust
  48     /// fn im_too_long() {
  49     /// println!("");
  50     /// // ... 100 more LoC
  51     /// println!("");
  52     /// }
  53     /// ```
  54     pub TOO_MANY_LINES,
  55     pedantic,
  56     "functions with too many lines"
  57 }
  58
  59 declare_clippy_lint! {
  60     /// **What it does:** Checks for public functions that dereference raw pointer
  61     /// arguments but are not marked unsafe.
  62     ///
  63     /// **Why is this bad?** The function should probably be marked `unsafe`, since
  64     /// for an arbitrary raw pointer, there is no way of telling for sure if it is
  65     /// valid.
  66     ///
  67     /// **Known problems:**
  68     ///
  69     /// * It does not check functions recursively so if the pointer is passed to a
  70     /// private non-`unsafe` function which does the dereferencing, the lint won't
  71     /// trigger.
  72     /// * It only checks for arguments whose type are raw pointers, not raw pointers
  73     /// got from an argument in some other way (`fn foo(bar: &[*const u8])` or
  74     /// `some_argument.get_raw_ptr()`).
  75     ///
  76     /// **Example:**
  77     /// ```rust
  78     /// pub fn foo(x: *const u8) {
  79     ///     println!("{}", unsafe { *x });
  80     /// }
  81     /// ```
  82     pub NOT_UNSAFE_PTR_ARG_DEREF,
  83     correctness,
  84     "public functions dereferencing raw pointer arguments but not marked `unsafe`"
  85 }
  86
  87 #[derive(Copy, Clone)]
  88 pub struct Functions {
  89     threshold: u64,
  90     max_lines: u64,
  91 }
  92
  93 impl Functions {
  94     pub fn new(threshold: u64, max_lines: u64) -> Self {
  95         Self { threshold, max_lines }
  96     }
  97 }
  98
  99 impl_lint_pass!(Functions => [TOO_MANY_ARGUMENTS, TOO_MANY_LINES, NOT_UNSAFE_PTR_ARG_DEREF]);
 100
 101 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions {
 102     fn check_fn(
 103         &mut self,
 104         cx: &LateContext<'a, 'tcx>,
 105         kind: intravisit::FnKind<'tcx>,
 106         decl: &'tcx hir::FnDecl,
 107         body: &'tcx hir::Body,
 108         span: Span,
 109         hir_id: hir::HirId,
 110     ) {
 111         let is_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) {
 112             matches!(item.node, hir::ItemKind::Impl(_, _, _, _, Some(_), _, _))
 113         } else {
 114             false
 115         };
 116
 117         let unsafety = match kind {
 118             hir::intravisit::FnKind::ItemFn(_, _, hir::FnHeader { unsafety, .. }, _, _) => unsafety,
 119             hir::intravisit::FnKind::Method(_, sig, _, _) => sig.header.unsafety,
 120             hir::intravisit::FnKind::Closure(_) => return,
 121         };
 122
 123         // don't warn for implementations, it's not their fault
 124         if !is_impl {
 125             // don't lint extern functions decls, it's not their fault either
 126             match kind {
 127                 hir::intravisit::FnKind::Method(
 128                     _,
 129                     &hir::MethodSig {
 130                         header: hir::FnHeader { abi: Abi::Rust, .. },
 131                         ..
 132                     },
 133                     _,
 134                     _,
 135                 )
 136                 | hir::intravisit::FnKind::ItemFn(_, _, hir::FnHeader { abi: Abi::Rust, .. }, _, _) => {
 137                     self.check_arg_number(cx, decl, span)
 138                 },
 139                 _ => {},
 140             }
 141         }
 142
 143         self.check_raw_ptr(cx, unsafety, decl, body, hir_id);
 144         self.check_line_number(cx, span, body);
 145     }
 146
 147     fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) {
 148         if let hir::TraitItemKind::Method(ref sig, ref eid) = item.node {
 149             // don't lint extern functions decls, it's not their fault
 150             if sig.header.abi == Abi::Rust {
 151                 self.check_arg_number(cx, &sig.decl, item.span);
 152             }
 153
 154             if let hir::TraitMethod::Provided(eid) = *eid {
 155                 let body = cx.tcx.hir().body(eid);
 156                 self.check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.hir_id);
 157             }
 158         }
 159     }
 160 }
 161
 162 impl<'a, 'tcx> Functions {
 163     fn check_arg_number(self, cx: &LateContext<'_, '_>, decl: &hir::FnDecl, span: Span) {
 164         // Remove the function body from the span. We can't use `SourceMap::def_span` because the
 165         // argument list might span multiple lines.
 166         let span = if let Some(snippet) = snippet_opt(cx, span) {
 167             let snippet = snippet.split('{').nth(0).unwrap_or("").trim_end();
 168             if snippet.is_empty() {
 169                 span
 170             } else {
 171                 span.with_hi(BytePos(span.lo().0 + u32::try_from(snippet.len()).unwrap()))
 172             }
 173         } else {
 174             span
 175         };
 176
 177         let args = decl.inputs.len() as u64;
 178         if args > self.threshold {
 179             span_lint(
 180                 cx,
 181                 TOO_MANY_ARGUMENTS,
 182                 span,
 183                 &format!("this function has too many arguments ({}/{})", args, self.threshold),
 184             );
 185         }
 186     }
 187
 188     fn check_line_number(self, cx: &LateContext<'_, '_>, span: Span, body: &'tcx hir::Body) {
 189         if in_external_macro(cx.sess(), span) {
 190             return;
 191         }
 192
 193         let code_snippet = snippet(cx, body.value.span, "..");
 194         let mut line_count: u64 = 0;
 195         let mut in_comment = false;
 196         let mut code_in_line;
 197
 198         // Skip the surrounding function decl.
 199         let start_brace_idx = code_snippet.find('{').map_or(0, |i| i + 1);
 200         let end_brace_idx = code_snippet.rfind('}').unwrap_or_else(|| code_snippet.len());
 201         let function_lines = code_snippet[start_brace_idx..end_brace_idx].lines();
 202
 203         for mut line in function_lines {
 204             code_in_line = false;
 205             loop {
 206                 line = line.trim_start();
 207                 if line.is_empty() {
 208                     break;
 209                 }
 210                 if in_comment {
 211                     match line.find("*/") {
 212                         Some(i) => {
 213                             line = &line[i + 2..];
 214                             in_comment = false;
 215                             continue;
 216                         },
 217                         None => break,
 218                     }
 219                 } else {
 220                     let multi_idx = line.find("/*").unwrap_or_else(|| line.len());
 221                     let single_idx = line.find("//").unwrap_or_else(|| line.len());
 222                     code_in_line |= multi_idx > 0 && single_idx > 0;
 223                     // Implies multi_idx is below line.len()
 224                     if multi_idx < single_idx {
 225                         line = &line[multi_idx + 2..];
 226                         in_comment = true;
 227                         continue;
 228                     }
 229                     break;
 230                 }
 231             }
 232             if code_in_line {
 233                 line_count += 1;
 234             }
 235         }
 236
 237         if line_count > self.max_lines {
 238             span_lint(cx, TOO_MANY_LINES, span, "This function has a large number of lines.")
 239         }
 240     }
 241
 242     fn check_raw_ptr(
 243         self,
 244         cx: &LateContext<'a, 'tcx>,
 245         unsafety: hir::Unsafety,
 246         decl: &'tcx hir::FnDecl,
 247         body: &'tcx hir::Body,
 248         hir_id: hir::HirId,
 249     ) {
 250         let expr = &body.value;
 251         if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(hir_id) {
 252             let raw_ptrs = iter_input_pats(decl, body)
 253                 .zip(decl.inputs.iter())
 254                 .filter_map(|(arg, ty)| raw_ptr_arg(arg, ty))
 255                 .collect::<FxHashSet<_>>();
 256
 257             if !raw_ptrs.is_empty() {
 258                 let tables = cx.tcx.body_tables(body.id());
 259                 let mut v = DerefVisitor {
 260                     cx,
 261                     ptrs: raw_ptrs,
 262                     tables,
 263                 };
 264
 265                 hir::intravisit::walk_expr(&mut v, expr);
 266             }
 267         }
 268     }
 269 }
 270
 271 fn raw_ptr_arg(arg: &hir::Param, ty: &hir::Ty) -> Option<hir::HirId> {
 272     if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.node, &ty.node) {
 273         Some(id)
 274     } else {
 275         None
 276     }
 277 }
 278
 279 struct DerefVisitor<'a, 'tcx> {
 280     cx: &'a LateContext<'a, 'tcx>,
 281     ptrs: FxHashSet<hir::HirId>,
 282     tables: &'a ty::TypeckTables<'tcx>,
 283 }
 284
 285 impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> {
 286     fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
 287         match expr.node {
 288             hir::ExprKind::Call(ref f, ref args) => {
 289                 let ty = self.tables.expr_ty(f);
 290
 291                 if type_is_unsafe_function(self.cx, ty) {
 292                     for arg in args {
 293                         self.check_arg(arg);
 294                     }
 295                 }
 296             },
 297             hir::ExprKind::MethodCall(_, _, ref args) => {
 298                 let def_id = self.tables.type_dependent_def_id(expr.hir_id).unwrap();
 299                 let base_type = self.cx.tcx.type_of(def_id);
 300
 301                 if type_is_unsafe_function(self.cx, base_type) {
 302                     for arg in args {
 303                         self.check_arg(arg);
 304                     }
 305                 }
 306             },
 307             hir::ExprKind::Unary(hir::UnDeref, ref ptr) => self.check_arg(ptr),
 308             _ => (),
 309         }
 310
 311         hir::intravisit::walk_expr(self, expr);
 312     }
 313     fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> {
 314         intravisit::NestedVisitorMap::None
 315     }
 316 }
 317
 318 impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
 319     fn check_arg(&self, ptr: &hir::Expr) {
 320         if let hir::ExprKind::Path(ref qpath) = ptr.node {
 321             if let Res::Local(id) = self.cx.tables.qpath_res(qpath, ptr.hir_id) {
 322                 if self.ptrs.contains(&id) {
 323                     span_lint(
 324                         self.cx,
 325                         NOT_UNSAFE_PTR_ARG_DEREF,
 326                         ptr.span,
 327                         "this public function dereferences a raw pointer but is not marked `unsafe`",
 328                     );
 329                 }
 330             }
 331         }
 332     }
 333 }