Report REFERENCED_BEFORE_DECLARATION for if statement/element, pattern variable declarations.

Change-Id: I844f186d504abf69b7524017ac9be27fdcef5ff6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272385
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/analyzer/lib/src/dart/ast/ast.dart

@@ -6548,7 +6548,8 @@ abstract class IdentifierImpl extends CommentReferableExpressionImpl
   bool get isAssignable => true;
 }
 
-class IfElementImpl extends CollectionElementImpl implements IfElement {
+class IfElementImpl extends CollectionElementImpl
+    implements IfElement, IfElementOrStatementImpl<CollectionElementImpl> {
   @override
   final Token ifKeyword;
 
@@ -6613,7 +6614,13 @@ class IfElementImpl extends CollectionElementImpl implements IfElement {
   Token get endToken => _elseElement?.endToken ?? _thenElement.endToken;
 
   @override
-  Expression get expression => _condition;
+  ExpressionImpl get expression => _condition;
+
+  @override
+  CollectionElementImpl? get ifFalse => elseElement;
+
+  @override
+  CollectionElementImpl get ifTrue => thenElement;
 
   @override
   CollectionElementImpl get thenElement => _thenElement;
@@ -6652,12 +6659,30 @@ class IfElementImpl extends CollectionElementImpl implements IfElement {
   }
 }
 
+abstract class IfElementOrStatementImpl<E extends AstNodeImpl>
+    implements AstNodeImpl {
+  /// Return the `case` clause used to match a pattern against the [expression].
+  CaseClauseImpl? get caseClause;
+
+  /// Return the expression used to either determine which of the statements is
+  /// executed next or to compute the value matched against the pattern in the
+  /// `case` clause.
+  ExpressionImpl get expression;
+
+  /// The node that is executed if the condition evaluates to `false`.
+  E? get ifFalse;
+
+  /// The node that is executed if the condition evaluates to `true`.
+  E get ifTrue;
+}
+
 /// An if statement.
 ///
 ///    ifStatement ::=
 ///        'if' '(' [Expression] [CaseClause]? ')'[Statement]
 ///        ('else' [Statement])?
-class IfStatementImpl extends StatementImpl implements IfStatement {
+class IfStatementImpl extends StatementImpl
+    implements IfStatement, IfElementOrStatementImpl<StatementImpl> {
   @override
   final Token ifKeyword;
 
@@ -6729,7 +6754,13 @@ class IfStatementImpl extends StatementImpl implements IfStatement {
   }
 
   @override
-  Expression get expression => _condition;
+  ExpressionImpl get expression => _condition;
+
+  @override
+  StatementImpl? get ifFalse => elseStatement;
+
+  @override
+  StatementImpl get ifTrue => thenStatement;
 
   @override
   StatementImpl get thenStatement => _thenStatement;

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

@@ -606,35 +606,13 @@ class _AssignedVariablesVisitor extends RecursiveAstVisitor<void> {
   }
 
   @override
-  void visitIfElement(IfElement node) {
-    node.condition.accept(this);
-    assignedVariables.beginNode();
-    node.thenElement.accept(this);
-    assignedVariables.endNode(node);
-    node.elseElement?.accept(this);
+  void visitIfElement(covariant IfElementImpl node) {
+    _visitIf(node);
   }
 
   @override
   void visitIfStatement(covariant IfStatementImpl node) {
-    node.condition.accept(this);
-
-    var caseClause = node.caseClause;
-    if (caseClause != null) {
-      var guardedPattern = caseClause.guardedPattern;
-      assignedVariables.beginNode();
-      for (var variable in guardedPattern.variables.values) {
-        assignedVariables.declare(variable);
-      }
-      guardedPattern.whenClause?.accept(this);
-      node.thenStatement.accept(this);
-      assignedVariables.endNode(node);
-      node.elseStatement?.accept(this);
-    } else {
-      assignedVariables.beginNode();
-      node.thenStatement.accept(this);
-      assignedVariables.endNode(node);
-      node.elseStatement?.accept(this);
-    }
+    _visitIf(node);
   }
 
   @override
@@ -642,6 +620,16 @@ class _AssignedVariablesVisitor extends RecursiveAstVisitor<void> {
     throw StateError('Should not visit top level declarations');
   }
 
+  @override
+  void visitPatternVariableDeclarationStatement(
+    covariant PatternVariableDeclarationStatementImpl node,
+  ) {
+    for (var variable in node.declaration.elements) {
+      assignedVariables.declare(variable);
+    }
+    super.visitPatternVariableDeclarationStatement(node);
+  }
+
   @override
   void visitPostfixExpression(PostfixExpression node) {
     super.visitPostfixExpression(node);
@@ -777,6 +765,28 @@ class _AssignedVariablesVisitor extends RecursiveAstVisitor<void> {
       throw StateError('Unrecognized for loop parts');
     }
   }
+
+  void _visitIf(IfElementOrStatementImpl node) {
+    node.expression.accept(this);
+
+    var caseClause = node.caseClause;
+    if (caseClause != null) {
+      var guardedPattern = caseClause.guardedPattern;
+      assignedVariables.beginNode();
+      for (var variable in guardedPattern.variables.values) {
+        assignedVariables.declare(variable);
+      }
+      guardedPattern.whenClause?.accept(this);
+      node.ifTrue.accept(this);
+      assignedVariables.endNode(node);
+      node.ifFalse?.accept(this);
+    } else {
+      assignedVariables.beginNode();
+      node.ifTrue.accept(this);
+      assignedVariables.endNode(node);
+      node.ifFalse?.accept(this);
+    }
+  }
 }
 
 /// The flow analysis based implementation of [LocalVariableTypeProvider].

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

@@ -800,18 +800,14 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     });
   }
 
+  @override
+  void visitIfElement(covariant IfElementImpl node) {
+    _visitIf(node);
+  }
+
   @override
   void visitIfStatement(covariant IfStatementImpl node) {
-    var caseClause = node.caseClause;
-    if (caseClause != null) {
-      node.expression.accept(this);
-      _resolveGuardedPattern(caseClause.guardedPattern, then: () {
-        node.thenStatement.accept(this);
-      });
-      node.elseStatement?.accept(this);
-    } else {
-      node.visitChildren(this);
-    }
+    _visitIf(node);
   }
 
   @override
@@ -1267,6 +1263,7 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
       _define(element);
       element.isFinal = node.keyword?.keyword == Keyword.FINAL;
       element.hasImplicitType = node.type == null;
+      element.type = node.type?.type ?? _dynamicType;
       node.declaredElement = element;
     }
   }
