CQ. Fix scoping of for-loop variables in initializers

Ensure loop-local variables introduced by `for` parts are bound
consistently during resolution, while still reporting reads that occur
before the declaration point.

The resolver now predeclares `ForPartsWithDeclarations` variables in the
loop’s local scope before traversing the initializer/condition/updaters.
This makes lexical lookup within `forLoopParts` resolve to the
loop-local element (even in `var x = x`), and allows later verification
to correctly report `REFERENCED_BEFORE_DECLARATION` instead of treating
the name as outer/undefined.

Also align foreach resolution by defining the declared loop variable
only after visiting the iterable expression, and update error
verification to hide not-yet-declared elements consistently for blocks,
switch members, and `for` parts via a generalized
`HiddenElements.forElements` helper.

Change-Id: Ifab327737bb033fe05f1d765196796924378ac5e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480120
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>

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

@@ -696,6 +696,16 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     });
   }
 
+  @override
+  void visitForEachPartsWithDeclaration(
+    covariant ForEachPartsWithDeclarationImpl node,
+  ) {
+    node.iterable.accept(this);
+    node.loopVariable.accept(this);
+    var fragment = node.loopVariable.declaredFragment!;
+    _define(fragment.element);
+  }
+
   @override
   void visitForEachPartsWithPattern(
     covariant ForEachPartsWithPatternImpl node,
@@ -717,9 +727,9 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
   }
 
   @override
