OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

shadowing detection

Browse Source
This commit is contained in:
llogiq 2015-08-21 17:11:34 +02:00
parent 209e6981a3
commit 5225feceaa
5 changed files with 97 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
README.md
View File

@ -44,9 +44,9 @@ ptr_arg | allow | fn arguments of the type `&Vec<...>` or `&S
range_step_by_zero | warn | using Range::step_by(0), which produces an infinite iterator
redundant_closure | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
result_unwrap_used | allow | using `Result.unwrap()`, which might be better handled
shadow_foreign | warn | The name is re-bound without even using the original value
shadow_reuse | allow | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1`
shadow_same | allow | rebinding a name to itself, e.g. `let mut x = &mut x`
shadow_unrelated | warn | The name is re-bound without even using the original value
single_match | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
str_to_string | warn | using `to_string()` on a str, which should be `to_owned()`
string_add | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead

4
src/lib.rs
View File

@ -76,9 +76,9 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_lint_pass(box matches::MatchPass as LintPassObject);
reg.register_lint_group("shadow", vec![
shadow::SHADOW_FOREIGN,
shadow::SHADOW_REUSE,
shadow::SHADOW_SAME,
shadow::SHADOW_UNRELATED,
]);
reg.register_lint_group("clippy", vec![
@ -114,9 +114,9 @@ pub fn plugin_registrar(reg: &mut Registry) {
ranges::RANGE_STEP_BY_ZERO,
returns::LET_AND_RETURN,
returns::NEEDLESS_RETURN,
shadow::SHADOW_FOREIGN,
shadow::SHADOW_REUSE,
shadow::SHADOW_SAME,
shadow::SHADOW_UNRELATED,
strings::STRING_ADD,
strings::STRING_ADD_ASSIGN,
types::BOX_VEC,

3
src/methods.rs
View File

@ -41,7 +41,8 @@ impl LintPass for MethodsPass {
if obj_ty.sty == ty::TyStr {
span_lint(cx, STR_TO_STRING, expr.span, "`str.to_owned()` is faster");
} else if match_type(cx, obj_ty, &STRING_PATH) {
span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op");
span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op; use \
`clone()` to make a copy");
}
}
}

152
src/shadow.rs
View File

@ -1,3 +1,4 @@
use std::ops::Deref;
use syntax::ast::*;
use syntax::codemap::Span;
use syntax::visit::FnKind;
@ -10,7 +11,7 @@ declare_lint!(pub SHADOW_SAME, Allow,
declare_lint!(pub SHADOW_REUSE, Allow,
"rebinding a name to an expression that re-uses the original value, e.g. \
`let x = x + 1`");
declare_lint!(pub SHADOW_FOREIGN, Warn,
declare_lint!(pub SHADOW_UNRELATED, Warn,
"The name is re-bound without even using the original value");
#[derive(Copy, Clone)]
@ -18,7 +19,7 @@ pub struct ShadowPass;
impl LintPass for ShadowPass {
fn get_lints(&self) -> LintArray {
lint_array!(SHADOW_SAME, SHADOW_REUSE, SHADOW_FOREIGN)
lint_array!(SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED)
}
fn check_fn(&mut self, cx: &Context, _: FnKind, decl: &FnDecl,
@ -44,10 +45,6 @@ fn named(pat: &Pat) -> Option<Name> {
} else { None }
}
fn add(bindings: &mut Vec<Name>, pat: &Pat) {
named(pat).map(|name| bindings.push(name));
}
fn check_block(cx: &Context, block: &Block, bindings: &mut Vec<Name>) {
let len = bindings.len();
for stmt in &block.stmts {
@ -65,41 +62,53 @@ fn check_block(cx: &Context, block: &Block, bindings: &mut Vec<Name>) {
fn check_decl(cx: &Context, decl: &Decl, bindings: &mut Vec<Name>) {
if in_external_macro(cx, decl.span) { return; }
if let DeclLocal(ref local) = decl.node {
let Local{ ref pat, ref ty, ref init, id: _, span: _ } = **local;
if let &Some(ref t) = ty { check_ty(cx, t, bindings); }
named(pat).map(|name| if bindings.contains(&name) {
if let &Some(ref o) = init {
if in_external_macro(cx, o.span) { return; }
check_expr(cx, o, bindings);
bindings.push(name);
lint_shadow(cx, name, decl.span, pat.span, o);
}
});
add(bindings, pat);
if let &Some(ref o) = init {
check_expr(cx, o, bindings)
}
let Local{ ref pat, ref ty, ref init, id: _, span } = **local;
if let &Some(ref t) = ty { check_ty(cx, t, bindings) }
check_pat(cx, pat, init, span, bindings);
if let &Some(ref o) = init { check_expr(cx, o, bindings) }
}
}
fn lint_shadow(cx: &Context, name: Name, span: Span, lspan: Span, init: &Expr) {
if is_self_shadow(name, init) {
span_lint(cx, SHADOW_SAME, span, &format!(
"{} is shadowed by itself in {}",
snippet(cx, lspan, "_"),
snippet(cx, init.span, "..")));
} else {
if contains_self(name, init) {
span_lint(cx, SHADOW_REUSE, span, &format!(
"{} is shadowed by {} which reuses the original value",
snippet(cx, lspan, "_"),
snippet(cx, init.span, "..")));
} else {
span_lint(cx, SHADOW_FOREIGN, span, &format!(
"{} is shadowed by {} in this declaration",
snippet(cx, lspan, "_"),
snippet(cx, init.span, "..")));
fn check_pat<T>(cx: &Context, pat: &Pat, init: &Option<T>, span: Span,
bindings: &mut Vec<Name>) where T: Deref<Target=Expr> {
//TODO: match more stuff / destructuring
named(pat).map(|name| {
if let &Some(ref o) = init {
if !in_external_macro(cx, o.span) {
check_expr(cx, o, bindings);
}
}
if bindings.contains(&name) {
lint_shadow(cx, name, span, pat.span, init);
}
bindings.push(name);
});
}
fn lint_shadow<T>(cx: &Context, name: Name, span: Span, lspan: Span, init:
&Option<T>) where T: Deref<Target=Expr> {
if let &Some(ref expr) = init {
if is_self_shadow(name, expr) {
span_lint(cx, SHADOW_SAME, span, &format!(
"{} is shadowed by itself in {}",
snippet(cx, lspan, "_"),
snippet(cx, expr.span, "..")));
} else {
if contains_self(name, expr) {
span_lint(cx, SHADOW_REUSE, span, &format!(
"{} is shadowed by {} which reuses the original value",
snippet(cx, lspan, "_"),
snippet(cx, expr.span, "..")));
} else {
span_lint(cx, SHADOW_UNRELATED, span, &format!(
"{} is shadowed by {} in this declaration",
snippet(cx, lspan, "_"),
snippet(cx, expr.span, "..")));
}
}
} else {
span_lint(cx, SHADOW_UNRELATED, span, &format!(
"{} is shadowed in this declaration", snippet(cx, lspan, "_")));
}
}
@ -120,26 +129,21 @@ fn check_expr(cx: &Context, expr: &Expr, bindings: &mut Vec<Name>) {
check_block(cx, then, bindings);
if let &Some(ref o) = otherwise { check_expr(cx, o, bindings); }
},
ExprIfLet(ref pat, ref e, ref block, ref otherwise) => {
check_expr(cx, e, bindings);
let len = bindings.len();
add(bindings, pat);
check_block(cx, block, bindings);
if let &Some(ref o) = otherwise { check_expr(cx, o, bindings); }
bindings.truncate(len);
},
ExprWhile(ref cond, ref block, _) => {
check_expr(cx, cond, bindings);
check_block(cx, block, bindings);
},
ExprWhileLet(ref pat, ref e, ref block, _) |
ExprForLoop(ref pat, ref e, ref block, _) => {
check_expr(cx, e, bindings);
let len = bindings.len();
add(bindings, pat);
check_block(cx, block, bindings);
bindings.truncate(len);
},
ExprMatch(ref init, ref arms, _) =>
for ref arm in arms {
for ref pat in &arm.pats {
//TODO: This is ugly, but needed to get the right type
check_pat(cx, pat, &Some(&**init), pat.span, bindings);
}
if let Some(ref guard) = arm.guard {
check_expr(cx, guard, bindings);
}
check_expr(cx, &*arm.body, bindings);
},
_ => ()
}
}
@ -169,12 +173,15 @@ fn is_self_shadow(name: Name, expr: &Expr) -> bool {
map_or(false, |ref e| is_self_shadow(name, e)),
ExprUnary(op, ref inner) => (UnUniq == op || UnDeref == op) &&
is_self_shadow(name, inner),
ExprPath(_, ref path) => path.segments.len() == 1 &&
path.segments[0].identifier.name == name,
ExprPath(_, ref path) => path_eq_name(name, path),
_ => false,
}
}
fn path_eq_name(name: Name, path: &Path) -> bool {
path.segments.len() == 1 && path.segments[0].identifier.name == name
}
fn contains_self(name: Name, expr: &Expr) -> bool {
match expr.node {
ExprUnary(_, ref e) | ExprParen(ref e) | ExprField(ref e, _) |
@ -193,13 +200,11 @@ fn contains_self(name: Name, expr: &Expr) -> bool {
ExprIf(ref cond, ref then, ref otherwise) =>
contains_self(name, cond) || contains_block_self(name, then) ||
otherwise.as_ref().map_or(false, |ref e| contains_self(name, e)),
ExprIfLet(_, ref e, ref block, ref otherwise) =>
contains_self(name, e) || contains_block_self(name, block) ||
otherwise.as_ref().map_or(false, |ref o| contains_self(name, o)),
ExprWhile(ref e, ref block, _) |
ExprWhileLet(_, ref e, ref block, _) |
ExprForLoop(_, ref e, ref block, _) =>
ExprWhile(ref e, ref block, _) =>
contains_self(name, e) || contains_block_self(name, block),
ExprMatch(ref e, ref arms, _) =>
arms.iter().any(|ref arm| arm.pats.iter().any(|ref pat|
contains_pat_self(name, pat))) || contains_self(name, e),
ExprPath(_, ref path) => path.segments.len() == 1 &&
path.segments[0].identifier.name == name,
_ => false
@ -211,6 +216,9 @@ fn contains_block_self(name: Name, block: &Block) -> bool {
match stmt.node {
StmtDecl(ref decl, _) =>
if let DeclLocal(ref local) = decl.node {
//TODO: We don't currently handle the case where the name
//is shadowed wiithin the block; this means code including this
//degenerate pattern will get the wrong warning.
if let Some(ref init) = local.init {
if contains_self(name, init) { return true; }
}
@ -222,3 +230,25 @@ fn contains_block_self(name: Name, block: &Block) -> bool {
}
if let Some(ref e) = block.expr { contains_self(name, e) } else { false }
}
fn contains_pat_self(name: Name, pat: &Pat) -> bool {
match pat.node {
PatIdent(_, ref ident, ref inner) => name == ident.node.name ||
inner.as_ref().map_or(false, |ref p| contains_pat_self(name, p)),
PatEnum(_, ref opats) => opats.as_ref().map_or(false,
|pats| pats.iter().any(|p| contains_pat_self(name, p))),
PatQPath(_, ref path) => path_eq_name(name, path),
PatStruct(_, ref fieldpats, _) => fieldpats.iter().any(
|ref fp| contains_pat_self(name, &fp.node.pat)),
PatTup(ref ps) => ps.iter().any(|ref p| contains_pat_self(name, p)),
PatBox(ref p) |
PatRegion(ref p, _) => contains_pat_self(name, p),
PatRange(ref from, ref until) =>
contains_self(name, from) || contains_self(name, until),
PatVec(ref pre, ref opt, ref post) =>
pre.iter().any(|ref p| contains_pat_self(name, p)) ||
opt.as_ref().map_or(false, |ref p| contains_pat_self(name, p)) ||
post.iter().any(|ref p| contains_pat_self(name, p)),
_ => false,
}
}

2
tests/compile-fail/approx_const.rs
View File

@ -2,7 +2,7 @@
#![plugin(clippy)]
#[deny(approx_constant)]
#[allow(unused, shadow_foreign)]
#[allow(unused, shadow_unrelated)]
fn main() {
let my_e = 2.7182; //~ERROR approximate value of `f{32, 64}::E` found
let almost_e = 2.718; //~ERROR approximate value of `f{32, 64}::E` found