"Remove dead code" quick fix doesn't properly handle a dead body of an "if" statement #60536

stereotype441 · 2025-04-14T18:16:13Z

Starting with the following file:

main() {
  if (false) print('Should never happen');
  print('Should always happen');
}

Place the cursor over the statement print('Should never happen'); (which should be grayed out, since it is dead code). Select the "Remove dead code" quick fix.

Expected result: the whole if statement should be deleted, resulting in:

main() {
  print('Should always happen);
}

Observed result: just the dead statement is deleted, resulting in:

main() {
  if (false)
  print('Should always happen');
}

This is particularly unfortunate because it constitutes a behavioral change: after the dead code removal, print('Should always happen'); will not execute.

I think this issue is particularly important because when the sound flow analysis language feature is enabled, a lot of code will start getting flagged as dead code, for example:

f(int i) {
  if (i == null) throw StateError('Unexpected null');
  ...other code...
}

If we don't address this, and a user upgrades to a language version that includes the sound flow analysis feature, they might try to use "dart fix" to automatically clean up the resulting dead code, and get a really unexpected result.

The text was updated successfully, but these errors were encountered:

bwilkerson · 2025-04-14T18:37:32Z

I agree that this is a bug. Automated code changes generally shouldn't change the semantics of the code.

It's probably a result of a general policy to not remove code that might have an observable effect. That probably influenced the author to remove only the highlighted code. In the case of dead code in the then-body of an if, that could include the condition in the if statement. Maybe it's rare enough that we should make an exception here (where the dead code is the whole body of a larger statement). Or maybe we should do something different like replacing the dead code with {}.

Consider, for example:

void f(int i) {
  if (i == null)
    throw StateError('Unexpected null');
  else
    print(i);
}

We probably don't want to delete the whole if at that point. We could potentially invert the condition and remove the dead code and the following else keyword. Anyway, just another case to consider.

Fortunately, the 'remove dead code' fix can't be bulk applied, so that it won't be as bad as it otherwise could be.

stereotype441 · 2025-04-15T12:45:39Z

Consider, for example:

void f(int i) {
if (i == null)
throw StateError('Unexpected null');
else
print(i);
}
We probably don't want to delete the whole if at that point. We could potentially invert the condition and remove the dead code and the following else keyword. Anyway, just another case to consider.

Yeah, that's a good example. I would support inverting the condition in this case.

Fortunately, the 'remove dead code' fix can't be bulk applied, so that it won't be as bad as it otherwise could be.

I suppose, although for me that's rather unfortunate, becau 8000 se I was hoping to bulk apply this fix in order to prepare google3 for the rollout of sound-flow-analysis. I wonder how many other people will want to do the same to their codebases when sound-flow-analysis rolls out.

bwilkerson · 2025-04-15T14:41:35Z

The reason it isn't enabled for bulk application is because it isn't clearly the right thing to do in every case.

We have, at various points in the past, talked about providing support for specifying a specific fix for a specific diagnostic when running dart fix. That would presumably allow the user to select any fix, whether it was allowed for bulk fix by default or not. Unfortunately it hasn't been considered a high enough priority yet to be implemented.

stereotype441 added devexp-quick-fix Issues with analysis server (quick) fixes devexp-server Issues related to some aspect of the analysis server labels Apr 14, 2025

bwilkerson added P2 A bug or feature request we're likely to work on area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. labels Apr 14, 2025

stereotype441 self-assigned this Apr 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"Remove dead code" quick fix doesn't properly handle a dead body of an "if" statement #60536

"Remove dead code" quick fix doesn't properly handle a dead body of an "if" statement #60536

"Remove dead code" quick fix doesn't properly handle a dead body of an "if" statement #60536

"Remove dead code" quick fix doesn't properly handle a dead body of an "if" statement #60536

Comments