Add a "legacy type asserter," manage broken test cases

Change-Id: Ic046805c3246ad7e5e7c56fb9188d6ebd197e2fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105560
Commit-Queue: Mike Fairhurst <mfairhurst@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

@@ -20,6 +20,7 @@ import 'package:analyzer/src/dart/constant/utilities.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/handle.dart';
 import 'package:analyzer/src/dart/element/inheritance_manager2.dart';
+import 'package:analyzer/src/dart/resolver/legacy_type_asserter.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/error/inheritance_override.dart';
 import 'package:analyzer/src/error/pending_error.dart';
@@ -201,6 +202,9 @@ class LibraryAnalyzer {
         }
       });
     }
+
+    assert(units.values.every(LegacyTypeAsserter.assertLegacyTypes));
+
     timerLibraryAnalyzerVerify.stop();
 
     // Return full results.

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

@@ -0,0 +1,170 @@
+// Copyright (c) 2019, 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 'dart:collection';
+
+import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/src/dart/element/type.dart';
+
+/// A visitor to assert that legacy libraries deal with legacy types.
+///
+/// Intended to be used via the static method
+/// [LegacyTypeAsserter.assertLegacyTypes], inside an `assert()` node.
+///
+/// Has a defense against being accidentally run outside of an assert statement,
+/// but that can be overridden if needed.
+///
+/// Checks that the static type of every node, as well as the elements of many
+/// nodes, have legacy types, and asserts that the legacy types are deep legacy
+/// types.
+class LegacyTypeAsserter extends GeneralizingAstVisitor {
+  // TODO(mfairhurst): remove custom equality/hashCode once both use nullability
+  Set<DartType> visitedTypes = LinkedHashSet<DartType>(
+      equals: (a, b) =>
+          a == b &&
+          (a as TypeImpl).nullabilitySuffix ==
+              (b as TypeImpl).nullabilitySuffix,
+      hashCode: (a) =>
+          a.hashCode * 11 + (a as TypeImpl).nullabilitySuffix.hashCode);
+
+  LegacyTypeAsserter({bool requireIsDebug = true}) {
+    if (requireIsDebug) {
+      bool isDebug = false;
+
+      assert(() {
+        isDebug = true;
+        return true;
+      }());
+
+      if (!isDebug) {
+        throw UnsupportedError(
+            'Legacy type asserter is being run outside of a debug environment');
+      }
+    }
+  }
+
+  @override
+  visitClassDeclaration(ClassDeclaration node) {
+    _assertLegacyType(node.element?.type);
+    super.visitClassDeclaration(node);
+  }
+
+  @override
+  visitClassMember(ClassMember node) {
+    final element = node.element;
+    if (element is ExecutableElement) {
+      _assertLegacyType(element?.type);
+    }
+    super.visitClassMember(node);
+  }
+
+  @override
+  visitCompilationUnit(CompilationUnit node) {
+    if (!node.featureSet.isEnabled(Feature.non_nullable)) {
+      super.visitCompilationUnit(node);
+    }
+  }
+
+  @override
+  visitDeclaredIdentifier(DeclaredIdentifier node) {
+    _assertLegacyType(node.element?.type);
+    super.visitDeclaredIdentifier(node);
+  }
+
+  @override
+  visitEnumDeclaration(EnumDeclaration node) {
+    _assertLegacyType(node.element?.type);
+    super.visitEnumDeclaration(node);
+  }
+
+  @override
+  visitExpression(Expression e) {
+    _assertLegacyType(e.staticType);
+    super.visitExpression(e);
+  }
+
+  @override
+  visitFormalParameter(FormalParameter node) {
+    _assertLegacyType(node.element?.type);
+    super.visitFormalParameter(node);
+  }
+
+  @override
+  visitFunctionDeclaration(FunctionDeclaration node) {
+    _assertLegacyType(node.element?.type);
+    super.visitFunctionDeclaration(node);
+  }
+
+  @override
+  visitTypeAnnotation(TypeAnnotation node) {
+    _assertLegacyType(node.type);
+    super.visitTypeAnnotation(node);
+  }
+
+  @override
+  visitTypeName(TypeName node) {
+    _assertLegacyType(node.type);
+    super.visitTypeName(node);
+  }
+
+  @override
+  visitVariableDeclaration(VariableDeclaration node) {
+    _assertLegacyType(node.element?.type);
+    super.visitVariableDeclaration(node);
+  }
+
+  void _assertLegacyType(DartType type) {
+    if (type == null) {
+      return;
+    }
+
+    if (type.isDynamic || type.isVoid) {
+      return;
+    }
+
+    if (type.isBottom && type.isDartCoreNull) {
+      // Never?, which is ok.
+      //
+      // Note: we could allow Null? and Null, but we really should be able to
+      // guarantee that we are only working with Null*, so that's what this
+      // currently does.
+      return;
+    }
+
+    if (visitedTypes.contains(type)) {
+      return;
+    }
+
+    visitedTypes.add(type);
+
+    if (type is TypeParameterType) {
+      _assertLegacyType(type.bound);
+    } else if (type is InterfaceType) {
+      type.typeArguments.forEach(_assertLegacyType);
+      type.typeParameters.map((param) => param.type).forEach(_assertLegacyType);
+    } else if (type is FunctionType) {
+      _assertLegacyType(type.returnType);
+      type.parameters.map((param) => param.type).forEach(_assertLegacyType);
+      type.typeArguments.forEach(_assertLegacyType);
+      type.typeParameters.map((param) => param.type).forEach(_assertLegacyType);
+    }
+
+    if ((type as TypeImpl).nullabilitySuffix == NullabilitySuffix.star) {
+      return;
+    }
+
+    throw AssertionError('Expected all legacy types, but got '
+        '${(type as TypeImpl).toString(withNullability: true)} '
+        '(${type.runtimeType})');
+  }
+
+  static bool assertLegacyTypes(CompilationUnit compilationUnit) {
+    new LegacyTypeAsserter().visitCompilationUnit(compilationUnit);
+    return true;
+  }
+}

