Name Shadowing

[rust-lang]

Name Shadowing

[Manishearth]

I think shadowing is quite liberally used in Rust, so we should tread lightly here.

I think your proposed scheme should work well.

I think especially in nested libsyntax code it's quite common to shadow and re-shadow stuff like ty and expr because they end up being used a lot. But those users can just disable the lint.

[llogiq]

I know that many don't see shadowing as a problem. But I maintain that it does make code harder to visually parse than strictly necessary (where does that foo come from again?). And often, reusing a binding stems from lazyness to think up a more specific name.

Btw. scopes are a good antidote to shadowing – if a binding is out of scope, it cannot shadow anything. Thus, reducing scopes also reduces shadowing.

I'd really like to see how many warnings libsyntax would generate with shadow_foreign turned on. 😄

[Manishearth]

Sounds good to me.

[llogiq]

I see you added [T-middle]. Is there something that would ease building this?

I figure it would be possible to implement this by walking the AST of any fn and keeping a chain of sets of local bindings (one for each scope), but perhaps there is an easier solution?

[Manishearth]

I guess an AST walk would work. I was thinking along the lines of ExprUseVisitor but that's not exactly what we want anyway.

[llogiq]

@Manishearth Now I have a very strange compiler error:

error: duplicate specification of lint shadow_same
error: duplicate specification of lint shadow_reuse
error: duplicate specification of lint shadow_foreign

I'm quite sure I have only one definition of each lint. Perhaps a rustup --channel=nightly will fix it.

[birkenfeld]

This comes from get_lints() returning the same lint name(s) for multiple passes.

[llogiq]

Ah, I had registered the lint twice. Thanks, @birkenfeld.