[analyzer] Improve dead_code detection for for statements/elements.

Special care has to be taken when reporting dead code for `for`
statements and `for` elements, because the "updaters" part of a
for-loop appears before the body in source, but is executed after the
body. This means that if the end of the loop body is unreachable, then
a separate dead_code range might need to be generated to cover the
updaters.

Previously, this was accomplished by special-case logic in
`NullSafetyDeadCodeVerifier.flowEnd`: if the parent of
`_firstDeadNode` was a `ForStatement` (or a `Block` inside a
`ForStatement`), extra logic would be triggered to emit a dead_code
range to cover the updaters. This mostly worked, but it didn't handle
a `Block` inside a `Block` inside a `ForStatement`, and I couldn't
figure out a good way to generalize it to handle `ForElement`s.

The new approach is for the resolver to make two additional calls into
`NullSafetyDeadCodeVerifier`, just after visiting the "condition" part
of the loop and just before visiting the updaters. Combining together
information gathered at these two times, the
`NullSafetyDeadCodeVerifier` can determine whether or not it's
necessary to emit a separate dead_code range to cover the updaters.

Pre-existing tests in `dead_code_test.dart` still pass, and I've added
a bunch of tests of the new functionality.

These improvements pave the way for improvements to the analysis
server's "remove dead code" fix, which I want to do in order to
prepare for the rollout of sound flow analysis.

Bug: #60438
Change-Id: I6d2c43fd7c06adcd4de22d42144b033319e949a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424180
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>

Author: stereotype441

pkg/analyzer/lib/src/dart/resolver/for_resolver.dart

@@ -253,10 +253,16 @@ class ForResolver {
       );
     }
 
+    var deadCodeForPartsState =
+        _resolver.nullSafetyDeadCodeVerifier.for_conditionEnd();
     _resolver.flowAnalysis.for_bodyBegin(node, condition);
     visitBody();
 
     _resolver.flowAnalysis.flow?.for_updaterBegin();
+    _resolver.nullSafetyDeadCodeVerifier.for_updaterBegin(
+      forParts.updaters,
+      deadCodeForPartsState,
+    );
     for (var updater in forParts.updaters) {
       _resolver.analyzeExpression(updater, _resolver.operations.unknownType);
       _resolver.popRewrite();

pkg/analyzer/lib/src/error/dead_code_verifier.dart

@@ -25,6 +25,17 @@ typedef _CatchClausesVerifierReporter =
       List<Object> arguments,
     );
 
+/// State information captured by [NullSafetyDeadCodeVerifier.for_conditionEnd]
+/// for later use by [NullSafetyDeadCodeVerifier.for_updaterBegin].
+class DeadCodeForPartsState {
+  /// The value of [NullSafetyDeadCodeVerifier._firstDeadNode] at the time of
+  /// the call to [NullSafetyDeadCodeVerifier.for_conditionEnd]
+  final AstNode? _firstDeadNodeAsOfConditionEnd;
+
+  DeadCodeForPartsState._({required AstNode? firstDeadNodeAsOfConditionEnd})
+    : _firstDeadNodeAsOfConditionEnd = firstDeadNodeAsOfConditionEnd;
+}
+
 /// A visitor that finds dead code, other than unreachable code that is
 /// handled in [NullSafetyDeadCodeVerifier].
 class DeadCodeVerifier extends RecursiveAstVisitor<void> {
@@ -302,13 +313,6 @@ class NullSafetyDeadCodeVerifier {
         }
       } else if (parent is ForParts) {
         node = parent.updaters.last;
-      } else if (parent is ForStatement) {
-        _reportForUpdaters(parent);
-      } else if (parent is Block) {
-        var grandParent = parent.parent;
-        if (grandParent is ForStatement) {
-          _reportForUpdaters(grandParent);
-        }
       } else if (parent is BinaryExpression) {
         offset = parent.operator.offset;
         node = parent.rightOperand;
@@ -330,6 +334,46 @@ class NullSafetyDeadCodeVerifier {
     _firstDeadNode = null;
   }
 
+  /// Performs the necessary dead code analysis when reaching the end of the
+  /// `condition` part of [ForParts].
+  ///
+  /// Some state is returned, which should be passed to [for_updaterBegin] after
+  /// visiting the body of the loop.
+  DeadCodeForPartsState for_conditionEnd() {
+    // Capture the state of this class so that `for_updaterBegin` can use it to
+    // decide whether it's necessary to create an extra dead code report for the
+    // updaters.
+    return DeadCodeForPartsState._(
+      firstDeadNodeAsOfConditionEnd: _firstDeadNode,
+    );
+  }
+
+  /// Performs the necessary dead code analysis when reaching the beginning of
+  /// the `updaters` part of [ForParts].
+  ///
+  /// [state] should be the state returned by [for_conditionEnd].
+  void for_updaterBegin(
+    NodeListImpl<ExpressionImpl> updaters,
+    DeadCodeForPartsState state,
+  ) {
+    var isReachable = _flowAnalysis?.flow?.isReachable ?? true;
+    if (!isReachable && state._firstDeadNodeAsOfConditionEnd == null) {
+      // A dead code range started either at the beginning of the loop body or
+      // somewhere inside it, and so the updaters are dead. Since the updaters
+      // appear textually before the loop body, they need their own dead code
+      // warning.
+      var beginToken = updaters.beginToken;
+      var endToken = updaters.endToken;
+      if (beginToken != null && endToken != null) {
+        _errorReporter.atOffset(
+          offset: beginToken.offset,
+          length: endToken.end - beginToken.offset,
+          errorCode: WarningCode.DEAD_CODE,
+        );
+      }
+    }
+  }
+
   /// Rewites [_firstDeadNode] if it is equal to [oldNode], as [oldNode] is
   /// being rewritten into [newNode] in the syntax tree.
   void maybeRewriteFirstDeadNode(AstNode oldNode, AstNode newNode) {
@@ -428,22 +472,6 @@ class NullSafetyDeadCodeVerifier {
     return false;
   }
 
-  void _reportForUpdaters(ForStatement node) {
-    var forParts = node.forLoopParts;
-    if (forParts is ForParts) {
-      var updaters = forParts.updaters;
-      var beginToken = updaters.beginToken;
-      var endToken = updaters.endToken;
-      if (beginToken != null && endToken != null) {
-        _errorReporter.atOffset(
-          offset: beginToken.offset,
-          length: endToken.end - beginToken.offset,
-          errorCode: WarningCode.DEAD_CODE,
-        );
-      }
-    }
-  }
-
   void _verifyUnassignedSimpleIdentifier(
     AstNode node,
     Expression? target,