From b190aa7debabc5b9d81b73b3f72701ece99d5ff0 Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Mon, 21 Dec 2015 01:03:12 -0800 Subject: [PATCH 1/2] Implement #507 Make `used_underscore_binding` lint compatible with MacroAttributes expansions. TODO: Add a good test for this. --- src/misc.rs | 27 ++++++++++++++----- tests/compile-fail/used_underscore_binding.rs | 11 +++++++- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/misc.rs b/src/misc.rs index 3a17ff0f0..139dfde36 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -3,7 +3,7 @@ use syntax::ptr::P; use rustc_front::hir::*; use reexport::*; use rustc_front::util::{is_comparison_binop, binop_to_string}; -use syntax::codemap::{Span, Spanned}; +use syntax::codemap::{Span, Spanned, ExpnFormat}; use rustc_front::intravisit::FnKind; use rustc::middle::ty; use rustc::middle::const_eval::ConstVal::Float; @@ -11,7 +11,7 @@ use rustc::middle::const_eval::eval_const_expr_partial; use rustc::middle::const_eval::EvalHint::ExprTypeChecked; use utils::{get_item_name, match_path, snippet, get_parent_expr, span_lint}; -use utils::{span_help_and_lint, in_external_macro, walk_ptrs_ty, is_integer_literal}; +use utils::{span_help_and_lint, walk_ptrs_ty, is_integer_literal}; /// **What it does:** This lint checks for function arguments and let bindings denoted as `ref`. It is `Warn` by default. /// @@ -345,6 +345,9 @@ impl LintPass for UsedUnderscoreBinding { impl LateLintPass for UsedUnderscoreBinding { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + if in_attributes_expansion(cx, expr) { // Don't lint things expanded by #[derive(...)], etc + return; + } let needs_lint = match expr.node { ExprPath(_, ref path) => { let ident = path.segments.last() @@ -352,7 +355,7 @@ impl LateLintPass for UsedUnderscoreBinding { .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 + && ident.name != ident.unhygienic_name //not in bang macro && is_used(cx, expr) }, ExprField(_, spanned) => { @@ -362,9 +365,6 @@ impl LateLintPass for UsedUnderscoreBinding { }, _ => false }; - if in_external_macro(cx, expr.span) { - return - } if needs_lint { cx.span_lint(USED_UNDERSCORE_BINDING, expr.span, "used binding which is prefixed with an underscore. A leading underscore \ @@ -373,6 +373,8 @@ impl LateLintPass for UsedUnderscoreBinding { } } +/// Heuristic to see if an expression is used. Should be compatible with `unused_variables`'s idea +/// of what it means for an expression to be "used". fn is_used(cx: &LateContext, expr: &Expr) -> bool { if let Some(ref parent) = get_parent_expr(cx, expr) { match parent.node { @@ -385,3 +387,16 @@ fn is_used(cx: &LateContext, expr: &Expr) -> bool { true } } + +/// Test whether an expression is in a macro expansion (e.g. something generated by #[derive(...)] +/// or the like) +fn in_attributes_expansion(cx: &LateContext, expr: &Expr) -> bool { + cx.sess().codemap().with_expn_info(expr.span.expn_id, |info_opt| { + info_opt.map_or(false, |info| { + match info.callee.format { + ExpnFormat::MacroAttribute(_) => true, + _ => false, + } + }) + }) +} diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index 49e1aa99c..fd1b3bfc1 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -9,7 +9,15 @@ fn prefix_underscore(_foo: u32) -> u32 { /// Test that we lint even if the use is within a macro expansion fn in_macro(_foo: u32) { - println!("{}", _foo); // doesn't warn, nut should #507 + println!("{}", _foo); //~ ERROR used binding which is prefixed with an underscore +} + +// TODO: This doesn't actually correctly test this. Need to find a #[derive(...)] which sets off +// the lint if the `in_attributes_expansion` test isn't there +/// Test that we do not lint for unused underscores in a MacroAttribute expansion +#[derive(Clone)] +struct MacroAttributesTest { + _foo: u32, } // Struct for testing use of fields prefixed with an underscore @@ -68,6 +76,7 @@ fn non_variables() { fn main() { let foo = 0u32; + let _ = MacroAttributesTest{_foo: 0}; // tests of unused_underscore lint let _ = prefix_underscore(foo); in_macro(foo); From b6766a0dcf66650a24820929c20be237e37230bc Mon Sep 17 00:00:00 2001 From: Devon Hollowood Date: Mon, 21 Dec 2015 01:40:19 -0800 Subject: [PATCH 2/2] Add RustcEncodable test --- Cargo.toml | 1 + tests/compile-fail/used_underscore_binding.rs | 9 --------- tests/used_underscore_binding_macro.rs | 16 ++++++++++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 tests/used_underscore_binding_macro.rs diff --git a/Cargo.toml b/Cargo.toml index 5b1332b18..609f847b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ compiletest_rs = "0.0.11" regex = "*" regex_macros = "*" lazy_static = "*" +rustc-serialize = "0.3" [features] diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index fd1b3bfc1..39a33c968 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -12,14 +12,6 @@ fn in_macro(_foo: u32) { println!("{}", _foo); //~ ERROR used binding which is prefixed with an underscore } -// TODO: This doesn't actually correctly test this. Need to find a #[derive(...)] which sets off -// the lint if the `in_attributes_expansion` test isn't there -/// Test that we do not lint for unused underscores in a MacroAttribute expansion -#[derive(Clone)] -struct MacroAttributesTest { - _foo: u32, -} - // Struct for testing use of fields prefixed with an underscore struct StructFieldTest { _underscore_field: u32, @@ -76,7 +68,6 @@ fn non_variables() { fn main() { let foo = 0u32; - let _ = MacroAttributesTest{_foo: 0}; // tests of unused_underscore lint let _ = prefix_underscore(foo); in_macro(foo); diff --git a/tests/used_underscore_binding_macro.rs b/tests/used_underscore_binding_macro.rs new file mode 100644 index 000000000..4170f907b --- /dev/null +++ b/tests/used_underscore_binding_macro.rs @@ -0,0 +1,16 @@ +#![feature(plugin)] +#![plugin(clippy)] + +extern crate rustc_serialize; + +/// Test that we do not lint for unused underscores in a MacroAttribute expansion +#[deny(used_underscore_binding)] +#[derive(RustcEncodable)] +struct MacroAttributesTest { + _foo: u32, +} + +#[test] +fn macro_attributes_test() { + let _ = MacroAttributesTest{_foo: 0}; +}