|
1 | | -use crate::utils::{iter_input_pats, span_lint, type_is_unsafe_function}; |
| 1 | +use crate::utils::{iter_input_pats, snippet, span_lint, type_is_unsafe_function}; |
2 | 2 | use matches::matches; |
3 | 3 | use rustc::hir; |
4 | 4 | use rustc::hir::def::Def; |
5 | 5 | use rustc::hir::intravisit; |
6 | | -use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; |
| 6 | +use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; |
7 | 7 | use rustc::ty; |
8 | 8 | use rustc::{declare_tool_lint, lint_array}; |
9 | 9 | use rustc_data_structures::fx::FxHashSet; |
@@ -31,6 +31,29 @@ declare_clippy_lint! { |
31 | 31 | "functions with too many arguments" |
32 | 32 | } |
33 | 33 |
|
| 34 | +/// **What it does:** Checks for functions with a large amount of lines. |
| 35 | +/// |
| 36 | +/// **Why is this bad?** Functions with a lot of lines are harder to understand |
| 37 | +/// due to having to look at a larger amount of code to understand what the |
| 38 | +/// function is doing. Consider splitting the body of the function into |
| 39 | +/// multiple functions. |
| 40 | +/// |
| 41 | +/// **Known problems:** None. |
| 42 | +/// |
| 43 | +/// **Example:** |
| 44 | +/// ``` rust |
| 45 | +/// fn im_too_long() { |
| 46 | +/// println!(""); |
| 47 | +/// // ... 100 more LoC |
| 48 | +/// println!(""); |
| 49 | +/// } |
| 50 | +/// ``` |
| 51 | +declare_clippy_lint! { |
| 52 | + pub TOO_MANY_LINES, |
| 53 | + pedantic, |
| 54 | + "functions with too many lines" |
| 55 | +} |
| 56 | + |
34 | 57 | /// **What it does:** Checks for public functions that dereferences raw pointer |
35 | 58 | /// arguments but are not marked unsafe. |
36 | 59 | /// |
@@ -62,17 +85,18 @@ declare_clippy_lint! { |
62 | 85 | #[derive(Copy, Clone)] |
63 | 86 | pub struct Functions { |
64 | 87 | threshold: u64, |
| 88 | + max_lines: u64, |
65 | 89 | } |
66 | 90 |
|
67 | 91 | impl Functions { |
68 | | - pub fn new(threshold: u64) -> Self { |
69 | | - Self { threshold } |
| 92 | + pub fn new(threshold: u64, max_lines: u64) -> Self { |
| 93 | + Self { threshold, max_lines } |
70 | 94 | } |
71 | 95 | } |
72 | 96 |
|
73 | 97 | impl LintPass for Functions { |
74 | 98 | fn get_lints(&self) -> LintArray { |
75 | | - lint_array!(TOO_MANY_ARGUMENTS, NOT_UNSAFE_PTR_ARG_DEREF) |
| 99 | + lint_array!(TOO_MANY_ARGUMENTS, TOO_MANY_LINES, NOT_UNSAFE_PTR_ARG_DEREF) |
76 | 100 | } |
77 | 101 |
|
78 | 102 | fn name(&self) -> &'static str { |
@@ -123,6 +147,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { |
123 | 147 | } |
124 | 148 |
|
125 | 149 | self.check_raw_ptr(cx, unsafety, decl, body, nodeid); |
| 150 | + self.check_line_number(cx, span); |
126 | 151 | } |
127 | 152 |
|
128 | 153 | fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) { |
@@ -153,6 +178,72 @@ impl<'a, 'tcx> Functions { |
153 | 178 | } |
154 | 179 | } |
155 | 180 |
|
| 181 | + fn check_line_number(self, cx: &LateContext<'_, '_>, span: Span) { |
| 182 | + if in_external_macro(cx.sess(), span) { |
| 183 | + return; |
| 184 | + } |
| 185 | + |
| 186 | + let code_snippet = snippet(cx, span, ".."); |
| 187 | + let mut line_count: u64 = 0; |
| 188 | + let mut in_comment = false; |
| 189 | + let mut code_in_line; |
| 190 | + |
| 191 | + // Skip the surrounding function decl. |
| 192 | + let start_brace_idx = match code_snippet.find('{') { |
| 193 | + Some(i) => i + 1, |
| 194 | + None => 0, |
| 195 | + }; |
| 196 | + let end_brace_idx = match code_snippet.find('}') { |
| 197 | + Some(i) => i, |
| 198 | + None => code_snippet.len(), |
| 199 | + }; |
| 200 | + let function_lines = code_snippet[start_brace_idx..end_brace_idx].lines(); |
| 201 | + |
| 202 | + for mut line in function_lines { |
| 203 | + code_in_line = false; |
| 204 | + loop { |
| 205 | + line = line.trim_start(); |
| 206 | + if line.is_empty() { |
| 207 | + break; |
| 208 | + } |
| 209 | + if in_comment { |
| 210 | + match line.find("*/") { |
| 211 | + Some(i) => { |
| 212 | + line = &line[i + 2..]; |
| 213 | + in_comment = false; |
| 214 | + continue; |
| 215 | + }, |
| 216 | + None => break, |
| 217 | + } |
| 218 | + } else { |
| 219 | + let multi_idx = match line.find("/*") { |
| 220 | + Some(i) => i, |
| 221 | + None => line.len(), |
| 222 | + }; |
| 223 | + let single_idx = match line.find("//") { |
| 224 | + Some(i) => i, |
| 225 | + None => line.len(), |
| 226 | + }; |
| 227 | + code_in_line |= multi_idx > 0 && single_idx > 0; |
| 228 | + // Implies multi_idx is below line.len() |
| 229 | + if multi_idx < single_idx { |
| 230 | + line = &line[multi_idx + 2..]; |
| 231 | + in_comment = true; |
| 232 | + continue; |
| 233 | + } |
| 234 | + break; |
| 235 | + } |
| 236 | + } |
| 237 | + if code_in_line { |
| 238 | + line_count += 1; |
| 239 | + } |
| 240 | + } |
| 241 | + |
| 242 | + if line_count > self.max_lines { |
| 243 | + span_lint(cx, TOO_MANY_LINES, span, "This function has a large number of lines.") |
| 244 | + } |
| 245 | + } |
| 246 | + |
156 | 247 | fn check_raw_ptr( |
157 | 248 | self, |
158 | 249 | cx: &LateContext<'a, 'tcx>, |
@@ -183,7 +274,7 @@ impl<'a, 'tcx> Functions { |
183 | 274 | } |
184 | 275 |
|
185 | 276 | fn raw_ptr_arg(arg: &hir::Arg, ty: &hir::Ty) -> Option<ast::NodeId> { |
186 | | - if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.node, &ty.node) { |
| 277 | + if let (&hir::PatKind::Binding(_, id, _, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.node, &ty.node) { |
187 | 278 | Some(id) |
188 | 279 | } else { |
189 | 280 | None |
|
0 commit comments