From c5affa2efc697f478828c81def852f70968bb329 Mon Sep 17 00:00:00 2001 From: mcarton Date: Wed, 8 Jun 2016 22:00:42 +0200 Subject: [PATCH 1/4] Whitelist Nan in `DOC_MARKDOWN` --- clippy_lints/src/utils/conf.rs | 2 +- tests/compile-fail/doc.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index e773cc0e025..851764edc28 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -151,7 +151,7 @@ define_Conf! { /// Lint: CYCLOMATIC_COMPLEXITY. The maximum cyclomatic complexity a function can have ("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25 => u64), /// Lint: DOC_MARKDOWN. The list of words this lint should not consider as identifiers needing ticks - ("doc-valid-idents", doc_valid_idents, ["MiB", "GiB", "TiB", "PiB", "EiB", "GitHub"] => Vec), + ("doc-valid-idents", doc_valid_idents, ["MiB", "GiB", "TiB", "PiB", "EiB", "GitHub", "NaN"] => Vec), /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have ("too-many-arguments-threshold", too_many_arguments_threshold, 7 => u64), /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs index d3b1c037f47..415bcb2e661 100755 --- a/tests/compile-fail/doc.rs +++ b/tests/compile-fail/doc.rs @@ -46,6 +46,7 @@ fn test_emphasis() { /// 32kib 32Mib 32Gib 32Tib 32Pib 32Eib /// 32kB 32MB 32GB 32TB 32PB 32EB /// 32kb 32Mb 32Gb 32Tb 32Pb 32Eb +/// NaN /// be_sure_we_got_to_the_end_of_it //~^ ERROR: you should put `be_sure_we_got_to_the_end_of_it` between ticks fn test_units() { From ce2b96abe98ba23362d91094007b2b932cdd89e0 Mon Sep 17 00:00:00 2001 From: mcarton Date: Thu, 9 Jun 2016 00:22:59 +0200 Subject: [PATCH 2/4] Fix yet another FP in `USELESS_LET_IF_SEQ` The block expression before the assignment must be `None`. --- clippy_lints/src/let_if_seq.rs | 1 + tests/compile-fail/let_if_seq.rs | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs index a85cb52f2dc..2f98d10dee0 100644 --- a/clippy_lints/src/let_if_seq.rs +++ b/clippy_lints/src/let_if_seq.rs @@ -150,6 +150,7 @@ impl<'a, 'tcx, 'v> hir::intravisit::Visitor<'v> for UsedVisitor<'a, 'tcx> { fn check_assign<'e>(cx: &LateContext, decl: hir::def_id::DefId, block: &'e hir::Block) -> Option<&'e hir::Expr> { if_let_chain! {[ + block.expr.is_none(), let Some(expr) = block.stmts.iter().last(), let hir::StmtSemi(ref expr, _) = expr.node, let hir::ExprAssign(ref var, ref value) = expr.node, diff --git a/tests/compile-fail/let_if_seq.rs b/tests/compile-fail/let_if_seq.rs index 0b086faf077..b49e5a26122 100644 --- a/tests/compile-fail/let_if_seq.rs +++ b/tests/compile-fail/let_if_seq.rs @@ -98,6 +98,15 @@ fn main() { toto = 2; } + // found in libcore, the inner if is not a statement but the block's expr + let mut ch = b'x'; + if f() { + ch = b'*'; + if f() { + ch = b'?'; + } + } + // baz needs to be mut let mut baz = 0; //~^ ERROR `if _ { .. } else { .. }` is an expression From 3ae39145fc763a798404dd9301e8d8f045fdbf4e Mon Sep 17 00:00:00 2001 From: mcarton Date: Thu, 9 Jun 2016 00:24:44 +0200 Subject: [PATCH 3/4] Fix false-positive in `LET_AND_RETURN` If the declaration has a type, it might be required for coercion to happen. --- clippy_lints/src/returns.rs | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index d7893821263..7bb468166b8 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -3,7 +3,7 @@ use syntax::ast::*; use syntax::codemap::{Span, Spanned}; use syntax::visit::FnKind; -use utils::{span_lint, span_lint_and_then, snippet_opt, match_path_ast, in_external_macro}; +use utils::{span_note_and_lint, span_lint_and_then, snippet_opt, match_path_ast, in_external_macro}; /// **What it does:** This lint checks for return statements at the end of a block. /// @@ -95,29 +95,23 @@ impl ReturnPass { let Some(ref retexpr) = block.expr, let StmtKind::Decl(ref decl, _) = stmt.node, let DeclKind::Local(ref local) = decl.node, + local.ty.is_none(), let Some(ref initexpr) = local.init, let PatKind::Ident(_, Spanned { node: id, .. }, _) = local.pat.node, let ExprKind::Path(_, ref path) = retexpr.node, - match_path_ast(path, &[&id.name.as_str()]) + match_path_ast(path, &[&id.name.as_str()]), + !in_external_macro(cx, initexpr.span), ], { - self.emit_let_lint(cx, retexpr.span, initexpr.span); + span_note_and_lint(cx, + LET_AND_RETURN, + retexpr.span, + "returning the result of a let binding from a block. \ + Consider returning the expression directly.", + initexpr.span, + "this expression can be directly returned"); } } } - - fn emit_let_lint(&mut self, cx: &EarlyContext, lint_span: Span, note_span: Span) { - if in_external_macro(cx, note_span) { - return; - } - let mut db = span_lint(cx, - LET_AND_RETURN, - lint_span, - "returning the result of a let binding from a block. Consider returning the \ - expression directly."); - if cx.current_level(LET_AND_RETURN) != Level::Allow { - db.span_note(note_span, "this expression can be directly returned"); - } - } } impl LintPass for ReturnPass { From e9360f76751b8b60ec14520d23f1edb8263963cd Mon Sep 17 00:00:00 2001 From: mcarton Date: Thu, 9 Jun 2016 23:05:48 +0200 Subject: [PATCH 4/4] Fix suggestions for `REVERSE_RANGE_LOOP` --- clippy_lints/src/loops.rs | 10 +++++++++- tests/compile-fail/for_loop.rs | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 5dcea35e5a7..5d431e47465 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -444,6 +444,11 @@ fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) { if sup { let start_snippet = snippet(cx, start.span, "_"); let end_snippet = snippet(cx, end.span, "_"); + let dots = if limits == ast::RangeLimits::Closed { + "..." + } else { + ".." + }; span_lint_and_then(cx, REVERSE_RANGE_LOOP, @@ -454,7 +459,10 @@ fn check_for_loop_reverse_range(cx: &LateContext, arg: &Expr, expr: &Expr) { "consider using the following if \ you are attempting to iterate \ over this range in reverse", - format!("({}..{}).rev()", end_snippet, start_snippet)); + format!("({end}{dots}{start}).rev()", + end=end_snippet, + dots=dots, + start=start_snippet)); }); } else if eq && limits != ast::RangeLimits::Closed { // if they are equal, it's also problematic - this loop diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index d35beb617e0..411a4b11c17 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -169,7 +169,7 @@ fn main() { for i in 10...0 { //~^ERROR this range is empty so this for loop will never run //~|HELP consider - //~|SUGGESTION (0..10).rev() + //~|SUGGESTION (0...10).rev() println!("{}", i); }