Migration: add support for Angular's @ViewChild() and @ContentChild() annotations.

Bug: https://github.com/dart-lang/sdk/issues/45661
Change-Id: Ie77d720677fb032585e17fe13358afe597e702bd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209820
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2021-08-11 01:55:41 +00:00 committed by commit-bot@chromium.org
parent 650dfa19ff
commit f66037acce
5 changed files with 166 additions and 16 deletions

View file

@ -240,6 +240,7 @@ abstract class EdgeOriginInfo {
enum EdgeOriginKind {
alreadyMigratedType,
alwaysNullableType,
angularAnnotation,
argumentErrorCheckNotNull,
callTearOff,
compoundAssignment,
@ -272,7 +273,6 @@ enum EdgeOriginKind {
nonNullableUsage,
nonNullAssertion,
nullabilityComment,
optionalAnnotation,
optionalFormalParameter,
parameterInheritance,
quiverCheckNotNull,

View file

@ -64,6 +64,19 @@ class AlwaysNullableTypeOrigin extends EdgeOrigin {
EdgeOriginKind get kind => EdgeOriginKind.alwaysNullableType;
}
/// Edge origin resulting from the presence of an Angular annotation such as
/// `@Optional()`, `@ViewChild(...)`, or `@ContentChild(...)`.
class AngularAnnotationOrigin extends EdgeOrigin {
AngularAnnotationOrigin(Source? source, AstNode node) : super(source, node);
@override
String get description =>
"annotated with an Angular annotation indicating nullability";
@override
EdgeOriginKind get kind => EdgeOriginKind.angularAnnotation;
}
/// Edge origin resulting from the presence of a call to
/// `ArgumentError.checkNotNull`.
///
@ -542,18 +555,6 @@ class NullabilityCommentOrigin extends EdgeOrigin {
EdgeOriginKind get kind => EdgeOriginKind.nullabilityComment;
}
/// Edge origin resulting from the presence of an Angular `@Optional()`
/// annotation.
class OptionalAnnotationOrigin extends EdgeOrigin {
OptionalAnnotationOrigin(Source? source, AstNode node) : super(source, node);
@override
String get description => "annotated with Angular's @Optional() annotation";
@override
EdgeOriginKind get kind => EdgeOriginKind.optionalAnnotation;
}
/// Edge origin resulting from the presence of an optional formal parameter.
///
/// For example, in the following code snippet:

View file

@ -465,9 +465,13 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType>
_variables!.recordDecoratedElementType(
declaredElement.variable, decoratedType.returnType);
} else {
_variables!.recordDecoratedElementType(
declaredElement.variable, decoratedType.positionalParameters![0],
var type = decoratedType.positionalParameters![0];
_variables!.recordDecoratedElementType(declaredElement.variable, type,
soft: true);
if (_hasAngularChildAnnotation(node.metadata)) {
_graph.makeNullable(
type!.node!, AngularAnnotationOrigin(source, node));
}
}
}
return null;
@ -668,6 +672,12 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType>
_variables!.recordDecoratedElementType(declaredElement, type);
variable.initializer?.accept(this);
}
var parent = node.parent;
if (parent is FieldDeclaration) {
if (_hasAngularChildAnnotation(parent.metadata)) {
_graph.makeNullable(type!.node!, AngularAnnotationOrigin(source, node));
}
}
return null;
}
@ -800,7 +810,7 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType>
element.enclosingElement.name == 'Optional' &&
_isAngularUri(element.librarySource.uri)) {
_graph.makeNullable(
decoratedType!.node!, OptionalAnnotationOrigin(source, node));
decoratedType!.node!, AngularAnnotationOrigin(source, node));
}
}
if (declaredElement.isNamed) {
@ -893,6 +903,23 @@ class NodeBuilder extends GeneralizingAstVisitor<DecoratedType>
.recordDecoratedDirectSupertypes(declaredElement, decoratedSupertypes);
}
/// Determines if the given [metadata] contains a reference to one of the
/// Angular annotations `ViewChild` or `ContentChild`, either of which implies
/// nullability of the underlying property.
bool _hasAngularChildAnnotation(NodeList<Annotation> metadata) {
for (var annotation in metadata) {
var element = annotation.element;
if (element is ConstructorElement) {
var name = element.enclosingElement.name;
if ((name == 'ViewChild' || name == 'ContentChild') &&
_isAngularUri(element.librarySource.uri)) {
return true;
}
}
}
return false;
}
/// Determines whether the given [uri] comes from the Angular package.
bool _isAngularUri(Uri uri) {
if (uri.scheme != 'package') return false;

View file

@ -55,9 +55,15 @@ class AbstractContextTest with ResourceProviderMixin {
addPackageFile(
internalUris ? 'third_party.dart_src.angular.angular' : 'angular',
'angular.dart', '''
class ContentChild {
const ContentChild(Object selector, {Object? read});
}
class Optional {
const Optional();
}
class ViewChild {
const ViewChild(Object selector, {Object? read});
}
''');
if (internalUris) {
addPackageFile('angular', 'angular.dart', '''

View file

@ -356,6 +356,37 @@ g() {
await _checkSingleFileChanges(content, expected);
}
Future<void> test_angular_contentChild_field() async {
addAngularPackage();
var content = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
// Initialize this.bar in the constructor just so the migration tool doesn't
// decide to make it nullable due to the lack of initializer.
MyComponent(this.bar);
@ContentChild('foo')
Element bar;
}
''';
var expected = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
// Initialize this.bar in the constructor just so the migration tool doesn't
// decide to make it nullable due to the lack of initializer.
MyComponent(this.bar);
@ContentChild('foo')
Element? bar;
}
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_angular_optional_constructor_param() async {
addAngularPackage();
var content = '''
@ -415,6 +446,91 @@ class MyComponent {
await _checkSingleFileChanges(content, expected);
}
Future<void> test_angular_viewChild_field() async {
addAngularPackage();
var content = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
// Initialize this.bar in the constructor just so the migration tool doesn't
// decide to make it nullable due to the lack of initializer.
MyComponent(this.bar);
@ViewChild('foo')
Element bar;
}
''';
var expected = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
// Initialize this.bar in the constructor just so the migration tool doesn't
// decide to make it nullable due to the lack of initializer.
MyComponent(this.bar);
@ViewChild('foo')
Element? bar;
}
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_angular_viewChild_field_internal() async {
addAngularPackage(internalUris: true);
var content = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
// Initialize this.bar in the constructor just so the migration tool doesn't
// decide to make it nullable due to the lack of initializer.
MyComponent(this.bar);
@ViewChild('foo')
Element bar;
}
''';
var expected = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
// Initialize this.bar in the constructor just so the migration tool doesn't
// decide to make it nullable due to the lack of initializer.
MyComponent(this.bar);
@ViewChild('foo')
Element? bar;
}
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_angular_viewChild_setter() async {
addAngularPackage();
var content = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
@ViewChild('foo')
set bar(Element element) {}
}
''';
var expected = '''
import 'dart:html';
import 'package:angular/angular.dart';
class MyComponent {
@ViewChild('foo')
set bar(Element? element) {}
}
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_argumentError_checkNotNull_implies_non_null_intent() async {
var content = '''
void f(int i) {