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] identity_conversion removes parenthesis with inner expression, that needs them. #4750

Closed
soruh opened this issue Oct 28, 2019 · 3 comments · Fixed by #5900
Closed
Labels
C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages 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 28, 2019

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

This

let int_c = i32::from(int_a + int_b) * 3;

will be turned into this

let int_c = int_a + int_b * 3;

, which is obviously incorrect.
It should instead be turned into this:

let int_c = (int_a + int_b) * 3;

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)
@flip1995 flip1995 added L-suggestion Lint: Improving, adding or fixing lint suggestions C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Oct 28, 2019
@phansch phansch added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Oct 28, 2019
@basil-cow
Copy link
Contributor

Is it ok to just always emit (...) regardless of whether they are needed?

@flip1995
Copy link
Member

There is the unused_parens lint, that would deal with this afterwards. But there already is

pub enum Sugg<'a> {
/// An expression that never needs parenthesis such as `1337` or `[0; 42]`.
NonParen(Cow<'a, str>),
/// An expression that does not fit in other variants.
MaybeParen(Cow<'a, str>),
/// A binary operator expression, including `as`-casts and explicit type
/// coercion.
BinOp(AssocOp, Cow<'a, str>),
}

which can be helpful on fixing this issue.

@basil-cow
Copy link
Contributor

Cool, I'll make a pr once clippy is buildable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages 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.

4 participants