Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use identifier hygiene in HIR #1012

Merged
merged 2 commits into from
Jun 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "clippy"
version = "0.0.76"
version = "0.0.77"
authors = [
"Manish Goregaokar <manishsmail@gmail.com>",
"Andre Bogus <bogusandre@gmail.com>",
Expand All @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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 <manishsmail@gmail.com>",
Expand Down
47 changes: 31 additions & 16 deletions clippy_lints/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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
}
Expand All @@ -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));
}
}
}
Expand Down Expand Up @@ -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,
}
}
9 changes: 4 additions & 5 deletions clippy_lints/src/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand All @@ -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<Vec<P<Pat>>>),
PatKind::Struct(_, ref pfields, _) => {
if let Some(ref init_struct) = *init {
if let ExprStruct(_, ref efields, _) = init_struct.node {
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}
}
Expand Down
17 changes: 15 additions & 2 deletions tests/compile-fail/used_underscore_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down