From e6cbe970c83f623609cad396aa8113e2340adea9 Mon Sep 17 00:00:00 2001 From: mcarton Date: Wed, 15 Jun 2016 16:27:56 +0200 Subject: [PATCH 1/2] 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 3d9795b0c38f..495325764690 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -10,8 +10,8 @@ use rustc_const_math::ConstFloat; 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 @@ impl LateLintPass for UsedUnderscoreBinding { } 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 @@ impl LateLintPass for UsedUnderscoreBinding { _ => 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 0954d92cf9a5..9838ce0202cf 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 c571906c53b8..c3700d1b1cd4 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 From 92c02bd4afdb691a762060a393603321933be34e Mon Sep 17 00:00:00 2001 From: mcarton Date: Tue, 21 Jun 2016 16:26:56 +0200 Subject: [PATCH 2/2] Bump to 0.0.77 --- CHANGELOG.md | 6 +++++- Cargo.toml | 4 ++-- clippy_lints/Cargo.toml | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81102a900e5c..c52cbdbc6757 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,11 @@ # Change Log All notable changes to this project will be documented in this file. -## 0.0.76 — TBD +## 0.0.77 — 2016-06-21 +* Rustup to *rustc 1.11.0-nightly (5522e678b 2016-06-20)* +* New lints: [`stutter`] and [`iter_nth`] + +## 0.0.76 — 2016-06-10 * Rustup to *rustc 1.11.0-nightly (7d2f75a95 2016-06-09)* * `cargo clippy` now automatically defines the `clippy` feature diff --git a/Cargo.toml b/Cargo.toml index 60862865b43f..e52342d3860d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.0.76" +version = "0.0.77" authors = [ "Manish Goregaokar ", "Andre Bogus ", @@ -25,7 +25,7 @@ test = false [dependencies] regex_macros = { version = "0.1.33", optional = true } # begin automatic update -clippy_lints = { version = "0.0.76", path = "clippy_lints" } +clippy_lints = { version = "0.0.77", path = "clippy_lints" } # end automatic update [dev-dependencies] diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 2f748d1ec3cb..a1012e94ee5a 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clippy_lints" # begin automatic update -version = "0.0.76" +version = "0.0.77" # end automatic update authors = [ "Manish Goregaokar ",