@@ -1543,6 +1540,19 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     }
   }
 
+  void _visitIf(IfElementOrStatementImpl node) {
+    var caseClause = node.caseClause;
+    if (caseClause != null) {
+      node.expression.accept(this);
+      _resolveGuardedPattern(caseClause.guardedPattern, then: () {
+        node.ifTrue.accept(this);
+      });
+      node.ifFalse?.accept(this);
+    } else {
+      node.visitChildren(this);
+    }
+  }
+
   /// Make the given [holder] be the current one while running [f].
   void _withElementHolder(ElementHolder holder, void Function() f) {
     var previousHolder = _elementHolder;

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

@@ -6,6 +6,7 @@ import 'dart:collection';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/generated/engine.dart';
@@ -23,7 +24,11 @@ class BlockScope {
       if (statement is LabeledStatement) {
         statement = statement.statement;
       }
-      if (statement is VariableDeclarationStatement) {
+      if (statement is PatternVariableDeclarationStatementImpl) {
+        for (var variable in statement.declaration.elements) {
+          yield variable;
+        }
+      } else if (statement is VariableDeclarationStatement) {
         NodeList<VariableDeclaration> variables = statement.variables.variables;
         int variableCount = variables.length;
         for (int j = 0; j < variableCount; j++) {

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

@@ -859,6 +859,14 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     super.visitGenericTypeAlias(node);
   }
 
+  @override
+  void visitGuardedPattern(covariant GuardedPatternImpl node) {
+    _withHiddenElementsGuardedPattern(node, () {
+      node.pattern.accept(this);
+    });
+    node.whenClause?.accept(this);
+  }
+
   @override
   void visitImplementsClause(ImplementsClause node) {
     node.interfaces.forEach(_checkForImplicitDynamicType);
@@ -1044,6 +1052,16 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     super.visitNativeFunctionBody(node);
   }
 
+  @override
+  void visitPatternVariableDeclarationStatement(
+    covariant PatternVariableDeclarationStatementImpl node,
+  ) {
+    super.visitPatternVariableDeclarationStatement(node);
+    for (var variable in node.declaration.elements) {
+      _hiddenElements?.declare(variable);
+    }
+  }
+
   @override
   void visitPostfixExpression(PostfixExpression node) {
     var operand = node.operand;
@@ -5271,6 +5289,17 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     }
   }
 
+  void _withHiddenElementsGuardedPattern(
+      GuardedPatternImpl guardedPattern, void Function() f) {
+    _hiddenElements =
+        HiddenElements.forGuardedPattern(_hiddenElements, guardedPattern);
+    try {
+      f();
+    } finally {
+      _hiddenElements = _hiddenElements!.outerElements;
+    }
+  }
+
   /// Return [FieldElement]s that are declared in the [ClassDeclaration] with
   /// the given [constructor], but are not initialized.
   static List<FieldElement> computeNotInitializedFields(
@@ -5327,6 +5356,16 @@ class HiddenElements {
     _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);
+  }
+
   /// Return `true` if this set of elements contains the given [element].
   bool contains(Element element) {
     if (_elements.contains(element)) {

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

@@ -4405,26 +4405,13 @@ class ScopeResolverVisitor extends UnifyingAstVisitor<void> {
   }
 
   @override
-  void visitIfStatement(covariant IfStatementImpl node) {
-    node.condition.accept(this);
+  void visitIfElement(covariant IfElementImpl node) {
+    _visitIf(node);
+  }
 
-    var caseClause = node.caseClause;
-    if (caseClause != null) {
-      var guardedPattern = caseClause.guardedPattern;
-      guardedPattern.pattern.accept(this);
-      _withNameScope(() {
-        var patternVariables = guardedPattern.variables;
-        for (var variable in patternVariables.values) {
-          _define(variable);
-        }
-        guardedPattern.whenClause?.accept(this);
-        visitStatementInScope(node.thenStatement);
-      });
-      visitStatementInScope(node.elseStatement);
-    } else {
-      visitStatementInScope(node.thenStatement);
-      visitStatementInScope(node.elseStatement);
-    }
+  @override
+  void visitIfStatement(covariant IfStatementImpl node) {
+    _visitIf(node);
   }
 
   @override
@@ -4719,6 +4706,27 @@ class ScopeResolverVisitor extends UnifyingAstVisitor<void> {
     }
   }
 
+  void _visitIf(IfElementOrStatementImpl node) {
+    node.expression.accept(this);
+
+    var caseClause = node.caseClause;
+    if (caseClause != null) {
+      var guardedPattern = caseClause.guardedPattern;
+      _withNameScope(() {
+        var patternVariables = guardedPattern.variables;
+        for (var variable in patternVariables.values) {
+          _define(variable);
+        }
+        guardedPattern.accept(this);
+        node.ifTrue.accept(this);
+      });
+      node.ifFalse?.accept(this);
+    } else {
+      node.ifTrue.accept(this);
+      node.ifFalse?.accept(this);
+    }
+  }
+
   void _withDeclaredLocals(
     AstNode node,
     List<Statement> statements,

pkg/analyzer/lib/src/test_utilities/find_node.dart

@@ -32,14 +32,22 @@ class FindNode {
     var nodes = <GuardedPattern>[];
     unit.accept(
       FunctionAstVisitor(
-        ifStatement: (node) {
-          var caseClause = node.caseClause;
-          if (caseClause != null) {
-            nodes.add(caseClause.guardedPattern);
-          }
+        guardedPattern: (node) {
+          nodes.add(node);
+        },
+      ),
+    );
+    return nodes.single;
+  }
+
+  /// Returns the [IfStatement], there must be only one.
+  IfStatement get singleIfStatement {
+    var nodes = <IfStatement>[];
+    unit.accept(
+      FunctionAstVisitor(
+        ifStatement: (node) {
+          nodes.add(node);
         },
-        switchPatternCase: (node) => nodes.add(node.guardedPattern),
-        switchExpressionCase: (node) => nodes.add(node.guardedPattern),
       ),
     );
     return nodes.single;

pkg/analyzer/lib/src/test_utilities/function_ast_visitor.dart

@@ -12,6 +12,7 @@ class FunctionAstVisitor extends RecursiveAstVisitor<void> {
   final void Function(FunctionDeclarationStatement)?
       functionDeclarationStatement;
   final void Function(FunctionExpression, bool)? functionExpression;
+  final void Function(GuardedPattern)? guardedPattern;
   final void Function(IfStatement)? ifStatement;
   final void Function(Label)? label;
   final void Function(MethodInvocation)? methodInvocation;
@@ -28,6 +29,7 @@ class FunctionAstVisitor extends RecursiveAstVisitor<void> {
     this.declaredIdentifier,
     this.functionDeclarationStatement,
     this.functionExpression,
+    this.guardedPattern,
     this.ifStatement,
     this.label,
     this.methodInvocation,
@@ -71,6 +73,12 @@ class FunctionAstVisitor extends RecursiveAstVisitor<void> {
     super.visitFunctionExpression(node);
   }
 
+  @override
+  void visitGuardedPattern(GuardedPattern node) {
+    guardedPattern?.call(node);
+    super.visitGuardedPattern(node);
+  }
+
   @override
   void visitIfStatement(IfStatement node) {
     ifStatement?.call(node);

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

@@ -2,18 +2,19 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analyzer/src/error/codes.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import 'context_collection_resolution.dart';
 
 main() {
   defineReflectiveSuite(() {
-    defineReflectiveTests(IfElementTest);
+    defineReflectiveTests(IfElementResolutionTest);
   });
 }
 
 @reflectiveTest
-class IfElementTest extends PatternsResolutionTest {
+class IfElementResolutionTest extends PatternsResolutionTest {
   test_caseClause() async {
     await assertNoErrorsInCode(r'''
 void f(Object x) {
@@ -48,6 +49,154 @@ IfElement
 ''');
   }
 
+  test_caseClause_variables_scope() async {
+    // Each `guardedPattern` introduces a new case scope which is where the
+    // variables defined by that case's pattern are bound.
+    // There is no initializing expression for the variables in a case pattern,
+    // but they are considered initialized after the entire case pattern,
+    // before the guard expression if there is one. However, all pattern
+    // variables are in scope in the entire pattern.
+    await assertErrorsInCode(r'''
+const a = 0;
+void f(Object x) {
+  [
+    if (x case [int a, == a] when a > 0)
+      a
+    else
+      a
+  ];
+}
+''', [
+      error(CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION, 62, 1,
+          contextMessages: [message('/home/test/lib/test.dart', 56, 1)]),
+    ]);
+
+    final node = findNode.ifElement('if');
+    assertResolvedNodeText(node, r'''
+IfElement
+  ifKeyword: if
+  leftParenthesis: (
+  condition: SimpleIdentifier
+    token: x
+    staticElement: self::@function::f::@parameter::x
+    staticType: Object
+  caseClause: CaseClause
+    caseKeyword: case
+    guardedPattern: GuardedPattern
+      pattern: ListPattern
+        leftBracket: [
+        elements
+          VariablePattern
+            type: NamedType
+              name: SimpleIdentifier
+                token: int
+                staticElement: dart:core::@class::int
+                staticType: null
+              type: int
+            name: a
+            declaredElement: a@56
+              type: int
+          RelationalPattern
+            operator: ==
+            operand: SimpleIdentifier
+              token: a
+              staticElement: a@56
+              staticType: int
+            element: dart:core::@class::Object::@method::==
+        rightBracket: ]
+        requiredType: List<Object?>
+      whenClause: WhenClause
+        whenKeyword: when
+        expression: BinaryExpression
+          leftOperand: SimpleIdentifier
+            token: a
+            staticElement: a@56
+            staticType: int
+          operator: >
+          rightOperand: IntegerLiteral
+            literal: 0
+            parameter: dart:core::@class::num::@method::>::@parameter::other
+            staticType: int
+          staticElement: dart:core::@class::num::@method::>
+          staticInvokeType: bool Function(num)
+          staticType: bool
+  rightParenthesis: )
+  thenElement: SimpleIdentifier
+    token: a
+    staticElement: a@56
+    staticType: int
+  elseKeyword: else
+  elseElement: SimpleIdentifier
+    token: a
+    staticElement: self::@getter::a
+    staticType: int
+''');
+  }
+
+  test_caseClause_variables_single() async {
+    await assertErrorsInCode(r'''
+void f(Object x) {
+  [
+    if (x case int a when a > 0)
+      a
+    else
+      a // error
+  ];
+}
+''', [
+      error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 79, 1),
+    ]);
+
+    final node = findNode.ifElement('if');
+    assertResolvedNodeText(node, r'''
+IfElement
+  ifKeyword: if
+  leftParenthesis: (
+  condition: SimpleIdentifier
+    token: x
+    staticElement: self::@function::f::@parameter::x
+    staticType: Object
+  caseClause: CaseClause
+    caseKeyword: case
+    guardedPattern: GuardedPattern
+      pattern: VariablePattern
+        type: NamedType
+          name: SimpleIdentifier
+            token: int
+            staticElement: dart:core::@class::int
+            staticType: null
+          type: int
+        name: a
+        declaredElement: a@42
+          type: int
+      whenClause: WhenClause
+        whenKeyword: when
+        expression: BinaryExpression
+          leftOperand: SimpleIdentifier
+            token: a
+            staticElement: a@42
+            staticType: int
+          operator: >
+          rightOperand: IntegerLiteral
+            literal: 0
+            parameter: dart:core::@class::num::@method::>::@parameter::other
+            staticType: int
+          staticElement: dart:core::@class::num::@method::>
+          staticInvokeType: bool Function(num)
+          staticType: bool
+  rightParenthesis: )
+  thenElement: SimpleIdentifier
+    token: a
+    staticElement: a@42
+    staticType: int
+  elseKeyword: else
+  elseElement: SimpleIdentifier
+    token: a
+    staticElement: <null>
+    staticType: dynamic
+''');
+  }
+
   test_rewrite_caseClause_pattern() async {
     await assertNoErrorsInCode(r'''
 void f(Object x, int Function() a) {

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

@@ -755,6 +755,101 @@ IfStatement
 ''');
   }
 
+  test_caseClause_variables_scope() async {
+    // Each `guardedPattern` introduces a new case scope which is where the
+    // variables defined by that case's pattern are bound.
+    // There is no initializing expression for the variables in a case pattern,
+    // but they are considered initialized after the entire case pattern,
+    // before the guard expression if there is one. However, all pattern
+    // variables are in scope in the entire pattern.
+    await assertErrorsInCode(r'''
+const a = 0;
+void f(Object? x) {
+  if (x case [int a, == a] when a > 0) {
+    a;
+  } else {
+    a;
+  }
+}
+''', [
+      error(CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION, 57, 1,
+          contextMessages: [message('/home/test/lib/test.dart', 51, 1)]),
+    ]);
+
+    final node = findNode.ifStatement('if');
+    assertResolvedNodeText(node, r'''
+IfStatement
+  ifKeyword: if
+  leftParenthesis: (
+  condition: SimpleIdentifier
+    token: x
+    staticElement: self::@function::f::@parameter::x
+    staticType: Object?
+  caseClause: CaseClause
+    caseKeyword: case
+    guardedPattern: GuardedPattern
+      pattern: ListPattern
+        leftBracket: [
+        elements
+          VariablePattern
+            type: NamedType
+              name: SimpleIdentifier
+                token: int
+                staticElement: dart:core::@class::int
+                staticType: null
+              type: int
+            name: a
+            declaredElement: a@51
+              type: int
+          RelationalPattern
+            operator: ==
+            operand: SimpleIdentifier
+              token: a
+              staticElement: a@51
+              staticType: int
+            element: dart:core::@class::Object::@method::==
+        rightBracket: ]
+        requiredType: List<Object?>
+      whenClause: WhenClause
+        whenKeyword: when
+        expression: BinaryExpression
+          leftOperand: SimpleIdentifier
+            token: a
+            staticElement: a@51
+            staticType: int
+          operator: >
+          rightOperand: IntegerLiteral
+            literal: 0
+            parameter: dart:core::@class::num::@method::>::@parameter::other
+            staticType: int
+          staticElement: dart:core::@class::num::@method::>
+          staticInvokeType: bool Function(num)
+          staticType: bool
+  rightParenthesis: )
+  thenStatement: Block
+    leftBracket: {
+    statements
+      ExpressionStatement
+        expression: SimpleIdentifier
+          token: a
+          staticElement: a@51
+          staticType: int
+        semicolon: ;
+    rightBracket: }
+  elseKeyword: else
+  elseStatement: Block
+    leftBracket: {
+    statements
+      ExpressionStatement
+        expression: SimpleIdentifier
+          token: a
+          staticElement: self::@getter::a
+          staticType: int
+        semicolon: ;
+    rightBracket: }
+''');
+  }
+
   test_caseClause_variables_single() async {
     await assertErrorsInCode(r'''
 void f(Object? x) {

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

@@ -15,6 +15,37 @@ main() {
 
 @reflectiveTest
 class ReferencedBeforeDeclarationTest extends PubPackageResolutionTest {
+  test_block_patternVariable_after() async {
+    await assertErrorsInCode(r'''
+var v = 0;
+void f() {
+  v;
+  var [var v] = [0];
+}
+''', [
+      error(CompileTimeErrorCode.REFERENCED_BEFORE_DECLARATION, 24, 1,
+          contextMessages: [message('/home/test/lib/test.dart', 38, 1)]),
+    ]);
+
+    var node = findNode.simple('v;');
+    assertResolvedNodeText(node, r'''
+SimpleIdentifier
+  token: v
+  staticElement: v@38
+  staticType: dynamic
+''');
+  }
+
+  test_block_patternVariable_before() async {
+    await assertNoErrorsInCode(r'''
+var v = 0;
+void f() {
+  var [var v] = [0];
+  v;
+}
+''');
+  }
+
   test_cascade_after_declaration() async {
     await assertNoErrorsInCode(r'''
 testRequestHandler() {}