From 232dd43fe932db6db21e41007cd05dd490590fa0 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Tue, 3 Sep 2019 06:26:49 +0200 Subject: [PATCH] Fix occurrences of `too_many_lines` violations --- clippy_lints/src/ranges.rs | 143 ++++++++++++------------ clippy_lints/src/types.rs | 162 +++++++++++++++------------- clippy_lints/src/utils/inspector.rs | 2 + clippy_lints/src/write.rs | 1 + 4 files changed, 166 insertions(+), 142 deletions(-) diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 804f86a9a964..7b6b6cbc8f83 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -151,75 +151,82 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges { } } - // exclusive range plus one: `x..(y+1)` - if_chain! { - if let Some(higher::Range { - start, - end: Some(end), - limits: RangeLimits::HalfOpen - }) = higher::range(cx, expr); - if let Some(y) = y_plus_one(end); - then { - let span = if expr.span.from_expansion() { - expr.span - .ctxt() - .outer_expn_data() - .call_site - } else { - expr.span - }; - span_lint_and_then( - cx, - RANGE_PLUS_ONE, - span, - "an inclusive range would be more readable", - |db| { - let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); - if let Some(is_wrapped) = &snippet_opt(cx, span) { - if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') { - db.span_suggestion( - span, - "use", - format!("({}..={})", start, end), - Applicability::MaybeIncorrect, - ); - } else { - db.span_suggestion( - span, - "use", - format!("{}..={}", start, end), - Applicability::MachineApplicable, // snippet - ); - } - } - }, - ); - } - } + check_exclusive_range_plus_one(cx, expr); + check_inclusive_range_minus_one(cx, expr); + } +} - // inclusive range minus one: `x..=(y-1)` - if_chain! { - if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(cx, expr); - if let Some(y) = y_minus_one(end); - then { - span_lint_and_then( - cx, - RANGE_MINUS_ONE, - expr.span, - "an exclusive range would be more readable", - |db| { - let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); - db.span_suggestion( - expr.span, - "use", - format!("{}..{}", start, end), - Applicability::MachineApplicable, // snippet - ); - }, - ); - } +// exclusive range plus one: `x..(y+1)` +fn check_exclusive_range_plus_one(cx: &LateContext<'_, '_>, expr: &Expr) { + if_chain! { + if let Some(higher::Range { + start, + end: Some(end), + limits: RangeLimits::HalfOpen + }) = higher::range(cx, expr); + if let Some(y) = y_plus_one(end); + then { + let span = if expr.span.from_expansion() { + expr.span + .ctxt() + .outer_expn_data() + .call_site + } else { + expr.span + }; + span_lint_and_then( + cx, + RANGE_PLUS_ONE, + span, + "an inclusive range would be more readable", + |db| { + let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); + let end = Sugg::hir(cx, y, "y"); + if let Some(is_wrapped) = &snippet_opt(cx, span) { + if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') { + db.span_suggestion( + span, + "use", + format!("({}..={})", start, end), + Applicability::MaybeIncorrect, + ); + } else { + db.span_suggestion( + span, + "use", + format!("{}..={}", start, end), + Applicability::MachineApplicable, // snippet + ); + } + } + }, + ); + } + } +} + +// inclusive range minus one: `x..=(y-1)` +fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr) { + if_chain! { + if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(cx, expr); + if let Some(y) = y_minus_one(end); + then { + span_lint_and_then( + cx, + RANGE_MINUS_ONE, + expr.span, + "an exclusive range would be more readable", + |db| { + let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); + let end = Sugg::hir(cx, y, "y"); + db.span_suggestion( + expr.span, + "use", + format!("{}..{}", start, end), + Applicability::MachineApplicable, // snippet + ); + }, + ); } } } diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 28d337d3cd67..3c2111c55fb5 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1159,83 +1159,97 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Casts { } } if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { - match (cast_from.is_integral(), cast_to.is_integral()) { - (true, false) => { - let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); - let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.sty { - 32 - } else { - 64 - }; - if is_isize_or_usize(cast_from) || from_nbits >= to_nbits { - span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64); - } - if from_nbits < to_nbits { - span_lossless_lint(cx, expr, ex, cast_from, cast_to); - } - }, - (false, true) => { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - &format!("casting {} to {} may truncate the value", cast_from, cast_to), - ); - if !cast_to.is_signed() { - span_lint( - cx, - CAST_SIGN_LOSS, - expr.span, - &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to), - ); - } - }, - (true, true) => { - check_loss_of_sign(cx, expr, ex, cast_from, cast_to); - check_truncation_and_wrapping(cx, expr, cast_from, cast_to); - check_lossless(cx, expr, ex, cast_from, cast_to); - }, - (false, false) => { - if let (&ty::Float(FloatTy::F64), &ty::Float(FloatTy::F32)) = (&cast_from.sty, &cast_to.sty) { - span_lint( - cx, - CAST_POSSIBLE_TRUNCATION, - expr.span, - "casting f64 to f32 may truncate the value", - ); - } - if let (&ty::Float(FloatTy::F32), &ty::Float(FloatTy::F64)) = (&cast_from.sty, &cast_to.sty) { - span_lossless_lint(cx, expr, ex, cast_from, cast_to); - } - }, - } + lint_numeric_casts(cx, expr, ex, cast_from, cast_to); } - if_chain! { - if let ty::RawPtr(from_ptr_ty) = &cast_from.sty; - if let ty::RawPtr(to_ptr_ty) = &cast_to.sty; - if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty); - if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty); - if from_layout.align.abi < to_layout.align.abi; - // with c_void, we inherently need to trust the user - if !is_c_void(cx, from_ptr_ty.ty); - // when casting from a ZST, we don't know enough to properly lint - if !from_layout.is_zst(); - then { - span_lint( - cx, - CAST_PTR_ALIGNMENT, - expr.span, - &format!( - "casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)", - cast_from, - cast_to, - from_layout.align.abi.bytes(), - to_layout.align.abi.bytes(), - ), - ); - } + lint_cast_ptr_alignment(cx, expr, cast_from, cast_to); + } + } +} + +fn lint_numeric_casts<'tcx>( + cx: &LateContext<'_, 'tcx>, + expr: &Expr, + cast_expr: &Expr, + cast_from: Ty<'tcx>, + cast_to: Ty<'tcx>, +) { + match (cast_from.is_integral(), cast_to.is_integral()) { + (true, false) => { + let from_nbits = int_ty_to_nbits(cast_from, cx.tcx); + let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.sty { + 32 + } else { + 64 + }; + if is_isize_or_usize(cast_from) || from_nbits >= to_nbits { + span_precision_loss_lint(cx, expr, cast_from, to_nbits == 64); } + if from_nbits < to_nbits { + span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); + } + }, + (false, true) => { + span_lint( + cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + &format!("casting {} to {} may truncate the value", cast_from, cast_to), + ); + if !cast_to.is_signed() { + span_lint( + cx, + CAST_SIGN_LOSS, + expr.span, + &format!("casting {} to {} may lose the sign of the value", cast_from, cast_to), + ); + } + }, + (true, true) => { + check_loss_of_sign(cx, expr, cast_expr, cast_from, cast_to); + check_truncation_and_wrapping(cx, expr, cast_from, cast_to); + check_lossless(cx, expr, cast_expr, cast_from, cast_to); + }, + (false, false) => { + if let (&ty::Float(FloatTy::F64), &ty::Float(FloatTy::F32)) = (&cast_from.sty, &cast_to.sty) { + span_lint( + cx, + CAST_POSSIBLE_TRUNCATION, + expr.span, + "casting f64 to f32 may truncate the value", + ); + } + if let (&ty::Float(FloatTy::F32), &ty::Float(FloatTy::F64)) = (&cast_from.sty, &cast_to.sty) { + span_lossless_lint(cx, expr, cast_expr, cast_from, cast_to); + } + }, + } +} + +fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) { + if_chain! { + if let ty::RawPtr(from_ptr_ty) = &cast_from.sty; + if let ty::RawPtr(to_ptr_ty) = &cast_to.sty; + if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty); + if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty); + if from_layout.align.abi < to_layout.align.abi; + // with c_void, we inherently need to trust the user + if !is_c_void(cx, from_ptr_ty.ty); + // when casting from a ZST, we don't know enough to properly lint + if !from_layout.is_zst(); + then { + span_lint( + cx, + CAST_PTR_ALIGNMENT, + expr.span, + &format!( + "casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)", + cast_from, + cast_to, + from_layout.align.abi.bytes(), + to_layout.align.abi.bytes(), + ), + ); } } } diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index b48ef7d293b4..ba0e56c9987a 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -144,6 +144,7 @@ fn has_attr(sess: &Session, attrs: &[Attribute]) -> bool { } #[allow(clippy::similar_names)] +#[allow(clippy::too_many_lines)] fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr, indent: usize) { let ind = " ".repeat(indent); println!("{}+", ind); @@ -396,6 +397,7 @@ fn print_item(cx: &LateContext<'_, '_>, item: &hir::Item) { } #[allow(clippy::similar_names)] +#[allow(clippy::too_many_lines)] fn print_pat(cx: &LateContext<'_, '_>, pat: &hir::Pat, indent: usize) { let ind = " ".repeat(indent); println!("{}+", ind); diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index bcf688c1914c..7d72a21ac036 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -320,6 +320,7 @@ impl FmtStr { /// ```rust,ignore /// (Some("string to write: {}"), Some(buf)) /// ``` +#[allow(clippy::too_many_lines)] fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option, Option) { use fmt_macros::*; let tts = tts.clone();