-  void visitForPartsWithDeclarations(ForPartsWithDeclarations node) {
+  void visitForStatement(covariant ForStatementImpl node) {
     _withNameScope(() {
-      super.visitForPartsWithDeclarations(node);
+      super.visitForStatement(node);
     });
   }
 

pkg/analyzer/lib/src/generated/error_verifier.dart

@@ -420,7 +420,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
 
   @override
   void visitBlock(covariant BlockImpl node) {
-    _withHiddenElements(node.statements, () {
+    _withHiddenElementsForStatements(node.statements, () {
       _duplicateDefinitionVerifier.checkStatements(node.statements);
       super.visitBlock(node);
     });
@@ -984,6 +984,13 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     super.visitForEachPartsWithIdentifier(node);
   }
 
+  @override
+  void visitForElement(covariant ForElementImpl node) {
+    _withHiddenElementsForForParts(node.forLoopParts, () {
+      super.visitForElement(node);
+    });
+  }
+
   @override
   void visitFormalParameterList(covariant FormalParameterListImpl node) {
     _duplicateDefinitionVerifier.checkParameters(node);
@@ -1000,6 +1007,13 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     super.visitForPartsWithDeclarations(node);
   }
 
+  @override
+  void visitForStatement(covariant ForStatementImpl node) {
+    _withHiddenElementsForForParts(node.forLoopParts, () {
+      super.visitForStatement(node);
+    });
+  }
+
   @override
   void visitFunctionDeclaration(covariant FunctionDeclarationImpl node) {
     var fragment = node.declaredFragment!;
@@ -1724,7 +1738,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
 
   @override
   void visitSwitchCase(covariant SwitchCaseImpl node) {
-    _withHiddenElements(node.statements, () {
+    _withHiddenElementsForStatements(node.statements, () {
       _duplicateDefinitionVerifier.checkStatements(node.statements);
       super.visitSwitchCase(node);
     });
@@ -1732,7 +1746,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
 
   @override
   void visitSwitchDefault(covariant SwitchDefaultImpl node) {
-    _withHiddenElements(node.statements, () {
+    _withHiddenElementsForStatements(node.statements, () {
       _duplicateDefinitionVerifier.checkStatements(node.statements);
       super.visitSwitchDefault(node);
     });
@@ -1746,7 +1760,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
 
   @override
   void visitSwitchPatternCase(covariant SwitchPatternCaseImpl node) {
-    _withHiddenElements(node.statements, () {
+    _withHiddenElementsForStatements(node.statements, () {
       _duplicateDefinitionVerifier.checkStatements(node.statements);
       super.visitSwitchPatternCase(node);
     });
@@ -6908,28 +6922,59 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     }
   }
 
-  void _withHiddenElements(List<Statement> statements, void Function() f) {
-    _hiddenElements = HiddenElements(_hiddenElements, statements);
+  void _withHiddenElements(HiddenElements hiddenElements, void Function() f) {
+    var outerElements = _hiddenElements;
+    _hiddenElements = hiddenElements;
     try {
       f();
     } finally {
-      _hiddenElements = _hiddenElements!.outerElements;
+      _hiddenElements = outerElements;
     }
   }
 
+  void _withHiddenElementsForForParts(
+    ForLoopParts forLoopParts,
+    void Function() f,
+  ) {
+    if (forLoopParts is ForPartsWithDeclarations) {
+      _withHiddenElements(
+        HiddenElements.forElements(
+          _hiddenElements,
+          forLoopParts.variables.variables.map(
+            (variable) => variable.declaredFragment!.element,
+          ),
+        ),
+        f,
+      );
+    } else {
+      f();
+    }
+  }
+
+  void _withHiddenElementsForStatements(
+    List<Statement> statements,
+    void Function() f,
+  ) {
+    _withHiddenElements(
+      HiddenElements.forElements(
+        _hiddenElements,
+        BlockScope.elementsInStatements(statements),
+      ),
+      f,
+    );
+  }
+
   void _withHiddenElementsGuardedPattern(
     GuardedPatternImpl guardedPattern,
     void Function() f,
   ) {
-    _hiddenElements = HiddenElements.forGuardedPattern(
-      _hiddenElements,
-      guardedPattern,
+    _withHiddenElements(
+      HiddenElements.forElements(
+        _hiddenElements,
+        guardedPattern.variables.values,
+      ),
+      f,
     );
-    try {
-      f();
-    } finally {
-      _hiddenElements = _hiddenElements!.outerElements;
-    }
   }
 
   /// Executes [f] with [state] as the current [ThisContext].
@@ -6973,20 +7018,9 @@ class HiddenElements {
   final Set<Element> _elements = {};
 
   /// Initialize a newly created set of hidden elements to include all of the
-  /// elements defined in the set of [outerElements] and all of the elements
-  /// declared in the given [statements].
-  HiddenElements(this.outerElements, List<Statement> statements) {
-    _initializeElements(statements);
-  }
-
-  /// Initialize a newly created set of hidden elements to include all of the
-  /// elements defined in the set of [outerElements] and all of the elements
-  /// declared in the given [guardedPattern].
-  HiddenElements.forGuardedPattern(
-    this.outerElements,
-    GuardedPatternImpl guardedPattern,
-  ) {
-    _elements.addAll(guardedPattern.variables.values);
+  /// elements defined in [outerElements] and the given [elements].
+  HiddenElements.forElements(this.outerElements, Iterable<Element> elements) {
+    _elements.addAll(elements);
   }
 
   /// Return `true` if this set of elements contains the given [element].
@@ -7004,12 +7038,6 @@ class HiddenElements {
   void declare(Element element) {
     _elements.remove(element);
   }
-
-  /// Initialize the list of elements that are not yet declared to be all of the
-  /// elements declared somewhere in the given [statements].
-  void _initializeElements(List<Statement> statements) {
-    _elements.addAll(BlockScope.elementsInStatements(statements));
-  }
 }
 
 /// Information to pass from from the defining unit to augmentations.

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

@@ -5190,6 +5190,7 @@ class ScopeResolverVisitor extends UnifyingAstVisitor<void> {
     try {
       nameScope = LocalScope(nameScope);
       node.nameScope = nameScope;
+      _predeclareForPartsVariables(node.forLoopParts);
       node.forLoopParts.accept(this);
       node.body.accept(this);
     } finally {
@@ -5225,6 +5226,7 @@ class ScopeResolverVisitor extends UnifyingAstVisitor<void> {
       nameScope = LocalScope(nameScope);
       _implicitLabelScope = _implicitLabelScope.nest(node);
       node.nameScope = nameScope;
+      _predeclareForPartsVariables(node.forLoopParts);
       node.forLoopParts.accept(this);
       _visitStatementInScope(node.body);
     } finally {
@@ -5771,6 +5773,20 @@ class ScopeResolverVisitor extends UnifyingAstVisitor<void> {
     }
   }
 
+  /// Predeclare `for`-parts variables so lexical lookup during traversal of
+  /// `forLoopParts` (initializer, condition, updaters) binds to loop-local
+  /// elements.
+  ///
+  /// This is only about binding. Reads that occur before the declaration point
+  /// are still reported later by error verification.
+  void _predeclareForPartsVariables(ForLoopParts forLoopParts) {
+    if (forLoopParts is ForPartsWithDeclarations) {
+      for (var variable in forLoopParts.variables.variables) {
+        _define(variable.declaredFragment!.element);
+      }
+    }
+  }
+
   /// Visits a documentation comment with a [DocumentationCommentScope] that encloses the
   /// current [nameScope].
   void _visitDocumentationComment(CommentImpl? node) {

pkg/analyzer/test/src/dart/resolution/for_element_test.dart

@@ -1034,7 +1034,6 @@ SimpleIdentifier
 ''');
   }
 
-  @failingTest
   test_scope_variables_initializer_uses_outer_sameName() async {
     await assertErrorsInCode(
       r'''

pkg/analyzer/test/src/dart/resolution/for_statement_test.dart

@@ -2415,7 +2415,6 @@ ForStatement
 ''');
   }
 
-  @failingTest
   test_scope_variables_initializer_uses_outer_sameName() async {
     await assertErrorsInCode(
       r'''

pkg/analyzer/test/src/diagnostics/referenced_before_declaration_test.dart

@@ -67,6 +67,44 @@ main() {
 ''');
   }
 
+  test_forElement_forPartsWithDeclarations_initializer() async {
+    await assertErrorsInCode(
+      r'''
+void f() {
+  [for (var x = x;;) x];
+}
+''',
+      [
+        error(
+          diag.referencedBeforeDeclaration,
+          27,
+          1,
+          contextMessages: [message(testFile, 23, 1)],
+        ),
+      ],
+    );
+  }
+
+  test_forStatement_forPartsWithDeclarations_initializer() async {
+    await assertErrorsInCode(
+      r'''
+void f() {
+  for (var x = x;;) {
+    x;
+  }
+}
+''',
+      [
+        error(
+          diag.referencedBeforeDeclaration,
+          26,
+          1,
+          contextMessages: [message(testFile, 22, 1)],
+        ),
+      ],
+    );
+  }
+
   test_hideInBlock_comment() async {
     await assertErrorsInCode(
       r'''

pkg/analyzer/test/src/diagnostics/undefined_identifier_test.dart

@@ -278,19 +278,6 @@ f() {
     );
   }
 
-  test_forStatement_ForPartsWithDeclarations_initializer() async {
-    await assertErrorsInCode(
-      '''
-void f() {
-  for (var x = x;;) {
-    x;
-  }
-}
-''',
-      [error(diag.undefinedIdentifier, 26, 1)],
-    );
-  }
-
   test_forStatement_inBody() async {
     await assertNoErrorsInCode('''
 f() {

tests/language/loop/for_test.dart

@@ -88,5 +88,6 @@ void testShadowLocal() {
   List<int> x = [1, 2, 3];
   for (var x = x; ;) break;
   //           ^
+  // [analyzer] COMPILE_TIME_ERROR.REFERENCED_BEFORE_DECLARATION
   // [cfe] Local variable 'x' can't be referenced before it is declared.
 }