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 = match code_snippet.find('{') {
 200             Some(i) => i + 1,
 201             None => 0,
 202         };
 203         let end_brace_idx = match code_snippet.rfind('}') {
 204             Some(i) => i,
 205             None => code_snippet.len(),
 206         };
 207         let function_lines = code_snippet[start_brace_idx..end_brace_idx].lines();
 208
 209         for mut line in function_lines {
 210             code_in_line = false;
 211             loop {
 212                 line = line.trim_start();
 213                 if line.is_empty() {
 214                     break;
 215                 }
 216                 if in_comment {
 217                     match line.find("*/") {
 218                         Some(i) => {
 219                             line = &line[i + 2..];
 220                             in_comment = false;
 221                             continue;
 222                         },
 223                         None => break,
 224                     }
 225                 } else {
 226                     let multi_idx = match line.find("/*") {
 227                         Some(i) => i,
 228                         None => line.len(),
 229                     };
 230                     let single_idx = match line.find("//") {
 231                         Some(i) => i,
 232                         None => line.len(),
 233                     };
 234                     code_in_line |= multi_idx > 0 && single_idx > 0;
 235                     // Implies multi_idx is below line.len()
 236                     if multi_idx < single_idx {
 237                         line = &line[multi_idx + 2..];
 238                         in_comment = true;
 239                         continue;
 240                     }
 241                     break;
 242                 }
 243             }
 244             if code_in_line {
 245                 line_count += 1;
 246             }
 247         }
 248
 249         if line_count > self.max_lines {
 250             span_lint(cx, TOO_MANY_LINES, span, "This function has a large number of lines.")
 251         }
 252     }
 253
 254     fn check_raw_ptr(
 255         self,
 256         cx: &LateContext<'a, 'tcx>,
 257         unsafety: hir::Unsafety,
 258         decl: &'tcx hir::FnDecl,
 259         body: &'tcx hir::Body,
 260         hir_id: hir::HirId,
 261     ) {
 262         let expr = &body.value;
 263         if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(hir_id) {
 264             let raw_ptrs = iter_input_pats(decl, body)
 265                 .zip(decl.inputs.iter())
 266                 .filter_map(|(arg, ty)| raw_ptr_arg(arg, ty))
 267                 .collect::<FxHashSet<_>>();
 268
 269             if !raw_ptrs.is_empty() {
 270                 let tables = cx.tcx.body_tables(body.id());
 271                 let mut v = DerefVisitor {
 272                     cx,
 273                     ptrs: raw_ptrs,
 274                     tables,
 275                 };
 276
 277                 hir::intravisit::walk_expr(&mut v, expr);
 278             }
 279         }
 280     }
 281 }
 282
 283 fn raw_ptr_arg(arg: &hir::Param, ty: &hir::Ty) -> Option<hir::HirId> {
 284     if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.node, &ty.node) {
 285         Some(id)
 286     } else {
 287         None
 288     }
 289 }
 290
 291 struct DerefVisitor<'a, 'tcx> {
 292     cx: &'a LateContext<'a, 'tcx>,
 293     ptrs: FxHashSet<hir::HirId>,
 294     tables: &'a ty::TypeckTables<'tcx>,
 295 }
 296
 297 impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> {
 298     fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
 299         match expr.node {
 300             hir::ExprKind::Call(ref f, ref args) => {
 301                 let ty = self.tables.expr_ty(f);
 302
 303                 if type_is_unsafe_function(self.cx, ty) {
 304                     for arg in args {
 305                         self.check_arg(arg);
 306                     }
 307                 }
 308             },
 309             hir::ExprKind::MethodCall(_, _, ref args) => {
 310                 let def_id = self.tables.type_dependent_def_id(expr.hir_id).unwrap();
 311                 let base_type = self.cx.tcx.type_of(def_id);
 312
 313                 if type_is_unsafe_function(self.cx, base_type) {
 314                     for arg in args {
 315                         self.check_arg(arg);
 316                     }
 317                 }
 318             },
 319             hir::ExprKind::Unary(hir::UnDeref, ref ptr) => self.check_arg(ptr),
 320             _ => (),
 321         }
 322
 323         hir::intravisit::walk_expr(self, expr);
 324     }
 325     fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> {
 326         intravisit::NestedVisitorMap::None
 327     }
 328 }
 329
 330 impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
 331     fn check_arg(&self, ptr: &hir::Expr) {
 332         if let hir::ExprKind::Path(ref qpath) = ptr.node {
 333             if let Res::Local(id) = self.cx.tables.qpath_res(qpath, ptr.hir_id) {
 334                 if self.ptrs.contains(&id) {
 335                     span_lint(
 336                         self.cx,
 337                         NOT_UNSAFE_PTR_ARG_DEREF,
 338                         ptr.span,
 339                         "this public function dereferences a raw pointer but is not marked `unsafe`",
 340                     );
 341                 }
 342             }
 343         }
 344     }
 345 }