diff --git a/.travis.yml b/.travis.yml index d456497d997..d7b26225518 100644 --- a/.travis.yml +++ b/.travis.yml @@ -39,40 +39,45 @@ install: matrix: fast_finish: true include: + # Builds that are executed for every PR - os: osx # run base tests on both platforms env: BASE_TESTS=true - os: linux env: BASE_TESTS=true - os: windows env: CARGO_INCREMENTAL=0 BASE_TESTS=true + + # Builds that are only executed when a PR is r+ed or a try build is started + # We don't want to run these always because they go towards + # the build limit within the Travis rust-lang account. - env: INTEGRATION=rust-lang/cargo - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-random/rand - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-lang-nursery/stdsimd - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-lang/rustfmt - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-lang-nursery/futures-rs - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-lang-nursery/failure - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-lang-nursery/log - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-lang-nursery/chalk - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=rust-lang/rls - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=chronotope/chrono - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=serde-rs/serde - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=Geal/nom - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=hyperium/hyper - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) - env: INTEGRATION=bluss/rust-itertools - if: repo =~ /^rust-lang\/rust-clippy$/ + if: repo =~ /^rust-lang\/rust-clippy$/ AND branch IN (auto, try) allow_failures: - os: windows env: CARGO_INCREMENTAL=0 BASE_TESTS=true diff --git a/CHANGELOG.md b/CHANGELOG.md index 44616a55610..59a18e9d4c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -982,6 +982,7 @@ All notable changes to this project will be documented in this file. [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments +[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str [`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index cc44b514ea7..2ef2a184746 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -64,6 +64,7 @@ impl LintPass for AssignOps { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps { + #[allow(clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) { match &expr.node { hir::ExprKind::AssignOp(op, lhs, rhs) => { diff --git a/clippy_lints/src/bit_mask.rs b/clippy_lints/src/bit_mask.rs index d4e30376199..f052ad6e5ac 100644 --- a/clippy_lints/src/bit_mask.rs +++ b/clippy_lints/src/bit_mask.rs @@ -177,6 +177,7 @@ fn check_compare(cx: &LateContext<'_, '_>, bit_op: &Expr, cmp_op: BinOpKind, cmp } } +#[allow(clippy::too_many_lines)] fn check_bit_mask( cx: &LateContext<'_, '_>, bit_op: BinOpKind, diff --git a/clippy_lints/src/blacklisted_name.rs b/clippy_lints/src/blacklisted_name.rs index 9606b2eda32..74d45505a4b 100644 --- a/clippy_lints/src/blacklisted_name.rs +++ b/clippy_lints/src/blacklisted_name.rs @@ -44,7 +44,7 @@ impl LintPass for BlackListedName { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlackListedName { fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) { - if let PatKind::Binding(_, _, ident, _) = pat.node { + if let PatKind::Binding(.., ident, _) = pat.node { if self.blacklist.contains(&ident.name.to_string()) { span_lint( cx, diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index c704a635425..41afe5ce0d7 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -286,7 +286,7 @@ fn bindings<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat) -> FxHashMap<LocalI bindings_impl(cx, pat, map); } }, - PatKind::Binding(_, _, ident, ref as_pat) => { + PatKind::Binding(.., ident, ref as_pat) => { if let Entry::Vacant(v) = map.entry(ident.as_str()) { v.insert(cx.tables.pat_ty(pat)); } diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index 57291dd24ee..2602ad45986 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -59,7 +59,7 @@ impl LintPass for EqOp { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp { - #[allow(clippy::similar_names)] + #[allow(clippy::similar_names, clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { if let ExprKind::Binary(op, ref left, ref right) = e.node { if in_macro(e.span) { diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index f0557154f90..4dbb390cd50 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -81,7 +81,7 @@ fn check_closure(cx: &LateContext<'_, '_>, expr: &Expr) { _ => (), } for (a1, a2) in iter_input_pats(decl, body).zip(args) { - if let PatKind::Binding(_, _, ident, _) = a1.pat.node { + if let PatKind::Binding(.., ident, _) = a1.pat.node { // XXXManishearth Should I be checking the binding mode here? if let ExprKind::Path(QPath::Resolved(None, ref p)) = a2.node { if p.segments.len() != 1 { diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index cb69e96c8e4..b6e0480d986 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -1,9 +1,9 @@ -use crate::utils::{iter_input_pats, span_lint, type_is_unsafe_function}; +use crate::utils::{iter_input_pats, snippet, span_lint, type_is_unsafe_function}; use matches::matches; use rustc::hir; use rustc::hir::def::Def; use rustc::hir::intravisit; -use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; use rustc::ty; use rustc::{declare_tool_lint, lint_array}; use rustc_data_structures::fx::FxHashSet; @@ -31,6 +31,29 @@ declare_clippy_lint! { "functions with too many arguments" } +/// **What it does:** Checks for functions with a large amount of lines. +/// +/// **Why is this bad?** Functions with a lot of lines are harder to understand +/// due to having to look at a larger amount of code to understand what the +/// function is doing. Consider splitting the body of the function into +/// multiple functions. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ``` rust +/// fn im_too_long() { +/// println!(""); +/// // ... 100 more LoC +/// println!(""); +/// } +/// ``` +declare_clippy_lint! { + pub TOO_MANY_LINES, + pedantic, + "functions with too many lines" +} + /// **What it does:** Checks for public functions that dereferences raw pointer /// arguments but are not marked unsafe. /// @@ -62,17 +85,18 @@ declare_clippy_lint! { #[derive(Copy, Clone)] pub struct Functions { threshold: u64, + max_lines: u64, } impl Functions { - pub fn new(threshold: u64) -> Self { - Self { threshold } + pub fn new(threshold: u64, max_lines: u64) -> Self { + Self { threshold, max_lines } } } impl LintPass for Functions { fn get_lints(&self) -> LintArray { - lint_array!(TOO_MANY_ARGUMENTS, NOT_UNSAFE_PTR_ARG_DEREF) + lint_array!(TOO_MANY_ARGUMENTS, TOO_MANY_LINES, NOT_UNSAFE_PTR_ARG_DEREF) } fn name(&self) -> &'static str { @@ -123,6 +147,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { } self.check_raw_ptr(cx, unsafety, decl, body, nodeid); + self.check_line_number(cx, span); } fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) { @@ -153,6 +178,72 @@ impl<'a, 'tcx> Functions { } } + fn check_line_number(self, cx: &LateContext<'_, '_>, span: Span) { + if in_external_macro(cx.sess(), span) { + return; + } + + let code_snippet = snippet(cx, span, ".."); + let mut line_count: u64 = 0; + let mut in_comment = false; + let mut code_in_line; + + // Skip the surrounding function decl. + let start_brace_idx = match code_snippet.find('{') { + Some(i) => i + 1, + None => 0, + }; + let end_brace_idx = match code_snippet.find('}') { + Some(i) => i, + None => code_snippet.len(), + }; + let function_lines = code_snippet[start_brace_idx..end_brace_idx].lines(); + + for mut line in function_lines { + code_in_line = false; + loop { + line = line.trim_start(); + if line.is_empty() { + break; + } + if in_comment { + match line.find("*/") { + Some(i) => { + line = &line[i + 2..]; + in_comment = false; + continue; + }, + None => break, + } + } else { + let multi_idx = match line.find("/*") { + Some(i) => i, + None => line.len(), + }; + let single_idx = match line.find("//") { + Some(i) => i, + None => line.len(), + }; + code_in_line |= multi_idx > 0 && single_idx > 0; + // Implies multi_idx is below line.len() + if multi_idx < single_idx { + line = &line[multi_idx + 2..]; + in_comment = true; + continue; + } + break; + } + } + if code_in_line { + line_count += 1; + } + } + + if line_count > self.max_lines { + span_lint(cx, TOO_MANY_LINES, span, "This function has a large number of lines.") + } + } + fn check_raw_ptr( self, cx: &LateContext<'a, 'tcx>, @@ -183,7 +274,7 @@ impl<'a, 'tcx> Functions { } fn raw_ptr_arg(arg: &hir::Arg, ty: &hir::Ty) -> Option<ast::NodeId> { - if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.node, &ty.node) { + if let (&hir::PatKind::Binding(_, id, _, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.node, &ty.node) { Some(id) } else { None diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index 59e036c715e..a6d34f2c7a2 100644 --- a/clippy_lints/src/large_enum_variant.rs +++ b/clippy_lints/src/large_enum_variant.rs @@ -94,10 +94,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant { |db| { if variant.fields.len() == 1 { let span = match def.variants[i].node.data { - VariantData::Struct(ref fields, _) | VariantData::Tuple(ref fields, _) => { + VariantData::Struct(ref fields, ..) | VariantData::Tuple(ref fields, ..) => { fields[0].ty.span }, - VariantData::Unit(_) => unreachable!(), + VariantData::Unit(..) => unreachable!(), }; if let Some(snip) = snippet_opt(cx, span) { db.span_suggestion( diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs index f1aed79847f..a4f69e32171 100644 --- a/clippy_lints/src/let_if_seq.rs +++ b/clippy_lints/src/let_if_seq.rs @@ -73,7 +73,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetIfSeq { if_chain! { if let Some(expr) = it.peek(); if let hir::StmtKind::Local(ref local) = stmt.node; - if let hir::PatKind::Binding(mode, canonical_id, ident, None) = local.pat.node; + if let hir::PatKind::Binding(mode, canonical_id, _, ident, None) = local.pat.node; if let hir::StmtKind::Expr(ref if_) = expr.node; if let hir::ExprKind::If(ref cond, ref then, ref else_) = if_.node; if !used_in_expr(cx, canonical_id, cond); diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0ec872c80be..5a9364eddb6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -292,6 +292,7 @@ pub fn read_conf(reg: &rustc_plugin::Registry<'_>) -> Conf { } } +#[allow(clippy::too_many_lines)] #[rustfmt::skip] pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { let mut store = reg.sess.lint_store.borrow_mut(); @@ -429,7 +430,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new( conf.blacklisted_names.iter().cloned().collect() )); - reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold)); + reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold, conf.too_many_lines_threshold)); reg.register_early_lint_pass(box doc::Doc::new(conf.doc_valid_idents.iter().cloned().collect())); reg.register_late_lint_pass(box neg_multiply::NegMultiply); reg.register_early_lint_pass(box unsafe_removed_from_name::UnsafeNameRemoval); @@ -530,6 +531,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { enum_glob_use::ENUM_GLOB_USE, enum_variants::MODULE_NAME_REPETITIONS, enum_variants::PUB_ENUM_VARIANT_NAMES, + functions::TOO_MANY_LINES, if_not_else::IF_NOT_ELSE, infinite_iter::MAYBE_INFINITE_ITER, items_after_statements::ITEMS_AFTER_STATEMENTS, diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 4d6cc75135e..dcd1a4e0a61 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -472,6 +472,7 @@ impl LintPass for Pass { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + #[allow(clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { // we don't want to check expanded macros if in_macro(expr.span) { @@ -968,7 +969,7 @@ fn detect_manual_memcpy<'a, 'tcx>( }) = higher::range(cx, arg) { // the var must be a single name - if let PatKind::Binding(_, canonical_id, _, _) = pat.node { + if let PatKind::Binding(_, canonical_id, _, _, _) = pat.node { let print_sum = |arg1: &Offset, arg2: &Offset| -> String { match (&arg1.value[..], arg1.negate, &arg2.value[..], arg2.negate) { ("0", _, "0", _) => "".into(), @@ -1066,6 +1067,7 @@ fn detect_manual_memcpy<'a, 'tcx>( /// Check for looping over a range and then indexing a sequence with it. /// The iteratee must be a range literal. +#[allow(clippy::too_many_lines)] fn check_for_loop_range<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat, @@ -1084,7 +1086,7 @@ fn check_for_loop_range<'a, 'tcx>( }) = higher::range(cx, arg) { // the var must be a single name - if let PatKind::Binding(_, canonical_id, ident, _) = pat.node { + if let PatKind::Binding(_, canonical_id, _, ident, _) = pat.node { let mut visitor = VarVisitor { cx, var: canonical_id, @@ -1635,7 +1637,7 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr) -> Option<NodeId let node_str = cx.tcx.hir().get(node_id); if_chain! { if let Node::Binding(pat) = node_str; - if let PatKind::Binding(bind_ann, _, _, _) = pat.node; + if let PatKind::Binding(bind_ann, ..) = pat.node; if let BindingAnnotation::Mutable = bind_ann; then { return Some(node_id); @@ -1668,7 +1670,7 @@ fn check_for_mutation( fn pat_is_wild<'tcx>(pat: &'tcx PatKind, body: &'tcx Expr) -> bool { match *pat { PatKind::Wild => true, - PatKind::Binding(_, _, ident, None) if ident.as_str().starts_with('_') => { + PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => { let mut visitor = UsedVisitor { var: ident.name, used: false, @@ -2093,7 +2095,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> { // Look for declarations of the variable if let StmtKind::Local(ref local) = stmt.node { if local.pat.id == self.var_id { - if let PatKind::Binding(_, _, ident, _) = local.pat.node { + if let PatKind::Binding(.., ident, _) = local.pat.node { self.name = Some(ident.name); self.state = if let Some(ref init) = local.init { @@ -2284,7 +2286,7 @@ impl<'tcx> Visitor<'tcx> for LoopNestVisitor { if self.nesting != Unknown { return; } - if let PatKind::Binding(_, _, span_name, _) = pat.node { + if let PatKind::Binding(.., span_name, _) = pat.node { if self.iterator == span_name.name { self.nesting = RuledOut; return; diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 49bd8f650e5..5699870c307 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -70,13 +70,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { then { match closure_body.arguments[0].pat.node { hir::PatKind::Ref(ref inner, _) => if let hir::PatKind::Binding( - hir::BindingAnnotation::Unannotated, _, name, None + hir::BindingAnnotation::Unannotated, .., name, None ) = inner.node { if ident_eq(name, closure_expr) { lint(cx, e.span, args[0].span); } }, - hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, name, None) => { + hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, .., name, None) => { match closure_expr.node { hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner) => { if ident_eq(name, inner) && !cx.tables.expr_ty(inner).is_box() { diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 6ef07316691..9cb160685ca 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -338,7 +338,7 @@ fn check_single_match_opt_like( } print::to_string(print::NO_ANN, |s| s.print_qpath(path, false)) }, - PatKind::Binding(BindingAnnotation::Unannotated, _, ident, None) => ident.to_string(), + PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => ident.to_string(), PatKind::Path(ref path) => print::to_string(print::NO_ANN, |s| s.print_qpath(path, false)), _ => return, }; @@ -657,7 +657,7 @@ fn is_ref_some_arm(arm: &Arm) -> Option<BindingAnnotation> { if_chain! { if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pats[0].node; if pats.len() == 1 && match_qpath(path, &paths::OPTION_SOME); - if let PatKind::Binding(rb, _, ident, _) = pats[0].node; + if let PatKind::Binding(rb, .., ident, _) = pats[0].node; if rb == BindingAnnotation::Ref || rb == BindingAnnotation::RefMut; if let ExprKind::Call(ref e, ref args) = remove_blocks(&arm.body).node; if let ExprKind::Path(ref some_path) = e.node; diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 20ffc1fd406..4ce2ae03b9f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1005,6 +1005,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } /// Checks for the `OR_FUN_CALL` lint. +#[allow(clippy::too_many_lines)] fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Span, name: &str, args: &[hir::Expr]) { /// Check for `unwrap_or(T::new())` or `unwrap_or(T::default())`. fn check_unwrap_or_default( @@ -1151,6 +1152,7 @@ fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Spa } /// Checks for the `EXPECT_FUN_CALL` lint. +#[allow(clippy::too_many_lines)] fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Span, name: &str, args: &[hir::Expr]) { // Strip `&`, `as_ref()` and `as_str()` off `arg` until we're left with either a `String` or // `&str` diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index c15fba76869..99cdba9402f 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -264,8 +264,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } for arg in iter_input_pats(decl, body) { match arg.pat.node { - PatKind::Binding(BindingAnnotation::Ref, _, _, _) - | PatKind::Binding(BindingAnnotation::RefMut, _, _, _) => { + PatKind::Binding(BindingAnnotation::Ref, ..) | PatKind::Binding(BindingAnnotation::RefMut, ..) => { span_lint( cx, TOPLEVEL_REF_ARG, @@ -282,7 +281,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) { if_chain! { if let StmtKind::Local(ref l) = s.node; - if let PatKind::Binding(an, _, i, None) = l.pat.node; + if let PatKind::Binding(an, .., i, None) = l.pat.node; if let Some(ref init) = l.init; then { if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut { @@ -445,7 +444,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { } fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) { - if let PatKind::Binding(_, _, ident, Some(ref right)) = pat.node { + if let PatKind::Binding(.., ident, Some(ref right)) = pat.node { if let PatKind::Wild = right.node { span_lint( cx, diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index 206a1465a46..777d2f683f0 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -89,7 +89,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrow { return; } if_chain! { - if let PatKind::Binding(BindingAnnotation::Ref, _, name, _) = pat.node; + if let PatKind::Binding(BindingAnnotation::Ref, .., name, _) = pat.node; if let ty::Ref(_, tam, mutbl) = cx.tables.pat_ty(pat).sty; if mutbl == MutImmutable; if let ty::Ref(_, _, mutbl) = tam.sty; diff --git a/clippy_lints/src/needless_borrowed_ref.rs b/clippy_lints/src/needless_borrowed_ref.rs index bf2857d9288..eae8ed541e2 100644 --- a/clippy_lints/src/needless_borrowed_ref.rs +++ b/clippy_lints/src/needless_borrowed_ref.rs @@ -76,7 +76,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrowedRef { if let PatKind::Ref(ref sub_pat, MutImmutable) = pat.node; // Check sub_pat got a `ref` keyword (excluding `ref mut`). - if let PatKind::Binding(BindingAnnotation::Ref, _, spanned_name, ..) = sub_pat.node; + if let PatKind::Binding(BindingAnnotation::Ref, .., spanned_name, _) = sub_pat.node; then { span_lint_and_then(cx, NEEDLESS_BORROWED_REFERENCE, pat.span, "this pattern takes a reference on something that is being de-referenced", diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 73c0ed72d3b..77a6aeba53b 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -73,6 +73,7 @@ macro_rules! need { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { + #[allow(clippy::too_many_lines)] fn check_fn( &mut self, cx: &LateContext<'a, 'tcx>, @@ -163,7 +164,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { // Ignore `self`s. if idx == 0 { - if let PatKind::Binding(_, _, ident, ..) = arg.pat.node { + if let PatKind::Binding(.., ident, _) = arg.pat.node { if ident.as_str() == "self" { continue; } diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 3bdf2c38d23..fbf60db28ee 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -158,6 +158,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> { ); } } + #[allow(clippy::too_many_lines)] fn check_name(&mut self, span: Span, name: Name) { let interned_name = name.as_str(); if interned_name.chars().any(char::is_uppercase) { diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index b990b7ab2bd..5a54971f49e 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -151,6 +151,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PointerPass { } } +#[allow(clippy::too_many_lines)] fn check_fn(cx: &LateContext<'_, '_>, decl: &FnDecl, fn_id: NodeId, opt_body_id: Option<BodyId>) { let fn_def_id = cx.tcx.hir().local_def_id(fn_id); let sig = cx.tcx.fn_sig(fn_def_id); diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 7ac147c8ac1..5c994d8a2bc 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -80,6 +80,7 @@ impl LintPass for RedundantClone { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { + #[allow(clippy::too_many_lines)] fn check_fn( &mut self, cx: &LateContext<'a, 'tcx>, diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 1f341bd22ee..722f64405c7 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -108,7 +108,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_fn<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, decl: &'tcx FnDecl, body: &'tcx Body) { let mut bindings = Vec::new(); for arg in iter_input_pats(decl, body) { - if let PatKind::Binding(_, _, ident, _) = arg.pat.node { + if let PatKind::Binding(.., ident, _) = arg.pat.node { bindings.push((ident.name, ident.span)) } } @@ -172,7 +172,7 @@ fn check_pat<'a, 'tcx>( ) { // TODO: match more stuff / destructuring match pat.node { - PatKind::Binding(_, _, ident, ref inner) => { + PatKind::Binding(.., ident, ref inner) => { let name = ident.name; if is_binding(cx, pat.hir_id) { let mut new_binding = true; diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index 1f9f369cfe4..b8ab32491a3 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -96,7 +96,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { // Matches statements which initializes vectors. For example: `let mut vec = Vec::with_capacity(10)` if_chain! { if let StmtKind::Local(ref local) = stmt.node; - if let PatKind::Binding(BindingAnnotation::Mutable, _, variable_name, None) = local.pat.node; + if let PatKind::Binding(BindingAnnotation::Mutable, .., variable_name, None) = local.pat.node; if let Some(ref init) = local.init; if let Some(ref len_arg) = Self::is_vec_with_capacity(init); diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index af7fd11c6e5..7b532cdb172 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -77,7 +77,7 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) { // let t = foo(); if let StmtKind::Local(ref tmp) = w[0].node; if let Some(ref tmp_init) = tmp.init; - if let PatKind::Binding(_, _, ident, None) = tmp.pat.node; + if let PatKind::Binding(.., ident, None) = tmp.pat.node; // foo() = bar(); if let StmtKind::Semi(ref first) = w[1].node; diff --git a/clippy_lints/src/transmute.rs b/clippy_lints/src/transmute.rs index 90cfcd56c3b..56e1aa4a4e1 100644 --- a/clippy_lints/src/transmute.rs +++ b/clippy_lints/src/transmute.rs @@ -226,7 +226,7 @@ impl LintPass for Transmute { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute { - #[allow(clippy::similar_names)] + #[allow(clippy::similar_names, clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { if let ExprKind::Call(ref path_expr, ref args) = e.node { if let ExprKind::Path(ref qpath) = path_expr.node { diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 94c83ed5720..b99e68e8946 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -186,7 +186,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypePass { check_fn_decl(cx, decl); } - fn check_struct_field(&mut self, cx: &LateContext<'_, '_>, field: &StructField) { + fn check_struct_field(&mut self, cx: &LateContext<'_, '_>, field: &hir::StructField) { check_ty(cx, &field.ty, false); } @@ -240,13 +240,14 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath, path: &[&str]) /// /// The parameter `is_local` distinguishes the context of the type; types from /// local bindings should only be checked for the `BORROWED_BOX` lint. -fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { - if in_macro(ast_ty.span) { +#[allow(clippy::too_many_lines)] +fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) { + if in_macro(hir_ty.span) { return; } - match ast_ty.node { + match hir_ty.node { TyKind::Path(ref qpath) if !is_local => { - let hir_id = cx.tcx.hir().node_to_hir_id(ast_ty.id); + let hir_id = cx.tcx.hir().node_to_hir_id(hir_ty.id); let def = cx.tables.qpath_def(qpath, hir_id); if let Some(def_id) = opt_def_id(def) { if Some(def_id) == cx.tcx.lang_items().owned_box() { @@ -254,7 +255,7 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { span_help_and_lint( cx, BOX_VEC, - ast_ty.span, + hir_ty.span, "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`", "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.", ); @@ -275,26 +276,24 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { if Some(def_id) == cx.tcx.lang_items().owned_box(); // At this point, we know ty is Box<T>, now get T if let Some(ref last) = last_path_segment(ty_qpath).args; - if let Some(ty) = last.args.iter().find_map(|arg| match arg { + if let Some(boxed_ty) = last.args.iter().find_map(|arg| match arg { GenericArg::Type(ty) => Some(ty), GenericArg::Lifetime(_) => None, }); - if let TyKind::Path(ref ty_qpath) = ty.node; - let def = cx.tables.qpath_def(ty_qpath, ty.hir_id); - if let Some(def_id) = opt_def_id(def); - let boxed_type = cx.tcx.type_of(def_id); - if boxed_type.is_sized(cx.tcx.at(ty.span), cx.param_env); then { - span_lint_and_sugg( - cx, - VEC_BOX, - ast_ty.span, - "`Vec<T>` is already on the heap, the boxing is unnecessary.", - "try", - format!("Vec<{}>", boxed_type), - Applicability::MaybeIncorrect, - ); - return; // don't recurse into the type + let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty); + if ty_ty.is_sized(cx.tcx.at(ty.span), cx.param_env) { + span_lint_and_sugg( + cx, + VEC_BOX, + hir_ty.span, + "`Vec<T>` is already on the heap, the boxing is unnecessary.", + "try", + format!("Vec<{}>", ty_ty), + Applicability::MachineApplicable, + ); + return; // don't recurse into the type + } } } } else if match_def_path(cx.tcx, def_id, &paths::OPTION) { @@ -302,7 +301,7 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { span_lint( cx, OPTION_OPTION, - ast_ty.span, + hir_ty.span, "consider using `Option<T>` instead of `Option<Option<T>>` or a custom \ enum if you need to distinguish all 3 cases", ); @@ -312,7 +311,7 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { span_help_and_lint( cx, LINKEDLIST, - ast_ty.span, + hir_ty.span, "I see you're using a LinkedList! Perhaps you meant some other data structure?", "a VecDeque might work", ); @@ -360,7 +359,7 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { }, } }, - TyKind::Rptr(ref lt, ref mut_ty) => check_ty_rptr(cx, ast_ty, is_local, lt, mut_ty), + TyKind::Rptr(ref lt, ref mut_ty) => check_ty_rptr(cx, hir_ty, is_local, lt, mut_ty), // recurse TyKind::Slice(ref ty) | TyKind::Array(ref ty, _) | TyKind::Ptr(MutTy { ref ty, .. }) => { check_ty(cx, ty, is_local) @@ -374,7 +373,7 @@ fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) { } } -fn check_ty_rptr(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool, lt: &Lifetime, mut_ty: &MutTy) { +fn check_ty_rptr(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool, lt: &Lifetime, mut_ty: &MutTy) { match mut_ty.ty.node { TyKind::Path(ref qpath) => { let hir_id = cx.tcx.hir().node_to_hir_id(mut_ty.ty.id); @@ -410,7 +409,7 @@ fn check_ty_rptr(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool, lt: span_lint_and_sugg( cx, BORROWED_BOX, - ast_ty.span, + hir_ty.span, "you seem to be trying to use `&Box<T>`. Consider using just `&T`", "try", format!( @@ -1324,7 +1323,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeComplexityPass { self.check_fndecl(cx, decl); } - fn check_struct_field(&mut self, cx: &LateContext<'a, 'tcx>, field: &'tcx StructField) { + fn check_struct_field(&mut self, cx: &LateContext<'a, 'tcx>, field: &'tcx hir::StructField) { // enum variants are also struct fields now self.check_type(cx, &field.ty); } @@ -1968,7 +1967,7 @@ impl LintPass for ImplicitHasher { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher { - #[allow(clippy::cast_possible_truncation)] + #[allow(clippy::cast_possible_truncation, clippy::too_many_lines)] fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) { use syntax_pos::BytePos; diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 19c3f3ad230..264a5463225 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -194,6 +194,7 @@ struct PrintVisitor { } impl<'tcx> Visitor<'tcx> for PrintVisitor { + #[allow(clippy::too_many_lines)] fn visit_expr(&mut self, expr: &Expr) { print!(" if let ExprKind::"); let current = format!("{}.node", self.current); @@ -506,12 +507,13 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { } } + #[allow(clippy::too_many_lines)] fn visit_pat(&mut self, pat: &Pat) { print!(" if let PatKind::"); let current = format!("{}.node", self.current); match pat.node { PatKind::Wild => println!("Wild = {};", current), - PatKind::Binding(anno, _, ident, ref sub) => { + PatKind::Binding(anno, .., ident, ref sub) => { let anno_pat = match anno { BindingAnnotation::Unannotated => "BindingAnnotation::Unannotated", BindingAnnotation::Mutable => "BindingAnnotation::Mutable", diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 55256a25427..09d204a562c 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -148,6 +148,8 @@ define_Conf! { (literal_representation_threshold, "literal_representation_threshold", 16384 => u64), /// Lint: TRIVIALLY_COPY_PASS_BY_REF. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. (trivial_copy_size_limit, "trivial_copy_size_limit", None => Option<u64>), + /// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have + (too_many_lines_threshold, "too_many_lines_threshold", 100 => u64), } impl Default for Conf { diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index a176830be26..53876fef579 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -193,7 +193,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { (&PatKind::TupleStruct(ref lp, ref la, ls), &PatKind::TupleStruct(ref rp, ref ra, rs)) => { self.eq_qpath(lp, rp) && over(la, ra, |l, r| self.eq_pat(l, r)) && ls == rs }, - (&PatKind::Binding(ref lb, _, ref li, ref lp), &PatKind::Binding(ref rb, _, ref ri, ref rp)) => { + (&PatKind::Binding(ref lb, .., ref li, ref lp), &PatKind::Binding(ref rb, .., ref ri, ref rp)) => { lb == rb && li.name.as_str() == ri.name.as_str() && both(lp, rp, |l, r| self.eq_pat(l, r)) }, (&PatKind::Path(ref l), &PatKind::Path(ref r)) => self.eq_qpath(l, r), @@ -389,7 +389,7 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { .hash(&mut self.s); } - #[allow(clippy::many_single_char_names)] + #[allow(clippy::many_single_char_names, clippy::too_many_lines)] pub fn hash_expr(&mut self, e: &Expr) { if let Some(e) = constant_simple(self.cx, self.tables, e) { return e.hash(&mut self.s); diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index 758d1d2d365..508bf26bab9 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -420,7 +420,7 @@ fn print_pat(cx: &LateContext<'_, '_>, pat: &hir::Pat, indent: usize) { println!("{}+", ind); match pat.node { hir::PatKind::Wild => println!("{}Wild", ind), - hir::PatKind::Binding(ref mode, _, ident, ref inner) => { + hir::PatKind::Binding(ref mode, .., ident, ref inner) => { println!("{}Binding", ind); println!("{}mode: {:?}", ind, mode); println!("{}name: {}", ind, ident.name); diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index ee3356fdc82..e68cefe2bc4 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -373,7 +373,7 @@ pub fn get_item_name(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<Name> { /// Get the name of a `Pat`, if any pub fn get_pat_name(pat: &Pat) -> Option<Name> { match pat.node { - PatKind::Binding(_, _, ref spname, _) => Some(spname.name), + PatKind::Binding(.., ref spname, _) => Some(spname.name), PatKind::Path(ref qpath) => single_segment_path(qpath).map(|ps| ps.ident.name), PatKind::Box(ref p) | PatKind::Ref(ref p, _) => get_pat_name(&*p), _ => None, @@ -1008,7 +1008,7 @@ pub fn opt_def_id(def: Def) -> Option<DefId> { } pub fn is_self(slf: &Arg) -> bool { - if let PatKind::Binding(_, _, name, _) = slf.pat.node { + if let PatKind::Binding(.., name, _) = slf.pat.node { name.name == keywords::SelfLower.name() } else { false @@ -1038,7 +1038,7 @@ pub fn is_try(expr: &Expr) -> Option<&Expr> { if_chain! { if let PatKind::TupleStruct(ref path, ref pat, None) = arm.pats[0].node; if match_qpath(path, &paths::RESULT_OK[1..]); - if let PatKind::Binding(_, defid, _, None) = pat[0].node; + if let PatKind::Binding(_, defid, _, _, None) = pat[0].node; if let ExprKind::Path(QPath::Resolved(None, ref path)) = arm.body.node; if let Def::Local(lid) = path.def; if lid == defid; @@ -1087,7 +1087,7 @@ pub fn is_allowed(cx: &LateContext<'_, '_>, lint: &'static Lint, id: NodeId) -> pub fn get_arg_name(pat: &Pat) -> Option<ast::Name> { match pat.node { - PatKind::Binding(_, _, ident, None) => Some(ident.name), + PatKind::Binding(.., ident, None) => Some(ident.name), PatKind::Ref(ref subpat, _) => get_arg_name(subpat), _ => None, } diff --git a/src/driver.rs b/src/driver.rs index 1ce6f6b7c49..b41895f3c2b 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -20,6 +20,7 @@ fn show_version() { println!(env!("CARGO_PKG_VERSION")); } +#[allow(clippy::too_many_lines)] pub fn main() { rustc_driver::init_rustc_env_logger(); exit( diff --git a/tests/ui-toml/functions_maxlines/clippy.toml b/tests/ui-toml/functions_maxlines/clippy.toml new file mode 100644 index 00000000000..951dbb523d9 --- /dev/null +++ b/tests/ui-toml/functions_maxlines/clippy.toml @@ -0,0 +1 @@ +too-many-lines-threshold = 1 diff --git a/tests/ui-toml/functions_maxlines/test.rs b/tests/ui-toml/functions_maxlines/test.rs new file mode 100644 index 00000000000..cd0e0082586 --- /dev/null +++ b/tests/ui-toml/functions_maxlines/test.rs @@ -0,0 +1,45 @@ +#![warn(clippy::too_many_lines)] + +// This function should be considered one line. +fn many_comments_but_one_line_of_code() { + /* println!("This is good."); */ + // println!("This is good."); + /* */ // println!("This is good."); + /* */ // println!("This is good."); + /* */ // println!("This is good."); + /* */ // println!("This is good."); + /* println!("This is good."); + println!("This is good."); + println!("This is good."); */ + println!("This is good."); +} + +// This should be considered two and a fail. +fn too_many_lines() { + println!("This is bad."); + println!("This is bad."); +} + +// This should be considered one line. +#[rustfmt::skip] +fn comment_starts_after_code() { + let _ = 5; /* closing comment. */ /* + this line shouldn't be counted theoretically. + */ +} + +// This should be considered one line. +fn comment_after_code() { + let _ = 5; /* this line should get counted once. */ +} + +// This should fail since it is technically two lines. +#[rustfmt::skip] +fn comment_before_code() { + let _ = "test"; + /* This comment extends to the front of + teh code but this line should still count. */ let _ = 5; +} + +// This should be considered one line. +fn main() {} diff --git a/tests/ui-toml/functions_maxlines/test.stderr b/tests/ui-toml/functions_maxlines/test.stderr new file mode 100644 index 00000000000..0669e99370b --- /dev/null +++ b/tests/ui-toml/functions_maxlines/test.stderr @@ -0,0 +1,23 @@ +error: This function has a large number of lines. + --> $DIR/test.rs:18:1 + | +LL | / fn too_many_lines() { +LL | | println!("This is bad."); +LL | | println!("This is bad."); +LL | | } + | |_^ + | + = note: `-D clippy::too-many-lines` implied by `-D warnings` + +error: This function has a large number of lines. + --> $DIR/test.rs:38:1 + | +LL | / fn comment_before_code() { +LL | | let _ = "test"; +LL | | /* This comment extends to the front of +LL | | teh code but this line should still count. */ let _ = 5; +LL | | } + | |_^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 05a04fb377a..ec2ac20684b 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `third-party` +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `third-party` error: aborting due to previous error diff --git a/tests/ui/complex_types.rs b/tests/ui/complex_types.rs index cfece3768ef..be61fb6b9be 100644 --- a/tests/ui/complex_types.rs +++ b/tests/ui/complex_types.rs @@ -1,5 +1,5 @@ #![warn(clippy::all)] -#![allow(unused, clippy::needless_pass_by_value)] +#![allow(unused, clippy::needless_pass_by_value, clippy::vec_box)] #![feature(associated_type_defaults)] type Alias = Vec<Vec<Box<(u32, u32, u32, u32)>>>; // no warning here diff --git a/tests/ui/functions_maxlines.rs b/tests/ui/functions_maxlines.rs new file mode 100644 index 00000000000..ada35abde99 --- /dev/null +++ b/tests/ui/functions_maxlines.rs @@ -0,0 +1,162 @@ +#![warn(clippy::too_many_lines)] + +fn good_lines() { + /* println!("This is good."); */ + // println!("This is good."); + /* */ // println!("This is good."); + /* */ // println!("This is good."); + /* */ // println!("This is good."); + /* */ // println!("This is good."); + /* println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); */ + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); + println!("This is good."); +} + +fn bad_lines() { + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); + println!("This is bad."); +} + +fn main() {} diff --git a/tests/ui/functions_maxlines.stderr b/tests/ui/functions_maxlines.stderr new file mode 100644 index 00000000000..dfa6a1cf3c5 --- /dev/null +++ b/tests/ui/functions_maxlines.stderr @@ -0,0 +1,16 @@ +error: This function has a large number of lines. + --> $DIR/functions_maxlines.rs:58:1 + | +LL | / fn bad_lines() { +LL | | println!("This is bad."); +LL | | println!("This is bad."); +LL | | println!("This is bad."); +... | +LL | | println!("This is bad."); +LL | | } + | |_^ + | + = note: `-D clippy::too-many-lines` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/matches.rs b/tests/ui/matches.rs index 013f12a1a0e..a40b80378ef 100644 --- a/tests/ui/matches.rs +++ b/tests/ui/matches.rs @@ -1,6 +1,6 @@ #![feature(exclusive_range_pattern)] #![warn(clippy::all)] -#![allow(unused, clippy::redundant_pattern_matching)] +#![allow(unused, clippy::redundant_pattern_matching, clippy::too_many_lines)] #![warn(clippy::match_same_arms)] fn dummy() {} diff --git a/tests/ui/single_char_pattern.fixed b/tests/ui/single_char_pattern.fixed index 220b855ead5..2dd21790389 100644 --- a/tests/ui/single_char_pattern.fixed +++ b/tests/ui/single_char_pattern.fixed @@ -1,5 +1,7 @@ // run-rustfix +#![allow(unused_must_use)] + use std::collections::HashSet; fn main() { diff --git a/tests/ui/single_char_pattern.rs b/tests/ui/single_char_pattern.rs index 9650eb2af32..dc2f9fe4959 100644 --- a/tests/ui/single_char_pattern.rs +++ b/tests/ui/single_char_pattern.rs @@ -1,5 +1,7 @@ // run-rustfix +#![allow(unused_must_use)] + use std::collections::HashSet; fn main() { diff --git a/tests/ui/single_char_pattern.stderr b/tests/ui/single_char_pattern.stderr index 82ef00bfee8..0fcb203dbc1 100644 --- a/tests/ui/single_char_pattern.stderr +++ b/tests/ui/single_char_pattern.stderr @@ -1,5 +1,5 @@ error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:7:13 + --> $DIR/single_char_pattern.rs:9:13 | LL | x.split("x"); | ^^^ help: try using a char instead: `'x'` @@ -7,115 +7,115 @@ LL | x.split("x"); = note: `-D clippy::single-char-pattern` implied by `-D warnings` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:24:16 + --> $DIR/single_char_pattern.rs:26:16 | LL | x.contains("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:25:19 + --> $DIR/single_char_pattern.rs:27:19 | LL | x.starts_with("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:26:17 + --> $DIR/single_char_pattern.rs:28:17 | LL | x.ends_with("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:27:12 + --> $DIR/single_char_pattern.rs:29:12 | LL | x.find("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:28:13 + --> $DIR/single_char_pattern.rs:30:13 | LL | x.rfind("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:29:14 + --> $DIR/single_char_pattern.rs:31:14 | LL | x.rsplit("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:30:24 + --> $DIR/single_char_pattern.rs:32:24 | LL | x.split_terminator("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:31:25 + --> $DIR/single_char_pattern.rs:33:25 | LL | x.rsplit_terminator("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:32:17 + --> $DIR/single_char_pattern.rs:34:17 | LL | x.splitn(0, "x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:33:18 + --> $DIR/single_char_pattern.rs:35:18 | LL | x.rsplitn(0, "x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:34:15 + --> $DIR/single_char_pattern.rs:36:15 | LL | x.matches("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:35:16 + --> $DIR/single_char_pattern.rs:37:16 | LL | x.rmatches("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:36:21 + --> $DIR/single_char_pattern.rs:38:21 | LL | x.match_indices("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:37:22 + --> $DIR/single_char_pattern.rs:39:22 | LL | x.rmatch_indices("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:38:26 + --> $DIR/single_char_pattern.rs:40:26 | LL | x.trim_start_matches("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:39:24 + --> $DIR/single_char_pattern.rs:41:24 | LL | x.trim_end_matches("x"); | ^^^ help: try using a char instead: `'x'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:41:13 + --> $DIR/single_char_pattern.rs:43:13 | LL | x.split("/n"); | ^^^^ help: try using a char instead: `'/n'` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:46:31 + --> $DIR/single_char_pattern.rs:48:31 | LL | x.replace(";", ",").split(","); // issue #2978 | ^^^ help: try using a char instead: `','` error: single-character string constant used as pattern - --> $DIR/single_char_pattern.rs:47:19 + --> $DIR/single_char_pattern.rs:49:19 | LL | x.starts_with("/x03"); // issue #2996 | ^^^^^^ help: try using a char instead: `'/x03'` diff --git a/tests/ui/vec_box_sized.fixed b/tests/ui/vec_box_sized.fixed new file mode 100644 index 00000000000..a56dac8aa23 --- /dev/null +++ b/tests/ui/vec_box_sized.fixed @@ -0,0 +1,36 @@ +// run-rustfix + +#![allow(dead_code)] + +struct SizedStruct(i32); +struct UnsizedStruct([i32]); + +/// The following should trigger the lint +mod should_trigger { + use super::SizedStruct; + + struct StructWithVecBox { + sized_type: Vec<SizedStruct>, + } + + struct A(Vec<SizedStruct>); + struct B(Vec<Vec<u32>>); +} + +/// The following should not trigger the lint +mod should_not_trigger { + use super::UnsizedStruct; + + struct C(Vec<Box<UnsizedStruct>>); + + struct StructWithVecBoxButItsUnsized { + unsized_type: Vec<Box<UnsizedStruct>>, + } + + struct TraitVec<T: ?Sized> { + // Regression test for #3720. This was causing an ICE. + inner: Vec<Box<T>>, + } +} + +fn main() {} diff --git a/tests/ui/vec_box_sized.rs b/tests/ui/vec_box_sized.rs index 884761675fc..32d1e940f27 100644 --- a/tests/ui/vec_box_sized.rs +++ b/tests/ui/vec_box_sized.rs @@ -1,17 +1,36 @@ -struct SizedStruct { - _a: i32, +// run-rustfix + +#![allow(dead_code)] + +struct SizedStruct(i32); +struct UnsizedStruct([i32]); + +/// The following should trigger the lint +mod should_trigger { + use super::SizedStruct; + + struct StructWithVecBox { + sized_type: Vec<Box<SizedStruct>>, + } + + struct A(Vec<Box<SizedStruct>>); + struct B(Vec<Vec<Box<(u32)>>>); } -struct UnsizedStruct { - _a: [i32], -} +/// The following should not trigger the lint +mod should_not_trigger { + use super::UnsizedStruct; -struct StructWithVecBox { - sized_type: Vec<Box<SizedStruct>>, -} + struct C(Vec<Box<UnsizedStruct>>); -struct StructWithVecBoxButItsUnsized { - unsized_type: Vec<Box<UnsizedStruct>>, + struct StructWithVecBoxButItsUnsized { + unsized_type: Vec<Box<UnsizedStruct>>, + } + + struct TraitVec<T: ?Sized> { + // Regression test for #3720. This was causing an ICE. + inner: Vec<Box<T>>, + } } fn main() {} diff --git a/tests/ui/vec_box_sized.stderr b/tests/ui/vec_box_sized.stderr index 21b515fa486..b33880b46bd 100644 --- a/tests/ui/vec_box_sized.stderr +++ b/tests/ui/vec_box_sized.stderr @@ -1,10 +1,22 @@ error: `Vec<T>` is already on the heap, the boxing is unnecessary. - --> $DIR/vec_box_sized.rs:10:17 + --> $DIR/vec_box_sized.rs:13:21 | -LL | sized_type: Vec<Box<SizedStruct>>, - | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>` +LL | sized_type: Vec<Box<SizedStruct>>, + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>` | = note: `-D clippy::vec-box` implied by `-D warnings` -error: aborting due to previous error +error: `Vec<T>` is already on the heap, the boxing is unnecessary. + --> $DIR/vec_box_sized.rs:16:14 + | +LL | struct A(Vec<Box<SizedStruct>>); + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Vec<SizedStruct>` + +error: `Vec<T>` is already on the heap, the boxing is unnecessary. + --> $DIR/vec_box_sized.rs:17:18 + | +LL | struct B(Vec<Vec<Box<(u32)>>>); + | ^^^^^^^^^^^^^^^ help: try: `Vec<u32>` + +error: aborting due to 3 previous errors