Warn about unnecessary parentheses upon assignment

[milibopp]

Fixes #12350.

Parentheses around assignment statements such as

let mut a = (0);
a = (1);
a += (2);

are not necessary and therefore an unnecessary_parens warning is raised when
statements like this occur.

NOTE: In let declarations this does not work as intended. Is it possible that they do not count as assignment expressions (ExprAssign)? (edit: this is fixed by now)

Furthermore, there are some cases that I fixed in the rest of the code, where parentheses could potentially enhance readability. Compare these lines:

a = b == c;
a = (b == c);

Thus, after having worked on this I'm not entirely sure, whether we should go through with this patch or not. Probably a matter of debate. ;)

[huonw]

These lines could be

let version_ok = vec::raw::buf_as_slice(cvbuf, minsz,
                                        |buf0| buf0 == encoder::metadata_encoding_version);

[milibopp]

fixed

[alexcrichton]

When dealing with let foo = (bar) you'll have to look for StmtDecl which is a statement instead of an expression.

[milibopp]

Thanks for the advice. Let's see if the tests reveal more unnecessary parentheses that way.

[milibopp]

Done. There were actually a lot more cases of declarations violating the intended lint. The set of changes throughout the standard library should serve as examples of code to judge whether this PR should be accepted or not.

[alexcrichton]

Alas, good old cfg'd out blocks of code hitting the new warning.

[pnkfelix]

@alexcrichton is it worth discussing whether x = y == z; is actually preferable to x = (y == z);? Or do we just land this and see if people complain?

[milibopp]

I only saw one error in bors' logs and fixed it. Is this all or did bors just not go beyond that first one? Especially since some of the test runs were apparently aborted.

[pnkfelix]

@aepsil0n bors aborts the other runs when one fails, so yeah, you do not necessarily know if the fixes you added will actually fix other platforms.

[alexcrichton]

@pnkfelix I figured it was rare enough that we could see how people feel after this lands.

[milibopp]

I do not understand why compile-fail/lint-unnecessary-parens.rs actually fails here. It's supposed to do that, so why does the test suite consider this an error?

[huonw]

There's an extra warning being generated:

warning: variable `a` is assigned to, but never used, #[warn(unused_variable)] on by default'

(One of the rather-too-long-lines toward the end of the test log in the buildbot failure link starts with "error: unexpected compiler error or warning" and that contains the above message (this is the test runner saying that the compiler gave extra output it wasn't expecting).)

You could name the variable _a (leading underscore silences that warning) or add an #[allow(unused_variable)]; attribute to the test.

[milibopp]

I see, thanks for the explanation. Will fix asap.

EDIT: should work now

[alexcrichton]

(needs a rebase)

[milibopp]

Rebased and resolved a couple of conflicts.