Skip to content

Do the opposite of let_and_return #12512

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

Open
amab8901 opened this issue Mar 20, 2024 · 0 comments
Open

Do the opposite of let_and_return #12512

amab8901 opened this issue Mar 20, 2024 · 0 comments
Labels
A-lint Area: New lints

Comments

@amab8901
Copy link

amab8901 commented Mar 20, 2024

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!() or println!() 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!() and println!() 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
}
@amab8901 amab8901 added the A-lint Area: New lints label Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

1 participant