Patterns flow analysis: promote to non-nullable when matched value is non-nullable.

Judging by several test cases that have shown up in co19 tests and
informal discussions, it appears to be a common expectation that a
pattern like `int? x?` or `int? x!` should promote `x` to
non-nullable.  Previous to this change, this didn't work in general.
Consider:

    Object? o = ...;
    switch (o) {
      case int? x?:
        print(x.isEven); // (1)
    }

At (1), the null-check happens *before* the required type check of the
variable pattern; therefore it promotes the matched value type to
`Object`.  This is not a subtype of the required type of the variable
pattern (which is `int?`), therefore, previous to this change, `x` was
not promoted.

With this change, since the matched value type of `Object` is
non-nullable, the required type of the variable pattern is
re-interpreted as its non-nullable counterpart, `int`.

In a fully null-safe program, this is sound, because if the matched
value type is non-nullable, that guarantees that the matched value is
not `null`, and therefore it is equivalent to type check against the
non-nullable counterpart of the required type.

In a program that is not fully null-safe, the matched value might have
originated in a non-null-safe library (and thus might be `null` in
violation of its static type).  So, strictly speaking, it is not sound
to re-interpret the required type of the pattern as its non-nullable
counterpart.  However, the only way this unsoundness can manifest is
for the matched value to be promoted to non-nullable when it is in
fact `null`, and that is precisely the sort of unsoundness thta we
permit in mixed-mode programs.  So this change won't result in
unsoundness escalation.

Bug: https://github.com/dart-lang/sdk/issues/51644
Change-Id: I9479e3c29e12f2a62a9e165b32c3480d7e299c29
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287040
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

@@ -4519,6 +4519,13 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
       required Type knownType,
       bool matchFailsIfWrongType = true,
       bool matchMayFailEvenIfCorrectType = false}) {
+    if (operations.classifyType(matchedType) ==
+        TypeClassification.nonNullable) {
+      // The matched type is non-nullable, so promote to a non-nullable type.
+      // This allows for code like `case int? x?` to promote `x` to
+      // non-nullable.
+      knownType = operations.promoteToNonNull(knownType);
+    }
     _PatternContext<Type> context = _stack.last as _PatternContext<Type>;
     ReferenceWithType<Type> matchedValueReference =
         context.createReference(matchedType);

pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart

@@ -521,6 +521,10 @@ mixin TypeAnalyzer<
               requiredType: staticType);
     }
     flow.promoteForPattern(matchedType: matchedType, knownType: staticType);
+    // The promotion may have made the matched type even more specific than
+    // either `matchedType` or `staticType`, so fetch it again and use that
+    // in the call to `declaredVariablePattern` below.
+    matchedType = flow.getMatchedValueType();
     bool isImplicitlyTyped = declaredType == null;
     // TODO(paulberry): are we handling _isFinal correctly?
     int promotionKey = context.patternVariablePromotionKeys[variableName] =

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

@@ -8992,6 +8992,38 @@ main() {
             ]),
           ]);
         });
+
+        test('Complex example', () {
+          // This is based on the code sample from
+          // https://github.com/dart-lang/sdk/issues/51644, except that the type
+          // of the scrutinee has been changed from `dynamic` to `Object?`.
+          var a1 = Var('a', identity: 'a1');
+          var a2 = Var('a', identity: 'a2');
+          var a3 = Var('a', identity: 'a3');
+          var a = PatternVariableJoin('a', expectedComponents: [a1, a2, a3]);
+          h.run([
+            switch_(expr('Object?'), [
+              switchStatementMember([
+                a1
+                    .pattern(type: 'String?')
+                    .nullCheck
+                    .when(a1.expr.is_('Never'))
+                    .switchCase,
+                a2
+                    .pattern(type: 'String?')
+                    .when(a2.expr.notEq(nullLiteral))
+                    .switchCase,
+                a3
+                    .pattern(type: 'String?')
+                    .nullAssert
+                    .when(a3.expr.eq(intLiteral(1)))
+                    .switchCase,
+              ], [
+                checkPromoted(a, 'String'),
+              ]),
+            ]),
+          ]);
+        });
       });
 
       group(
@@ -9151,6 +9183,31 @@ main() {
         ]);
       });
 
+      test('Promotes to non-nullable if matched type is non-nullable', () {
+        // When the matched value type is non-nullable, and the variable's
+        // declared type is nullable, a successful match promotes the variable.
+        // This allows a case pattern of the form `T? x?` to promote `x` to
+        // non-nullable `T`.
+        var x = Var('x');
+        h.run([
+          ifCase(expr('Object'), x.pattern(type: 'int?'), [
+            checkPromoted(x, 'int'),
+          ]),
+        ]);
+      });
+
+      test('Does not promote to non-nullable if matched type is `Null`', () {
+        // Since `Null` is handled specially by `TypeOperations.classifyType`,
+        // make sure that we don't accidentally promote the variable to
+        // non-nullable when the matched value type is `Null`.
+        var x = Var('x');
+        h.run([
+          ifCase(expr('Null'), x.pattern(type: 'int?'), [
+            checkNotPromoted(x),
+          ]),
+        ]);
+      });
+
       group('Demonstrated type:', () {
         test('Subtype of matched value type', () {
           var x = Var('x');

pkg/_fe_analyzer_shared/test/mini_ast.dart

@@ -877,6 +877,7 @@ class MiniAstOperations
     'Object': false,
     'Object?': false,
     'String': false,
+    'String?': false,
   };
 
   static final Map<String, Type> _coreGlbs = {
@@ -921,6 +922,7 @@ class MiniAstOperations
     'int': Type('int'),
     'int?': Type('int?'),
     'num': Type('num'),
+    'String?': Type('String?'),
     'List<int>': Type('List<int>'),
   };