rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.96k stars 1.57k forks source link

Restrict Usage of Shadowing #2928

Closed silversolver1 closed 4 years ago

silversolver1 commented 4 years ago

As per current example in the book: https://doc.rust-lang.org/book/ch03-01-variables-and-mutability.html, the following is currently acceptable:

fn main() { //Example One
    let x = 5;

    let x = x + 1;

    let x = x * 2;

    println!("The value of x is: {}", x);
}

I think that this behavior should be foregone (ie not allowed) in favor of forcing the user to do something like the following:

fn main() { //Example Two
    let mut x = 5;

    x = x + 1;

    x = x * 2;

    println!("The value of x is: {}", x);    
}

My rationale is as follows:

Shadowing definitely has its uses when declaring a variable of the same name that is a different type or perhaps sometimes even when assigning a new value of the same type to a variable of the same name (if used appropriately).

However, as in Example One, I do not think it is appropriate that shadowing can currently be used to de facto conduct operations on variables that are supposed to be (and technically are) immutable.

I think that perhaps the best practice would instead be that if the user wants to change or do operations on a variable, that they should then do so (and have to do so) clearly: by first marking that variable as mutable (using let mut) and then, and only then, being able to conduct operations on it (at least when the type is not changing, as in the example).

I don't think this particular usage of shadowing, which effectively allows the user to hide behind the concepts of "declarations" or "creating a variable" to bypass properly utilizing mutability, is beneficial. I also think it violates the Rust-like principle of writing better and more correct code by design.

Thank you for your time.

Lokathor commented 4 years ago

Clippy has the ability to lint against variable shadowing, if you'd like to do that.

burdges commented 4 years ago

I think https://users.rust-lang.org/ and https://github.com/rust-unofficial/patterns/ are two better venues for this. I'll bite however:

In fact, repeated rebinding (shadowing) should be strongly preferred over mutability because they permit less, like no &mut self methods, no &mut borrows, including. by FnMut closures, no usage across loop, etc., and thus inherently give more informative error messages.

I've mostly found rebindings improve code readability for several reasons. Yes, rebinding could make code less readable than using new names, but mostly only if the code gets longer than usually desirable, and actual mutation would always be worse. And field puns make rebinding essential.

silversolver1 commented 4 years ago

Thanks for the references to clippy and the patterns repository.

I do agree that in Rust's current implementation of shadowing what I'm bringing up is more pattern-related than anything else. I'm just not certain the official resource of the Rust book should be promoting a particular usage of shadowing (see Example One) that could very easily be misconstrued as a good or reasonable alternative to the proper and clear use of mutability.

In any case, I'll close this for now as it seems that most want to leave this up to the user. Thanks.