pkg/analyzer/lib/src/generated/testing/ast_test_factory.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/dart/analysis/features.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/standard_ast_factory.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ -287,6 +288,22 @@ class AstTestFactory {
           endToken: TokenFactory.tokenFromType(TokenType.EOF),
           featureSet: null);
 
+  static CompilationUnit compilationUnit9(
+          {String scriptTag,
+          List<Directive> directives,
+          List<CompilationUnitMember> declarations,
+          FeatureSet featureSet}) =>
+      astFactory.compilationUnit2(
+          beginToken: TokenFactory.tokenFromType(TokenType.EOF),
+          scriptTag:
+              scriptTag == null ? null : AstTestFactory.scriptTag(scriptTag),
+          directives: directives == null ? new List<Directive>() : directives,
+          declarations: declarations == null
+              ? new List<CompilationUnitMember>()
+              : declarations,
+          endToken: TokenFactory.tokenFromType(TokenType.EOF),
+          featureSet: featureSet);
+
   static ConditionalExpression conditionalExpression(Expression condition,
           Expression thenExpression, Expression elseExpression) =>
       astFactory.conditionalExpression(

pkg/analyzer/test/src/dart/resolver/legacy_type_asserter_test.dart

@@ -0,0 +1,184 @@
+// Copyright (c) 2019, 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/dart/analysis/features.dart';
+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/dart/resolver/legacy_type_asserter.dart';
+import 'package:analyzer/src/generated/resolver.dart';
+import 'package:analyzer/src/generated/testing/ast_test_factory.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../ast/parse_base.dart';
+import '../resolution/driver_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(LegacyTypeAsserterTest);
+  });
+}
+
+/// Tests for the [ExitDetector] that require that the control flow and spread
+/// experiments be enabled.
+@reflectiveTest
+class LegacyTypeAsserterTest extends DriverResolutionTest {
+  TypeProvider typeProvider;
+  setUp() async {
+    await super.setUp();
+    typeProvider = await this.driver.currentSession.typeProvider;
+  }
+
+  test_nullableUnit_expressionStaticType_bottom() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = BottomTypeImpl.instance;
+    try {
+      LegacyTypeAsserter.assertLegacyTypes(unit);
+      fail('expected an exception');
+    } on AssertionError {
+      // expected
+    }
+  }
+
+  test_nullableUnit_expressionStaticType_bottomQuestion() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = BottomTypeImpl.instanceNullable;
+    LegacyTypeAsserter.assertLegacyTypes(unit);
+  }
+
+  test_nullableUnit_expressionStaticType_dynamic() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = typeProvider.dynamicType;
+    LegacyTypeAsserter.assertLegacyTypes(unit);
+  }
+
+  test_nullableUnit_expressionStaticType_nonNull() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = (typeProvider.intType as TypeImpl)
+        .withNullability(NullabilitySuffix.none);
+    try {
+      LegacyTypeAsserter.assertLegacyTypes(unit);
+      fail('expected an exception');
+    } on AssertionError {
+      // expected
+    }
+  }
+
+  test_nullableUnit_expressionStaticType_nonNullTypeArgument() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = typeProvider.listType.instantiate([
+      (typeProvider.intType as TypeImpl)
+          .withNullability(NullabilitySuffix.question)
+    ]);
+
+    try {
+      LegacyTypeAsserter.assertLegacyTypes(unit);
+      fail('expected an exception');
+    } on AssertionError {
+      // expected
+    }
+  }
+
+  test_nullableUnit_expressionStaticType_nonNullTypeParameter() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    final listType = typeProvider.listType;
+    listType.typeParameters[0] = TypeParameterElementImpl('E', 0)
+      ..type = (listType.typeParameters[0].type as TypeImpl)
+          .withNullability(NullabilitySuffix.none) as TypeParameterTypeImpl;
+    identifier.staticType = listType;
+    expect(
+        (listType as dynamic)
+            .typeParameters[0]
+            .type
+            .toString(withNullability: true),
+        'E');
+    try {
+      LegacyTypeAsserter.assertLegacyTypes(unit);
+      fail('expected an exception');
+    } on AssertionError {
+      // expected
+    }
+  }
+
+  test_nullableUnit_expressionStaticType_nonNullTypeParameterBound() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    final listType = typeProvider.listType;
+    (listType.typeParameters[0] as TypeParameterElementImpl).bound =
+        (typeProvider.intType as TypeImpl)
+            .withNullability(NullabilitySuffix.none);
+    identifier.staticType = listType;
+    expect(
+        (listType as dynamic)
+            .typeParameters[0]
+            .type
+            .bound
+            .toString(withNullability: true),
+        'int');
+    try {
+      LegacyTypeAsserter.assertLegacyTypes(unit);
+      fail('expected an exception');
+    } on AssertionError {
+      // expected
+    }
+  }
+
+  test_nullableUnit_expressionStaticType_null() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = typeProvider.nullType;
+    LegacyTypeAsserter.assertLegacyTypes(unit);
+  }
+
+  test_nullableUnit_expressionStaticType_question() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = (typeProvider.intType as TypeImpl)
+        .withNullability(NullabilitySuffix.question);
+    try {
+      LegacyTypeAsserter.assertLegacyTypes(unit);
+      fail('expected an exception');
+    } on AssertionError {
+      // expected
+    }
+  }
+
+  test_nullableUnit_expressionStaticType_star() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = (typeProvider.intType as TypeImpl)
+        .withNullability(NullabilitySuffix.star);
+    LegacyTypeAsserter.assertLegacyTypes(unit);
+  }
+
+  test_nullableUnit_expressionStaticType_void() async {
+    var identifier = AstTestFactory.identifier3('foo');
+    var unit = _wrapExpression(identifier);
+    identifier.staticType = VoidTypeImpl.instance;
+    LegacyTypeAsserter.assertLegacyTypes(unit);
+  }
+
+  CompilationUnit _wrapExpression(Expression e, {bool nonNullable = false}) {
+    return AstTestFactory.compilationUnit9(
+        declarations: [
+          AstTestFactory.functionDeclaration(
+              null,
+              null,
+              null,
+              AstTestFactory.functionExpression2(
+                  null, AstTestFactory.expressionFunctionBody(e)))
+        ],
+        featureSet: FeatureSet.forTesting(
+            additionalFeatures: nonNullable ? [Feature.non_nullable] : []));
+  }
+}

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

