Join if statements with if case and boolean should add `when` or continue it

[FMorschel]

trafficstars

If you have the following code, in both inner if there is a Join if statement but when it does so, the case statements are broken.

if (x case int v) {
  if (x == 0) ;
}
if (x case int()) {
  if (x == 0) ;
}

I'd expect either this to know the value for the outer if (since it knows it is there) or to revert the changes if it detects this happens.

[dart-github-bot]

Summary: The issue reports that the "Join if statement" refactoring breaks case statements within nested if statements. The user expects the refactoring to either correctly handle the nested case statements or revert the changes to avoid breaking the code.

[FMorschel]

After some more thinking.

The current output if we join the first two:

if (x && x == 0) {
}

It would be fine if the output was:

if (x case int v when x == 0) {
}

[FMorschel]

A little more testing. If the first one already had a case, it removes it completely today.

if (x case int v when true) {
  if (x == 0) ;
}

Becomes:

if (x && x == 0) {
;    /// <--- should also remove this since it is useless (only happens when the inner `if` ends in `;`)
}

[tenhobi]

I finished implementation of this, will add PR soon. :)

[lrhn]

I think it is safe to convert

if (e case p when b) {
  if (b2) stmt
}

into

if (e case p when b && b2) stmt

(and if the original had no when clause, then just ... when b2).

If the inner if is also a case condition, that won't work, sadly.

Other than that it should follow the same rules for when it applies as combining two plain if conditions with &&.

(So: if inner is case expression, you cannot combine. If outer is, you combine into when clause. I don't see any extra complications.)

[tenhobi]

@lrhn i have it working when inner is not if-case. It's a good point that I should also check whether the inner is if-case and if so, don't assist. I will add test for this too. 👍 Btw. I think we can also merge if outer is normal if and inner is if-case, right?

if (a) {
  if (b case c when d) {} 
}

into

if (b case c when a && d) {}

[lrhn]

I think we can also merge if outer is normal if and inner is if-case, right?

Not in general. It changes the evaluation order. If the inner expression depends on promotion from the outer test, that test can't be moved to after the pattern.

[tenhobi]

You are right, I will update the implementation according to what you say. :)

[FMorschel]

One more thing I realised. @lrhn please say so if this should be a separate issue.

Could we add the option to join inner if with else:

if (a) {
} else {
  if (b) {
  } else if (c) {
  }
}

Could become:

if (a) {
} else if (b) {
} else if (c) {
}

Not sure if it makes sense for the current message we have for the assist, but anyway.

[bwilkerson]

I would make that a different assist with a different message.

[tenhobi]

This have been merged in https://dart-review.googlesource.com/c/sdk/+/390632, can you @FMorschel please close this issue? :)