nonminimal_bool is very hard to read

[jyn514]

warning: this boolean expression can be simplified
  --> crates/ra_parser/src/grammar/items.rs:24:11
   |
24 |     while !p.at(EOF) && !(stop_on_r_curly && p.at(T!['}'])) {
   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `!(stop_on_r_curly && p.at(T!['}']) || p.at(EOF))`

b || a is not the same as !a && b.

[mati865]

I don't see what is wrong: https://docs.google.com/spreadsheets/d/1pSdvDJbmO9bJYtSNNAcWKyDY5Q6tSho16geeHE-0LQ0/edit?usp=sharing

[jyn514]

Oh I misread the change: I thought it was !(stop_on_r_curly && p.at(T!['}'])) || p.at(EOF) but it was actually !(stop_on_r_curly && p.at(T!['}']) || p.at(EOF)).

[JohnTitor]

Hm, indeed the suggestion is hard a bit to read but it depends on the complexity of the original code. What we should do here?

[jyn514]

A good start would be to keep the original order of the conditions: if it were !(p.at(EOF) || stop_on_r_curly && p.at(T!['}'])) I think I would have noticed the difference.

[jyn514]

That order also reads nicer to my eye, although it might just be because I'm familiar with the code.

[JohnTitor]

Yeah, I agree with not to change the ordering, at least this case.

[JohnTitor]

Heh, I found this comment btw:

rust-clippy/clippy_lints/src/booleans.rs

Lines 418 to 419 in 920cdb5

	// nonminimal_bool can produce minimal but
	// not human readable expressions (#3141)

[JohnTitor]

So, let's deal with the order problem only here, others should be tracked in #3141.