From 06f30a61dd2a948c58907ef9ebb80b040e930450 Mon Sep 17 00:00:00 2001 From: Johannes Linke Date: Fri, 1 Jan 2016 17:48:19 +0100 Subject: [PATCH 1/3] Add "warn/allow by default" to lint descriptions where it was missing. --- src/attrs.rs | 2 +- src/block_in_if_condition.rs | 4 ++-- src/escape.rs | 2 +- src/loops.rs | 4 ++-- src/minmax.rs | 2 +- src/misc.rs | 2 +- src/mut_reference.rs | 2 +- src/strings.rs | 2 +- src/types.rs | 5 +++-- src/zero_div_zero.rs | 2 +- 10 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/attrs.rs b/src/attrs.rs index 0882f3af41f..f0dbf390ebb 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -8,7 +8,7 @@ use syntax::attr::*; use syntax::ast::{Attribute, MetaList, MetaWord}; use utils::{in_macro, match_path, span_lint, BEGIN_UNWIND}; -/// **What it does:** This lint warns on items annotated with `#[inline(always)]`, unless the annotated function is empty or simply panics. +/// **What it does:** This lint `Warn`s on items annotated with `#[inline(always)]`, unless the annotated function is empty or simply panics. /// /// **Why is this bad?** While there are valid uses of this annotation (and once you know when to use it, by all means `allow` this lint), it's a common newbie-mistake to pepper one's code with it. /// diff --git a/src/block_in_if_condition.rs b/src/block_in_if_condition.rs index f7c181f5fd8..03265635b1d 100644 --- a/src/block_in_if_condition.rs +++ b/src/block_in_if_condition.rs @@ -3,7 +3,7 @@ use rustc::lint::{LateLintPass, LateContext, LintArray, LintPass}; use rustc_front::intravisit::{Visitor, walk_expr}; use utils::*; -/// **What it does:** This lint checks for `if` conditions that use blocks to contain an expression. +/// **What it does:** This lint checks for `if` conditions that use blocks to contain an expression. It is `Warn` by default. /// /// **Why is this bad?** It isn't really rust style, same as using parentheses to contain expressions. /// @@ -15,7 +15,7 @@ declare_lint! { "braces can be eliminated in conditions that are expressions, e.g `if { true } ...`" } -/// **What it does:** This lint checks for `if` conditions that use blocks containing statements, or conditions that use closures with blocks. +/// **What it does:** This lint checks for `if` conditions that use blocks containing statements, or conditions that use closures with blocks. It is `Warn` by default. /// /// **Why is this bad?** Using blocks in the condition makes it hard to read. /// diff --git a/src/escape.rs b/src/escape.rs index 63894cda9be..21bf30e131c 100644 --- a/src/escape.rs +++ b/src/escape.rs @@ -14,7 +14,7 @@ use utils::span_lint; pub struct EscapePass; -/// **What it does:** This lint checks for usage of `Box` where an unboxed `T` would work fine +/// **What it does:** This lint checks for usage of `Box` where an unboxed `T` would work fine. It is `Warn` by default. /// /// **Why is this bad?** This is an unnecessary allocation, and bad for performance /// diff --git a/src/loops.rs b/src/loops.rs index 5c552d2ce42..9f103f4e7a8 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -50,7 +50,7 @@ declare_lint!{ pub EXPLICIT_ITER_LOOP, Warn, declare_lint!{ pub ITER_NEXT_LOOP, Warn, "for-looping over `_.next()` which is probably not intended" } -/// **What it does:** This lint detects `loop + match` combinations that are easier written as a `while let` loop. +/// **What it does:** This lint detects `loop + match` combinations that are easier written as a `while let` loop. It is `Warn` by default. /// /// **Why is this bad?** The `while let` loop is usually shorter and more readable /// @@ -85,7 +85,7 @@ declare_lint!{ pub UNUSED_COLLECT, Warn, "`collect()`ing an iterator without using the result; this is usually better \ written as a for loop" } -/// **What it does:** This lint checks for loops over ranges `x..y` where both `x` and `y` are constant and `x` is greater or equal to `y`, unless the range is reversed or has a negative `.step_by(_)`. +/// **What it does:** This lint checks for loops over ranges `x..y` where both `x` and `y` are constant and `x` is greater or equal to `y`, unless the range is reversed or has a negative `.step_by(_)`. It is `Warn` by default. /// /// **Why is it bad?** Such loops will either be skipped or loop until wrap-around (in debug code, this may `panic!()`). Both options are probably not intended. /// diff --git a/src/minmax.rs b/src/minmax.rs index ac8d6f05272..b63e839612c 100644 --- a/src/minmax.rs +++ b/src/minmax.rs @@ -8,7 +8,7 @@ use consts::{Constant, constant_simple}; use utils::{match_def_path, span_lint}; use self::MinMax::{Min, Max}; -/// **What it does:** This lint checks for expressions where `std::cmp::min` and `max` are used to clamp values, but switched so that the result is constant. +/// **What it does:** This lint checks for expressions where `std::cmp::min` and `max` are used to clamp values, but switched so that the result is constant. It is `Warn` by default. /// /// **Why is this bad?** This is in all probability not the intended outcome. At the least it hurts readability of the code. /// diff --git a/src/misc.rs b/src/misc.rs index 139dfde3681..92276961d11 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -281,7 +281,7 @@ impl LateLintPass for ModuloOne { } } -/// **What it does:** This lint checks for patterns in the form `name @ _`. +/// **What it does:** This lint checks for patterns in the form `name @ _`. It is `Warn` by default. /// /// **Why is this bad?** It's almost always more readable to just use direct bindings. /// diff --git a/src/mut_reference.rs b/src/mut_reference.rs index 9ba9782336a..133462071e4 100644 --- a/src/mut_reference.rs +++ b/src/mut_reference.rs @@ -4,7 +4,7 @@ use utils::span_lint; use rustc::middle::ty::{TypeAndMut, TypeVariants, MethodCall, TyS}; use syntax::ptr::P; -/// **What it does:** This lint detects giving a mutable reference to a function that only requires an immutable reference. +/// **What it does:** This lint detects giving a mutable reference to a function that only requires an immutable reference. It is `Warn` by default. /// /// **Why is this bad?** The immutable reference rules out all other references to the value. Also the code misleads about the intent of the call site. /// diff --git a/src/strings.rs b/src/strings.rs index b567d949330..d60a045aa75 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -11,7 +11,7 @@ use eq_op::is_exp_equal; use utils::{match_type, span_lint, walk_ptrs_ty, get_parent_expr}; use utils::STRING_PATH; -/// **What it does:** This lint matches code of the form `x = x + y` (without `let`!) +/// **What it does:** This lint matches code of the form `x = x + y` (without `let`!). It is `Allow` by default. /// /// **Why is this bad?** Because this expression needs another copy as opposed to `x.push_str(y)` (in practice LLVM will usually elide it, though). Despite [llogiq](https://github.com/llogiq)'s reservations, this lint also is `allow` by default, as some people opine that it's more readable. /// diff --git a/src/types.rs b/src/types.rs index 9bc50643259..f332659188b 100644 --- a/src/types.rs +++ b/src/types.rs @@ -17,7 +17,7 @@ use utils::{LL_PATH, VEC_PATH}; #[allow(missing_copy_implementations)] pub struct TypePass; -/// **What it does:** This lint checks for use of `Box>` anywhere in the code. +/// **What it does:** This lint checks for use of `Box>` anywhere in the code. It is `Warn` by default. /// /// **Why is this bad?** `Vec` already keeps its contents in a separate area on the heap. So if you `Box` it, you just add another level of indirection without any benefit whatsoever. /// @@ -26,7 +26,8 @@ pub struct TypePass; /// **Example:** `struct X { values: Box> }` declare_lint!(pub BOX_VEC, Warn, "usage of `Box>`, vector elements are already on the heap"); -/// **What it does:** This lint checks for usage of any `LinkedList`, suggesting to use a `Vec` or a `VecDeque` (formerly called `RingBuf`). + +/// **What it does:** This lint checks for usage of any `LinkedList`, suggesting to use a `Vec` or a `VecDeque` (formerly called `RingBuf`). It is `Warn` by default. /// /// **Why is this bad?** Gankro says: /// diff --git a/src/zero_div_zero.rs b/src/zero_div_zero.rs index c4d7cf4a589..5a4d3931606 100644 --- a/src/zero_div_zero.rs +++ b/src/zero_div_zero.rs @@ -9,7 +9,7 @@ use consts::{Constant, constant_simple, FloatWidth}; /// 0.0/0.0 with std::f32::NaN or std::f64::NaN, depending on the precision. pub struct ZeroDivZeroPass; -/// **What it does:** This lint checks for `0.0 / 0.0` +/// **What it does:** This lint checks for `0.0 / 0.0`. It is `Warn` by default. /// /// **Why is this bad?** It's less readable than `std::f32::NAN` or `std::f64::NAN` /// From b287739c0b2e4d6202d94eb2a20a385a69c74d63 Mon Sep 17 00:00:00 2001 From: Johannes Linke Date: Fri, 1 Jan 2016 17:48:46 +0100 Subject: [PATCH 2/3] Remove reference to a fixed issue --- src/map_clone.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/map_clone.rs b/src/map_clone.rs index b1ba47a9b54..ef992ad086c 100644 --- a/src/map_clone.rs +++ b/src/map_clone.rs @@ -8,7 +8,7 @@ use utils::{walk_ptrs_ty, walk_ptrs_ty_depth}; /// /// **Why is this bad?** It makes the code less readable. /// -/// **Known problems:** False negative: The lint currently misses mapping `Clone::clone` directly. Issue #436 is tracking this. +/// **Known problems:** None /// /// **Example:** `x.map(|e| e.clone());` declare_lint!(pub MAP_CLONE, Warn, From f89e4005784abfe0b71a2c24fbd2fa007e57a61a Mon Sep 17 00:00:00 2001 From: Johannes Linke Date: Fri, 1 Jan 2016 17:49:01 +0100 Subject: [PATCH 3/3] Minor documentation cleanups --- src/escape.rs | 4 ++-- src/mut_reference.rs | 8 ++++---- src/precedence.rs | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/escape.rs b/src/escape.rs index 21bf30e131c..4b6a8258dbe 100644 --- a/src/escape.rs +++ b/src/escape.rs @@ -16,9 +16,9 @@ pub struct EscapePass; /// **What it does:** This lint checks for usage of `Box` where an unboxed `T` would work fine. It is `Warn` by default. /// -/// **Why is this bad?** This is an unnecessary allocation, and bad for performance +/// **Why is this bad?** This is an unnecessary allocation, and bad for performance. It is only necessary to allocate if you wish to move the box into something. /// -/// It is only necessary to allocate if you wish to move the box into something. +/// **Known problems:** None /// /// **Example:** /// diff --git a/src/mut_reference.rs b/src/mut_reference.rs index 133462071e4..ddfd9ddcc13 100644 --- a/src/mut_reference.rs +++ b/src/mut_reference.rs @@ -36,7 +36,7 @@ impl LateLintPass for UnnecessaryMutPassed { match borrowed_table.node_types.get(&fn_expr.id) { Some(function_type) => { if let ExprPath(_, ref path) = fn_expr.node { - check_arguments(cx, &arguments, function_type, + check_arguments(cx, &arguments, function_type, &format!("{}", path)); } } @@ -50,7 +50,7 @@ impl LateLintPass for UnnecessaryMutPassed { ExprMethodCall(ref name, _, ref arguments) => { let method_call = MethodCall::expr(e.id); match borrowed_table.method_map.get(&method_call) { - Some(method_type) => check_arguments(cx, &arguments, method_type.ty, + Some(method_type) => check_arguments(cx, &arguments, method_type.ty, &format!("{}", name.node.as_str())), None => unreachable!(), // Just like above, this should never happen. }; @@ -68,9 +68,9 @@ fn check_arguments(cx: &LateContext, arguments: &[P], type_definition: &Ty TypeVariants::TyRef(_, TypeAndMut {mutbl: MutImmutable, ..}) | TypeVariants::TyRawPtr(TypeAndMut {mutbl: MutImmutable, ..}) => { if let ExprAddrOf(MutMutable, _) = argument.node { - span_lint(cx, UNNECESSARY_MUT_PASSED, + span_lint(cx, UNNECESSARY_MUT_PASSED, argument.span, &format!("The function/method \"{}\" \ - doesn't need a mutable reference", + doesn't need a mutable reference", name)); } } diff --git a/src/precedence.rs b/src/precedence.rs index 39a3e9e56c2..3aae9fd0d6c 100644 --- a/src/precedence.rs +++ b/src/precedence.rs @@ -4,7 +4,7 @@ use syntax::ast::*; use utils::{span_lint, snippet}; -/// **What it does:** This lint checks for operations where precedence may be unclear and `Warn`'s about them by default, suggesting to add parentheses. Currently it catches the following: +/// **What it does:** This lint checks for operations where precedence may be unclear and `Warn`s about them by default, suggesting to add parentheses. Currently it catches the following: /// * mixed usage of arithmetic and bit shifting/combining operators without parentheses /// * a "negative" numeric literal (which is really a unary `-` followed by a numeric literal) followed by a method call /// @@ -33,17 +33,17 @@ impl EarlyLintPass for Precedence { if let ExprBinary(Spanned { node: op, ..}, ref left, ref right) = expr.node { if !is_bit_op(op) { return; } match (is_arith_expr(left), is_arith_expr(right)) { - (true, true) => span_lint(cx, PRECEDENCE, expr.span, + (true, true) => span_lint(cx, PRECEDENCE, expr.span, &format!("operator precedence can trip the unwary. \ Consider parenthesizing your expression:\ `({}) {} ({})`", snippet(cx, left.span, ".."), op.to_string(), snippet(cx, right.span, ".."))), - (true, false) => span_lint(cx, PRECEDENCE, expr.span, + (true, false) => span_lint(cx, PRECEDENCE, expr.span, &format!("operator precedence can trip the unwary. \ Consider parenthesizing your expression:\ `({}) {} {}`", snippet(cx, left.span, ".."), op.to_string(), snippet(cx, right.span, ".."))), - (false, true) => span_lint(cx, PRECEDENCE, expr.span, + (false, true) => span_lint(cx, PRECEDENCE, expr.span, &format!("operator precedence can trip the unwary. \ Consider parenthesizing your expression:\ `{} {} ({})`", snippet(cx, left.span, ".."),