From e6cbe970c83f623609cad396aa8113e2340adea9 Mon Sep 17 00:00:00 2001 From: mcarton Date: Wed, 15 Jun 2016 16:27:56 +0200 Subject: [PATCH] Don't use identifier hygiene in HIR --- clippy_lints/src/misc.rs | 47 ++++++++++++------- clippy_lints/src/shadow.rs | 9 ++-- tests/compile-fail/used_underscore_binding.rs | 17 ++++++- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 3d9795b0c38..49532576469 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -10,8 +10,8 @@ use syntax::codemap::{Span, Spanned, ExpnFormat}; use syntax::ptr::P; use utils::{ - get_item_name, get_parent_expr, implements_trait, is_integer_literal, match_path, snippet, - span_lint, span_lint_and_then, walk_ptrs_ty + get_item_name, get_parent_expr, implements_trait, in_macro, is_integer_literal, match_path, + snippet, span_lint, span_lint_and_then, walk_ptrs_ty }; /// **What it does:** This lint checks for function arguments and let bindings denoted as `ref`. @@ -405,15 +405,18 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { } let binding = match expr.node { ExprPath(_, ref path) => { - let segment = path.segments + let binding = path.segments .last() .expect("path should always have at least one segment") - .name; - if segment.as_str().starts_with('_') && - !segment.as_str().starts_with("__") && - segment != segment.unhygienize() && // not in bang macro - is_used(cx, expr) { - Some(segment.as_str()) + .name + .as_str(); + if binding.starts_with('_') && + !binding.starts_with("__") && + binding != "_result" && // FIXME: #944 + is_used(cx, expr) && + // don't lint if the declaration is in a macro + non_macro_local(cx, &cx.tcx.expect_def(expr.id)) { + Some(binding) } else { None } @@ -429,13 +432,11 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { _ => None, }; if let Some(binding) = binding { - if binding != "_result" { // FIXME: #944 - span_lint(cx, - 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.", binding)); - } + span_lint(cx, + 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.", binding)); } } } @@ -463,3 +464,17 @@ fn in_attributes_expansion(cx: &LateContext, expr: &Expr) -> bool { }) }) } + +/// Test whether `def` is a variable defined outside a macro. +fn non_macro_local(cx: &LateContext, def: &def::Def) -> bool { + match *def { + def::Def::Local(_, id) | def::Def::Upvar(_, id, _, _) => { + if let Some(span) = cx.tcx.map.opt_span(id) { + !in_macro(cx, span) + } else { + true + } + } + _ => false, + } +} diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 0954d92cf9a..9838ce0202c 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -66,7 +66,7 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, block: &Block) { let mut bindings = Vec::new(); for arg in &decl.inputs { if let PatKind::Binding(_, ident, _) = arg.pat.node { - bindings.push((ident.node.unhygienize(), ident.span)) + bindings.push((ident.node, ident.span)) } } check_block(cx, block, &mut bindings); @@ -120,7 +120,7 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, bind // TODO: match more stuff / destructuring match pat.node { PatKind::Binding(_, ref ident, ref inner) => { - let name = ident.node.unhygienize(); + let name = ident.node; if is_binding(cx, pat) { let mut new_binding = true; for tup in bindings.iter_mut() { @@ -139,7 +139,6 @@ fn check_pat(cx: &LateContext, pat: &Pat, init: &Option<&Expr>, span: Span, bind check_pat(cx, p, init, span, bindings); } } - // PatEnum(Path, Option>>), PatKind::Struct(_, ref pfields, _) => { if let Some(ref init_struct) = *init { if let ExprStruct(_, ref efields, _) = init_struct.node { @@ -327,7 +326,7 @@ fn is_self_shadow(name: Name, expr: &Expr) -> bool { } fn path_eq_name(name: Name, path: &Path) -> bool { - !path.global && path.segments.len() == 1 && path.segments[0].name.unhygienize() == name + !path.global && path.segments.len() == 1 && path.segments[0].name.as_str() == name.as_str() } struct ContainsSelf { @@ -337,7 +336,7 @@ struct ContainsSelf { impl<'v> Visitor<'v> for ContainsSelf { fn visit_name(&mut self, _: Span, name: Name) { - if self.name == name.unhygienize() { + if self.name == name { self.result = true; } } diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index c571906c53b..c3700d1b1cd 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -5,14 +5,27 @@ #![allow(blacklisted_name)] #![deny(used_underscore_binding)] +macro_rules! test_macro { + () => {{ + let _foo = 42; + _foo + 1 + }} +} + /// Test that we lint if we use a binding with a single leading underscore fn prefix_underscore(_foo: u32) -> u32 { _foo + 1 //~ ERROR used binding `_foo` which is prefixed with an underscore } -/// Test that we lint even if the use is within a macro expansion +/// Test that we lint if we use a `_`-variable defined outside within a macro expansion fn in_macro(_foo: u32) { - println!("{}", _foo); //~ ERROR used binding `_foo` which is prefixed with an underscore + println!("{}", _foo); + //~^ ERROR used binding `_foo` which is prefixed with an underscore + assert_eq!(_foo, _foo); + //~^ ERROR used binding `_foo` which is prefixed with an underscore + //~| ERROR used binding `_foo` which is prefixed with an underscore + + test_macro!() + 1; } // Struct for testing use of fields prefixed with an underscore -- 2.44.0