Flow analysis: rework some testing logic in preparation for pattern support.

- Variable types are no longer specified in the call to the `Var`
  constructor; they are now specified in the call to `declare`.  This
  paves the way for supporting variable pattern syntax, in which a
  single variable might appear in multiple variable patterns, and have
  its type specified in each pattern.  The properties `isFinal` and
  `isLate` are also moved to `declare` for consistency.

- Variables with inferred types are now specified by simply not
  including a type in `declare`; it's no longer necessary to specify
  `isImplicitlyTyped: true`.

- `declare` now supports an `expectInferredType` argument to allow the
  inferred type of an implicitly typed variable to be tested.

- The tests now check that variables are assigned a type before flow
  analysis requests it; previously this was not tested, and the flow
  analysis tests sometimes did things in the wrong order.  (The
  analyzer and CFE have always done this in the proper order though).

- The tests now support some of the crazy types that arise during type
  parameter promotion, e.g. they can now distinguish `(T&int)?` from
  `T&(int?)`.

- Flow analysis tests now properly replicate the analyzer and CFE
  behaviors for converting the static type of an initializer
  expression to the corresponding inferred variable type: (a) `Null`
  is converted to `dynamic`, and (b) type parameter promotions are
  dropped.

Note that this last behavior (dropping type parameter promotions) has
a lot of subtleties, and I'm not convinced the CFE and analyzer do it
soundly in all cases (I've already found one such soundness bug:
https://github.com/dart-lang/sdk/issues/49691).  In a later CL, I plan
to add a more thorough set of language tests to verify that we don't
have other lurking soundness issues.

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

pkg/_fe_analyzer_shared/test/mini_ast.dart

@@ -59,11 +59,17 @@ Statement checkUnassigned(Var variable, bool expectedUnassignedState) =>
 
 Statement continue_() => new _Continue();
 
-Statement declare(Var variable, {required bool initialized}) =>
-    new _Declare(variable, initialized ? expr(variable.type.type) : null);
-
-Statement declareInitialized(Var variable, Expression initializer) =>
-    new _Declare(variable, initializer);
+Statement declare(Var variable,
+        {bool isLate = false,
+        bool isFinal = false,
+        String? type,
+        Expression? initializer,
+        String? expectInferredType}) =>
+    new _Declare(variable, initializer,
+        isLate: isLate,
+        isFinal: isFinal,
+        declaredType: type == null ? null : Type(type),
+        expectInferredType: expectInferredType);
 
 Statement do_(List<Statement> body, Expression condition) =>
     _Do(block(body), condition);
@@ -669,20 +675,31 @@ abstract class TryStatement extends Statement implements TryBuilder {
 /// analysis testing.
 class Var implements Promotable {
   final String name;
-  final Type type;
-  final bool isFinal;
-  final bool isImplicitlyTyped;
-  final bool isLate;
 
-  Var(this.name, String typeStr,
-      {this.isFinal = false,
-      this.isImplicitlyTyped = false,
-      this.isLate = false})
-      : type = Type(typeStr);
+  /// The type of the variable, or `null` if it is not yet known.
+  Type? _type;
+
+  Var(this.name);
 
   /// Creates an L-value representing a reference to this variable.
   LValue get expr => new _VariableReference(this, null);
 
+  /// Gets the type if known; otherwise throws an exception.
+  Type get type {
+    if (_type == null) {
+      throw 'Type not yet known';
+    } else {
+      return _type!;
+    }
+  }
+
+  set type(Type value) {
+    if (_type != null) {
+      throw 'Type already set';
+    }
+    _type = value;
+  }
+
   @override
   void preVisit(AssignedVariables<Node, Var> assignedVariables) {}
 
@@ -692,7 +709,7 @@ class Var implements Promotable {
       new _VariableReference(this, callback);
 
   @override
-  String toString() => '$type $name';
+  String toString() => 'var $name';
 
   /// Creates an expression representing a write to this variable.
   Expression write(Expression? value) => expr.write(value);
@@ -992,10 +1009,18 @@ class _Continue extends Statement {
 }
 
 class _Declare extends Statement {
+  final bool isLate;
+  final bool isFinal;
+  final Type? declaredType;
   final Var variable;
   final Expression? initializer;
+  final String? expectInferredType;
 
-  _Declare(this.variable, this.initializer);
+  _Declare(this.variable, this.initializer,
+      {required this.isLate,
+      required this.isFinal,
+      required this.declaredType,
+      this.expectInferredType});
 
   @override
   void preVisit(AssignedVariables<Node, Var> assignedVariables) {
@@ -1005,22 +1030,28 @@ class _Declare extends Statement {
 
   @override
   String toString() {
-    var latePart = variable.isLate ? 'late ' : '';
-    var finalPart = variable.isFinal ? 'final ' : '';
-    var initializerPart = initializer != null ? ' = $initializer' : '';
-    return '$latePart$finalPart$variable${initializerPart};';
+    var parts = <String>[
+      if (isLate) 'late',
+      if (isFinal) 'final',
+      if (declaredType != null) declaredType!.type else if (!isFinal) 'var',
+      variable.name,
+      if (initializer != null) '= $initializer'
+    ];
+    return '${parts.join(' ')};';
   }
 
   @override
   void visit(Harness h) {
     h.irBuilder.atom(variable.name);
     h.typeAnalyzer.analyzeVariableDeclaration(
-        this, variable.type, variable, initializer,
-        isFinal: variable.isFinal, isLate: variable.isLate);
+        this, declaredType, variable, initializer,
+        isFinal: isFinal, isLate: isLate);
+    var expectInferredType = this.expectInferredType;
+    if (expectInferredType != null) {
+      expect(variable.type.type, expectInferredType);
+    }
     h.irBuilder.apply(
-        ['declare', if (variable.isLate) 'late', if (variable.isFinal) 'final']
-            .join('_'),
-        2);
+        ['declare', if (isLate) 'late', if (isFinal) 'final'].join('_'), 2);
   }
 }
 
@@ -1360,6 +1391,8 @@ class _MiniAstTypeAnalyzer {
 
   late final Type boolType = Type('bool');
 
+  late final Type dynamicType = Type('dynamic');
+
   late final Type neverType = Type('Never');
 
   late final Type nullType = Type('Null');
@@ -1629,18 +1662,21 @@ class _MiniAstTypeAnalyzer {
   }
 
   void analyzeVariableDeclaration(
-      Statement node, Type type, Var variable, Expression? initializer,
+      Statement node, Type? declaredType, Var variable, Expression? initializer,
       {required bool isFinal, required bool isLate}) {
     if (initializer == null) {
       handleNoInitializer();
       flow.declare(variable, false);
+      variable.type = declaredType ?? dynamicType;
     } else {
       var initializerType = analyzeExpression(initializer);
       flow.declare(variable, true);
+      variable.type =
+          declaredType ?? variableTypeFromInitializerType(initializerType);
       flow.initialize(variable, initializerType, initializer,
           isFinal: isFinal,
           isLate: isLate,
-          isImplicitlyTyped: variable.isImplicitlyTyped);
+          isImplicitlyTyped: declaredType == null);
     }
   }
 
@@ -1702,6 +1738,22 @@ class _MiniAstTypeAnalyzer {
     return _harness.getMember(receiverType, memberName);
   }
 
+  /// Computes the type that should be inferred for an implicitly typed variable
+  /// whose initializer expression has static type [type].
+  Type variableTypeFromInitializerType(Type type) {
+    // Variables whose initializer has type `Null` receive the inferred type
+    // `dynamic`.
+    if (_harness.classifyType(type) == TypeClassification.nullOrEquivalent) {
+      type = dynamicType;
+    }
+    // Variables whose initializer type includes a promoted type variable
+    // receive the nearest supertype that could be expressed in Dart source code
+    // (e.g. `T&int` is demoted to `T`).
+    // TODO(paulberry): add language tests to verify that the behavior of
+    // `type.recursivelyDemote` matches what the analyzer and CFE do.
+    return type.recursivelyDemote(covariant: true) ?? type;
+  }
+
   _PropertyElement _lookupMember(
       Expression node, Type receiverType, String memberName) {
     return lookupInterfaceMember(node, receiverType, memberName);

pkg/_fe_analyzer_shared/test/mini_types.dart

@@ -11,7 +11,8 @@ import 'package:test/test.dart';
 /// Representation of a function type suitable for unit testing of code in the
 /// `_fe_analyzer_shared` package.
 ///
-/// Optional and named parameters are not (yet) supported.
+/// Optional parameters, named parameters, and type parameters are not (yet)
+/// supported.
 class FunctionType extends Type {
   /// The return type.
   final Type returnType;
@@ -22,7 +23,25 @@ class FunctionType extends Type {
   FunctionType(this.returnType, this.positionalParameters) : super._();
 
   @override
-  String get type => '$returnType Function(${positionalParameters.join(', ')})';
+  Type? recursivelyDemote({required bool covariant}) {
+    Type? newReturnType = returnType.recursivelyDemote(covariant: covariant);
+    List<Type>? newPositionalParameters =
+        positionalParameters.recursivelyDemote(covariant: !covariant);
+    if (newReturnType == null && newPositionalParameters == null) {
+      return null;
+    }
+    return FunctionType(newReturnType ?? returnType,
+        newPositionalParameters ?? positionalParameters);
+  }
+
+  @override
+  String _toString({required bool allowSuffixes}) {
+    var result = '$returnType Function(${positionalParameters.join(', ')})';
+    if (!allowSuffixes) {
+      result = '($result)';
+    }
+    return result;
+  }
 }
 
 /// Representation of a "simple" type suitable for unit testing of code in the
@@ -40,7 +59,14 @@ class NonFunctionType extends Type {
   NonFunctionType(this.name, {this.args = const []}) : super._();
 
   @override
-  String get type {
+  Type? recursivelyDemote({required bool covariant}) {
+    List<Type>? newArgs = args.recursivelyDemote(covariant: covariant);
+    if (newArgs == null) return null;
+    return NonFunctionType(name, args: newArgs);
+  }
+
+  @override
+  String _toString({required bool allowSuffixes}) {
     if (args.isEmpty) {
       return name;
     } else {
@@ -62,7 +88,17 @@ class PromotedTypeVariableType extends Type {
   PromotedTypeVariableType(this.innerType, this.promotion) : super._();
 
   @override
-  String get type => '$innerType&$promotion';
+  Type? recursivelyDemote({required bool covariant}) =>
+      covariant ? innerType : new NonFunctionType('Never');
+
+  @override
+  String _toString({required bool allowSuffixes}) {
+    var result = '$innerType&${promotion._toString(allowSuffixes: false)}';
+    if (!allowSuffixes) {
+      result = '($result)';
+    }
+    return result;
+  }
 }
 
 /// Representation of a nullable type suitable for unit testing of code in the
@@ -76,7 +112,20 @@ class QuestionType extends Type {
   QuestionType(this.innerType) : super._();
 
   @override
-  String get type => '$innerType?';
+  Type? recursivelyDemote({required bool covariant}) {
+    Type? newInnerType = innerType.recursivelyDemote(covariant: covariant);
+    if (newInnerType == null) return null;
+    return QuestionType(newInnerType);
+  }
+
+  @override
+  String _toString({required bool allowSuffixes}) {
+    var result = '$innerType?';
+    if (!allowSuffixes) {
+      result = '($result)';
+    }
+    return result;
+  }
 }
 
 /// Representation of a "star" type suitable for unit testing of code in the
@@ -87,7 +136,20 @@ class StarType extends Type {
   StarType(this.innerType) : super._();
 
   @override
-  String get type => '$innerType*';
+  Type? recursivelyDemote({required bool covariant}) {
+    Type? newInnerType = innerType.recursivelyDemote(covariant: covariant);
+    if (newInnerType == null) return null;
+    return StarType(newInnerType);
+  }
+
+  @override
+  String _toString({required bool allowSuffixes}) {
+    var result = '$innerType*';
+    if (!allowSuffixes) {
+      result = '($result)';
+    }
+    return result;
+  }
 }
 
 /// Representation of a type suitable for unit testing of code in the
@@ -118,7 +180,7 @@ abstract class Type {
     return type.hashCode;
   }
 
-  String get type;
+  String get type => _toString(allowSuffixes: true);
 
   @override
   bool operator ==(Object other) {
@@ -130,9 +192,22 @@ abstract class Type {
     return other is Type && this.type == other.type;
   }
 
+  /// Finds the nearest type that doesn't involve any type parameter promotion.
+  /// If `covariant` is `true`, a supertype will be returned (replacing promoted
+  /// type parameters with their unpromoted counterparts); otherwise a subtype
+  /// will be returned (replacing promoted type parameters with `Never`).
+  ///
+  /// Returns `null` if this type is already free from type promotion.
+  Type? recursivelyDemote({required bool covariant});
+
   @override
   String toString() => type;
 
+  /// Returns a string representation of this type.  If `allowSuffixes` is
+  /// `false`, then the result will be surrounded in parenthesis if it would
+  /// otherwise have ended in a suffix.
+  String _toString({required bool allowSuffixes});
+
   /// Executes [callback] while temporarily allowing types to be compared using
   /// `==` and `hashCode`.
   static T withComparisonsAllowed<T>(T Function() callback) {
@@ -152,7 +227,10 @@ class UnknownType extends Type {
   const UnknownType() : super._();
 
   @override
-  String get type => '?';
+  Type? recursivelyDemote({required bool covariant}) => null;
+
+  @override
+  String _toString({required bool allowSuffixes}) => '?';
 }
 
 class _TypeParser {
@@ -181,22 +259,16 @@ class _TypeParser {
     fail('Error parsing type `$_typeStr` at token $_currentToken: $message');
   }
 
-  Type _parseNullability(Type innerType) {
+  Type? _parseSuffix(Type type) {
     if (_currentToken == '?') {
       _next();
-      return QuestionType(innerType);
+      return QuestionType(type);
     } else if (_currentToken == '*') {
       _next();
-      return StarType(innerType);
-    } else {
-      return innerType;
-    }
-  }
-
-  Type? _parseSuffix(Type type) {
-    if (_currentToken == '&') {
+      return StarType(type);
+    } else if (_currentToken == '&') {
       _next();
-      var promotion = _parseType();
+      var promotion = _parseUnsuffixedType();
       return PromotedTypeVariableType(type, promotion);
     } else if (_currentToken == 'Function') {
       _next();
@@ -216,7 +288,7 @@ class _TypeParser {
         }
       }
       _next();
-      return _parseNullability(FunctionType(type, parameterTypes));
+      return FunctionType(type, parameterTypes);
     } else {
       return null;
     }
@@ -224,19 +296,43 @@ class _TypeParser {
 
   Type _parseType() {
     // We currently accept the following grammar for types:
-    //   type := identifier typeArgs? nullability suffix* | `?`
+    //   type := unsuffixedType nullability suffix*
+    //   unsuffixedType := identifier typeArgs?
+    //                   | `?`
+    //                   | `(` type `)`
     //   typeArgs := `<` type (`,` type)* `>`
     //   nullability := (`?` | `*`)?
-    //   suffix := `Function` `(` type (`,` type)* `)` suffix
-    //           | `&` type
+    //   suffix := `Function` `(` type (`,` type)* `)`
+    //           | `?`
+    //           | `*`
+    //           | `&` unsuffixedType
     // TODO(paulberry): support more syntax if needed
+    var result = _parseUnsuffixedType();
+    while (true) {
+      var newResult = _parseSuffix(result);
+      if (newResult == null) break;
+      result = newResult;
+    }
+    return result;
+  }
+
+  Type _parseUnsuffixedType() {
     if (_currentToken == '?') {
       _next();
       return const UnknownType();
     }
+    if (_currentToken == '(') {
+      _next();
+      var type = _parseType();
+      if (_currentToken != ')') {
+        _parseFailure('Expected `)`');
+      }
+      _next();
+      return type;
+    }
     var typeName = _currentToken;
     if (_identifierRegexp.matchAsPrefix(typeName) == null) {
-      _parseFailure('Expected an identifier or `?`');
+      _parseFailure('Expected an identifier, `?`, or `(`');
     }
     _next();
     List<Type> typeArgs;
@@ -255,13 +351,7 @@ class _TypeParser {
     } else {
       typeArgs = const [];
     }
-    var result = _parseNullability(NonFunctionType(typeName, args: typeArgs));
-    while (true) {
-      var newResult = _parseSuffix(result);
-      if (newResult == null) break;
-      result = newResult;
-    }
-    return result;
+    return NonFunctionType(typeName, args: typeArgs);
   }
 
   static Type parse(String typeStr) {
@@ -293,3 +383,22 @@ class _TypeParser {
     return result;
   }
 }
+
+extension on List<Type> {
+  /// Calls [Type.recursivelyDemote] to translate every list member into a type
+  /// that doesn't involve any type promotion.  If no type would be changed by
+  /// this operation, returns `null`.
+  List<Type>? recursivelyDemote({required bool covariant}) {
+    List<Type>? newList;
+    for (int i = 0; i < length; i++) {
+      Type type = this[i];
+      Type? newType = type.recursivelyDemote(covariant: covariant);
+      if (newList == null) {
+        if (newType == null) continue;
+        newList = sublist(0, i);
+      }
+      newList.add(newType ?? type);
+    }
+    return newList;
+  }
+}

pkg/_fe_analyzer_shared/test/mini_types_test.dart

@@ -0,0 +1,135 @@
+// 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:test/test.dart';
+
+import 'mini_types.dart';
+
+main() {
+  group('recursivelyDemote:', () {
+    group('FunctionType:', () {
+      group('return type:', () {
+        test('unchanged', () {
+          expect(Type('int Function()').recursivelyDemote(covariant: true),
+              isNull);
+          expect(Type('int Function()').recursivelyDemote(covariant: false),
+              isNull);
+        });
+
+        test('covariant', () {
+          expect(
+              Type('T&int Function()').recursivelyDemote(covariant: true)!.type,
+              'T Function()');
+        });
+
+        test('contravariant', () {
+          expect(
+              Type('T&int Function()')
+                  .recursivelyDemote(covariant: false)!
+                  .type,
+              'Never Function()');
+        });
+      });
+
+      group('positional parameters:', () {
+        test('unchanged', () {
+          expect(
+              Type('void Function(int, String)')
+                  .recursivelyDemote(covariant: true),
+              isNull);
+          expect(
+              Type('void Function(int, String)')
+                  .recursivelyDemote(covariant: false),
+              isNull);
+        });
+
+        test('covariant', () {
+          expect(
+              Type('void Function(T&int, String)')
+                  .recursivelyDemote(covariant: true)!
+                  .type,
+              'void Function(Never, String)');
+        });
+
+        test('contravariant', () {
+          expect(
+              Type('void Function(T&int, String)')
+                  .recursivelyDemote(covariant: false)!
+                  .type,
+              'void Function(T, String)');
+        });
+      });
+    });
+
+    group('NonFunctionType', () {
+      test('unchanged', () {
+        expect(Type('int').recursivelyDemote(covariant: true), isNull);
+        expect(Type('int').recursivelyDemote(covariant: false), isNull);
+      });
+
+      group('type parameters:', () {
+        test('unchanged', () {
+          expect(Type('Map<int, String>').recursivelyDemote(covariant: true),
+              isNull);
+          expect(Type('Map<int, String>').recursivelyDemote(covariant: false),
+              isNull);
+        });
+
+        test('covariant', () {
+          expect(
+              Type('Map<T&int, String>')
+                  .recursivelyDemote(covariant: true)!
+                  .type,
+              'Map<T, String>');
+        });
+
+        test('contravariant', () {
+          expect(
+              Type('Map<T&int, String>')
+                  .recursivelyDemote(covariant: false)!
+                  .type,
+              'Map<Never, String>');
+        });
+      });
+    });
+
+    group('QuestionType:', () {
+      test('unchanged', () {
+        expect(Type('int?').recursivelyDemote(covariant: true), isNull);
+        expect(Type('int?').recursivelyDemote(covariant: false), isNull);
+      });
+
+      test('covariant', () {
+        expect(Type('(T&int)?').recursivelyDemote(covariant: true)!.type, 'T?');
+      });
+
+      test('contravariant', () {
+        // Note: we don't normalize `Never?` to `Null`.
+        expect(Type('(T&int)?').recursivelyDemote(covariant: false)!.type,
+            'Never?');
+      });
+    });
+
+    group('StarType:', () {
+      test('unchanged', () {
+        expect(Type('int*').recursivelyDemote(covariant: true), isNull);
+        expect(Type('int*').recursivelyDemote(covariant: false), isNull);
+      });
+
+      test('covariant', () {
+        expect(Type('(T&int)*').recursivelyDemote(covariant: true)!.type, 'T*');
+      });
+
+      test('contravariant', () {
+        expect(Type('(T&int)*').recursivelyDemote(covariant: false)!.type,
+            'Never*');
+      });
+    });
+
+    test('UnknownType:', () {
+      expect(Type('?').recursivelyDemote(covariant: true), isNull);
+      expect(Type('?').recursivelyDemote(covariant: false), isNull);
+    });
+  });
+}