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

[cargo fix] unit_arg removes calls to functions with side-effects #4741

Closed
soruh opened this issue Oct 27, 2019 · 3 comments · Fixed by #4455
Closed

[cargo fix] unit_arg removes calls to functions with side-effects #4741

soruh opened this issue Oct 27, 2019 · 3 comments · Fixed by #4455
Labels
A-documentation Area: Adding or improving documentation good-first-issue These issues are a good way to get started with Clippy I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@soruh
Copy link

soruh commented Oct 27, 2019

When running cargo fix --clippy -Z unstable-options the following happens:

fn function_with_side_effects_but_unit_return() {
    print("This is a side-effect!");
}

This

fn some_function() -> Result<()> {
    // ...
    Ok(function_with_side_effects_but_unit_return())
}

will be turned into this

fn some_function() -> Result<()> {
    // ...
    Ok(())
}

but it should either be turned into this

fn some_function() -> Result<()> {
    // ...
    function_with_side_effects_but_unit_return();
    Ok(())
}

or not be changed at all.

This behavior results from the unit_arg Lint, so that probably should be changed to no longer being MachineApplicable or it's "fixing" logic should be adjusted to produce the above result.

I don't know if the "Known problems" section is used for problems when using cargo fix, but I think while this is being looked into it might make sense to add this there.

Only tested on this version:

$ cargo clippy -V
clippy 0.0.212 (e8d5a9e 2019-10-22)
$ cargo -V
cargo 1.40.0-nightly (3a9abe3f0 2019-10-15)
$ rustc -V
rustc 1.40.0-nightly (4a8c5b20c 2019-10-23)
@soruh soruh changed the title [cargo ] unit_arg removes calls to functions with side-effects [cargo fix] unit_arg removes calls to functions with side-effects Oct 27, 2019
@flip1995
Copy link
Member

Fix is in #4455, but is awaiting review.

@soruh
Copy link
Author

soruh commented Oct 27, 2019

Ah sorry, I seem to have missed that.
Do you think It would make sense to note this under the "Known Problems" section of the doc, while your PR is being reviewed?

@flip1995
Copy link
Member

flip1995 commented Oct 27, 2019

Yeah, I think it would make sense to turn this into MaybeIncorrect and add a note to the documentation, until #4455 is reviewed. Also a note can be added to the lint, that the unit returning expression might need to be preserved.

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy A-documentation Area: Adding or improving documentation labels Oct 27, 2019
@phansch phansch added L-suggestion Lint: Improving, adding or fixing lint suggestions I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied labels Apr 26, 2020
@bors bors closed this as completed in 9fdcb13 May 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation good-first-issue These issues are a good way to get started with Clippy I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants