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 <scheglov@google.com>
Commit-Queue: Mike Fairhurst <mfairhurst@google.com>
diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
index 0e6d519..d95f632 100644
--- a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
@@ -683,8 +683,8 @@
.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 @@
.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) {
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 858e83f..7f78c28 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -46,13 +46,19 @@
*/
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,7 +82,6 @@
}
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 =
@@ -84,7 +89,9 @@
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 @@
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 @@
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 @@
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 @@
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 @@
.map((TypeParameterElement parameter) => parameter.type)
.toList();
type = type.substitute2(argumentTypes, parameterTypes);
+ } else if (typeArguments == null && typeParameters != null) {
+ type = typeSystem.instantiateToBounds(type);
}
return type;
}
diff --git a/pkg/analyzer/lib/src/summary/link.dart b/pkg/analyzer/lib/src/summary/link.dart
index 3c6353c..10656bd 100644
--- a/pkg/analyzer/lib/src/summary/link.dart
+++ b/pkg/analyzer/lib/src/summary/link.dart
@@ -2225,7 +2225,7 @@
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(
diff --git a/pkg/analyzer/lib/src/summary/resynthesize.dart b/pkg/analyzer/lib/src/summary/resynthesize.dart
index 170a534..5c72f8a 100644
--- a/pkg/analyzer/lib/src/summary/resynthesize.dart
+++ b/pkg/analyzer/lib/src/summary/resynthesize.dart
@@ -1600,8 +1600,12 @@
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);
diff --git a/pkg/analyzer/lib/src/task/dart.dart b/pkg/analyzer/lib/src/task/dart.dart
index a1c7232..50ff551 100644
--- a/pkg/analyzer/lib/src/task/dart.dart
+++ b/pkg/analyzer/lib/src/task/dart.dart
@@ -5125,8 +5125,12 @@
// 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.
diff --git a/pkg/analyzer/test/generated/non_error_resolver_test.dart b/pkg/analyzer/test/generated/non_error_resolver_test.dart
index 73c2001..4a6062f 100644
--- a/pkg/analyzer/test/generated/non_error_resolver_test.dart
+++ b/pkg/analyzer/test/generated/non_error_resolver_test.dart
@@ -4829,6 +4829,87 @@
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 {
