Report an error when a variable pattern binds in one branch of logical-or, but not in another.

Only check for if-case is implements for now.

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

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -679,6 +679,8 @@ CompileTimeErrorCode.MISSING_DEFAULT_VALUE_FOR_PARAMETER_WITH_ANNOTATION:
   status: hasFix
 CompileTimeErrorCode.MISSING_REQUIRED_ARGUMENT:
   status: hasFix
+CompileTimeErrorCode.MISSING_VARIABLE_PATTERN:
+  status: needsEvaluation
 CompileTimeErrorCode.MIXIN_APPLICATION_CONCRETE_SUPER_INVOKED_MEMBER_TYPE:
   status: needsEvaluation
 CompileTimeErrorCode.MIXIN_APPLICATION_NO_CONCRETE_SUPER_INVOKED_MEMBER:

pkg/analyzer/lib/error/error.dart

@@ -306,6 +306,7 @@ const List<ErrorCode> errorCodeValues = [
   CompileTimeErrorCode.MISSING_DEFAULT_VALUE_FOR_PARAMETER_POSITIONAL,
   CompileTimeErrorCode.MISSING_DEFAULT_VALUE_FOR_PARAMETER_WITH_ANNOTATION,
   CompileTimeErrorCode.MISSING_REQUIRED_ARGUMENT,
+  CompileTimeErrorCode.MISSING_VARIABLE_PATTERN,
   CompileTimeErrorCode.MIXIN_APPLICATION_CONCRETE_SUPER_INVOKED_MEMBER_TYPE,
   CompileTimeErrorCode.MIXIN_APPLICATION_NO_CONCRETE_SUPER_INVOKED_MEMBER,
   CompileTimeErrorCode.MIXIN_APPLICATION_NOT_IMPLEMENTED_INTERFACE,

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

@@ -2,6 +2,7 @@
 // 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:_fe_analyzer_shared/src/type_inference/variable_bindings.dart';
 import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ -89,6 +90,8 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
   /// enclosing element.
   ElementHolder _elementHolder;
 
+  _PatternContext? _patternContext;
+
   factory ResolutionVisitor({
     required CompilationUnitElementImpl unitElement,
     required AnalysisErrorListener errorListener,
@@ -171,6 +174,26 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     });
   }
 
+  @override
+  void visitBinaryPattern(covariant BinaryPatternImpl node) {
+    final isOr = node.operator.type == TokenType.BAR;
+    final binder = _patternContext!.binder;
+    if (isOr) {
+      binder.startAlternatives();
+      binder.startAlternative(node.leftOperand);
+    }
+    node.leftOperand.accept(this);
+    if (isOr) {
+      binder.finishAlternative();
+      binder.startAlternative(node.rightOperand);
+    }
+    node.rightOperand.accept(this);
+    if (isOr) {
+      binder.finishAlternative();
+      binder.finishAlternatives();
+    }
+  }
+
   @override
   void visitBlock(Block node) {
     var outerScope = _nameScope;
@@ -185,6 +208,13 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     }
   }
 
+  @override
+  void visitCaseClause(CaseClause node) {
+    _withPatternContext(() {
+      super.visitCaseClause(node);
+    });
+  }
+
   @override
   void visitCatchClause(covariant CatchClauseImpl node) {
     var exceptionTypeNode = node.exceptionType;
@@ -1103,6 +1133,13 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     });
   }
 
+  @override
+  void visitSwitchPatternCase(SwitchPatternCase node) {
+    _withPatternContext(() {
+      super.visitSwitchPatternCase(node);
+    });
+  }
+
   @override
   void visitTypeParameter(TypeParameter node) {
     var element = node.declaredElement as TypeParameterElementImpl;
@@ -1182,15 +1219,23 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
   void visitVariablePattern(covariant VariablePatternImpl node) {
     node.type?.accept(this);
 
-    if (node.name.lexeme != '_') {
-      final element = VariablePatternElementImpl(
-        node.name.lexeme,
-        node.name.offset,
-      );
+    final name = node.name.lexeme;
+    if (name != '_') {
+      final patternContext = _patternContext!;
+      var element = patternContext.variables[name];
+      if (element == null) {
+        element = VariablePatternElementImpl(
+          name,
+          node.name.offset,
+        );
+        patternContext.variables[name] = element;
+        _elementHolder.enclose(element);
+        _define(element);
+        element.isFinal = node.keyword?.keyword == Keyword.FINAL;
+        element.hasImplicitType = node.type == null;
+      }
       node.declaredElement = element;
-      _elementHolder.enclose(element);
-      element.isFinal = node.keyword?.keyword == Keyword.FINAL;
-      element.hasImplicitType = node.type == null;
+      patternContext.binder.add(node, element);
     }
   }
 
@@ -1476,6 +1521,19 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     }
   }
 
+  /// Run [f] with a new pattern variable binder.
+  void _withPatternContext(void Function() f) {
+    final saved = _patternContext;
+    try {
+      final patternContext = _PatternContext(this);
+      _patternContext = patternContext;
+      f();
+      patternContext.binder.finish();
+    } finally {
+      _patternContext = saved;
+    }
+  }
+
   /// We always build local elements for [VariableDeclarationStatement]s and
   /// [FunctionDeclarationStatement]s in blocks, because invalid code might try
   /// to use forward references.
@@ -1500,3 +1558,47 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     }
   }
 }
+
+class _PatternContext
+    implements
+        VariableBindingCallbacks<DartPatternImpl, VariablePatternElementImpl,
+            DartType>,
+        VariableBinderErrors<DartPatternImpl, VariablePatternElementImpl> {
+  final ResolutionVisitor visitor;
+  final Map<String, VariablePatternElementImpl> variables = {};
+  late final VariableBinder<DartPatternImpl, VariablePatternElementImpl,
+      DartType> binder = VariableBinder(this);
+
+  _PatternContext(this.visitor);
+
+  @override
+  VariableBinderErrors<DartPatternImpl, VariablePatternElementImpl>
+      get errors => this;
+
+  @override
+  void assertInErrorRecovery() {
+    // TODO: implement assertInErrorRecovery
+    throw UnimplementedError();
+  }
+
+  @override
+  void matchVarOverlap({
+    required DartPatternImpl pattern,
+    required DartPatternImpl previousPattern,
+  }) {
+    // TODO: implement matchVarOverlap
+    throw UnimplementedError();
+  }
+
+  @override
+  void missingMatchVar(
+    DartPatternImpl alternative,
+    VariablePatternElementImpl variable,
+  ) {
+    visitor._errorReporter.reportErrorForNode(
+      CompileTimeErrorCode.MISSING_VARIABLE_PATTERN,
+      alternative,
+      [variable.name],
+    );
+  }
+}

pkg/analyzer/lib/src/error/codes.g.dart

@@ -2725,6 +2725,16 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
     hasPublishedDocs: true,
   );
 
