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

New Lint - Pointless mutation #13977

Open
kinnison opened this issue Jan 10, 2025 · 3 comments
Open

New Lint - Pointless mutation #13977

kinnison opened this issue Jan 10, 2025 · 3 comments
Labels
A-lint Area: New lints

Comments

@kinnison
Copy link
Contributor

What it does

The lint should detect situations in which newly created values are mutated and then immediately dropped during assignments.

Advantage

Would spot situations such as the example below and warn the user that it is not acting as the author intended.

Drawbacks

Deliberate discarding of values may end up being slightly less ergonomic in some corner cases.

Example

This example is possibly a bit wordy, but is a simplified example of something encountered during a code review at work:

    struct Thingy {
        foo: Option<(u32, u32)>,
    }

    let mut thingy = Thingy { foo: None };
    thingy.foo = Some((0, 1));

    // This operation is effectively useless (no side-effects etc.)
    thingy.foo.unwrap().1 = 2;

    // What intuitively we'd expected to pass
    assert_eq!(thingy.foo.unwrap().1, 2);

The lint is clearly meant to trigger on the thingy.foo.unwrap().1 = 2; line and probably ought to suggest:

Could be written as:

    thingy.foo.as_mut().unwrap().1 = 2;

Though the exact details of the suggestion would likely tie the lint to only working on replacing the innards of Options or similar.

@kinnison kinnison added the A-lint Area: New lints label Jan 10, 2025
@kinnison
Copy link
Contributor Author

Some extra thoughts:

If the thing being unwrapped wasn't Copy then we'd get a use-after-move error anyway, so this only seems to apply in cases where the assignment is into an lvalue which was copied.

@GuillaumeGomez
Copy link
Member

I think it's a valid case.

@kinnison
Copy link
Contributor Author

If someone could guide me into how I might go about building such a lint (eg. pointing me at anything which might be similar) I'm prepared to have a go at a solution myself; though it'd be my first time in the clippy codebase.

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

2 participants