[cfe] Improve error message on incomplete joint pattern variables

Closes #51604
Closes #51962

Part of #49749

Change-Id: I9ada7a3a0194964af5c9e2afea1c2ea3ead67641
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292890
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>

Author: chloestefantsova

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -7519,6 +7519,62 @@ Message _withArgumentsInvokeNonFunction(String name) {
       arguments: {'name': name});
 }
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        String
+            name)> templateJointPatternVariableNotInAll = const Template<
+        Message Function(String name)>(
+    problemMessageTemplate:
+        r"""The variable '#name' is available in some, but not all cases that share this body.""",
+    withArguments: _withArgumentsJointPatternVariableNotInAll);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String name)> codeJointPatternVariableNotInAll =
+    const Code<Message Function(String name)>("JointPatternVariableNotInAll",
+        analyzerCodes: <String>[
+      "INVALID_PATTERN_VARIABLE_IN_SHARED_CASE_SCOPE"
+    ]);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsJointPatternVariableNotInAll(String name) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  return new Message(codeJointPatternVariableNotInAll,
+      problemMessage:
+          """The variable '${name}' is available in some, but not all cases that share this body.""",
+      arguments: {'name': name});
+}
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        String
+            name)> templateJointPatternVariableWithLabelDefault = const Template<
+        Message Function(String name)>(
+    problemMessageTemplate:
+        r"""The variable '#name' is not available because there is a label or 'default' case.""",
+    withArguments: _withArgumentsJointPatternVariableWithLabelDefault);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String name)>
+    codeJointPatternVariableWithLabelDefault =
+    const Code<Message Function(String name)>(
+        "JointPatternVariableWithLabelDefault",
+        analyzerCodes: <String>[
+      "INVALID_PATTERN_VARIABLE_IN_SHARED_CASE_SCOPE"
+    ]);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsJointPatternVariableWithLabelDefault(String name) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  return new Message(codeJointPatternVariableWithLabelDefault,
+      problemMessage:
+          """The variable '${name}' is not available because there is a label or 'default' case.""",
+      arguments: {'name': name});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<
     Message Function(

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

@@ -7836,10 +7836,12 @@ class BodyBuilder extends StackListenerImpl
 
     List<VariableDeclaration>? jointPatternVariables;
     List<VariableDeclaration>? jointPatternVariablesWithMismatchingFinality;
+    List<VariableDeclaration>? jointPatternVariablesNotInAll;
     enterLocalScope(switchCaseScope!);
     if (expressionCount > 1) {
-      for (ExpressionOrPatternGuardCase expressionOrPattern
-          in expressionOrPatterns) {
+      for (int i = 0; i < expressionOrPatterns.length; i++) {
+        ExpressionOrPatternGuardCase expressionOrPattern =
+            expressionOrPatterns[i];
         PatternGuard? patternGuard = expressionOrPattern.patternGuard;
         if (patternGuard != null) {
           if (jointPatternVariables == null) {
@@ -7850,6 +7852,12 @@ class BodyBuilder extends StackListenerImpl
                     variable.fileOffset, variable.name!)
                   ..isFinal = variable.isFinal
             ];
+            if (i != 0) {
+              // The previous heads were non-pattern ones, so no variables can
+              // be joined.
+              (jointPatternVariablesNotInAll ??= [])
+                  .addAll(jointPatternVariables);
+            }
           } else {
             Map<String, VariableDeclaration> patternVariablesByName = {
               for (VariableDeclaration variable
@@ -7859,19 +7867,34 @@ class BodyBuilder extends StackListenerImpl
             for (VariableDeclaration jointVariable in jointPatternVariables) {
               String jointVariableName = jointVariable.name!;
               VariableDeclaration? patternVariable =
-                  patternVariablesByName[jointVariableName];
+                  patternVariablesByName.remove(jointVariableName);
               if (patternVariable != null) {
                 if (patternVariable.isFinal != jointVariable.isFinal) {
                   (jointPatternVariablesWithMismatchingFinality ??= [])
                       .add(jointVariable);
                 }
+              } else {
+                (jointPatternVariablesNotInAll ??= []).add(jointVariable);
+              }
+            }
+            if (patternVariablesByName.isNotEmpty) {
+              for (VariableDeclaration variable
+                  in patternVariablesByName.values) {
+                VariableDeclaration jointVariable = forest
+                    .createVariableDeclaration(
+                        variable.fileOffset, variable.name!)
+                  ..isFinal = variable.isFinal;
+                (jointPatternVariablesNotInAll ??= []).add(jointVariable);
+                jointPatternVariables.add(jointVariable);
               }
             }
           }
         } else {
           // It's a non-pattern head, so no variables can be joined.
-          jointPatternVariables = null;
-          break;
+          if (jointPatternVariables != null) {
+            (jointPatternVariablesNotInAll ??= [])
+                .addAll(jointPatternVariables);
+          }
         }
       }
       if (jointPatternVariables != null) {
@@ -7895,11 +7918,17 @@ class BodyBuilder extends StackListenerImpl
       exitLocalScope(expectedScopeKinds: const [ScopeKind.caseHead]);
       enterLocalScope(switchCaseScope);
     }
+    push(jointPatternVariablesNotInAll ?? NullValues.VariableDeclarationList);
     push(jointPatternVariablesWithMismatchingFinality ??
         NullValues.VariableDeclarationList);
     push(jointPatternVariables ?? NullValues.VariableDeclarationList);
 
+    createAndEnterLocalScope(
+        debugName: "switch-case-body", kind: ScopeKind.switchCaseBody);
+
     assert(checkState(firstToken, [
+      ValueKinds.Scope,
+      ValueKinds.VariableDeclarationListOrNull,
       ValueKinds.VariableDeclarationListOrNull,
       ValueKinds.VariableDeclarationListOrNull,
       ValueKinds.Scope,
@@ -7986,56 +8015,87 @@ class BodyBuilder extends StackListenerImpl
     debugEvent("SwitchCase");
     assert(checkState(firstToken, [
       ...repeatedKind(ValueKinds.Statement, statementCount),
+      ValueKinds.Scope,
+      ValueKinds.VariableDeclarationListOrNull,
       ValueKinds.VariableDeclarationListOrNull,
       ValueKinds.VariableDeclarationListOrNull,
       ValueKinds.Scope,
       ValueKinds.LabelListOrNull,
       ValueKinds.Bool,
       ValueKinds.ExpressionOrPatternGuardCaseList,
     ]));
+
     // We always create a block here so that we later know that there's always
     // one synthetic block when we finish compiling the switch statement and
     // check this switch case to see if it falls through to the next case.
     Statement block = popBlock(statementCount, firstToken, null);
+    exitLocalScope(expectedScopeKinds: const [ScopeKind.switchCaseBody]);
     List<VariableDeclaration>? jointPatternVariables =
         pop() as List<VariableDeclaration>?;
     List<VariableDeclaration>? jointPatternVariablesWithMismatchingFinality =
         pop() as List<VariableDeclaration>?;
+    List<VariableDeclaration>? jointPatternVariablesNotInAll =
+        pop() as List<VariableDeclaration>?;
 
+    // The current scope should be the scope of the body of the switch case
+    // because we want to lookup the first use of the pattern variables
+    // specifically in the body of the case, as opposed to, for example, the
+    // guard in one of the heads of the case.
+    assert(
+        scope.kind == ScopeKind.switchCase ||
+            scope.kind == ScopeKind.jointVariables,
+        "Expected the current scope to be of kind '${ScopeKind.switchCase}' "
+        "or '${ScopeKind.jointVariables}', but got '${scope.kind}.");
+    Map<String, int>? usedNamesOffsets = scope.usedNames;
+
+    bool hasDefaultOrLabels = defaultKeyword != null || labelCount > 0;
+
+    List<VariableDeclaration>? usedJointPatternVariables;
     List<int>? jointVariableFirstUseOffsets;
     if (jointPatternVariables != null) {
-      List<VariableDeclaration> usedJointPatternVariables = [];
+      usedJointPatternVariables = [];
       Map<VariableDeclaration, int> firstUseOffsets = {};
-      Scope? jointVariablesScope = scope;
-      while (jointVariablesScope != null &&
-          jointVariablesScope.kind != ScopeKind.jointVariables) {
-        jointVariablesScope = jointVariablesScope.parent;
-      }
-      assert(jointVariablesScope != null,
-          "Can't find the scope the joint variables are declared in.");
       for (VariableDeclaration variable in jointPatternVariables) {
-        int? firstUseOffset = jointVariablesScope?.usedNames?[variable.name!];
+        int? firstUseOffset = usedNamesOffsets?[variable.name!];
         if (firstUseOffset != null) {
           usedJointPatternVariables.add(variable);
           firstUseOffsets[variable] = firstUseOffset;
         }
       }
-      jointPatternVariables = usedJointPatternVariables;
-      if (jointPatternVariablesWithMismatchingFinality != null) {
-        for (VariableDeclaration jointVariable in jointPatternVariables) {
+      if (jointPatternVariablesWithMismatchingFinality != null ||
+          jointPatternVariablesNotInAll != null ||
+          hasDefaultOrLabels) {
+        for (VariableDeclaration jointVariable in usedJointPatternVariables) {
           if (jointPatternVariablesWithMismatchingFinality
-              .contains(jointVariable)) {
+                  ?.contains(jointVariable) ??
+              false) {
             String jointVariableName = jointVariable.name!;
             addProblem(
                 fasta.templateJointPatternVariablesMismatch
                     .withArguments(jointVariableName),
                 firstUseOffsets[jointVariable]!,
                 jointVariableName.length);
           }
+          if (jointPatternVariablesNotInAll?.contains(jointVariable) ?? false) {
+            String jointVariableName = jointVariable.name!;
+            addProblem(
+                fasta.templateJointPatternVariableNotInAll
+                    .withArguments(jointVariableName),
+                firstUseOffsets[jointVariable]!,
+                jointVariableName.length);
+          }
+          if (hasDefaultOrLabels) {
+            String jointVariableName = jointVariable.name!;
+            addProblem(
+                fasta.templateJointPatternVariableWithLabelDefault
+                    .withArguments(jointVariableName),
+                firstUseOffsets[jointVariable]!,
+                jointVariableName.length);
+          }
         }
       }
       jointVariableFirstUseOffsets = [
-        for (VariableDeclaration variable in jointPatternVariables)
+        for (VariableDeclaration variable in usedJointPatternVariables)
           firstUseOffsets[variable]!
       ];
     }
@@ -8051,6 +8111,28 @@ class BodyBuilder extends StackListenerImpl
     bool containsPatterns = pop() as bool;
     List<ExpressionOrPatternGuardCase> expressionsOrPatternGuards =
         pop() as List<ExpressionOrPatternGuardCase>;
+
+    if (expressionCount == 1 &&
+        containsPatterns &&
+        hasDefaultOrLabels &&
+        usedNamesOffsets != null) {
+      PatternGuard? patternGuard =
+          expressionsOrPatternGuards.first.patternGuard;
+      if (patternGuard != null) {
+        for (VariableDeclaration variable
+            in patternGuard.pattern.declaredVariables) {
+          String variableName = variable.name!;
+          int? offset = usedNamesOffsets[variableName];
+          if (offset != null) {
+            addProblem(
+                fasta.templateJointPatternVariableWithLabelDefault
+                    .withArguments(variableName),
+                offset,
+                variableName.length);
+          }
+        }
+      }
+    }
     if (containsPatterns || libraryFeatures.patterns.isEnabled) {
       // If patterns are enabled, we always use the pattern switch encoding.
       // Otherwise, we use pattern switch encoding to handle the erroneous case
@@ -8072,7 +8154,7 @@ class BodyBuilder extends StackListenerImpl
           firstToken.charOffset, caseOffsets, patternGuards, block,
           isDefault: defaultKeyword != null,
           hasLabel: labels != null,
-          jointVariables: jointPatternVariables ?? [],
+          jointVariables: usedJointPatternVariables ?? [],
           jointVariableFirstUseOffsets: jointVariableFirstUseOffsets));
     } else {
       List<Expression> expressions = <Expression>[];

pkg/front_end/lib/src/fasta/scope.dart

@@ -81,6 +81,11 @@ enum ScopeKind {
   /// This scope kind is used in assertion checks.
   switchCase,
 
+  /// Scope for switch case bodies
+  ///
+  /// This is used to handle local variables of switch cases.
+  switchCaseBody,
+
   /// Scope for type parameters of declarations
   typeParameters,
 }

pkg/front_end/lib/src/fasta/type_inference/inference_visitor.dart

@@ -8371,6 +8371,7 @@ class InferenceVisitorImpl extends InferenceVisitorBase
 
     for (int caseIndex = 0; caseIndex < node.cases.length; caseIndex++) {
       PatternSwitchCase switchCase = node.cases[caseIndex];
+      List<VariableDeclaration> jointVariablesNotInAll = [];
       for (int headIndex = 0;
           headIndex < switchCase.patternGuards.length;
           headIndex++) {
@@ -8398,13 +8399,23 @@ class InferenceVisitorImpl extends InferenceVisitorBase
         };
         if (headIndex == 0) {
           for (VariableDeclaration jointVariable in switchCase.jointVariables) {
-            jointVariable.type = inferredVariableTypes[jointVariable.name!]!;
+            DartType? inferredType = inferredVariableTypes[jointVariable.name!];
+            if (inferredType != null) {
+              jointVariable.type = inferredType;
+            } else {
+              jointVariable.type = const InvalidType();
+              jointVariablesNotInAll.add(jointVariable);
+            }
           }
         } else {
           for (int i = 0; i < switchCase.jointVariables.length; ++i) {
             VariableDeclaration jointVariable = switchCase.jointVariables[i];
-            if (jointVariable.type !=
-                inferredVariableTypes[jointVariable.name!]) {
+            // The error on joint variables not present in all case heads is
+            // reported in BodyBuilder.
+            DartType? inferredType = inferredVariableTypes[jointVariable.name!];
+            if (!jointVariablesNotInAll.contains(jointVariable) &&
+                inferredType != null &&
+                jointVariable.type != inferredType) {
               jointVariable.initializer = helper.buildProblem(
                   templateJointPatternVariablesMismatch
                       .withArguments(jointVariable.name!),

pkg/front_end/messages.yaml

@@ -6827,3 +6827,15 @@ PatternVariableAssignmentInsideGuard:
       int i when (i = 5) > 0 => 0,
       _ => -1,
     };
+
+JointPatternVariableNotInAll:
+  problemMessage: "The variable '#name' is available in some, but not all cases that share this body."
+  analyzerCode: INVALID_PATTERN_VARIABLE_IN_SHARED_CASE_SCOPE
+  script: |
+    test(x) { switch(x) { case var a: case < 0: case [var a]: return a; default: return null; } }
+
+JointPatternVariableWithLabelDefault:
+  problemMessage: "The variable '#name' is not available because there is a label or 'default' case."
+  analyzerCode: INVALID_PATTERN_VARIABLE_IN_SHARED_CASE_SCOPE
+  script: |
+    test(x) { switch(x) { case var a: default: return a; } }