Fix a bug in optional const: missing instantiation to bounds on ctors

Part of #30384. This was usually not a problem as most cases involve
additional visitors which rewrite this (for instance, inference).

However, in some cases (appears to be summaries only), that extra
resolution is not needed/skipped, so the non-instantiated constructors
remain.

That makes the constantValue of annotations vulnerable to this, and the
resulting DartObject types will have 'TypeParameterType's for there
typeArguments.

Instantiate the types to bounds, and get the ctor from the type rather
than the element, inside of AstRewriter in order to ensure these
constructors are instantiated.

Change-Id: I65f55feb751e139e68c774786bfbc53788d32995
Reviewed-on: https://dart-review.googlesource.com/58800
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Mike Fairhurst <[email protected]>

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

@@ -683,8 +683,8 @@ class LibraryAnalyzer {
         .resolve(unit, unitElement);
 
     if (_libraryElement.context.analysisOptions.previewDart2) {
-      unit.accept(new AstRewriteVisitor(_libraryElement, source, _typeProvider,
-          AnalysisErrorListener.NULL_LISTENER));
+      unit.accept(new AstRewriteVisitor(_context.typeSystem, _libraryElement,
+          source, _typeProvider, AnalysisErrorListener.NULL_LISTENER));
     }
 
     // TODO(scheglov) remove EnumMemberBuilder class
@@ -737,8 +737,8 @@ class LibraryAnalyzer {
         .resolve(unit, unitElement);
 
     if (_libraryElement.context.analysisOptions.previewDart2) {
-      unit.accept(new AstRewriteVisitor(_libraryElement, file.source,
-          _typeProvider, AnalysisErrorListener.NULL_LISTENER));
+      unit.accept(new AstRewriteVisitor(_context.typeSystem, _libraryElement,
+          file.source, _typeProvider, AnalysisErrorListener.NULL_LISTENER));
     }
 
     for (var declaration in unit.declarations) {

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

@@ -46,13 +46,19 @@ export 'package:analyzer/src/generated/type_system.dart';
  */
 class AstRewriteVisitor extends ScopedVisitor {
   final bool addConstKeyword;
+  final TypeSystem typeSystem;
 
   /**
    * Initialize a newly created visitor.
    */
-  AstRewriteVisitor(LibraryElement definingLibrary, Source source,
-      TypeProvider typeProvider, AnalysisErrorListener errorListener,
-      {Scope nameScope, this.addConstKeyword: false})
+  AstRewriteVisitor(
+      this.typeSystem,
+      LibraryElement definingLibrary,
+      Source source,
+      TypeProvider typeProvider,
+      AnalysisErrorListener errorListener,
+      {Scope nameScope,
+      this.addConstKeyword: false})
       : super(definingLibrary, source, typeProvider, errorListener,
             nameScope: nameScope);
 
@@ -76,15 +82,16 @@ class AstRewriteVisitor extends ScopedVisitor {
       }
       Element element = nameScope.lookup(methodName, definingLibrary);
       if (element is ClassElement) {
-        ConstructorElement constructorElement = element.unnamedConstructor;
         AstFactory astFactory = new AstFactoryImpl();
         TypeName typeName = astFactory.typeName(methodName, node.typeArguments);
         ConstructorName constructorName =
             astFactory.constructorName(typeName, null, null);
         InstanceCreationExpression instanceCreationExpression =
             astFactory.instanceCreationExpression(
                 _getKeyword(node), constructorName, node.argumentList);
-        DartType type = _getType(element, node.typeArguments);
+        InterfaceType type = _getType(element, node.typeArguments);
+        ConstructorElement constructorElement =
+            type.lookUpConstructor(null, definingLibrary);
         methodName.staticElement = element;
         methodName.staticType = type;
         typeName.type = type;
@@ -111,7 +118,9 @@ class AstRewriteVisitor extends ScopedVisitor {
           InstanceCreationExpression instanceCreationExpression =
               astFactory.instanceCreationExpression(
                   _getKeyword(node), constructorName, node.argumentList);
-          DartType type = _getType(element, node.typeArguments);
+          InterfaceType type = _getType(element, node.typeArguments);
+          constructorElement =
+              type.lookUpConstructor(methodName.name, definingLibrary);
           methodName.staticElement = element;
           methodName.staticType = type;
           typeName.type = type;
@@ -129,8 +138,6 @@ class AstRewriteVisitor extends ScopedVisitor {
             astFactory.simpleIdentifier(methodName.token));
         Element prefixedElement = nameScope.lookup(identifier, definingLibrary);
         if (prefixedElement is ClassElement) {
-          ConstructorElement constructorElement =
-              prefixedElement.unnamedConstructor;
           TypeName typeName = astFactory.typeName(
               astFactory.prefixedIdentifier(target, node.operator, methodName),
               node.typeArguments);
@@ -139,7 +146,9 @@ class AstRewriteVisitor extends ScopedVisitor {
           InstanceCreationExpression instanceCreationExpression =
               astFactory.instanceCreationExpression(
                   _getKeyword(node), constructorName, node.argumentList);
-          DartType type = _getType(prefixedElement, node.typeArguments);
+          InterfaceType type = _getType(prefixedElement, node.typeArguments);
+          ConstructorElement constructorElement =
+              type.lookUpConstructor(null, definingLibrary);
           methodName.staticElement = element;
           methodName.staticType = type;
           typeName.type = type;
@@ -164,7 +173,9 @@ class AstRewriteVisitor extends ScopedVisitor {
             InstanceCreationExpression instanceCreationExpression =
                 astFactory.instanceCreationExpression(
                     _getKeyword(node), constructorName, node.argumentList);
-            DartType type = _getType(element, node.typeArguments);
+            InterfaceType type = _getType(element, node.typeArguments);
+            constructorElement =
+                type.lookUpConstructor(methodName.name, definingLibrary);
             methodName.staticElement = element;
             methodName.staticType = type;
             typeName.type = type;
@@ -206,6 +217,8 @@ class AstRewriteVisitor extends ScopedVisitor {
           .map((TypeParameterElement parameter) => parameter.type)
           .toList();
       type = type.substitute2(argumentTypes, parameterTypes);
+    } else if (typeArguments == null && typeParameters != null) {
+      type = typeSystem.instantiateToBounds(type);
     }
     return type;
   }

pkg/analyzer/lib/src/summary/link.dart

@@ -2225,7 +2225,7 @@ class ExprTypeComputer {
     UnlinkedExpr unlinkedConst = unlinkedExecutable.bodyExpr;
     var errorListener = AnalysisErrorListener.NULL_LISTENER;
     var astRewriteVisitor = new AstRewriteVisitor(
-        library, unit.source, typeProvider, errorListener);
+        linker.typeSystem, library, unit.source, typeProvider, errorListener);
     // TODO(paulberry): Do we need to pass a nameScope to
     // resolverVisitor to get type variables to resolve properly?
     var resolverVisitor = new ResolverVisitor(

pkg/analyzer/lib/src/summary/resynthesize.dart

@@ -1600,8 +1600,12 @@ class _UnitResynthesizer extends UnitResynthesizer with UnitResynthesizerMixin {
     var expression = new ExprBuilder(this, context, uc).build();
 
     if (expression != null && context.context.analysisOptions.previewDart2) {
-      astRewriteVisitor ??= new AstRewriteVisitor(libraryResynthesizer.library,
-          unit.source, typeProvider, AnalysisErrorListener.NULL_LISTENER,
+      astRewriteVisitor ??= new AstRewriteVisitor(
+          libraryResynthesizer.summaryResynthesizer.context.typeSystem,
+          libraryResynthesizer.library,
+          unit.source,
+          typeProvider,
+          AnalysisErrorListener.NULL_LISTENER,
           addConstKeyword: true);
       var container = astFactory.expressionStatement(expression, null);
       expression.accept(astRewriteVisitor);

pkg/analyzer/lib/src/task/dart.dart

@@ -5125,8 +5125,12 @@ class ResolveUnitTypeNamesTask extends SourceBasedAnalysisTask {
     // Re-write the AST to handle the optional new and const feature.
     //
     if (library.context.analysisOptions.previewDart2) {
-      unit.accept(new AstRewriteVisitor(library, unit.element.source,
-          typeProvider, AnalysisErrorListener.NULL_LISTENER));
+      unit.accept(new AstRewriteVisitor(
+          context.typeSystem,
+          library,
+          unit.element.source,
+          typeProvider,
+          AnalysisErrorListener.NULL_LISTENER));
     }
     //
     // Record outputs.

pkg/analyzer/test/generated/non_error_resolver_test.dart

@@ -4829,6 +4829,87 @@ class B {
     verify([source]);
   }
 
+  test_optionalNew_rewrite_instantiatesToBounds() async {
+    resetWith(
+        options: new AnalysisOptionsImpl()
+          ..previewDart2 = true
+          ..strongMode = true);
+    Source source = addSource(r'''
+import 'b.dart';
+
+@B.named1()
+@B.named2()
+@B.named3()
+@B.named4()
+@B.named5()
+@B.named6()
+@B.named7()
+@B.named8()
+main() {}
+''');
+    final aSource = addNamedSource("/a.dart", r'''
+class Unbounded<T> {
+  const Unbounded();
+  const Unbounded.named();
+}
+class Bounded<T extends String> {
+  const Bounded();
+  const Bounded.named();
+}
+''');
+    final bSource = addNamedSource("/b.dart", r'''
+import 'a.dart';
+import 'a.dart' as p;
+
+const unbounded1 = Unbounded();
+const unbounded2 = Unbounded.named();
+const unbounded3 = p.Unbounded();
+const unbounded4 = p.Unbounded.named();
+const bounded1 = Bounded();
+const bounded2 = Bounded.named();
+const bounded3 = p.Bounded();
+const bounded4 = p.Bounded.named();
+
+class B {
+  const B.named1({this.unbounded: unbounded1}) : bounded = null;
+  const B.named2({this.unbounded: unbounded2}) : bounded = null;
+  const B.named3({this.unbounded: unbounded3}) : bounded = null;
+  const B.named4({this.unbounded: unbounded4}) : bounded = null;
+  const B.named5({this.bounded: bounded1}) : unbounded = null;
+  const B.named6({this.bounded: bounded2}) : unbounded = null;
+  const B.named7({this.bounded: bounded3}) : unbounded = null;
+  const B.named8({this.bounded: bounded4}) : unbounded = null;
+
+  final Unbounded unbounded;
+  final Bounded bounded;
+}
+''');
+    final result = await computeAnalysisResult(source);
+    expect(result.unit.declarations, hasLength(1));
+    final mainDecl = result.unit.declarations[0];
+    expect(mainDecl.metadata, hasLength(8));
+    mainDecl.metadata.forEach((metadata) {
+      final value = metadata.elementAnnotation.computeConstantValue();
+      expect(value, isNotNull);
+      expect(value.type.toString(), 'B');
+      final unbounded = value.getField('unbounded');
+      final bounded = value.getField('bounded');
+      if (!unbounded.isNull) {
+        expect(bounded.isNull, true);
+        expect(unbounded.type.name, 'Unbounded');
+        expect(unbounded.type.typeArguments, hasLength(1));
+        expect(unbounded.type.typeArguments[0].isDynamic, isTrue);
+      } else {
+        expect(unbounded.isNull, true);
+        expect(bounded.type.name, 'Bounded');
+        expect(bounded.type.typeArguments, hasLength(1));
+        expect(bounded.type.typeArguments[0].name, 'String');
+      }
+    });
+    assertNoErrors(source);
+    verify([source]);
+  }
+
   test_optionalParameterInOperator_required() async {
     Source source = addSource(r'''
 class A {