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

Restrict Usage of Shadowing #2928

Closed
silversolver1 opened this issue May 16, 2020 · 3 comments
Closed

Restrict Usage of Shadowing #2928

silversolver1 opened this issue May 16, 2020 · 3 comments

Comments

@silversolver1
Copy link

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
Copy link
Contributor

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

@burdges
Copy link

burdges commented May 16, 2020

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants