warning lint on chained assignment

[hellow554]

(This is a first impl on https://users.rust-lang.org/t/warn-about-using-the-value-of-an-assignment-expression/31324, thanks to @sourcefrog )

Code like a = b = 3 is a "common" pattern in C, python and other languages. People who are coming to Rust may be surprised to see, that it doesn't work the intended way. There's no warning whatsoever, just a clippy lint:

warning: this let-binding has unit value
 --> src/main.rs:2:5
  |
2 |     let a;
  |     ^^^^^^

I managed to implement this:

warning: chaining assignment is not supported in Rust
 --> a.rs:4:5
  |
4 |     a = b = 3;
  |     ^^^^^^^^^
  |
  = note: `#[warn(chained_assignment)]` on by default

But I have a few questions:
What does the user actually want?
Does he want
1 a = 3; b = 3
2 a = b == 3

The first one only works if the value is Copy for obvious reasons.
The second one only works if the value implements PartialEq.

I would like to see this in the Rust compiler, what do you think? What is missing? Am I overlooking something? Is everything covered? Would it brake any existing code?

[hellow554]

Also I have a implemention issue right now:

warning: chaining assignment is not supported in Rust
  --> a.rs:11:5
   |
11 |     a = b = c = 3;
   |     ^^^^^^^^^^^^^

warning: chaining assignment is not supported in Rust
  --> a.rs:11:9
   |
11 |     a = b = c = 3;
   |         ^^^^^^^^^

How do I prevent the linter from visiting the expression again? The code is straight forwards ATM:

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ChainedAssignment {
    fn check_expr(&mut self, cx: &LateContext<'_, '_>, e: &hir::Expr) {
        if let hir::ExprKind::Assign(_, ref right) = e.node {
            if let hir::ExprKind::Assign(..) = right.node {
                let msg = "chaining assignment is not supported in Rust";
                let cs = cx.tcx.sess.source_map().def_span(e.span);
                let mut err = cx.struct_span_lint(CHAINED_ASSIGNMENT, cs, msg);
                err.emit();
            }    
        }    
    }    
}

[hellow554]

@rustbot modify labels: A-diagnostics A-suggestion-diagnostics T-compiler C-enhancement

[Centril]

If you write:

fn _foo() {
    let _z = 1;
    let mut _y = 2;
    let x = _y = _z; //~ WARN unused variable: `x`
}

you see a warning.

If on the other hand you actually use x at type which isn't () or ?T you will find out that the type is () through a type error. In this case the user might be confused as to why the type is ().

Lints in the compiler have a cost in terms of compile times, so we need to ensure that they are useful.
So my question: Is there a sizeable number of users who actually hit the latter and have been confused?

[estebank]

If on the other hand you actually use x at type which isn't () or ?T you will find out that the type is () through a type error. In this case the user might be confused as to why the type is ().

That would be a good extension of E0308 to explain what's happening in that case.

[sourcefrog]

Thanks for drafting this @hellow554 !

What does the user actually want? Does he want
1 a = 3; b = 3
2 a = b == 3

The first one only works if the value is Copy for obvious reasons.
The second one only works if the value implements PartialEq.

I can imagine people potentially wanting either of these, although my guess would be the first is more of a mistake due to not knowing this detail of the language, whereas the second might be a simple typo. I feel I don't often assign the boolean result of a comparison to a variable very often.

re #63556 (comment),
In my humble opinion it's not a problem that a = b = c = 3 gives you two warnings. It has two instances of the same bug. In general rustc doesn't seem to suppress multiple similar warnings originating from the same line (or does it?)

So my question: Is there a sizeable number of users who actually hit the latter and have been confused?

I'm not sure how you would measure the number of people who every got confused. You can find various web pages where people seem to be asking about it, such as https://stackoverflow.com/q/38810973/243712 and https://www.reddit.com/r/rust/comments/3lplt4/declaring_multiple_variables_at_once/.

None of them are the most-upvoted questions about Rust, so perhaps not that many people try it or people who try it figure it out.

It is a difference in Rust versus other languages people might previously have used including C, C++, Python, and JS, so it seems worth giving a specific message if it's cheap enough for the compiler.

[estebank]

In general rustc doesn't seem to suppress multiple similar warnings originating from the same line (or does it?)

It does in some cases. It is one of my current focuses: removing redundant errors. An old example of this is:

error: 4 positional arguments in format string, but no arguments were given
 --> src/main.rs:2:15
  |
2 |     println!("{} {} {} {}");
  |               ^^ ^^ ^^ ^^

But this is a nice to have gradual improvement and should not block other improvements/features.