analyzer: Correct dead_code report range when syntax tree is rewritten.

The range at which to report dead code is controlled by DeadCodeVerifier's _firstDeadNode field. But nodes can be rewritten during resolution, in which case this AstNode gets lost. We just have to update it as needed. Fixes #55642 Change-Id: I57c341b64f90fbea0cf251d712eb8f18cb84b2d9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400800 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
dart-lang · Dec 14, 2024 · 47269eb · 47269eb
1 parent ae07ae6
commit 47269eb
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 3 deletions.
diff --git a/pkg/analyzer/lib/src/error/dead_code_verifier.dart b/pkg/analyzer/lib/src/error/dead_code_verifier.dart
@@ -337,6 +337,14 @@ class NullSafetyDeadCodeVerifier {
     _firstDeadNode = null;
   }
 
+  /// 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) {
+    if (_firstDeadNode == oldNode) {
+      _firstDeadNode = newNode;
+    }
+  }
+
   void tryStatementEnter(TryStatement node) {
     var verifier = _CatchClausesVerifier(
       _typeSystem,

diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -1389,6 +1389,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
     inferenceLogWriter?.recordExpressionRewrite(
         oldExpression: oldNode, newExpression: newNode);
     NodeReplacer.replace(oldNode, newNode, parent: parent);
+    nullSafetyDeadCodeVerifier.maybeRewriteFirstDeadNode(oldNode, newNode);
   }
 
   PatternResult<DartType> resolveAssignedVariablePattern({

diff --git a/pkg/analyzer/test/src/diagnostics/dead_code_test.dart b/pkg/analyzer/test/src/diagnostics/dead_code_test.dart
@@ -17,6 +17,16 @@ main() {
 @reflectiveTest
 class DeadCodeTest extends PubPackageResolutionTest
     with DeadCodeTestCases_Language212 {
+  test_deadBlock_conditionalElse_recordPropertyAccess() async {
+    await assertErrorsInCode(r'''
+void f(({int x, int y}) p) {
+  true ? p.x : p.y;
+}
+''', [
+      error(WarningCode.DEAD_CODE, 44, 3),
+    ]);
+  }
+
   test_deadOperandLHS_or_recordPropertyAccess() async {
     await assertErrorsInCode(r'''
 void f(({bool b, }) r) {
@@ -402,7 +412,7 @@ f() {
     ]);
   }
 
-  test_deadBlock_conditionalIf() async {
+  test_deadBlock_conditionalThen() async {
     await assertErrorsInCode(r'''
 f() {
   false ? 1 : 2;
@@ -412,7 +422,7 @@ f() {
     ]);
   }
 
-  test_deadBlock_conditionalIf_debugConst() async {
+  test_deadBlock_conditionalThen_debugConst() async {
     await assertNoErrorsInCode(r'''
 const bool DEBUG = false;
 f() {
@@ -421,7 +431,7 @@ f() {
 ''');
   }
 
-  test_deadBlock_conditionalIf_nested() async {
+  test_deadBlock_conditionalThen_nested() async {
     // Test that a dead then-statement can't generate additional violations.
     await assertErrorsInCode(r'''
 f() {