Description
What it does
The opposite of let_and_return. It will find which parts of your code are directly instantiating something as an output return value without first assigning it to a variable, and it will tell you to first assign to a variable before returning the value.
Advantage
The let_and_return
lint's "Why is this bad?" explanation says the following:
It is just extraneous code. Remove it to make your code more rusty.
I strongly disagree with both of these sentences.
"extraneous code"
The word "extraneous" suggests that this extra code doesn't serve any purpose and has no value. But that code does indeed serve a purpose and bring value. Here are some reasons for using the let_and_return pattern:
- If you're debugging the code, you might want to add
dbg!()
orprintln!()
statement on the return value to see what value is being returned. - A new contributor to a repository wants to explore the repository to understand how it works.
dbg!()
andprintln!()
wrapper around the return value helps the new contributor understand how the data values get changed by each line of code, including the last line. - It's more convenient to put a wrapper around a single variable name in the end instead of having to put a dbg!() or println!() wrapper around a large unit of code (for example a struct that gets instantiated directly into the last line as a return value).
- The variable name chosen for the return value serves as a form of documentation since it explains to the one exploring the repository how this data should be interpreted and what purpose it serves in the broader context.
- The let_and_return pattern doesn't incur any performance penalties since the compiler will optimize it away during the compilation process.
- By doing as I suggested above (in "What it does" section), we can avoid/prevent the "Known Problem" that the current
let_and_return
lint admits to causing - namely deadlocks as elaborated in Using pattern match directly on LockResult causes deadlock rust#37612. - This makes it more convenient to refactor a chunk of code into a separate function (or vice versa) as the developer could just copy-paste the code and use it as it is.
"more rusty"
This seems like a vague explanation to why we should avoid let_and_return
pattern, as it doesn't explain what the advantages are (performance, security, maintainability, etc)
Drawbacks
- Getting fewer points in code golf where the goal is to implement a given algorithm as quickly as possible and with as few words as possible
- A few extra words to read in the source code
- Upfront work to implement this lint for Clippy maintainers and for end-user to apply this lint.
Example
fn foo() -> String {
String::new()
}
Could be written as:
fn foo() -> String {
let x = String::new();
x
}