From 6482840bc55055bc1597ae2aa7ed0f6abdad30fa Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Thu, 10 Dec 2015 12:54:43 -0800 Subject: [PATCH 01/14] Add tests --- tests/compile-fail/used_underscore_binding.rs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/compile-fail/used_underscore_binding.rs diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs new file mode 100644 index 000000000000..8c7fe4e00691 --- /dev/null +++ b/tests/compile-fail/used_underscore_binding.rs @@ -0,0 +1,22 @@ +#![feature(plugin)] +#![plugin(clippy)] +#[deny(used_underscore_binding)] + +fn main() { + let foo = 0u32; + prefix_underscore(foo); //should fail + non_prefix_underscore(foo); //should pass + unused_underscore(foo); //should pass +} + +fn prefix_underscore(_x: u32){ + println!("{}", _x + 1); //~Error: Used binding which is prefixed with an underscore +} + +fn non_prefix_underscore(some_foo: u32) { + println!("{}", some_foo + 1); +} + +fn unused_underscore(_foo: u32) { + println!("{}", 1); +} From 9de308ee1538ed35988dc6bcb0fd2faf43129133 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Fri, 11 Dec 2015 14:02:02 -0800 Subject: [PATCH 02/14] Add used_underscore_binding lint --- src/misc.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/misc.rs b/src/misc.rs index b5c3d0c514f1..7068aac781e1 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -9,6 +9,7 @@ use rustc::middle::ty; use rustc::middle::const_eval::ConstVal::Float; use rustc::middle::const_eval::eval_const_expr_partial; use rustc::middle::const_eval::EvalHint::ExprTypeChecked; +use rustc::middle::def::Def; use utils::{get_item_name, match_path, snippet, span_lint, walk_ptrs_ty, is_integer_literal}; use utils::span_help_and_lint; @@ -316,3 +317,36 @@ impl LateLintPass for PatternPass { } } } + +declare_lint!(pub USED_UNDERSCORE_BINDING, Warn, + "using a binding which is prefixed with an underscore"); + +#[derive(Copy, Clone)] +pub struct UsedUnderscoreBinding; + +impl LintPass for UsedUnderscoreBinding { + fn get_lints(&self) -> LintArray { + lint_array!(USED_UNDERSCORE_BINDING) + } +} + +impl LateLintPass for UsedUnderscoreBinding { + fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + let needs_lint = match expr.node { + ExprPath(_, ref path) => { + path.segments.last().unwrap().identifier.name.as_str().chars().next() == Some('_') && + (cx.tcx.def_map.borrow()).values().any(|res| match res.base_def { + Def::DefLocal(_, _) => true, + _ => false + }) + }, + ExprField(_, spanned) => spanned.node.as_str().chars().next() == Some('_'), + _ => false + }; + if needs_lint { + cx.span_lint(USED_UNDERSCORE_BINDING, expr.span, &format!( + "Used binding which is prefixed with an underscore. A leading underscore signals\ + that a binding will not be used.")); + } + } +} From 43b96d59ade2321bf55f2ee0f082b8f0b7080bfe Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Fri, 11 Dec 2015 15:08:59 -0800 Subject: [PATCH 03/14] Run update_lints.py --- README.md | 3 ++- src/lib.rs | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4e4100028529..015a7d9adf26 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 83 lints included in this crate: +There are 84 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -86,6 +86,7 @@ name [unstable_as_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice) | warn | as_slice is not stable and can be replaced by & v[..]see https://github.com/rust-lang/rust/issues/27729 [unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop [unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes) | warn | unused lifetimes in function definitions +[used_underscore_binding](https://github.com/Manishearth/rust-clippy/wiki#used_underscore_binding) | warn | using a binding which is prefixed with an underscore [useless_transmute](https://github.com/Manishearth/rust-clippy/wiki#useless_transmute) | warn | transmutes that have the same to and from types [while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop) | warn | `loop { if let { ... } else break }` can be written as a `while let` loop [while_let_on_iterator](https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator) | warn | using a while-let loop instead of a for loop on an iterator diff --git a/src/lib.rs b/src/lib.rs index 03e0cbea19c1..05181e69b2ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -120,6 +120,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(25)); reg.register_late_lint_pass(box escape::EscapePass); reg.register_early_lint_pass(box misc_early::MiscEarly); + reg.register_late_lint_pass(box misc::UsedUnderscoreBinding); reg.register_lint_group("clippy_pedantic", vec![ methods::OPTION_UNWRAP_USED, @@ -184,6 +185,7 @@ pub fn plugin_registrar(reg: &mut Registry) { misc::MODULO_ONE, misc::REDUNDANT_PATTERN, misc::TOPLEVEL_REF_ARG, + misc::USED_UNDERSCORE_BINDING, misc_early::UNNEEDED_FIELD_PATTERN, mut_reference::UNNECESSARY_MUT_PASSED, mutex_atomic::MUTEX_ATOMIC, From 609111269818f99918980c85753d0d1bcea488c3 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Sat, 12 Dec 2015 16:31:34 -0800 Subject: [PATCH 04/14] Update tests --- src/misc.rs | 2 +- tests/compile-fail/used_underscore_binding.rs | 38 +++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/misc.rs b/src/misc.rs index 7068aac781e1..5c3aeeb78994 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -345,7 +345,7 @@ impl LateLintPass for UsedUnderscoreBinding { }; if needs_lint { cx.span_lint(USED_UNDERSCORE_BINDING, expr.span, &format!( - "Used binding which is prefixed with an underscore. A leading underscore signals\ + "used binding which is prefixed with an underscore. A leading underscore signals\ that a binding will not be used.")); } } diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index 8c7fe4e00691..adc20d67841f 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -1,22 +1,30 @@ #![feature(plugin)] #![plugin(clippy)] -#[deny(used_underscore_binding)] +#![deny(clippy)] + +fn prefix_underscore(_x: u32) -> u32{ + _x + 1 //~ ERROR used binding which is prefixed with an underscore +} + +fn in_macro(_x: u32) { + println!("{}", _x); //~ ERROR used binding which is prefixed with an underscore +} + +fn non_prefix_underscore(some_foo: u32) -> u32 { + some_foo + 1 +} + +fn unused_underscore(_foo: u32) -> u32 { + 1 +} fn main() { let foo = 0u32; - prefix_underscore(foo); //should fail - non_prefix_underscore(foo); //should pass - unused_underscore(foo); //should pass + // tests of unused_underscore lint + let _ = prefix_underscore(foo); + in_macro(foo); + // possible false positives + let _ = non_prefix_underscore(foo); + let _ = unused_underscore(foo); } -fn prefix_underscore(_x: u32){ - println!("{}", _x + 1); //~Error: Used binding which is prefixed with an underscore -} - -fn non_prefix_underscore(some_foo: u32) { - println!("{}", some_foo + 1); -} - -fn unused_underscore(_foo: u32) { - println!("{}", 1); -} From aeb5a0e60cc7b61253fa486b4e3af4d537af7322 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Sat, 12 Dec 2015 17:51:58 -0800 Subject: [PATCH 05/14] Reduce false positives Add macro checking, and only lint for single leading underscores --- src/misc.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/misc.rs b/src/misc.rs index 5c3aeeb78994..f46a26c10e1d 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -334,19 +334,24 @@ impl LateLintPass for UsedUnderscoreBinding { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { let needs_lint = match expr.node { ExprPath(_, ref path) => { - path.segments.last().unwrap().identifier.name.as_str().chars().next() == Some('_') && - (cx.tcx.def_map.borrow()).values().any(|res| match res.base_def { + let ident = path.segments.last() + .expect("path should always have at least one segment") + .identifier; + ident.name.as_str().chars().next() == Some('_') //starts with '_' + && ident.name.as_str().chars().skip(1).next() != Some('_') //doesn't start with "__" + && ident.name != ident.unhygienic_name //not in macro + && cx.tcx.def_map.borrow().values().any(|res| match res.base_def { Def::DefLocal(_, _) => true, _ => false - }) + }) //local variable }, ExprField(_, spanned) => spanned.node.as_str().chars().next() == Some('_'), _ => false }; if needs_lint { - cx.span_lint(USED_UNDERSCORE_BINDING, expr.span, &format!( - "used binding which is prefixed with an underscore. A leading underscore signals\ - that a binding will not be used.")); + cx.span_lint(USED_UNDERSCORE_BINDING, expr.span, + "used binding which is prefixed with an underscore. A leading underscore\ + signals that a binding will not be used."); } } } From 92fba6bd2c7c5b1a08ff1399ae8357e51399a1c0 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Sat, 12 Dec 2015 21:35:35 -0800 Subject: [PATCH 06/14] Make clippy tests compatible with new lint --- tests/compile-fail/for_loop.rs | 3 ++- tests/compile-fail/range.rs | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 7a18c210d0c7..59b77f6421bd 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -16,7 +16,8 @@ impl Unrelated { #[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)] #[deny(unused_collect)] -#[allow(linkedlist,shadow_unrelated,unnecessary_mut_passed, cyclomatic_complexity)] +#[allow(linkedlist,shadow_unrelated,unnecessary_mut_passed, cyclomatic_complexity, + used_underscore_binding)] fn main() { let mut vec = vec![1, 2, 3, 4]; let vec2 = vec![1, 2, 3, 4]; diff --git a/tests/compile-fail/range.rs b/tests/compile-fail/range.rs index 2d731670cbe3..064d9f551732 100644 --- a/tests/compile-fail/range.rs +++ b/tests/compile-fail/range.rs @@ -22,8 +22,8 @@ fn main() { let y = NotARange; y.step_by(0); - let _v1 = vec![1,2,3]; - let _v2 = vec![4,5]; - let _x = _v1.iter().zip(0.._v1.len()); //~ERROR It is more idiomatic to use _v1.iter().enumerate() - let _y = _v1.iter().zip(0.._v2.len()); // No error + let v1 = vec![1,2,3]; + let v2 = vec![4,5]; + let _x = v1.iter().zip(0..v1.len()); //~ERROR It is more idiomatic to use v1.iter().enumerate() + let _y = v1.iter().zip(0..v2.len()); // No error } From b24e3aeea038cb207f28f868a6af1e36897d0e5b Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Sat, 12 Dec 2015 21:50:36 -0800 Subject: [PATCH 07/14] Add wiki docs, in line with #492 --- src/misc.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/misc.rs b/src/misc.rs index f46a26c10e1d..2558d6bb4bbb 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -318,6 +318,29 @@ impl LateLintPass for PatternPass { } } + +/// **What it does:** This lint checks for the use of bindings with a single leading underscore +/// +/// **Why is this bad?** A single leading underscore is usually used to indicate that a binding +/// will not be used. Using such a binding breaks this expectation. +/// +/// **Known problems:** This lint's idea of a "used" variable is not quite the same as in the +/// built-in `unused_variables` lint. For example, in the following code +/// ``` +/// fn foo(_y: u32) -> u32) { +/// let _x = 1; +/// _x +=1; +/// y +/// } +/// ``` +/// _x will trigger both the `unused_variables` lint and the `used_underscore_binding` lint. +/// +/// **Example**: +/// ``` +/// let _x = 0; +/// let y = _x + 1; // Here we are using `_x`, even though it has a leading underscore. +/// // We should rename `_x` to `x` +/// ``` declare_lint!(pub USED_UNDERSCORE_BINDING, Warn, "using a binding which is prefixed with an underscore"); From 6960bf2ebc711385048834863eda081bae2633db Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Sat, 12 Dec 2015 21:59:25 -0800 Subject: [PATCH 08/14] Make ExprField follow single-underscore rules --- src/misc.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/misc.rs b/src/misc.rs index 2558d6bb4bbb..89b4dbb4de6b 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -368,7 +368,11 @@ impl LateLintPass for UsedUnderscoreBinding { _ => false }) //local variable }, - ExprField(_, spanned) => spanned.node.as_str().chars().next() == Some('_'), + ExprField(_, spanned) => { + let name = spanned.node.as_str(); + name.chars().next() == Some('_') + && name.chars().skip(1).next() != Some('_') + }, _ => false }; if needs_lint { From e620a1d57cb1a49b1c6e7e45c19f9102dc0876d3 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Wed, 16 Dec 2015 15:28:06 -0800 Subject: [PATCH 09/14] Make suggested changes --- src/misc.rs | 8 ++------ tests/compile-fail/used_underscore_binding.rs | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/misc.rs b/src/misc.rs index 89b4dbb4de6b..3b093027f481 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -9,7 +9,6 @@ use rustc::middle::ty; use rustc::middle::const_eval::ConstVal::Float; use rustc::middle::const_eval::eval_const_expr_partial; use rustc::middle::const_eval::EvalHint::ExprTypeChecked; -use rustc::middle::def::Def; use utils::{get_item_name, match_path, snippet, span_lint, walk_ptrs_ty, is_integer_literal}; use utils::span_help_and_lint; @@ -327,7 +326,7 @@ impl LateLintPass for PatternPass { /// **Known problems:** This lint's idea of a "used" variable is not quite the same as in the /// built-in `unused_variables` lint. For example, in the following code /// ``` -/// fn foo(_y: u32) -> u32) { +/// fn foo(y: u32) -> u32) { /// let _x = 1; /// _x +=1; /// y @@ -363,10 +362,7 @@ impl LateLintPass for UsedUnderscoreBinding { ident.name.as_str().chars().next() == Some('_') //starts with '_' && ident.name.as_str().chars().skip(1).next() != Some('_') //doesn't start with "__" && ident.name != ident.unhygienic_name //not in macro - && cx.tcx.def_map.borrow().values().any(|res| match res.base_def { - Def::DefLocal(_, _) => true, - _ => false - }) //local variable + && cx.tcx.def_map.borrow().contains_key(&expr.id) //local variable }, ExprField(_, spanned) => { let name = spanned.node.as_str(); diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index adc20d67841f..5567f23a9ff7 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -2,7 +2,7 @@ #![plugin(clippy)] #![deny(clippy)] -fn prefix_underscore(_x: u32) -> u32{ +fn prefix_underscore(_x: u32) -> u32 { _x + 1 //~ ERROR used binding which is prefixed with an underscore } From 3533d3a22302269b90796a09f8baf292f6849c5d Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Thu, 17 Dec 2015 13:52:30 -0800 Subject: [PATCH 10/14] Add more tests --- tests/compile-fail/used_underscore_binding.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index 5567f23a9ff7..bc6b32807f35 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -2,22 +2,46 @@ #![plugin(clippy)] #![deny(clippy)] +/// Test that we lint if we use a binding with a single leading underscore fn prefix_underscore(_x: u32) -> u32 { _x + 1 //~ ERROR used binding which is prefixed with an underscore } +/// Test that we lint even if the use is within a macro expansion fn in_macro(_x: u32) { println!("{}", _x); //~ ERROR used binding which is prefixed with an underscore } +/// Test that we do not lint if the underscore is not a prefix fn non_prefix_underscore(some_foo: u32) -> u32 { some_foo + 1 } +/// Test that we do not lint if we do not use the binding fn unused_underscore(_foo: u32) -> u32 { 1 } +// Non-variable bindings with preceding underscore +fn _fn_test() {} +struct _StructTest; +enum _EnumTest { + _FieldA, + _FieldB(_StructTest) +} + +/// Test that we do not lint for non-variable bindings +fn non_variables() { + _fn_test(); + let _s = _StructTest; + let _e = match _EnumTest::_FieldB(_StructTest) { + _EnumTest::_FieldA => 0, + _EnumTest::_FieldB(_st) => 1, + }; + let f = _fn_test; + f(); +} + fn main() { let foo = 0u32; // tests of unused_underscore lint @@ -26,5 +50,6 @@ fn main() { // possible false positives let _ = non_prefix_underscore(foo); let _ = unused_underscore(foo); + non_variables(); } From 02cb24de82e7dda49e7ae612a968de548c67b8f0 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Fri, 18 Dec 2015 13:45:03 -0800 Subject: [PATCH 11/14] Remove local variable check --- src/misc.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/misc.rs b/src/misc.rs index 3b093027f481..9954e3582320 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -362,7 +362,6 @@ impl LateLintPass for UsedUnderscoreBinding { ident.name.as_str().chars().next() == Some('_') //starts with '_' && ident.name.as_str().chars().skip(1).next() != Some('_') //doesn't start with "__" && ident.name != ident.unhygienic_name //not in macro - && cx.tcx.def_map.borrow().contains_key(&expr.id) //local variable }, ExprField(_, spanned) => { let name = spanned.node.as_str(); From c8d78a70b3acbf255a7dfb9472361691e1e0f6cf Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Fri, 18 Dec 2015 13:47:12 -0800 Subject: [PATCH 12/14] Test that we do not lint for multiple underscores --- tests/compile-fail/used_underscore_binding.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index bc6b32807f35..0d822a29cee6 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -22,6 +22,11 @@ fn unused_underscore(_foo: u32) -> u32 { 1 } +///Test that we do not lint for multiple underscores +fn multiple_underscores(__x: u32) -> u32 { + __x + 1 +} + // Non-variable bindings with preceding underscore fn _fn_test() {} struct _StructTest; @@ -50,6 +55,7 @@ fn main() { // possible false positives let _ = non_prefix_underscore(foo); let _ = unused_underscore(foo); + let _ = multiple_underscores(foo); non_variables(); } From 98d21f9fc5f5dfb4391452b506d4a27ac1d452a4 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Fri, 18 Dec 2015 16:04:33 -0800 Subject: [PATCH 13/14] Make compatible with `unused_variables` lint --- src/misc.rs | 30 +++++++++++-------- tests/compile-fail/for_loop.rs | 7 ++--- tests/compile-fail/used_underscore_binding.rs | 29 +++++++++++------- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/misc.rs b/src/misc.rs index 9954e3582320..44d044a4384c 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -10,7 +10,8 @@ use rustc::middle::const_eval::ConstVal::Float; use rustc::middle::const_eval::eval_const_expr_partial; use rustc::middle::const_eval::EvalHint::ExprTypeChecked; -use utils::{get_item_name, match_path, snippet, span_lint, walk_ptrs_ty, is_integer_literal}; +use utils::{get_item_name, match_path, snippet, get_parent_expr, span_lint, walk_ptrs_ty, + is_integer_literal}; use utils::span_help_and_lint; /// **What it does:** This lint checks for function arguments and let bindings denoted as `ref`. It is `Warn` by default. @@ -323,16 +324,7 @@ impl LateLintPass for PatternPass { /// **Why is this bad?** A single leading underscore is usually used to indicate that a binding /// will not be used. Using such a binding breaks this expectation. /// -/// **Known problems:** This lint's idea of a "used" variable is not quite the same as in the -/// built-in `unused_variables` lint. For example, in the following code -/// ``` -/// fn foo(y: u32) -> u32) { -/// let _x = 1; -/// _x +=1; -/// y -/// } -/// ``` -/// _x will trigger both the `unused_variables` lint and the `used_underscore_binding` lint. +/// **Known problems:** None /// /// **Example**: /// ``` @@ -362,6 +354,7 @@ impl LateLintPass for UsedUnderscoreBinding { ident.name.as_str().chars().next() == Some('_') //starts with '_' && ident.name.as_str().chars().skip(1).next() != Some('_') //doesn't start with "__" && ident.name != ident.unhygienic_name //not in macro + && is_used(cx, expr) }, ExprField(_, spanned) => { let name = spanned.node.as_str(); @@ -372,8 +365,21 @@ impl LateLintPass for UsedUnderscoreBinding { }; if needs_lint { cx.span_lint(USED_UNDERSCORE_BINDING, expr.span, - "used binding which is prefixed with an underscore. A leading underscore\ + "used binding which is prefixed with an underscore. A leading underscore \ signals that a binding will not be used."); } } } + +fn is_used(cx: &LateContext, expr: &Expr) -> bool { + if let Some(ref parent) = get_parent_expr(cx, expr) { + match parent.node { + ExprAssign(_, ref rhs) => **rhs == *expr, + ExprAssignOp(_, _, ref rhs) => **rhs == *expr, + _ => is_used(cx, &parent) + } + } + else { + true + } +} diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 59b77f6421bd..f1c1adf6cc89 100644 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -16,8 +16,7 @@ impl Unrelated { #[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)] #[deny(unused_collect)] -#[allow(linkedlist,shadow_unrelated,unnecessary_mut_passed, cyclomatic_complexity, - used_underscore_binding)] +#[allow(linkedlist,shadow_unrelated,unnecessary_mut_passed, cyclomatic_complexity)] fn main() { let mut vec = vec![1, 2, 3, 4]; let vec2 = vec![1, 2, 3, 4]; @@ -180,8 +179,8 @@ fn main() { if false { _index = 0 }; for _v in &vec { _index += 1 } - let mut _index = 0; - { let mut _x = &mut _index; } + let mut index = 0; + { let mut _x = &mut index; } for _v in &vec { _index += 1 } let mut index = 0; diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index 0d822a29cee6..e787124dce7a 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -3,13 +3,13 @@ #![deny(clippy)] /// Test that we lint if we use a binding with a single leading underscore -fn prefix_underscore(_x: u32) -> u32 { - _x + 1 //~ ERROR used binding which is prefixed with an underscore +fn prefix_underscore(_foo: u32) -> u32 { + _foo + 1 //~ ERROR used binding which is prefixed with an underscore } /// Test that we lint even if the use is within a macro expansion -fn in_macro(_x: u32) { - println!("{}", _x); //~ ERROR used binding which is prefixed with an underscore +fn in_macro(_foo: u32) { + println!("{}", _foo); //~ ERROR used binding which is prefixed with an underscore } /// Test that we do not lint if the underscore is not a prefix @@ -17,14 +17,23 @@ fn non_prefix_underscore(some_foo: u32) -> u32 { some_foo + 1 } -/// Test that we do not lint if we do not use the binding -fn unused_underscore(_foo: u32) -> u32 { +/// Test that we do not lint if we do not use the binding (simple case) +fn unused_underscore_simple(_foo: u32) -> u32 { + 1 +} + +#[deny(unused_variables)] +/// Test that we do not lint if we do not use the binding (complex case). This checks for +/// compatibility with the built-in `unused_variables` lint. +fn unused_underscore_complex(mut _foo: u32) -> u32 { + _foo += 1; + _foo = 2; 1 } ///Test that we do not lint for multiple underscores -fn multiple_underscores(__x: u32) -> u32 { - __x + 1 +fn multiple_underscores(__foo: u32) -> u32 { + __foo + 1 } // Non-variable bindings with preceding underscore @@ -54,8 +63,8 @@ fn main() { in_macro(foo); // possible false positives let _ = non_prefix_underscore(foo); - let _ = unused_underscore(foo); + let _ = unused_underscore_simple(foo); + let _ = unused_underscore_complex(foo); let _ = multiple_underscores(foo); non_variables(); } - From bd82c082cb7734f037116d38f949ca354b597a29 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Fri, 18 Dec 2015 16:29:22 -0800 Subject: [PATCH 14/14] Add test for struct fields --- tests/compile-fail/used_underscore_binding.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index e787124dce7a..39a33c96876e 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -12,6 +12,17 @@ fn in_macro(_foo: u32) { println!("{}", _foo); //~ ERROR used binding which is prefixed with an underscore } +// Struct for testing use of fields prefixed with an underscore +struct StructFieldTest { + _underscore_field: u32, +} + +/// Test that we lint the use of a struct field which is prefixed with an underscore +fn in_struct_field() { + let mut s = StructFieldTest { _underscore_field: 0 }; + s._underscore_field += 1; //~ Error used binding which is prefixed with an underscore +} + /// Test that we do not lint if the underscore is not a prefix fn non_prefix_underscore(some_foo: u32) -> u32 { some_foo + 1 @@ -22,7 +33,6 @@ fn unused_underscore_simple(_foo: u32) -> u32 { 1 } -#[deny(unused_variables)] /// Test that we do not lint if we do not use the binding (complex case). This checks for /// compatibility with the built-in `unused_variables` lint. fn unused_underscore_complex(mut _foo: u32) -> u32 { @@ -61,6 +71,7 @@ fn main() { // tests of unused_underscore lint let _ = prefix_underscore(foo); in_macro(foo); + in_struct_field(); // possible false positives let _ = non_prefix_underscore(foo); let _ = unused_underscore_simple(foo);