@@ -5,9 +5,11 @@
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import 'exit_detector_test.dart' as exit_detector;
+import 'legacy_type_asserter_test.dart' as legacy_type_asserter;
 
 main() {
   defineReflectiveSuite(() {
     exit_detector.main();
+    legacy_type_asserter.main();
   }, name: 'resolver');
 }

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

@@ -27,8 +27,8 @@ class NonNullOptOutTest extends DriverResolutionTest {
     assertErrorsInCode('''
 // @dart = 2.2
 // NNBD syntax is not allowed
-f(x, y, z) { (x is String?) ? (x + y) : z; }
-''', [error(ParserErrorCode.EXPERIMENT_NOT_ENABLED, 70, 1)]);
+f(x, z) { (x is String?) ? x : z; }
+''', [error(ParserErrorCode.EXPERIMENT_NOT_ENABLED, 67, 1)]);
   }
 
   test_nnbd_optOut_late() async {
@@ -38,6 +38,17 @@ class C {
   // "late" is allowed as an identifier
   int late;
 }
+''');
+  }
+
+  @failingTest
+  test_nnbd_optOut_transformsOptedInSignatures() async {
+    // Failing because we don't transform opted out signatures.
+    await assertNoErrorsInCode('''
+// @dart = 2.2
+f(String x) {
+  x + null; // OK because we're in a nullable library.
+}
 ''');
   }
 }