Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix too_many_lines false positive #4490

Merged
merged 2 commits into from Sep 4, 2019
Merged

Fix too_many_lines false positive #4490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ae1a69
Fix `too_many_lines` false positive
Sep 3, 2019
232dd43
Fix occurrences of `too_many_lines` violations
Sep 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl<'a, 'tcx> Functions {
Some(i) => i + 1,
None => 0,
};
let end_brace_idx = match code_snippet.find('}') {
let end_brace_idx = match code_snippet.rfind('}') {
Some(i) => i,
None => code_snippet.len(),
};
Expand Down
139 changes: 73 additions & 66 deletions clippy_lints/src/ranges.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

// 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)`
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
);
},
);
}
// 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
);
},
);
}
}
}
Expand Down
162 changes: 88 additions & 74 deletions clippy_lints/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
),
);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/utils/inspector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/write.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<FmtStr>, Option<Expr>) {
use fmt_macros::*;
let tts = tts.clone();
Expand Down
1 change: 1 addition & 0 deletions tests/ui/functions_maxlines.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ fn good_lines() {
}

fn bad_lines() {
println!("Dont get confused by braces: {{}}");
println!("This is bad.");
println!("This is bad.");
println!("This is bad.");
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/functions_maxlines.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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!("Dont get confused by braces: {{}}");
LL | | println!("This is bad.");
LL | | println!("This is bad.");
... |
Expand Down