lint/noUselessElse incorrectly flags valid else

[nstepien]

Environment information

playground

What happened?

This an overly simplified example to trigger the bug:

https://biomejs.dev/playground/?indentStyle=space&quoteStyle=single&trailingComma=none&code=ZgB1AG4AYwB0AGkAbwBuACAAdABlAHMAdAAoACkAIAB7AAoAIAAgAGwAZQB0ACAAdgBhAGwAdQBlADsACgAKACAAIABpAGYAIAAoAGEAKQAgAHsACgAgACAAIAAgAHYAYQBsAHUAZQAgAD0AIAAxADsACgAgACAAfQAgAGUAbABzAGUAIABpAGYAIAAoAGIAKQAgAHsACgAgACAAIAAgAHIAZQB0AHUAcgBuACAAMgA7AAoAIAAgAH0AIABlAGwAcwBlACAAewAKACAAIAAgACAAdgBhAGwAdQBlACAAPQAgADMAOwAKACAAIAB9AAoACgAgACAAcgBlAHQAdQByAG4AIAB2AGEAbAB1AGUAOwAKAH0ACgA%3D

Expected result

If I follow biome's suggestions with the above code, I'll end up with

function test() {
  let value;

  if (a) {
    value = 1;
  } else if (b) {
    return 2;
  }

  value = 3;

  return value;
}

which breaks the previous logic.

If the a condition is truthy, then the last else should not run, regardless of what the b condition/block does.

Code of Conduct

• I agree to follow Biome's Code of Conduct

[Conaclos]

Actually the intended suggestion is:

function test() {
  let value;

  if (a) {
    value = 1;
  } else {
    if (b) {
      return 2;
    }
    value = 3;
  }

  return value;
}

[nstepien]

The CLI does not make it clear then.

  ! This else clause can be omitted.
  
     6 │   } else if (b) {
     7 │     return 2;
   > 8 │   } else {
       │     ^^^^^^
   > 9 │     value = 3;
  > 10 │   }
       │   ^
    11 │ 
    12 │   return value;

  i This if statement uses an early breaking statement.

     4 │   if (a) {
     5 │     value = 1;
   > 6 │   } else if (b) {
       │          ^^^^^^^^
   > 7 │     return 2;
   > 8 │   } else {
   > 9 │     value = 3;
  > 10 │   }
       │   ^
    11 │ 
    12 │   return value;

Personally I'm 👎 on suggesting to ADD indentation/nested code, the else in this case if fine IMHO, the code flow remains vertical.

[Conaclos]

Personally I'm 👎 on suggesting to ADD indentation/nested code, the else in this case if fine IMHO, the code flow remains vertical.

I think you are right. When I implemented the rule I was wondering the same thing. I chose this behavior because this was the simplest one (we just have to check the previous branch, not all of them). Let's think a bit about that and change the rule to ignore this kind of code.

[yphoenix]

Came here to report the same bug. If there is a branch before that doesn't return / break / continue then the rule can't be applied.