+  ///  Parameters:
+  ///  0: the name of the variable pattern
+  static const CompileTimeErrorCode MISSING_VARIABLE_PATTERN =
+      CompileTimeErrorCode(
+    'MISSING_VARIABLE_PATTERN',
+    "Variable pattern '{0}' is missing in this branch of the logical-or "
+        "pattern.",
+    correctionMessage: "Try declaring this variable pattern in the branch.",
+  );
+
   ///  Parameters:
   ///  0: the name of the class that appears in both "extends" and "with" clauses
   static const CompileTimeErrorCode MIXINS_SUPER_CLASS = CompileTimeErrorCode(

pkg/analyzer/messages.yaml

@@ -8373,6 +8373,12 @@ CompileTimeErrorCode:
         f(3, end: 5);
       }
       ```
+  MISSING_VARIABLE_PATTERN:
+    problemMessage: "Variable pattern '{0}' is missing in this branch of the logical-or pattern."
+    correctionMessage: "Try declaring this variable pattern in the branch."
+    comment: |-
+      Parameters:
+      0: the name of the variable pattern
   MIXINS_SUPER_CLASS:
     sharedName: IMPLEMENTS_SUPER_CLASS
     problemMessage: "'{0}' can't be used in both the 'extends' and 'with' clauses."

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

@@ -2,6 +2,7 @@
 // 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';
@@ -583,14 +584,16 @@ BinaryPattern
   }
 
   test_typed_named_as_inside_logicalOr_left() async {
-    await assertNoErrorsInCode(r'''
+    await assertErrorsInCode(r'''
 void f(x) {
   switch (x) {
     case int as | 2:
       break;
   }
 }
-''');
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 45, 1),
+    ]);
     final node = findNode.switchPatternCase('case').pattern;
     assertResolvedNodeText(node, r'''
 BinaryPattern
@@ -613,14 +616,16 @@ BinaryPattern
   }
 
   test_typed_named_as_inside_logicalOr_right() async {
-    await assertNoErrorsInCode(r'''
+    await assertErrorsInCode(r'''
 void f(x) {
   switch (x) {
     case 1 | int as:
       break;
   }
 }
-''');
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 36, 1),
+    ]);
     final node = findNode.switchPatternCase('case').pattern;
     assertResolvedNodeText(node, r'''
 BinaryPattern

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

@@ -0,0 +1,325 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// 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 '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(MissingVariablePatternTest);
+  });
+}
+
+@reflectiveTest
+class MissingVariablePatternTest extends PatternsResolutionTest {
+  test_ifCase_logicalOr2_both_direct() async {
+    await assertNoErrorsInCode(r'''
+void f(int x) {
+  if (x case final a | final a) {}
+}
+''');
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: hasImplicitType isFinal a@35
+      type: int
+  operator: |
+  rightOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: a@35
+''');
+  }
+
+  test_ifCase_logicalOr2_both_nested_logicalAnd() async {
+    await assertNoErrorsInCode(r'''
+void f(int x) {
+  if (x case 0 & final a | final a) {}
+}
+''');
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: ConstantPattern
+      expression: IntegerLiteral
+        literal: 0
+        staticType: int
+    operator: &
+    rightOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: hasImplicitType isFinal a@39
+        type: int
+  operator: |
+  rightOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: a@39
+''');
+  }
+
+  test_ifCase_logicalOr2_left() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case final a | 2) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 39, 1),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: hasImplicitType isFinal a@35
+      type: int
+  operator: |
+  rightOperand: ConstantPattern
+    expression: IntegerLiteral
+      literal: 2
+      staticType: int
+''');
+  }
+
+  test_ifCase_logicalOr2_right() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case 1 | final a) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 29, 1),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: ConstantPattern
+    expression: IntegerLiteral
+      literal: 1
+      staticType: int
+  operator: |
+  rightOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: hasImplicitType isFinal a@39
+      type: int
+''');
+  }
+
+  test_ifCase_logicalOr3_1() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case final a | 2 | 3) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 39, 1),
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 43, 1),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: hasImplicitType isFinal a@35
+        type: int
+    operator: |
+    rightOperand: ConstantPattern
+      expression: IntegerLiteral
+        literal: 2
+        staticType: int
+  operator: |
+  rightOperand: ConstantPattern
+    expression: IntegerLiteral
+      literal: 3
+      staticType: int
+''');
+  }
+
+  test_ifCase_logicalOr3_12() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case final a | final a | 3) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 49, 1),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: hasImplicitType isFinal a@35
+        type: int
+    operator: |
+    rightOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: a@35
+  operator: |
+  rightOperand: ConstantPattern
+    expression: IntegerLiteral
+      literal: 3
+      staticType: int
+''');
+  }
+
+  test_ifCase_logicalOr3_123() async {
+    await assertNoErrorsInCode(r'''
+void f(int x) {
+  if (x case final a | final a | final a) {}
+}
+''');
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: hasImplicitType isFinal a@35
+        type: int
+    operator: |
+    rightOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: a@35
+  operator: |
+  rightOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: a@35
+''');
+  }
+
+  test_ifCase_logicalOr3_13() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case final a | 2 | final a) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 39, 1),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: hasImplicitType isFinal a@35
+        type: int
+    operator: |
+    rightOperand: ConstantPattern
+      expression: IntegerLiteral
+        literal: 2
+        staticType: int
+  operator: |
+  rightOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: a@35
+''');
+  }
+
+  test_ifCase_logicalOr3_2() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case 1 | final a | 3) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 29, 1),
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 43, 1),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: ConstantPattern
+      expression: IntegerLiteral
+        literal: 1
+        staticType: int
+    operator: |
+    rightOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: hasImplicitType isFinal a@39
+        type: int
+  operator: |
+  rightOperand: ConstantPattern
+    expression: IntegerLiteral
+      literal: 3
+      staticType: int
+''');
+  }
+
+  test_ifCase_logicalOr3_23() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case 1 | final a | final a) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 29, 1),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: ConstantPattern
+      expression: IntegerLiteral
+        literal: 1
+        staticType: int
+    operator: |
+    rightOperand: VariablePattern
+      keyword: final
+      name: a
+      declaredElement: hasImplicitType isFinal a@39
+        type: int
+  operator: |
+  rightOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: a@39
+''');
+  }
+
+  test_ifCase_logicalOr3_3() async {
+    await assertErrorsInCode(r'''
+void f(int x) {
+  if (x case 1 | 2 | final a) {}
+}
+''', [
+      error(CompileTimeErrorCode.MISSING_VARIABLE_PATTERN, 29, 5),
+    ]);
+    final node = findNode.caseClause('case').pattern;
+    assertResolvedNodeText(node, r'''
+BinaryPattern
+  leftOperand: BinaryPattern
+    leftOperand: ConstantPattern
+      expression: IntegerLiteral
+        literal: 1
+        staticType: int
+    operator: |
+    rightOperand: ConstantPattern
+      expression: IntegerLiteral
+        literal: 2
+        staticType: int
+  operator: |
+  rightOperand: VariablePattern
+    keyword: final
+    name: a
+    declaredElement: hasImplicitType isFinal a@43
+      type: int
+''');
+  }
+}

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

@@ -452,6 +452,7 @@ import 'missing_required_param_test.dart' as missing_required_param;
 import 'missing_return_test.dart' as missing_return;
 import 'missing_size_annotation_carray_test.dart'
     as missing_size_annotation_carray;
+import 'missing_variable_pattern_test.dart' as missing_variable_pattern;
 import 'mixin_application_concrete_super_invoked_member_type_test.dart'
     as mixin_application_concrete_super_invoked_member_type;
 import 'mixin_application_no_concrete_super_invoked_member_test.dart'
@@ -1099,6 +1100,7 @@ main() {
     missing_required_param.main();
     missing_return.main();
     missing_size_annotation_carray.main();
+    missing_variable_pattern.main();
     mixin_application_concrete_super_invoked_member_type.main();
     mixin_application_no_concrete_super_invoked_member.main();
     mixin_application_not_implemented_interface.main();

pkg/analyzer/test/src/summary/resolved_ast_printer.dart

@@ -1410,12 +1410,16 @@ class ResolvedAstPrinter extends ThrowingAstVisitor<void> {
           element as VariablePatternElementImpl;
           _sink.write(_indent);
           _sink.write('declaredElement: ');
-          _writeIf(element.hasImplicitType, 'hasImplicitType ');
-          _writeIf(element.isFinal, 'isFinal ');
-          _sink.writeln('${element.name}@${element.nameOffset}');
-          _withIndent(() {
-            _writeType('type', element.type);
-          });
+          if (node.name.offset == element.nameOffset) {
+            _writeIf(element.hasImplicitType, 'hasImplicitType ');
+            _writeIf(element.isFinal, 'isFinal ');
+            _sink.writeln('${element.name}@${element.nameOffset}');
+            _withIndent(() {
+              _writeType('type', element.type);
+            });
+          } else {
+            _sink.writeln('${element.name}@${element.nameOffset}');
+          }
         }
       }
     });